fix: disallow #[test] on associated functions (noir-lang/noir#6449)

chore(ssa): Skip array_set pass for Brillig functions (noir-lang/noir#6513)
chore: Reverse ssa parser diff order (noir-lang/noir#6511)
chore: Parse negatives in SSA parser (noir-lang/noir#6510)
feat: avoid unnecessary ssa passes while loop unrolling (noir-lang/noir#6509)
fix(tests): Use a file lock as well as a mutex to isolate tests cases (noir-lang/noir#6508)
fix: set local_module before elaborating each trait (noir-lang/noir#6506)
fix: parse Slice type in SSa (noir-lang/noir#6507)
fix: perform arithmetic simplification through `CheckedCast` (noir-lang/noir#6502)
feat: SSA parser (noir-lang/noir#6489)
chore(test): Run test matrix on test_programs (noir-lang/noir#6429)
chore(ci): fix cargo deny (noir-lang/noir#6501)
feat: Deduplicate instructions across blocks (noir-lang/noir#6499)
chore: move tests for arithmetic generics closer to the code (noir-lang/noir#6497)
fix(docs): Fix broken links in oracles doc (noir-lang/noir#6488)
fix: Treat all parameters as possible aliases of each other (noir-lang/noir#6477)
chore: bump rust dependencies (noir-lang/noir#6482)
feat: use a full `BlackBoxFunctionSolver` implementation when execution brillig during acirgen (noir-lang/noir#6481)
chore(docs): Update How to Oracles (noir-lang/noir#5675)
chore: Release Noir(0.38.0) (noir-lang/noir#6422)
fix(ssa): Change array_set to not mutate slices coming from function inputs (noir-lang/noir#6463)
chore: update example to show how to split public inputs in bash (noir-lang/noir#6472)
fix: Discard optimisation that would change execution ordering or that is related to call outputs (noir-lang/noir#6461)
chore: proptest for `canonicalize` on infix type expressions (noir-lang/noir#6269)
fix: let formatter respect newlines between comments (noir-lang/noir#6458)
fix: check infix expression is valid in program input (noir-lang/noir#6450)
fix: don't crash on AsTraitPath with empty path (noir-lang/noir#6454)
fix(tests): Prevent EOF error while running test programs (noir-lang/noir#6455)
fix(sea): mem2reg to treat block input references as alias (noir-lang/noir#6452)
chore: revamp attributes (noir-lang/noir#6424)
feat!: Always Check Arithmetic Generics at Monomorphization (noir-lang/noir#6329)
chore: split path and import lookups (noir-lang/noir#6430)
fix(ssa): Resolve value IDs in terminator before comparing to array (noir-lang/noir#6448)
fix: right shift is not a regular division (noir-lang/noir#6400)

.noir-sync-commit

@@ -1 +1 @@
-0fc0c53ec183890370c69aa4148952b3123cb055
+35408ab303f1018c1e2c38e6ea55430a2c89dc4c

noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/mod.rs

@@ -212,6 +212,59 @@ impl std::fmt::Display for ProcedureId {
     }
 }
 
+impl ProcedureId {
+    pub(crate) fn to_debug_id(self) -> ProcedureDebugId {
+        ProcedureDebugId(match self {
+            ProcedureId::ArrayCopy => 0,
+            ProcedureId::ArrayReverse => 1,
+            ProcedureId::VectorCopy => 2,
+            ProcedureId::MemCopy => 3,
+            ProcedureId::PrepareVectorPush(true) => 4,
+            ProcedureId::PrepareVectorPush(false) => 5,
+            ProcedureId::VectorPopFront => 6,
+            ProcedureId::VectorPopBack => 7,
+            ProcedureId::PrepareVectorInsert => 8,
+            ProcedureId::VectorRemove => 9,
+            ProcedureId::CheckMaxStackDepth => 10,
+        })
+    }
+
+    pub fn from_debug_id(debug_id: ProcedureDebugId) -> Self {
+        let inner = debug_id.0;
+        match inner {
+            0 => ProcedureId::ArrayCopy,
+            1 => ProcedureId::ArrayReverse,
+            2 => ProcedureId::VectorCopy,
+            3 => ProcedureId::MemCopy,
+            4 => ProcedureId::PrepareVectorPush(true),
+            5 => ProcedureId::PrepareVectorPush(false),
+            6 => ProcedureId::VectorPopFront,
+            7 => ProcedureId::VectorPopBack,
+            8 => ProcedureId::PrepareVectorInsert,
+            9 => ProcedureId::VectorRemove,
+            10 => ProcedureId::CheckMaxStackDepth,
+            _ => panic!("Unsupported procedure debug ID of {inner} was supplied"),
+        }
+    }
+}
+
+impl std::fmt::Display for ProcedureId {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            ProcedureId::ArrayCopy => write!(f, "ArrayCopy"),
+            ProcedureId::ArrayReverse => write!(f, "ArrayReverse"),
+            ProcedureId::VectorCopy => write!(f, "VectorCopy"),
+            ProcedureId::MemCopy => write!(f, "MemCopy"),
+            ProcedureId::PrepareVectorPush(flag) => write!(f, "PrepareVectorPush({flag})"),
+            ProcedureId::VectorPopFront => write!(f, "VectorPopFront"),
+            ProcedureId::VectorPopBack => write!(f, "VectorPopBack"),
+            ProcedureId::PrepareVectorInsert => write!(f, "PrepareVectorInsert"),
+            ProcedureId::VectorRemove => write!(f, "VectorRemove"),
+            ProcedureId::CheckMaxStackDepth => write!(f, "CheckMaxStackDepth"),
+        }
+    }
+}
+
 pub(crate) fn compile_procedure<F: AcirField + DebugToString>(
     procedure_id: ProcedureId,
 ) -> BrilligArtifact<F> {

noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/array_set.rs

@@ -28,17 +28,21 @@ impl Ssa {
 
 impl Function {
     pub(crate) fn array_set_optimization(&mut self) {
+        if matches!(self.runtime(), RuntimeType::Brillig(_)) {
+            // Brillig is supposed to use refcounting to decide whether to mutate an array;
+            // array mutation was only meant for ACIR. We could use it with Brillig as well,
+            // but then some of the optimizations that we can do in ACIR around shared
+            // references have to be skipped, which makes it more cumbersome.
+            return;
+        }
+
         let reachable_blocks = self.reachable_blocks();
 
         if !self.runtime().is_entry_point() {
             assert_eq!(reachable_blocks.len(), 1, "Expected there to be 1 block remaining in Acir function for array_set optimization");
         }
 
-        let mut context = Context::new(
-            &self.dfg,
-            self.parameters(),
-            matches!(self.runtime(), RuntimeType::Brillig(_)),
-        );
+        let mut context = Context::new(&self.dfg);
 
         for block in reachable_blocks.iter() {
             context.analyze_last_uses(*block);
@@ -53,8 +57,6 @@ impl Function {
 
 struct Context<'f> {
     dfg: &'f DataFlowGraph,
-    function_parameters: &'f [ValueId],
-    is_brillig_runtime: bool,
     array_to_last_use: HashMap<ValueId, InstructionId>,
     instructions_that_can_be_made_mutable: HashSet<InstructionId>,
     // Mapping of an array that comes from a load and whether the address
@@ -64,15 +66,9 @@ struct Context<'f> {
 }
 
 impl<'f> Context<'f> {
-    fn new(
-        dfg: &'f DataFlowGraph,
-        function_parameters: &'f [ValueId],
-        is_brillig_runtime: bool,
-    ) -> Self {
+    fn new(dfg: &'f DataFlowGraph) -> Self {
         Context {
             dfg,
-            function_parameters,
-            is_brillig_runtime,
             array_to_last_use: HashMap::default(),
             instructions_that_can_be_made_mutable: HashSet::default(),
             arrays_from_load: HashMap::default(),
@@ -94,21 +90,12 @@ impl<'f> Context<'f> {
                         self.instructions_that_can_be_made_mutable.remove(&existing);
                     }
                 }
-                Instruction::ArraySet { array, value, .. } => {
+                Instruction::ArraySet { array, .. } => {
                     let array = self.dfg.resolve(*array);
 
                     if let Some(existing) = self.array_to_last_use.insert(array, *instruction_id) {
                         self.instructions_that_can_be_made_mutable.remove(&existing);
                     }
-                    if self.is_brillig_runtime {
-                        let value = self.dfg.resolve(*value);
-
-                        if let Some(existing) = self.inner_nested_arrays.get(&value) {
-                            self.instructions_that_can_be_made_mutable.remove(existing);
-                        }
-                        let result = self.dfg.instruction_results(*instruction_id)[0];
-                        self.inner_nested_arrays.insert(result, *instruction_id);
-                    }
 
                     // If the array we are setting does not come from a load we can safely mark it mutable.
                     // If the array comes from a load we may potentially being mutating an array at a reference
@@ -128,17 +115,13 @@ impl<'f> Context<'f> {
                         }
                     });
 
-                    // We cannot safely mutate slices that are inputs to the function, as they might be shared with the caller.
-                    // NB checking the block parameters is not enough, as we might have jumped into a parameterless blocks inside the function.
-                    let is_function_param = self.function_parameters.contains(&array);
-
                     let can_mutate = if let Some(is_from_param) = self.arrays_from_load.get(&array)
                     {
                         // If the array was loaded from a reference parameter, we cannot
                         // safely mark that array mutable as it may be shared by another value.
                         !is_from_param && is_return_block
                     } else {
-                        !is_array_in_terminator && !is_function_param
+                        !is_array_in_terminator
                     };
 
                     if can_mutate {

noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs

@@ -44,7 +44,7 @@ pub(crate) fn assert_normalized_ssa_equals(mut ssa: super::Ssa, expected: &str)
     let expected = trim_leading_whitespace_from_lines(expected);
 
     if ssa != expected {
-        println!("Got:\n~~~\n{}\n~~~\nExpected:\n~~~\n{}\n~~~", ssa, expected);
-        similar_asserts::assert_eq!(ssa, expected);
+        println!("Expected:\n~~~\n{expected}\n~~~\nGot:\n~~~\n{ssa}\n~~~");
+        similar_asserts::assert_eq!(expected, ssa);
     }
 }

noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs

@@ -452,7 +452,14 @@ impl<'context> Elaborator<'context> {
             }
             ItemKind::Impl(r#impl) => {
                 let module = self.module_id();
-                dc_mod::collect_impl(self.interner, generated_items, r#impl, self.file, module);
+                dc_mod::collect_impl(
+                    self.interner,
+                    generated_items,
+                    r#impl,
+                    self.file,
+                    module,
+                    &mut self.errors,
+                );
             }
 
             ItemKind::ModuleDecl(_)

noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs

@@ -97,9 +97,9 @@ pub fn collect_defs(
 
     errors.extend(collector.collect_functions(context, ast.functions, crate_id));
 
-    collector.collect_trait_impls(context, ast.trait_impls, crate_id);
+    errors.extend(collector.collect_trait_impls(context, ast.trait_impls, crate_id));
 
-    collector.collect_impls(context, ast.impls, crate_id);
+    errors.extend(collector.collect_impls(context, ast.impls, crate_id));
 
     collector.collect_attributes(
         ast.inner_attributes,
@@ -163,7 +163,13 @@ impl<'a> ModCollector<'a> {
         errors
     }
 
-    fn collect_impls(&mut self, context: &mut Context, impls: Vec<TypeImpl>, krate: CrateId) {
+    fn collect_impls(
+        &mut self,
+        context: &mut Context,
+        impls: Vec<TypeImpl>,
+        krate: CrateId,
+    ) -> Vec<(CompilationError, FileId)> {
+        let mut errors = Vec::new();
         let module_id = ModuleId { krate, local_id: self.module_id };
 
         for r#impl in impls {
@@ -173,16 +179,21 @@ impl<'a> ModCollector<'a> {
                 r#impl,
                 self.file_id,
                 module_id,
+                &mut errors,
             );
         }
+
+        errors
     }
 
     fn collect_trait_impls(
         &mut self,
         context: &mut Context,
         impls: Vec<NoirTraitImpl>,
         krate: CrateId,
-    ) {
+    ) -> Vec<(CompilationError, FileId)> {
+        let mut errors = Vec::new();
+
         for mut trait_impl in impls {
             let trait_name = trait_impl.trait_name.clone();
 
@@ -198,6 +209,13 @@ impl<'a> ModCollector<'a> {
             let module = ModuleId { krate, local_id: self.module_id };
 
             for (_, func_id, noir_function) in &mut unresolved_functions.functions {
+                if noir_function.def.attributes.is_test_function() {
+                    let error = DefCollectorErrorKind::TestOnAssociatedFunction {
+                        span: noir_function.name_ident().span(),
+                    };
+                    errors.push((error.into(), self.file_id));
+                }
+
                 let location = Location::new(noir_function.def.span, self.file_id);
                 context.def_interner.push_function(*func_id, &noir_function.def, module, location);
             }
@@ -224,6 +242,8 @@ impl<'a> ModCollector<'a> {
 
             self.def_collector.items.trait_impls.push(unresolved_trait_impl);
         }
+
+        errors
     }
 
     fn collect_functions(
@@ -1051,13 +1071,23 @@ pub fn collect_impl(
     r#impl: TypeImpl,
     file_id: FileId,
     module_id: ModuleId,
+    errors: &mut Vec<(CompilationError, FileId)>,
 ) {
     let mut unresolved_functions =
         UnresolvedFunctions { file_id, functions: Vec::new(), trait_id: None, self_type: None };
 
     for (method, _) in r#impl.methods {
         let doc_comments = method.doc_comments;
         let mut method = method.item;
+
+        if method.def.attributes.is_test_function() {
+            let error = DefCollectorErrorKind::TestOnAssociatedFunction {
+                span: method.name_ident().span(),
+            };
+            errors.push((error.into(), file_id));
+            continue;
+        }
+
         let func_id = interner.push_empty_fn();
         method.def.where_clause.extend(r#impl.where_clause.clone());
         let location = Location::new(method.span(), file_id);

noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/errors.rs

@@ -82,6 +82,8 @@ pub enum DefCollectorErrorKind {
     },
     #[error("{0}")]
     UnsupportedNumericGenericType(#[from] UnsupportedNumericGenericType),
+    #[error("The `#[test]` attribute may only be used on a non-associated function")]
+    TestOnAssociatedFunction { span: Span },
 }
 
 impl DefCollectorErrorKind {
@@ -291,6 +293,12 @@ impl<'a> From<&'a DefCollectorErrorKind> for Diagnostic {
                 diag
             }
             DefCollectorErrorKind::UnsupportedNumericGenericType(err) => err.into(),
+            DefCollectorErrorKind::TestOnAssociatedFunction { span } => Diagnostic::simple_error(
+                "The `#[test]` attribute is disallowed on `impl` methods".into(),
+                String::new(),
+                *span,
+            ),
+
         }
     }
 }

noir/noir-repo/compiler/noirc_frontend/src/tests.rs

@@ -3692,3 +3692,51 @@ fn allows_struct_with_generic_infix_type_as_main_input_3() {
     "#;
     assert_no_errors(src);
 }
+
+#[test]
+fn disallows_test_attribute_on_impl_method() {
+    let src = r#"
+    pub struct Foo {}
+    impl Foo {
+        #[test]
+        fn foo() {}
+    }
+
+    fn main() {}
+    "#;
+    let errors = get_program_errors(src);
+    assert_eq!(errors.len(), 1);
+
+    assert!(matches!(
+        errors[0].0,
+        CompilationError::DefinitionError(DefCollectorErrorKind::TestOnAssociatedFunction {
+            span: _
+        })
+    ));
+}
+
+#[test]
+fn disallows_test_attribute_on_trait_impl_method() {
+    let src = r#"
+    pub trait Trait {
+        fn foo() {}
+    }
+
+    pub struct Foo {}
+    impl Trait for Foo {
+        #[test]
+        fn foo() {}
+    }
+
+    fn main() {}
+    "#;
+    let errors = get_program_errors(src);
+    assert_eq!(errors.len(), 1);
+
+    assert!(matches!(
+        errors[0].0,
+        CompilationError::DefinitionError(DefCollectorErrorKind::TestOnAssociatedFunction {
+            span: _
+        })
+    ));
+}