fix: disallow returning constant values

compiler/noirc_errors/src/position.rs

@@ -119,4 +119,8 @@ impl Location {
     pub fn new(span: Span, file: FileId) -> Self {
         Self { span, file }
     }
+
+    pub fn dummy() -> Self {
+        Self { span: Span::single_char(0), file: FileId::dummy() }
+    }
 }

compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs

@@ -330,7 +330,7 @@ mod tests {
     fn create_test_environment() -> (Ssa, FunctionContext, BrilligContext) {
         let builder =
             FunctionBuilder::new("main".to_string(), Id::test_new(0), RuntimeType::Brillig);
-        let ssa = builder.finish();
+        let ssa = builder.finish(None);
         let mut brillig_context = create_context();
 
         let function_context = FunctionContext::new(ssa.main(), &mut brillig_context);

compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs

@@ -362,7 +362,7 @@ mod test {
 
         builder.terminate_with_return(vec![v8]);
 
-        let ssa = builder.finish();
+        let ssa = builder.finish(None);
         let func = ssa.main();
         let liveness = VariableLiveness::from_function(func);
 
@@ -505,7 +505,7 @@ mod test {
 
         builder.terminate_with_return(vec![v17]);
 
-        let ssa = builder.finish();
+        let ssa = builder.finish(None);
         let func = ssa.main();
 
         let liveness = VariableLiveness::from_function(func);

compiler/noirc_evaluator/src/errors.rs

@@ -9,7 +9,7 @@
 //! An Error of the latter is an error in the implementation of the compiler
 use acvm::acir::native_types::Expression;
 use iter_extended::vecmap;
-use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic};
+use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic, Location};
 use thiserror::Error;
 
 use crate::ssa::ir::dfg::CallStack;
@@ -67,6 +67,8 @@ pub enum InternalError {
     UndeclaredAcirVar { call_stack: CallStack },
     #[error("ICE: Expected {expected:?}, found {found:?}")]
     UnExpected { expected: String, found: String, call_stack: CallStack },
+    #[error("Returning constant value is not allowed")]
+    ReturnConstant { location: Location, call_stack: CallStack },
 }
 
 impl RuntimeError {
@@ -79,7 +81,8 @@ impl RuntimeError {
                 | InternalError::MissingArg { call_stack, .. }
                 | InternalError::NotAConstant { call_stack, .. }
                 | InternalError::UndeclaredAcirVar { call_stack }
-                | InternalError::UnExpected { call_stack, .. },
+                | InternalError::UnExpected { call_stack, .. }
+                | InternalError::ReturnConstant { call_stack, .. },
             )
             | RuntimeError::FailedConstraint { call_stack, .. }
             | RuntimeError::IndexOutOfBounds { call_stack, .. }
@@ -96,16 +99,23 @@ impl RuntimeError {
 impl From<RuntimeError> for FileDiagnostic {
     fn from(error: RuntimeError) -> FileDiagnostic {
         let call_stack = vecmap(error.call_stack(), |location| *location);
+        let file_id = match error {
+            RuntimeError::InternalError(InternalError::ReturnConstant { location, .. }) => {
+                location.file
+            }
+            _ => call_stack.last().map(|location| location.file).unwrap_or_default(),
+        };
         let diagnostic = error.into_diagnostic();
-        let file_id = call_stack.last().map(|location| location.file).unwrap_or_default();
-
         diagnostic.in_file(file_id).with_call_stack(call_stack)
     }
 }
 
 impl RuntimeError {
     fn into_diagnostic(self) -> Diagnostic {
         match self {
+            RuntimeError::InternalError(InternalError::ReturnConstant { ref location, .. }) => {
+               Diagnostic::simple_error(self.to_string(), "constant value".to_string(), location.span)
+            }
             RuntimeError::InternalError(cause) => {
                 Diagnostic::simple_error(
                     "Internal Consistency Evaluators Errors: \n

compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs

@@ -249,6 +249,11 @@ impl AcirContext {
         }
     }
 
+    /// True if the given AcirVar refers to a constant value
+    pub(crate) fn is_constant(&self, var: &AcirVar) -> bool {
+        matches!(self.vars[var], AcirVarData::Const(_))
+    }
+
     /// Adds a new Variable to context whose value will
     /// be constrained to be the negation of `var`.
     ///

compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs

@@ -32,6 +32,7 @@ use acvm::{
 use fxhash::FxHashMap as HashMap;
 use im::Vector;
 use iter_extended::{try_vecmap, vecmap};
+use noirc_errors::Location;
 use noirc_frontend::Distinctness;
 
 /// Context struct for the acir generation pass.
@@ -201,7 +202,7 @@ impl Context {
             self.convert_ssa_instruction(*instruction_id, dfg, ssa, &brillig, last_array_uses)?;
         }
 
-        self.convert_ssa_return(entry_block.unwrap_terminator(), dfg)?;
+        self.convert_ssa_return(entry_block.unwrap_terminator(), dfg, ssa.return_location)?;
 
         Ok(self.acir_context.finish(input_witness.collect()))
     }
@@ -1211,6 +1212,7 @@ impl Context {
         &mut self,
         terminator: &TerminatorInstruction,
         dfg: &DataFlowGraph,
+        return_location: Option<Location>,
     ) -> Result<(), InternalError> {
         let return_values = match terminator {
             TerminatorInstruction::Return { return_values } => return_values,
@@ -1221,6 +1223,12 @@ impl Context {
         // will expand the array if there is one.
         let return_acir_vars = self.flatten_value_list(return_values, dfg);
         for acir_var in return_acir_vars {
+            if self.acir_context.is_constant(&acir_var) {
+                return Err(InternalError::ReturnConstant {
+                    location: return_location.expect("return expression must have a location"),
+                    call_stack: self.acir_context.get_call_stack(),
+                });
+            }
             self.acir_context.return_var(acir_var)?;
         }
         Ok(())
@@ -1830,9 +1838,11 @@ mod tests {
         },
         FieldElement,
     };
+    use noirc_errors::{Location, Span};
 
     use crate::{
         brillig::Brillig,
+        errors::{InternalError, RuntimeError},
         ssa::{
             function_builder::FunctionBuilder,
             ir::{function::RuntimeType, map::Id, types::Type},
@@ -1858,15 +1868,13 @@ mod tests {
 
         builder.terminate_with_return(vec![array]);
 
-        let ssa = builder.finish();
+        let ssa = builder.finish(Some(Location::dummy()));
 
         let context = Context::new();
-        let mut acir = context.convert_ssa(ssa, Brillig::default(), &HashMap::default()).unwrap();
-
-        let expected_opcodes =
-            vec![Opcode::Arithmetic(&Expression::one() - &Expression::from(Witness(1)))];
-        assert_eq!(acir.take_opcodes(), expected_opcodes);
-        assert_eq!(acir.return_witnesses, vec![Witness(1)]);
+        let acir = context
+            .convert_ssa(ssa, Brillig::default(), &HashMap::default())
+            .expect_err("Return constant value");
+        assert!(matches!(acir, RuntimeError::InternalError(InternalError::ReturnConstant { .. })));
     }
 }
 //

compiler/noirc_evaluator/src/ssa/function_builder/mod.rs

@@ -87,9 +87,9 @@ impl FunctionBuilder {
     }
 
     /// Consume the FunctionBuilder returning all the functions it has generated.
-    pub(crate) fn finish(mut self) -> Ssa {
+    pub(crate) fn finish(mut self, return_location: Option<Location>) -> Ssa {
         self.finished_functions.push(self.current_function);
-        Ssa::new(self.finished_functions)
+        Ssa::new(self.finished_functions, return_location)
     }
 
     /// Add a parameter to the current function with the given parameter type.

compiler/noirc_evaluator/src/ssa/ir/dom.rs

@@ -300,7 +300,7 @@ mod tests {
         builder.switch_to_block(block3_id);
         builder.terminate_with_return(vec![]);
 
-        let ssa = builder.finish();
+        let ssa = builder.finish(None);
         let func = ssa.main();
         let block0_id = func.entry_block();
 
@@ -404,7 +404,7 @@ mod tests {
         builder.switch_to_block(block2_id);
         builder.terminate_with_jmp(block1_id, vec![]);
 
-        let ssa = builder.finish();
+        let ssa = builder.finish(None);
         let func = ssa.main();
         let block0_id = func.entry_block();
 

compiler/noirc_evaluator/src/ssa/ir/post_order.rs

@@ -146,7 +146,7 @@ mod tests {
         builder.switch_to_block(block_c_id);
         builder.terminate_with_jmp(block_f_id, vec![]);
 
-        let ssa = builder.finish();
+        let ssa = builder.finish(None);
         let func = ssa.main();
         let post_order = PostOrder::with_function(func);
         let block_a_id = func.entry_block();

compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs

@@ -215,7 +215,7 @@ mod test {
         let v2 = builder.insert_binary(v1, BinaryOp::Mul, three);
         builder.terminate_with_return(vec![v2]);
 
-        let mut ssa = builder.finish();
+        let mut ssa = builder.finish(None);
         let main = ssa.main_mut();
         let instructions = main.dfg[main.entry_block()].instructions();
         assert_eq!(instructions.len(), 2); // The final return is not counted
@@ -268,7 +268,7 @@ mod test {
         let arr = builder.current_function.dfg.make_array(vec![v1].into(), array_type);
         builder.terminate_with_return(vec![arr]);
 
-        let ssa = builder.finish().fold_constants();
+        let ssa = builder.finish(None).fold_constants();
         let main = ssa.main();
         let entry_block_id = main.entry_block();
         let entry_block = &main.dfg[entry_block_id];
@@ -313,7 +313,7 @@ mod test {
         let v2 = builder.insert_cast(v0, Type::unsigned(32));
         builder.insert_constrain(v1, v2, None);
 
-        let mut ssa = builder.finish();
+        let mut ssa = builder.finish(None);
         let main = ssa.main_mut();
         let instructions = main.dfg[main.entry_block()].instructions();
         assert_eq!(instructions.len(), 3);

compiler/noirc_evaluator/src/ssa/opt/die.rs

@@ -191,7 +191,7 @@ mod test {
         builder.insert_call(assert_constant_id, vec![v8], vec![]);
         builder.terminate_with_return(vec![v9]);
 
-        let ssa = builder.finish();
+        let ssa = builder.finish(None);
         let main = ssa.main();
 
         // The instruction count never includes the terminator instruction

compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs

@@ -728,7 +728,7 @@ mod test {
         builder.switch_to_block(b3);
         builder.terminate_with_return(vec![v1]);
 
-        let ssa = builder.finish();
+        let ssa = builder.finish(None);
         assert_eq!(ssa.main().reachable_blocks().len(), 4);
 
         // Expected output:
@@ -777,7 +777,7 @@ mod test {
         builder.switch_to_block(b2);
         builder.terminate_with_return(vec![]);
 
-        let ssa = builder.finish();
+        let ssa = builder.finish(None);
         assert_eq!(ssa.main().reachable_blocks().len(), 3);
 
         // Expected output:
@@ -826,7 +826,7 @@ mod test {
         builder.switch_to_block(b2);
         builder.terminate_with_return(vec![]);
 
-        let ssa = builder.finish();
+        let ssa = builder.finish(None);
 
         // Expected output:
         // fn main f0 {
@@ -893,7 +893,7 @@ mod test {
         builder.switch_to_block(b3);
         builder.terminate_with_return(vec![]);
 
-        let ssa = builder.finish();
+        let ssa = builder.finish(None);
 
         // Expected output:
         // fn main f0 {
@@ -1035,7 +1035,7 @@ mod test {
         let load = builder.insert_load(r1, Type::field());
         builder.terminate_with_return(vec![load]);
 
-        let ssa = builder.finish().flatten_cfg().mem2reg();
+        let ssa = builder.finish(None).flatten_cfg().mem2reg();
 
         // Expected results after mem2reg removes the allocation and each load and store:
         //
@@ -1135,7 +1135,7 @@ mod test {
         builder.switch_to_block(b2);
         builder.terminate_with_return(vec![]);
 
-        let ssa = builder.finish().flatten_cfg();
+        let ssa = builder.finish(None).flatten_cfg();
         let main = ssa.main();
 
         // Now assert that there is not a load between the allocate and its first store
@@ -1233,7 +1233,7 @@ mod test {
         builder.insert_constrain(v_false, v_true, None); // should not be removed
         builder.terminate_with_return(vec![]);
 
-        let ssa = builder.finish().flatten_cfg();
+        let ssa = builder.finish(None).flatten_cfg();
         let main = ssa.main();
 
         // Assert we have not incorrectly removed a constraint:
@@ -1316,7 +1316,7 @@ mod test {
         builder.insert_constrain(v12, v_true, None);
         builder.terminate_with_return(vec![]);
 
-        let ssa = builder.finish().flatten_cfg();
+        let ssa = builder.finish(None).flatten_cfg();
         let main = ssa.main();
 
         // Now assert that there is not an always-false constraint after flattening:
@@ -1436,7 +1436,7 @@ mod test {
         builder.switch_to_block(b4);
         builder.terminate_with_jmp(b5, vec![]);
 
-        let ssa = builder.finish().flatten_cfg().mem2reg().fold_constants();
+        let ssa = builder.finish(None).flatten_cfg().mem2reg().fold_constants();
 
         let main = ssa.main();
 

compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/branch_analysis.rs

@@ -169,7 +169,7 @@ mod test {
         builder.switch_to_block(b9);
         builder.terminate_with_return(vec![]);
 
-        let mut ssa = builder.finish();
+        let mut ssa = builder.finish(None);
         let function = ssa.main_mut();
         let cfg = ControlFlowGraph::with_function(function);
         let branch_ends = find_branch_ends(function, &cfg);
@@ -253,7 +253,7 @@ mod test {
         builder.switch_to_block(b15);
         builder.terminate_with_return(vec![]);
 
-        let mut ssa = builder.finish();
+        let mut ssa = builder.finish(None);
         let function = ssa.main_mut();
         let cfg = ControlFlowGraph::with_function(function);
         find_branch_ends(function, &cfg);

compiler/noirc_evaluator/src/ssa/opt/inlining.rs

@@ -141,7 +141,7 @@ impl InlineContext {
         context.inline_blocks(ssa);
 
         // Finally, we should have 1 function left representing the inlined version of the target function.
-        let mut new_ssa = self.builder.finish();
+        let mut new_ssa = self.builder.finish(ssa.return_location);
         assert_eq!(new_ssa.functions.len(), 1);
         new_ssa.functions.pop_first().unwrap().1
     }
@@ -535,7 +535,7 @@ mod test {
         let seventy_two = builder.field_constant(expected_return);
         builder.terminate_with_return(vec![seventy_two]);
 
-        let ssa = builder.finish();
+        let ssa = builder.finish(None);
         assert_eq!(ssa.functions.len(), 2);
 
         let inlined = ssa.inline_functions();
@@ -601,7 +601,7 @@ mod test {
         builder.terminate_with_return(vec![id2_v0]);
 
         // Done, now we test that we can successfully inline all functions.
-        let ssa = builder.finish();
+        let ssa = builder.finish(None);
         assert_eq!(ssa.functions.len(), 4);
 
         let inlined = ssa.inline_functions();
@@ -657,7 +657,7 @@ mod test {
         let v4 = builder.insert_binary(v0, BinaryOp::Mul, v3);
         builder.terminate_with_return(vec![v4]);
 
-        let ssa = builder.finish();
+        let ssa = builder.finish(None);
         assert_eq!(ssa.functions.len(), 2);
 
         // Expected SSA:
@@ -761,7 +761,7 @@ mod test {
         builder.switch_to_block(join_block);
         builder.terminate_with_return(vec![join_param]);
 
-        let ssa = builder.finish().inline_functions();
+        let ssa = builder.finish(None).inline_functions();
         // Expected result:
         // fn main f3 {
         //   b0(v0: u1):