diff --git a/tooling/ast_fuzzer/fuzz/src/targets/acir_vs_brillig.rs b/tooling/ast_fuzzer/fuzz/src/targets/acir_vs_brillig.rs index 3720e6419ad..08b89a79a00 100644 --- a/tooling/ast_fuzzer/fuzz/src/targets/acir_vs_brillig.rs +++ b/tooling/ast_fuzzer/fuzz/src/targets/acir_vs_brillig.rs @@ -5,11 +5,14 @@ use crate::{compare_results_compiled, compile_into_circuit_or_die, default_ssa_o use arbitrary::Arbitrary; use arbitrary::Unstructured; use color_eyre::eyre; +use noir_ast_fuzzer::Config; use noir_ast_fuzzer::compare::{CompareOptions, ComparePipelines}; use noir_ast_fuzzer::rewrite::change_all_functions_into_unconstrained; pub fn fuzz(u: &mut Unstructured) -> eyre::Result<()> { - let config = default_config(u)?; + // This target asserts that the ACIR and Brillig builds of a program agree, which + // `#[no_predicates]` breaks by design: see [`Config::avoid_no_predicates`]. + let config = Config { avoid_no_predicates: true, ..default_config(u)? }; let inputs = ComparePipelines::arb( u, diff --git a/tooling/ast_fuzzer/fuzz/src/targets/min_vs_full.rs b/tooling/ast_fuzzer/fuzz/src/targets/min_vs_full.rs index 3fb2e16f726..309470d2ed0 100644 --- a/tooling/ast_fuzzer/fuzz/src/targets/min_vs_full.rs +++ b/tooling/ast_fuzzer/fuzz/src/targets/min_vs_full.rs @@ -9,12 +9,16 @@ use crate::{ use arbitrary::{Arbitrary, Unstructured}; use color_eyre::eyre; use noir_ast_fuzzer::compare::{CompareOptions, ComparePipelines}; -use noir_ast_fuzzer::{compare::CompareResult, rewrite::change_all_functions_into_unconstrained}; +use noir_ast_fuzzer::{ + Config, compare::CompareResult, rewrite::change_all_functions_into_unconstrained, +}; use noirc_evaluator::ssa::minimal_passes; pub fn fuzz(u: &mut Unstructured) -> eyre::Result<()> { let passes = minimal_passes(); - let config = default_config(u)?; + // This target asserts that the minimally- and fully-compiled builds of a program agree, + // which `#[no_predicates]` breaks by design: see [`Config::avoid_no_predicates`]. + let config = Config { avoid_no_predicates: true, ..default_config(u)? }; let inputs = ComparePipelines::arb( u, diff --git a/tooling/ast_fuzzer/fuzz/src/targets/orig_vs_morph.rs b/tooling/ast_fuzzer/fuzz/src/targets/orig_vs_morph.rs index 22a7e2ff6cf..caf3b0428fd 100644 --- a/tooling/ast_fuzzer/fuzz/src/targets/orig_vs_morph.rs +++ b/tooling/ast_fuzzer/fuzz/src/targets/orig_vs_morph.rs @@ -7,6 +7,7 @@ use crate::targets::default_config; use crate::{compare_results_compiled, compile_into_circuit_or_die, default_ssa_options}; use arbitrary::{Arbitrary, Unstructured}; use color_eyre::eyre; +use noir_ast_fuzzer::Config; use noir_ast_fuzzer::compare::{CompareMorph, CompareOptions}; use noir_ast_fuzzer::rewrite; use noir_ast_fuzzer::scope::ScopeStack; @@ -17,7 +18,10 @@ use noirc_frontend::monomorphization::ast::{ use noirc_frontend::monomorphization::visitor::{visit_expr, visit_expr_be_mut}; pub fn fuzz(u: &mut Unstructured) -> eyre::Result<()> { - let config = default_config(u)?; + // This target asserts that a program and its value-preserving morph agree, but a morph + // may change the predicate structure around a call, which changes what a + // `#[no_predicates]` callee executes by design: see [`Config::avoid_no_predicates`]. + let config = Config { avoid_no_predicates: true, ..default_config(u)? }; let rules = rules::collect(&config); let max_rewrites = 10; let inputs = CompareMorph::arb( diff --git a/tooling/ast_fuzzer/src/lib.rs b/tooling/ast_fuzzer/src/lib.rs index e954aa9137d..5fdf84fef81 100644 --- a/tooling/ast_fuzzer/src/lib.rs +++ b/tooling/ast_fuzzer/src/lib.rs @@ -95,6 +95,32 @@ pub struct Config { pub avoid_match: bool, /// Avoid using the vector type. pub avoid_vectors: bool, + /// Avoid calling a constrained function from another constrained function. + /// + /// `main` has the lowest function ID, and the rule that keeps the constrained call graph + /// acyclic only lets a function call lower IDs, so without an exception for `main` no + /// constrained function other than `main` is reachable and every one of them is deleted + /// as unreachable. Lifting that makes the whole constrained call graph — ACIR calling + /// ACIR, `#[fold]`, `#[no_predicates]` — reachable, which is a region of the compiler + /// nothing else exercises. + pub avoid_constrained_calls: bool, + /// Avoid marking functions with `#[fold]`. + /// + /// A `#[fold]` function is compiled into its own ACIR circuit, which is a backend path + /// reached through `Opcode::Call` with its own argument marshalling and predicate + /// handling at the boundary. It only means anything for a constrained function that is + /// actually called, so it depends on [`Config::avoid_constrained_calls`] being off. + pub avoid_fold: bool, + /// Avoid marking functions with `#[no_predicates]`. + /// + /// The attribute inlines the callee's body only after the flattening pass, so the body + /// runs unpredicated: a call sitting in an untaken branch really executes in ACIR while + /// in Brillig it does not. That is the attribute's documented behavior, but it means a + /// target that asserts two builds of the same program agree (ACIR vs Brillig, or two + /// predicate structures the morph is entitled to change) reports a false positive for + /// any program whose `#[no_predicates]` function is fallible or side-effecting. Such + /// targets set this flag; targets that compare a pass against its own input keep it off. + pub avoid_no_predicates: bool, /// Only use comptime friendly expressions. pub comptime_friendly: bool, } @@ -167,6 +193,9 @@ impl Default for Config { avoid_constrain: false, avoid_match: false, avoid_vectors: false, + avoid_constrained_calls: false, + avoid_fold: false, + avoid_no_predicates: false, comptime_friendly: false, } } diff --git a/tooling/ast_fuzzer/src/program/func.rs b/tooling/ast_fuzzer/src/program/func.rs index e20191855b2..427d19cd594 100644 --- a/tooling/ast_fuzzer/src/program/func.rs +++ b/tooling/ast_fuzzer/src/program/func.rs @@ -77,6 +77,7 @@ pub(super) fn can_call( caller_returns_ref: bool, callee_id: FuncId, callee_decl: &FunctionDeclaration, + allow_constrained_calls: bool, ) -> bool { // Nobody should call `main`. if callee_id == Program::main_id() { @@ -87,8 +88,10 @@ pub(super) fn can_call( // Since the `limit` module currently inserts an `if ctx_limit == 0`, // returning a literal, it would violate this if the return has `&mut`, // therefore we don't make recursive calls from such functions, so the - // limit strategy is not applied to them. - if caller_returns_ref && caller_unconstrained { + // limit strategy is not applied to them. This holds whichever runtime the + // caller is in: `limit` keys the rewrite off whether the body makes a call, + // not off `unconstrained`. + if caller_returns_ref { return false; } @@ -108,6 +111,23 @@ pub(super) fn can_call( // recursion by only calling functions with lower IDs, // otherwise the inliner could get stuck. if !callee_decl.unconstrained { + // Flattening cannot keep a reference that crosses a constrained call boundary inside + // the `enable_side_effects` region it belongs to, and the SSA fails to validate after + // the pass. Vectors are excluded for the same reason they are between ACIR and + // Brillig: they are not a shape a constrained call passes cleanly. + if callee_decl.has_refs() || callee_decl.returns_vectors() { + return false; + } + + // `main` is the exception to the ordering rule below: it has the lowest ID, so + // the rule would forbid it from calling any ACIR function, and since nothing can + // call `main` and Brillig cannot call ACIR, no constrained function other than + // `main` would be reachable — `remove_unreachable_functions` would delete the + // whole constrained call graph. Nothing ever calls `main`, so letting it call any + // ACIR function cannot close a cycle. + if caller_id == Program::main_id() { + return allow_constrained_calls; + } // Higher calls lower, so we can use this rule to pick function parameters // as we create the declarations: we can pass functions already declared. return callee_id < caller_id; @@ -228,7 +248,14 @@ impl<'a> FunctionContext<'a> { // Consider calling any allowed global function. for (callee_id, callee_decl) in &ctx.function_declarations { - if !can_call(id, decl.unconstrained, decl.returns_refs(), *callee_id, callee_decl) { + if !can_call( + id, + decl.unconstrained, + decl.returns_refs(), + *callee_id, + callee_decl, + !ctx.config.avoid_constrained_calls, + ) { continue; } let produces = types::types_produced(&callee_decl.return_type); @@ -773,22 +800,13 @@ impl<'a> FunctionContext<'a> { // // The conversion returns a value that shares the string's storage, so the ownership // pass has to keep them apart; noir-claude#1201 was exactly a missing clone here. - (Type::String(len), Type::Array(tgt_len, item_type)) - if *len == *tgt_len - && matches!( - item_type.as_ref(), - Type::Integer(Signedness::Unsigned, IntegerBitSize::Eight) - ) => - { - let expr = self.call_str_as_bytes(src_expr, *len, tgt_type.clone()); - Ok(Some((expr, src_dyn))) - } - // Reinterpret a string as its byte array with `str_as_bytes`. // - // The conversion returns a value that shares the string's storage, so the ownership - // pass has to keep them apart; noir-claude#1201 was exactly a missing clone here. + // It prints as the method call `s.as_bytes()`, which only resolves against the + // standard library, so it is not available to the comptime targets: those + // elaborate the printed snippet on its own. (Type::String(len), Type::Array(tgt_len, item_type)) if *len == *tgt_len + && !self.config().comptime_friendly && matches!( item_type.as_ref(), Type::Integer(Signedness::Unsigned, IntegerBitSize::Eight) @@ -801,8 +819,13 @@ impl<'a> FunctionContext<'a> { // becomes dynamically sized in real Noir, and it is what promotes the value into // the runtime memory-block representation that vector intrinsics and the // reference-counting machinery operate on. + // + // Like `str_as_bytes` it prints as a method call, so it is unavailable to the + // comptime targets. (Type::Array(_, item_type), Type::Vector(tgt_item)) - if item_type == tgt_item && !self.in_no_dynamic => + if item_type == tgt_item + && !self.in_no_dynamic + && !self.config().comptime_friendly => { let expr = self.call_as_vector(src_expr, src_type.clone(), tgt_type.clone()); Ok(Some((expr, src_dyn))) @@ -1557,9 +1580,18 @@ impl<'a> FunctionContext<'a> { .current() .variables() .filter_map(|(id, (_, _, typ))| types::is_printable(typ).then_some((*id, typ.clone()))) - // TODO(#10499): comptime function representations are at the moment just "(function)" - // (disable printing functions if comptime_friendly is on) - .filter(|(_, typ)| !types::is_function(typ) || !self.config().comptime_friendly) + .filter(|(_, typ)| { + !types::is_function(typ) + // TODO(#10499): comptime function representations are at the moment + // just "(function)". + || (!self.config().comptime_friendly + // Only unconstrained code may call the print oracle directly. A + // constrained print is retargeted at a wrapper function by + // `wrap_oracle_prints_in_functions`, which cannot wrap a function + // value: it is passed as a tuple that flattens into several SSA + // values, and those would not match the wrapper's one parameter. + && self.unconstrained()) + }) .collect::>(); if opts.is_empty() { @@ -2421,7 +2453,10 @@ impl<'a> FunctionContext<'a> { .iter() .skip(1) // Can't call main. .filter_map(|(func_id, func)| { - let matches = func.return_type == *return_type.as_ref() + // `#[fold]` functions are not eligible as function values; see the candidate + // filter in `Context::gen_function_decl`. + let matches = func.inline_type != InlineType::Fold + && func.return_type == *return_type.as_ref() && func.unconstrained == *unconstrained && func.params.len() == param_types.len() && func diff --git a/tooling/ast_fuzzer/src/program/mod.rs b/tooling/ast_fuzzer/src/program/mod.rs index a87ddee03fb..483008f26aa 100644 --- a/tooling/ast_fuzzer/src/program/mod.rs +++ b/tooling/ast_fuzzer/src/program/mod.rs @@ -256,13 +256,18 @@ impl Context { self.function_declarations .iter() .filter_map(|(callee_id, callee)| { - can_call( - id, - unconstrained, - types::contains_reference(&return_type), - *callee_id, - callee, - ) + // A `#[fold]` function receives the recursion limit by value while every + // other function receives it by mutable reference, so its signature does + // not match the one a function pointer of that shape is rewritten to. + (callee.inline_type != InlineType::Fold + && can_call( + id, + unconstrained, + types::contains_reference(&return_type), + *callee_id, + callee, + !self.config.avoid_constrained_calls, + )) .then_some(*callee_id) }) .collect() @@ -328,13 +333,25 @@ impl Context { let inline_type = if is_main { InlineType::default() } else { + // A `#[fold]` function compiles into a separate ACIR circuit, so its signature + // has to cross a circuit boundary: `acir_gen` builds its parameters with + // `create_value_from_type`, which only understands numbers and arrays. + let can_be_folded = !self.config.avoid_fold + && types::can_be_main(&return_type) + && params.iter().all(|(_, _, _, typ, _)| types::can_be_main(typ)); + // Automatically include any new inline type, except where the compiler does not // support it: `#[fold]` compiles the function into a separate ACIR circuit, which // has no meaning for an unconstrained function, and `#[no_predicates]` acts on the // flattening pass that unconstrained code does not run. let choices = InlineType::iter() .filter(|it| { - !(unconstrained && matches!(it, InlineType::Fold | InlineType::NoPredicates)) + if unconstrained { + !matches!(it, InlineType::Fold | InlineType::NoPredicates) + } else { + (*it != InlineType::Fold || can_be_folded) + && (*it != InlineType::NoPredicates || !self.config.avoid_no_predicates) + } }) .collect::>(); *u.choose(&choices)? @@ -392,7 +409,12 @@ impl Context { return_visibility: decl.return_visibility, unconstrained: decl.unconstrained, inline_type: decl.inline_type, - is_entry_point: id == FuncId(0), // we only need main as an entry point + // `main`, plus every constrained function compiled into its own ACIR circuit. + // `acir_gen` emits one ACIR per entry point and `combine_artifacts` asserts that + // count against the entry points, so a `#[fold]` function has to be one. + // Mirrors `Monomorphizer::into_program`. + is_entry_point: id == Program::main_id() + || (!decl.unconstrained && decl.inline_type.is_entry_point()), allow_constant_return: false, }; self.functions.insert(id, func); diff --git a/tooling/ast_fuzzer/src/program/rewrite/limit.rs b/tooling/ast_fuzzer/src/program/rewrite/limit.rs index eb22ee3b43a..253f966df0a 100644 --- a/tooling/ast_fuzzer/src/program/rewrite/limit.rs +++ b/tooling/ast_fuzzer/src/program/rewrite/limit.rs @@ -9,7 +9,8 @@ use noirc_frontend::{ ast::BinaryOpKind, monomorphization::{ ast::{ - Call, Definition, Expression, FuncId, Function, Ident, IdentId, LocalId, Program, Type, + Call, Definition, Expression, FuncId, Function, Ident, IdentId, InlineType, LocalId, + Program, Type, }, visitor::visit_expr_mut, }, @@ -40,6 +41,15 @@ pub(crate) fn add_recursion_limit( ctx: &mut Context, u: &mut Unstructured, ) -> arbitrary::Result<()> { + // A `#[fold]` function is compiled into its own ACIR circuit, so its signature has to + // cross a circuit boundary and cannot contain a reference; it takes the limit by value. + let fold_functions = ctx + .functions + .iter() + .filter(|(_, func)| func.inline_type == InlineType::Fold) + .map(|(id, _)| *id) + .collect::>(); + // Collect functions potentially called from ACIR; they will need proxy functions. let called_from_acir = ctx.functions.values().filter(|func| !func.unconstrained).fold( HashSet::::new(), @@ -70,7 +80,7 @@ pub(crate) fn add_recursion_limit( // Rewrite functions. for (func_id, func) in &mut ctx.functions { - let mut limit_ctx = LimitContext::new(*func_id, func, &ctx.config); + let mut limit_ctx = LimitContext::new(*func_id, func, &ctx.config, &fold_functions); limit_ctx.rewrite_functions(u, &mut proxy_functions)?; } @@ -103,11 +113,21 @@ struct LimitContext<'a, 'b> { is_recursive: bool, next_local_id: u32, next_ident_id: u32, + /// Whether this function holds the limit by value rather than by mutable reference. + is_fold: bool, + /// Functions which hold the limit by value, so calls to them pass it that way. + fold_functions: &'b HashSet, } impl<'a, 'b> LimitContext<'a, 'b> { - fn new(func_id: FuncId, func: &'a mut Function, config: &'b Config) -> Self { + fn new( + func_id: FuncId, + func: &'a mut Function, + config: &'b Config, + fold_functions: &'b HashSet, + ) -> Self { let is_main = func_id == Program::main_id(); + let is_fold = func.inline_type == InlineType::Fold; // Recursive functions are those that call another function. let is_recursive = expr::has_call(&func.body); @@ -121,7 +141,17 @@ impl<'a, 'b> LimitContext<'a, 'b> { // traverse the AST to figure out what the next ID to use is. let (next_local_id, next_ident_id) = next_local_and_ident_id(func); - Self { func_id, func, config, is_main, is_recursive, next_local_id, next_ident_id } + Self { + func_id, + func, + config, + is_main, + is_recursive, + next_local_id, + next_ident_id, + is_fold, + fold_functions, + } } /// Rewrite the function and its proxy (if it has one). @@ -186,10 +216,16 @@ impl<'a, 'b> LimitContext<'a, 'b> { ) -> arbitrary::Result<()> { let limit_var = VariableId::Local(limit_id); - let limit_type = Rc::new(types::ref_mut(types::U32)); + // A `#[fold]` function takes the limit by value, because it is compiled into its own + // ACIR circuit and a reference cannot cross that boundary. It therefore cannot + // decrease its caller's budget, which is sound because the constrained call graph is + // acyclic — see `can_call` — so a constrained function can never recurse. + let by_value = self.is_fold; + let limit_type = Rc::new(if by_value { types::U32 } else { types::ref_mut(types::U32) }); + self.func.parameters.push(( limit_id, - false, + by_value, LIMIT_NAME.to_string(), limit_type.clone(), Visibility::Private, @@ -201,26 +237,32 @@ impl<'a, 'b> LimitContext<'a, 'b> { let limit_ident = expr::ident_inner( limit_var, self.next_ident_id(), - false, + by_value, LIMIT_NAME.to_string(), limit_type, ); let limit_expr = Expression::Ident(limit_ident.clone()); + // Reading the limit goes through a dereference unless we hold it by value. + let read_limit = + |expr: Expression| if by_value { expr } else { expr::deref(expr, types::U32) }; + expr::replace(&mut self.func.body, |mut body| { + let decreased = expr::binary( + read_limit(limit_expr.clone()), + BinaryOpKind::Subtract, + expr::u32_literal(1), + ); expr::prepend( &mut body, - expr::assign_ref( - limit_ident, - expr::binary( - expr::deref(limit_expr.clone(), types::U32), - BinaryOpKind::Subtract, - expr::u32_literal(1), - ), - ), + if by_value { + expr::assign_ident(limit_ident, decreased) + } else { + expr::assign_ref(limit_ident, decreased) + }, ); expr::if_else( - expr::equal(expr::deref(limit_expr.clone(), types::U32), expr::u32_literal(0)), + expr::equal(read_limit(limit_expr.clone()), expr::u32_literal(0)), default_return, body, self.func.return_type.clone(), @@ -234,7 +276,8 @@ impl<'a, 'b> LimitContext<'a, 'b> { /// In non-main we look at the limit and return a random value if it's zero, /// otherwise decrease it by one and continue with the original body. fn modify_body_when_non_recursive(&mut self, limit_id: LocalId) { - let limit_type = types::ref_mut(types::U32); + // See `modify_body_when_recursive` for why a `#[fold]` function takes it by value. + let limit_type = if self.is_fold { types::U32 } else { types::ref_mut(types::U32) }; self.func.parameters.push(( limit_id, false, @@ -341,6 +384,14 @@ impl<'a, 'b> LimitContext<'a, 'b> { other => unreachable!("unexpected call target definition: {}", other), }; + let callee_is_fold = match &ident.definition { + Definition::Function(id) => self.fold_functions.contains(id), + _ => false, + }; + // `main` keeps the limit in a local, and a `#[fold]` function receives it as + // a by-value parameter; everyone else holds a mutable reference to it. + let holds_by_value = self.is_main || self.is_fold; + types::unref_mut_rc(&mut ident.typ, |unref_mut_typ| { let Type::Function(mut param_types, ret, env, callee_unconstrained) = unref_mut_typ @@ -348,14 +399,14 @@ impl<'a, 'b> LimitContext<'a, 'b> { unreachable!("function type expected"); }; - if callee_unconstrained && !self.func.unconstrained { + if (callee_unconstrained && !self.func.unconstrained) || callee_is_fold { // Calling Brillig from ACIR: call the proxy if it's global. if let Some(proxy) = proxy { ident.name = proxy.name.clone(); ident.definition = Definition::Function(proxy.id); } // Pass the limit by value. - let limit_expr = if self.is_main { + let limit_expr = if holds_by_value { expr::ident( limit_var, self.next_ident_id(), @@ -380,8 +431,8 @@ impl<'a, 'b> LimitContext<'a, 'b> { } else { // Pass the limit by reference. let limit_type = types::ref_mut(types::U32); - let limit_expr = if self.is_main { - // In main we take a mutable reference to the limit. + let limit_expr = if holds_by_value { + // When we hold the limit itself, take a mutable reference to it. expr::ref_mut( expr::ident( limit_var, diff --git a/tooling/ast_fuzzer/src/program/rewrite/mod.rs b/tooling/ast_fuzzer/src/program/rewrite/mod.rs index 1be9babb1ab..c2d58e157f3 100644 --- a/tooling/ast_fuzzer/src/program/rewrite/mod.rs +++ b/tooling/ast_fuzzer/src/program/rewrite/mod.rs @@ -68,6 +68,10 @@ pub fn change_all_functions_into_unconstrained(mut program: Program) -> Program } // Modify the function. f.unconstrained = true; + // Only a constrained function is compiled into its own ACIR circuit, so an + // unconstrained one is an entry point only if it is `main`. This mirrors the + // `force_unconstrained` arm of `Monomorphizer::into_program`. + f.is_entry_point = f.id == Program::main_id(); // Modify any function pointers it takes. for (_, _, _, typ, _) in &mut f.parameters { types::unref_mut_rc(typ, |unref_mut_typ| { diff --git a/tooling/ast_fuzzer/src/program/scope.rs b/tooling/ast_fuzzer/src/program/scope.rs index b4cd4eb8b97..5eec7e724e8 100644 --- a/tooling/ast_fuzzer/src/program/scope.rs +++ b/tooling/ast_fuzzer/src/program/scope.rs @@ -230,9 +230,12 @@ mod tests { let scope1 = &stack.0[1]; assert_eq!(scope0.variable_ids().len(), 1); - assert_eq!(scope0.types_produced().len(), 5 + 2); // What we see plus upcasts from u32 to u64 and u128 + // What we see (the tuple, `Field`, `bool`, `[u32; 4]` and `u32`), plus upcasts from + // u32 to u64 and u128, plus the `[u32]` the array converts into. + assert_eq!(scope0.types_produced().len(), 5 + 2 + 1); assert_eq!(scope1.variable_ids().len(), 2); - assert_eq!(scope1.types_produced().len(), 5 + 2 + 1); + // The above plus `str<10>` and the `[u8; 10]` it converts into. + assert_eq!(scope1.types_produced().len(), 5 + 2 + 1 + 2); stack.exit(); assert_eq!(stack.0.len(), 1); diff --git a/tooling/ast_fuzzer/src/program/tests.rs b/tooling/ast_fuzzer/src/program/tests.rs index 3c8ac20d559..b2eb4e6c31a 100644 --- a/tooling/ast_fuzzer/src/program/tests.rs +++ b/tooling/ast_fuzzer/src/program/tests.rs @@ -454,3 +454,135 @@ fn test_generates_non_homogeneous_array_types() { ACIR's non-homogeneous array handling is unreachable from the fuzzer" ); } + +/// A `FunctionDeclaration` with no parameters, returning `Field`. +fn simple_decl(unconstrained: bool, inline_type: InlineType) -> FunctionDeclaration { + FunctionDeclaration { + name: "func".to_string(), + params: vec![], + return_type: Type::Field, + return_visibility: Visibility::Private, + inline_type, + unconstrained, + } +} + +#[test] +fn test_main_can_call_constrained_functions() { + use super::func::can_call; + + let acir = simple_decl(false, InlineType::Inline); + + // `main` has the lowest ID, so the "higher calls lower" rule alone would leave every + // constrained function unreachable and `remove_unreachable_functions` would delete them. + assert!( + can_call(Program::main_id(), false, false, FuncId(1), &acir, true), + "main should be able to call a constrained function" + ); + assert!( + !can_call(Program::main_id(), false, false, FuncId(1), &acir, false), + "the exception is off unless constrained calls are allowed" + ); + + // Between other constrained functions the ordering rule still keeps the graph acyclic. + assert!(can_call(FuncId(2), false, false, FuncId(1), &acir, true)); + assert!(!can_call(FuncId(1), false, false, FuncId(2), &acir, true)); + + // Nothing calls main, which is what makes the exception above safe. + assert!(!can_call(FuncId(1), false, false, Program::main_id(), &acir, true)); + + // Brillig still only calls Brillig. + assert!(!can_call(FuncId(1), true, false, FuncId(2), &acir, true)); +} + +#[test] +fn test_functions_returning_references_do_not_call() { + use super::func::can_call; + + let acir = simple_decl(false, InlineType::Inline); + let brillig = simple_decl(true, InlineType::Inline); + + // The recursion limit rewrite wraps the body of any function that makes a call in an + // `if ctx_limit == 0 { .. } else { .. }`, and an `if` cannot return a reference in ACIR. + // The rewrite keys off whether the body calls anything, not off the runtime. + assert!(!can_call(Program::main_id(), false, true, FuncId(1), &acir, true)); + assert!(!can_call(FuncId(2), true, true, FuncId(1), &brillig, true)); +} + +#[test] +fn test_generates_constrained_functions_other_than_main() { + let config = Config { avoid_constrained_calls: false, ..Config::default() }; + + // A deterministic byte source; the generator only needs entropy, not randomness. + let mut state = 0x2545_F491_4F6C_DD1Du64; + let mut data = vec![0u8; 1 << 20]; + for byte in &mut data { + state ^= state << 13; + state ^= state >> 7; + state ^= state << 17; + *byte = state as u8; + } + let mut u = Unstructured::new(&data); + + let mut found = 0; + for _ in 0..32 { + let Ok(program) = crate::arb_program(&mut u, config.clone()) else { break }; + found += program + .functions + .iter() + .filter(|f| !f.unconstrained && f.id != Program::main_id()) + .count(); + } + + assert!( + found > 0, + "every generated program consisted of `main` plus Brillig functions; \ + the whole constrained call graph is unreachable" + ); +} + +#[test] +fn test_avoid_no_predicates_generates_none() { + let config = Config { avoid_no_predicates: true, ..Config::default() }; + + // A deterministic byte source; the generator only needs entropy, not randomness. + let mut state = 0x2545_F491_4F6C_DD1Du64; + let mut data = vec![0u8; 1 << 20]; + for byte in &mut data { + state ^= state << 13; + state ^= state >> 7; + state ^= state << 17; + *byte = state as u8; + } + let mut u = Unstructured::new(&data); + + for _ in 0..32 { + let Ok(program) = crate::arb_program(&mut u, config.clone()) else { break }; + for func in &program.functions { + assert!( + !matches!(func.inline_type, InlineType::NoPredicates), + "function {} is `#[no_predicates]` despite `avoid_no_predicates`", + func.name + ); + } + } +} + +#[test] +fn test_types_produced_covers_the_builtin_conversions() { + use super::types::{U8, types_produced}; + + // `str_as_bytes` + let produced = types_produced(&Type::String(3)); + assert!( + produced.contains(&Type::Array(3, Rc::new(U8))), + "a string should offer the byte array it converts into: {produced:?}" + ); + + // `as_vector` + let produced = types_produced(&Type::Array(4, Rc::new(Type::Field))); + assert!( + produced.contains(&Type::Vector(Rc::new(Type::Field))), + "an array should offer the vector it converts into: {produced:?}" + ); +} diff --git a/tooling/ast_fuzzer/src/program/types.rs b/tooling/ast_fuzzer/src/program/types.rs index 0a31b077e9e..a0e523818a3 100644 --- a/tooling/ast_fuzzer/src/program/types.rs +++ b/tooling/ast_fuzzer/src/program/types.rs @@ -54,6 +54,12 @@ pub fn can_be_matched(typ: &Type) -> bool { /// Collect all the sub-types produced by a type. /// /// It's like a _power set_ of the type. +/// +/// This is what the producer index in [`Scope`](super::scope::Scope) is built from, so it +/// decides which variables are offered for a target type. It should stay in step with +/// `FunctionContext::gen_expr_from_source`: a type listed here that the generator cannot +/// actually reach costs coverage, because the chosen producer yields nothing and the +/// expression falls back to a literal. pub fn types_produced(typ: &Type) -> HashSet { /// Recursively visit subtypes. fn visit(acc: &mut HashSet, typ: &Type) { @@ -66,6 +72,10 @@ pub fn types_produced(typ: &Type) -> HashSet { match typ { Type::Array(len, item_type) => { + // `as_vector` turns a fixed-size array into a dynamically sized one. + // Only the vector itself is produced, not what the vector in turn + // produces, because that is as far as a single conversion gets us. + acc.insert(Type::Vector(item_type.clone())); if *len > 0 { visit(acc, item_type); } @@ -82,8 +92,11 @@ pub fn types_produced(typ: &Type) -> HashSet { visit(acc, item_type); } } - Type::String(_) => { - // Maybe it could produce substrings, but it would be an overkill to enumerate. + Type::String(len) => { + // `str_as_bytes` reinterprets the string as the array of its bytes. + // Maybe it could also produce substrings, but it would be an overkill + // to enumerate. + acc.insert(Type::Array(*len, Rc::new(U8))); } Type::Field => { // There are `try_to_*` methods, but let's consider only what is safe.