[1 changes] feat: Sync from aztec-packages (noir-lang/noir#6301)

fix: Allow array map on empty arrays (noir-lang/noir#6305)
fix: Display function name and body when inlining recursion limit hit (noir-lang/noir#6291)
feat(interpreter): Comptime derive generators (noir-lang/noir#6303)
fix: enforce correctness of decompositions performed at compile time (noir-lang/noir#6278)
feat: Warn about private types leaking in public functions and struct fields (noir-lang/noir#6296)
chore(docs): refactoring guides and some other nits (noir-lang/noir#6175)
fix: Do not warn on unused self in traits (noir-lang/noir#6298)
fix: Reject invalid expression with in CLI parser (noir-lang/noir#6287)

.noir-sync-commit

@@ -1 +1 @@
-ae87d287ab1fae0f999dfd0d1166fbddb927ba97
+70dcf4a25dcad10daeb427f0887d3a0bf10c9916

noir/noir-repo/.github/workflows/docs-pr.yml

@@ -80,7 +80,7 @@ jobs:
 
       - name: Build docs
         env:
-          MATOMO_ENV: staging # not really a secret, it will show in the footer anyway
+          ENV: staging # not really a secret, it will show in the footer anyway
         run: yarn workspaces foreach -Rpt --from docs run build
 
       - name: Upload artifact

noir/noir-repo/acvm-repo/acvm_js/build.sh

@@ -25,7 +25,7 @@ function run_if_available {
 require_command jq
 require_command cargo
 require_command wasm-bindgen
-#require_command wasm-opt
+require_command wasm-opt
 
 self_path=$(dirname "$(readlink -f "$0")")
 pname=$(cargo read-manifest | jq -r '.name')

noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs

@@ -9,7 +9,7 @@ use super::{
 };
 use acvm::acir::{brillig::MemoryAddress, AcirField};
 
-pub(crate) const MAX_STACK_SIZE: usize = 32768;
+pub(crate) const MAX_STACK_SIZE: usize = 16 * MAX_STACK_FRAME_SIZE;
 pub(crate) const MAX_STACK_FRAME_SIZE: usize = 2048;
 pub(crate) const MAX_SCRATCH_SPACE: usize = 64;
 

noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs

@@ -60,9 +60,7 @@ pub(super) fn simplify_call(
                 } else {
                     unreachable!("ICE: Intrinsic::ToRadix return type must be array")
                 };
-                let result_array = constant_to_radix(endian, field, 2, limb_count, dfg);
-
-                SimplifyResult::SimplifiedTo(result_array)
+                constant_to_radix(endian, field, 2, limb_count, dfg)
             } else {
                 SimplifyResult::None
             }
@@ -79,10 +77,7 @@ pub(super) fn simplify_call(
                 } else {
                     unreachable!("ICE: Intrinsic::ToRadix return type must be array")
                 };
-
-                let result_array = constant_to_radix(endian, field, radix, limb_count, dfg);
-
-                SimplifyResult::SimplifiedTo(result_array)
+                constant_to_radix(endian, field, radix, limb_count, dfg)
             } else {
                 SimplifyResult::None
             }
@@ -606,22 +601,29 @@ fn constant_to_radix(
     radix: u32,
     limb_count: u32,
     dfg: &mut DataFlowGraph,
-) -> ValueId {
+) -> SimplifyResult {
     let bit_size = u32::BITS - (radix - 1).leading_zeros();
     let radix_big = BigUint::from(radix);
     assert_eq!(BigUint::from(2u128).pow(bit_size), radix_big, "ICE: Radix must be a power of 2");
     let big_integer = BigUint::from_bytes_be(&field.to_be_bytes());
 
     // Decompose the integer into its radix digits in little endian form.
     let decomposed_integer = big_integer.to_radix_le(radix);
-    let mut limbs = vecmap(0..limb_count, |i| match decomposed_integer.get(i as usize) {
-        Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]),
-        None => FieldElement::zero(),
-    });
-    if endian == Endian::Big {
-        limbs.reverse();
+    if limb_count < decomposed_integer.len() as u32 {
+        // `field` cannot be represented as `limb_count` bits.
+        // defer error to acir_gen.
+        SimplifyResult::None
+    } else {
+        let mut limbs = vecmap(0..limb_count, |i| match decomposed_integer.get(i as usize) {
+            Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]),
+            None => FieldElement::zero(),
+        });
+        if endian == Endian::Big {
+            limbs.reverse();
+        }
+        let result_array = make_constant_array(dfg, limbs, Type::unsigned(bit_size));
+        SimplifyResult::SimplifiedTo(result_array)
     }
-    make_constant_array(dfg, limbs, Type::unsigned(bit_size))
 }
 
 fn to_u8_vec(dfg: &DataFlowGraph, values: im::Vector<Id<Value>>) -> Vec<u8> {

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

@@ -413,13 +413,14 @@ impl InlineContext {
     ) -> Vec<ValueId> {
         self.recursion_level += 1;
 
+        let source_function = &ssa.functions[&id];
+
         if self.recursion_level > RECURSION_LIMIT {
             panic!(
-                "Attempted to recur more than {RECURSION_LIMIT} times during function inlining."
+                "Attempted to recur more than {RECURSION_LIMIT} times during inlining function '{}': {}", source_function.name(), source_function
             );
         }
 
-        let source_function = &ssa.functions[&id];
         let mut context = PerFunctionContext::new(self, source_function);
 
         let parameters = source_function.parameters();
@@ -1091,6 +1092,30 @@ mod test {
         assert_eq!(main.reachable_blocks().len(), 4);
     }
 
+    #[test]
+    #[should_panic(
+        expected = "Attempted to recur more than 1000 times during inlining function 'main': acir(inline) fn main f0 {"
+    )]
+    fn unconditional_recursion() {
+        // fn main f1 {
+        //   b0():
+        //     call f1()
+        //     return
+        // }
+        let main_id = Id::test_new(0);
+        let mut builder = FunctionBuilder::new("main".into(), main_id);
+
+        let main = builder.import_function(main_id);
+        let results = builder.insert_call(main, Vec::new(), vec![]).to_vec();
+        builder.terminate_with_return(results);
+
+        let ssa = builder.finish();
+        assert_eq!(ssa.functions.len(), 1);
+
+        let inlined = ssa.inline_functions(i64::MAX);
+        assert_eq!(inlined.functions.len(), 0);
+    }
+
     #[test]
     fn inliner_disabled() {
         // brillig fn foo {

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

@@ -3,7 +3,9 @@ use std::{
     rc::Rc,
 };
 
-use crate::{ast::ItemVisibility, hir_def::traits::ResolvedTraitBound, StructField, TypeBindings};
+use crate::{
+    ast::ItemVisibility, hir_def::traits::ResolvedTraitBound, StructField, StructType, TypeBindings,
+};
 use crate::{
     ast::{
         BlockExpression, FunctionKind, GenericTypeArgs, Ident, NoirFunction, NoirStruct, Param,
@@ -53,7 +55,7 @@ mod unquote;
 
 use fm::FileId;
 use iter_extended::vecmap;
-use noirc_errors::{Location, Span};
+use noirc_errors::{Location, Span, Spanned};
 use types::bind_ordered_generics;
 
 use self::traits::check_trait_impl_method_matches_declaration;
@@ -398,13 +400,36 @@ impl<'context> Elaborator<'context> {
 
         self.run_function_lints(&func_meta, &modifiers);
 
+        // Check arg and return-value visibility of standalone functions.
+        if self.should_check_function_visibility(&func_meta, &modifiers) {
+            let name = Ident(Spanned::from(
+                func_meta.name.location.span,
+                self.interner.definition_name(func_meta.name.id).to_string(),
+            ));
+            for (_, typ, _) in func_meta.parameters.iter() {
+                self.check_type_is_not_more_private_then_item(
+                    &name,
+                    modifiers.visibility,
+                    typ,
+                    name.span(),
+                );
+            }
+            self.check_type_is_not_more_private_then_item(
+                &name,
+                modifiers.visibility,
+                func_meta.return_type(),
+                name.span(),
+            );
+        }
+
         self.introduce_generics_into_scope(func_meta.all_generics.clone());
 
         // The DefinitionIds for each parameter were already created in define_function_meta
         // so we need to reintroduce the same IDs into scope here.
         for parameter in &func_meta.parameter_idents {
             let name = self.interner.definition_name(parameter.id).to_owned();
-            self.add_existing_variable_to_scope(name, parameter.clone(), true);
+            let warn_if_unused = !(func_meta.trait_impl.is_some() && name == "self");
+            self.add_existing_variable_to_scope(name, parameter.clone(), warn_if_unused);
         }
 
         self.add_trait_constraints_to_scope(&func_meta);
@@ -1279,14 +1304,49 @@ impl<'context> Elaborator<'context> {
         let typ = self.resolve_type(alias.type_alias_def.typ);
 
         if visibility != ItemVisibility::Private {
-            self.check_aliased_type_is_not_more_private(name, visibility, &typ, span);
+            self.check_type_is_not_more_private_then_item(name, visibility, &typ, span);
         }
 
         self.interner.set_type_alias(alias_id, typ, generics);
         self.generics.clear();
     }
 
-    fn check_aliased_type_is_not_more_private(
+    /// Find the struct in the parent module so we can know its visibility
+    fn find_struct_visibility(&self, struct_type: &StructType) -> Option<ItemVisibility> {
+        let parent_module_id = struct_type.id.parent_module_id(self.def_maps);
+        let parent_module_data = self.get_module(parent_module_id);
+        let per_ns = parent_module_data.find_name(&struct_type.name);
+        per_ns.types.map(|(_, vis, _)| vis)
+    }
+
+    /// Check whether a functions return value and args should be checked for private type visibility.
+    fn should_check_function_visibility(
+        &self,
+        func_meta: &FuncMeta,
+        modifiers: &FunctionModifiers,
+    ) -> bool {
+        // Private functions don't leak anything.
+        if modifiers.visibility == ItemVisibility::Private {
+            return false;
+        }
+        // Implementing public traits on private types is okay, they can't be used unless the type itself is accessible.
+        if func_meta.trait_impl.is_some() {
+            return false;
+        }
+        // Public struct functions should not expose private types.
+        if let Some(struct_visibility) = func_meta.struct_id.and_then(|id| {
+            let struct_def = self.get_struct(id);
+            let struct_def = struct_def.borrow();
+            self.find_struct_visibility(&struct_def)
+        }) {
+            return struct_visibility != ItemVisibility::Private;
+        }
+        // Standalone functions should be checked
+        true
+    }
+
+    /// Check that an item such as a struct field or type alias is not more visible than the type it refers to.
+    fn check_type_is_not_more_private_then_item(
         &mut self,
         name: &Ident,
         visibility: ItemVisibility,
@@ -1302,11 +1362,7 @@ impl<'context> Elaborator<'context> {
                 // then it's either accessible (all good) or it's not, in which case a different
                 // error will happen somewhere else, but no need to error again here.
                 if struct_module_id.krate == self.crate_id {
-                    // Find the struct in the parent module so we can know its visibility
-                    let parent_module_id = struct_type.id.parent_module_id(self.def_maps);
-                    let parent_module_data = self.get_module(parent_module_id);
-                    let per_ns = parent_module_data.find_name(&struct_type.name);
-                    if let Some((_, aliased_visibility, _)) = per_ns.types {
+                    if let Some(aliased_visibility) = self.find_struct_visibility(&struct_type) {
                         if aliased_visibility < visibility {
                             self.push_err(ResolverError::TypeIsMorePrivateThenItem {
                                 typ: struct_type.name.to_string(),
@@ -1318,16 +1374,16 @@ impl<'context> Elaborator<'context> {
                 }
 
                 for generic in generics {
-                    self.check_aliased_type_is_not_more_private(name, visibility, generic, span);
+                    self.check_type_is_not_more_private_then_item(name, visibility, generic, span);
                 }
             }
             Type::Tuple(types) => {
                 for typ in types {
-                    self.check_aliased_type_is_not_more_private(name, visibility, typ, span);
+                    self.check_type_is_not_more_private_then_item(name, visibility, typ, span);
                 }
             }
             Type::Alias(alias_type, generics) => {
-                self.check_aliased_type_is_not_more_private(
+                self.check_type_is_not_more_private_then_item(
                     name,
                     visibility,
                     &alias_type.borrow().get_type(generics),
@@ -1336,17 +1392,17 @@ impl<'context> Elaborator<'context> {
             }
             Type::Function(args, return_type, env, _) => {
                 for arg in args {
-                    self.check_aliased_type_is_not_more_private(name, visibility, arg, span);
+                    self.check_type_is_not_more_private_then_item(name, visibility, arg, span);
                 }
-                self.check_aliased_type_is_not_more_private(name, visibility, return_type, span);
-                self.check_aliased_type_is_not_more_private(name, visibility, env, span);
+                self.check_type_is_not_more_private_then_item(name, visibility, return_type, span);
+                self.check_type_is_not_more_private_then_item(name, visibility, env, span);
             }
             Type::MutableReference(typ) | Type::Array(_, typ) | Type::Slice(typ) => {
-                self.check_aliased_type_is_not_more_private(name, visibility, typ, span);
+                self.check_type_is_not_more_private_then_item(name, visibility, typ, span);
             }
             Type::InfixExpr(left, _op, right) => {
-                self.check_aliased_type_is_not_more_private(name, visibility, left, span);
-                self.check_aliased_type_is_not_more_private(name, visibility, right, span);
+                self.check_type_is_not_more_private_then_item(name, visibility, left, span);
+                self.check_type_is_not_more_private_then_item(name, visibility, right, span);
             }
             Type::FieldElement
             | Type::Integer(..)
@@ -1383,6 +1439,22 @@ impl<'context> Elaborator<'context> {
                 }
             }
 
+            // Check that the a public struct doesn't have a private type as a public field.
+            if typ.struct_def.visibility != ItemVisibility::Private {
+                for field in &fields {
+                    let ident = Ident(Spanned::from(
+                        field.name.span(),
+                        format!("{}::{}", typ.struct_def.name, field.name),
+                    ));
+                    self.check_type_is_not_more_private_then_item(
+                        &ident,
+                        field.visibility,
+                        &field.typ,
+                        field.name.span(),
+                    );
+                }
+            }
+
             let fields_len = fields.len();
             self.interner.update_struct(*type_id, |struct_def| {
                 struct_def.set_fields(fields);

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

@@ -226,6 +226,10 @@ pub enum InterpreterError {
         location: Location,
         expression: String,
     },
+    UnknownArrayLength {
+        length: Type,
+        location: Location,
+    },
 
     // These cases are not errors, they are just used to prevent us from running more code
     // until the loop can be resumed properly. These cases will never be displayed to users.
@@ -299,7 +303,8 @@ impl InterpreterError {
             | InterpreterError::DuplicateGeneric { duplicate_location: location, .. }
             | InterpreterError::TypeAnnotationsNeededForMethodCall { location }
             | InterpreterError::CannotResolveExpression { location, .. }
-            | InterpreterError::CannotSetFunctionBody { location, .. } => *location,
+            | InterpreterError::CannotSetFunctionBody { location, .. }
+            | InterpreterError::UnknownArrayLength { location, .. } => *location,
 
             InterpreterError::FailedToParseMacro { error, file, .. } => {
                 Location::new(error.span(), *file)
@@ -635,6 +640,10 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic {
                 let msg = format!("`{expression}` is not a valid function body");
                 CustomDiagnostic::simple_error(msg, String::new(), location.span)
             }
+            InterpreterError::UnknownArrayLength { length, location } => {
+                let msg = format!("Could not determine array length `{length}`");
+                CustomDiagnostic::simple_error(msg, String::new(), location.span)
+            }
         }
     }
 }