Merge branch 'master' into aztec-packages

* master:
  fix: Allow array map on empty arrays (#6305)
  fix: Display function name and body when inlining recursion limit hit (#6291)
  feat(interpreter): Comptime derive generators (#6303)
  fix: enforce correctness of decompositions performed at compile time (#6278)
  feat: Warn about private types leaking in public functions and struct fields (#6296)
  chore(docs): refactoring guides and some other nits (#6175)

.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

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> {

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();
+        assert_eq!(inlined.functions.len(), 0);
+    }
+
     #[test]
     fn inliner_disabled() {
         // brillig fn foo {

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,6 +400,28 @@ 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
@@ -1280,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,
@@ -1303,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(),
@@ -1319,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),
@@ -1337,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(..)
@@ -1384,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);

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)
+            }
         }
     }
 }

compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs

@@ -63,6 +63,9 @@ impl<'local, 'context> Interpreter<'local, 'context> {
             "as_slice" => as_slice(interner, arguments, location),
             "ctstring_eq" => ctstring_eq(arguments, location),
             "ctstring_hash" => ctstring_hash(arguments, location),
+            "derive_pedersen_generators" => {
+                derive_generators(interner, arguments, return_type, location)
+            }
             "expr_as_array" => expr_as_array(interner, arguments, return_type, location),
             "expr_as_assert" => expr_as_assert(interner, arguments, return_type, location),
             "expr_as_assert_eq" => expr_as_assert_eq(interner, arguments, return_type, location),
@@ -2793,3 +2796,56 @@ fn ctstring_eq(arguments: Vec<(Value, Location)>, location: Location) -> IResult
 fn ctstring_hash(arguments: Vec<(Value, Location)>, location: Location) -> IResult<Value> {
     hash_item(arguments, location, get_ctstring)
 }
+
+fn derive_generators(
+    interner: &mut NodeInterner,
+    arguments: Vec<(Value, Location)>,
+    return_type: Type,
+    location: Location,
+) -> IResult<Value> {
+    let (domain_separator_string, starting_index) = check_two_arguments(arguments, location)?;
+
+    let domain_separator_location = domain_separator_string.1;
+    let (domain_separator_string, _) = get_array(interner, domain_separator_string)?;
+    let starting_index = get_u32(starting_index)?;
+
+    let domain_separator_string =
+        try_vecmap(domain_separator_string, |byte| get_u8((byte, domain_separator_location)))?;
+
+    let (size, elements) = match return_type.clone() {
+        Type::Array(size, elements) => (size, elements),
+        _ => panic!("ICE: Should only have an array return type"),
+    };
+
+    let Some(num_generators) = size.evaluate_to_u32() else {
+        return Err(InterpreterError::UnknownArrayLength { length: *size, location });
+    };
+
+    let generators = bn254_blackbox_solver::derive_generators(
+        &domain_separator_string,
+        num_generators,
+        starting_index,
+    );
+
+    let is_infinite = FieldElement::zero();
+    let x_field_name: Rc<String> = Rc::new("x".to_owned());
+    let y_field_name: Rc<String> = Rc::new("y".to_owned());
+    let is_infinite_field_name: Rc<String> = Rc::new("is_infinite".to_owned());
+    let mut results = Vector::new();
+    for gen in generators {
+        let x_big: BigUint = gen.x.into();
+        let x = FieldElement::from_be_bytes_reduce(&x_big.to_bytes_be());
+        let y_big: BigUint = gen.y.into();
+        let y = FieldElement::from_be_bytes_reduce(&y_big.to_bytes_be());
+        let mut embedded_curve_point_fields = HashMap::default();
+        embedded_curve_point_fields.insert(x_field_name.clone(), Value::Field(x));
+        embedded_curve_point_fields.insert(y_field_name.clone(), Value::Field(y));
+        embedded_curve_point_fields
+            .insert(is_infinite_field_name.clone(), Value::Field(is_infinite));
+        let embedded_curve_point_struct =
+            Value::Struct(embedded_curve_point_fields, *elements.clone());
+        results.push_back(embedded_curve_point_struct);
+    }
+
+    Ok(Value::Array(results, return_type))
+}