diff --git a/.github/workflows/gates_report.yml b/.github/workflows/gates_report.yml index 3d4bef1940e..0cc94a1a04d 100644 --- a/.github/workflows/gates_report.yml +++ b/.github/workflows/gates_report.yml @@ -1,88 +1,94 @@ -# name: Report gates diff - -# on: -# push: -# branches: -# - master -# pull_request: - -# jobs: -# build-nargo: -# runs-on: ubuntu-latest -# strategy: -# matrix: -# target: [x86_64-unknown-linux-gnu] - -# steps: -# - name: Checkout Noir repo -# uses: actions/checkout@v4 - -# - name: Setup toolchain -# uses: dtolnay/rust-toolchain@1.74.1 - -# - uses: Swatinem/rust-cache@v2 -# with: -# key: ${{ matrix.target }} -# cache-on-failure: true -# save-if: ${{ github.event_name != 'merge_group' }} - -# - name: Build Nargo -# run: cargo build --package nargo_cli --release - -# - name: Package artifacts -# run: | -# mkdir dist -# cp ./target/release/nargo ./dist/nargo -# 7z a -ttar -so -an ./dist/* | 7z a -si ./nargo-x86_64-unknown-linux-gnu.tar.gz - -# - name: Upload artifact -# uses: actions/upload-artifact@v4 -# with: -# name: nargo -# path: ./dist/* -# retention-days: 3 - - -# compare_gas_reports: -# needs: [build-nargo] -# runs-on: ubuntu-latest -# permissions: -# pull-requests: write - -# steps: -# - uses: actions/checkout@v4 - -# - name: Download nargo binary -# uses: actions/download-artifact@v4 -# with: -# name: nargo -# path: ./nargo - -# - name: Set nargo on PATH -# run: | -# nargo_binary="${{ github.workspace }}/nargo/nargo" -# chmod +x $nargo_binary -# echo "$(dirname $nargo_binary)" >> $GITHUB_PATH -# export PATH="$PATH:$(dirname $nargo_binary)" -# nargo -V - -# - name: Generate gates report -# working-directory: ./test_programs -# run: | -# ./gates_report.sh -# mv gates_report.json ../gates_report.json +name: Report gates diff + +on: + push: + branches: + - master + pull_request: + +jobs: + build-nargo: + runs-on: ubuntu-latest + strategy: + matrix: + target: [x86_64-unknown-linux-gnu] + + steps: + - name: Checkout Noir repo + uses: actions/checkout@v4 + + - name: Setup toolchain + uses: dtolnay/rust-toolchain@1.74.1 + + - uses: Swatinem/rust-cache@v2 + with: + key: ${{ matrix.target }} + cache-on-failure: true + save-if: ${{ github.event_name != 'merge_group' }} + + - name: Build Nargo + run: cargo build --package nargo_cli --release + + - name: Package artifacts + run: | + mkdir dist + cp ./target/release/nargo ./dist/nargo + 7z a -ttar -so -an ./dist/* | 7z a -si ./nargo-x86_64-unknown-linux-gnu.tar.gz + + - name: Upload artifact + uses: actions/upload-artifact@v4 + with: + name: nargo + path: ./dist/* + retention-days: 3 + + + compare_gates_reports: + needs: [build-nargo] + runs-on: ubuntu-latest + permissions: + pull-requests: write + + steps: + - uses: actions/checkout@v4 + + - name: Install `bb` + run: | + ./scripts/install_bb.sh + echo "$HOME/.bb/" >> $GITHUB_PATH + + - name: Download nargo binary + uses: actions/download-artifact@v4 + with: + name: nargo + path: ./nargo + + - name: Set nargo on PATH + run: | + nargo_binary="${{ github.workspace }}/nargo/nargo" + chmod +x $nargo_binary + echo "$(dirname $nargo_binary)" >> $GITHUB_PATH + export PATH="$PATH:$(dirname $nargo_binary)" + nargo -V + + - name: Generate gates report + working-directory: ./test_programs + run: | + ./rebuild.sh + ./gates_report.sh + mv gates_report.json ../gates_report.json -# - name: Compare gates reports -# id: gates_diff -# uses: vezenovm/noir-gates-diff@acf12797860f237117e15c0d6e08d64253af52b6 -# with: -# report: gates_report.json -# summaryQuantile: 0.9 # only display the 10% most significant circuit size diffs in the summary (defaults to 20%) - -# - name: Add gates diff to sticky comment -# if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target' -# uses: marocchino/sticky-pull-request-comment@v2 -# with: -# # delete the comment in case changes no longer impact circuit sizes -# delete: ${{ !steps.gates_diff.outputs.markdown }} -# message: ${{ steps.gates_diff.outputs.markdown }} + - name: Compare gates reports + id: gates_diff + uses: vezenovm/noir-gates-diff@acf12797860f237117e15c0d6e08d64253af52b6 + with: + report: gates_report.json + summaryQuantile: 0.9 # only display the 10% most significant circuit size diffs in the summary (defaults to 20%) + + - name: Add gates diff to sticky comment + if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target' + uses: marocchino/sticky-pull-request-comment@v2 + with: + # delete the comment in case changes no longer impact circuit sizes + delete: ${{ !steps.gates_diff.outputs.markdown }} + message: ${{ steps.gates_diff.outputs.markdown }} diff --git a/acvm-repo/acvm/src/compiler/transformers/csat.rs b/acvm-repo/acvm/src/compiler/transformers/csat.rs index 9e2e3091c74..12a37e3e37a 100644 --- a/acvm-repo/acvm/src/compiler/transformers/csat.rs +++ b/acvm-repo/acvm/src/compiler/transformers/csat.rs @@ -210,16 +210,16 @@ impl CSatTransformer { } } else { // No more usable elements left in the old opcode - opcode.linear_combinations = remaining_linear_terms; break; } } + opcode.linear_combinations.extend(remaining_linear_terms); + // Constraint this intermediate_opcode to be equal to the temp variable by adding it into the IndexMap // We need a unique name for our intermediate variable // XXX: Another optimization, which could be applied in another algorithm // If two opcodes have a large fan-in/out and they share a few common terms, then we should create intermediate variables for them // Do some sort of subset matching algorithm for this on the terms of the polynomial - let inter_var = Self::get_or_create_intermediate_vars( intermediate_variables, intermediate_opcode, diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 69e5f6ddfcc..c2fe7878bf8 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -56,6 +56,7 @@ pub(crate) fn optimize_into_acir( .run_pass(Ssa::defunctionalize, "After Defunctionalization:") .run_pass(Ssa::remove_paired_rc, "After Removing Paired rc_inc & rc_decs:") .run_pass(Ssa::inline_functions, "After Inlining:") + .run_pass(Ssa::resolve_is_unconstrained, "After Resolving IsUnconstrained:") // Run mem2reg with the CFG separated into blocks .run_pass(Ssa::mem2reg, "After Mem2Reg:") .run_pass(Ssa::as_slice_optimization, "After `as_slice` optimization") diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index 90cdbb650c2..b6e88a8d4b0 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -614,13 +614,44 @@ impl AcirContext { expr.push_multiplication_term(FieldElement::one(), lhs_witness, rhs_witness); self.add_data(AcirVarData::Expr(expr)) } - ( - AcirVarData::Expr(_) | AcirVarData::Witness(_), - AcirVarData::Expr(_) | AcirVarData::Witness(_), - ) => { + (AcirVarData::Expr(expression), AcirVarData::Witness(witness)) + | (AcirVarData::Witness(witness), AcirVarData::Expr(expression)) + if expression.is_linear() => + { + let mut expr = Expression::default(); + for term in expression.linear_combinations.iter() { + expr.push_multiplication_term(term.0, term.1, witness); + } + expr.push_addition_term(expression.q_c, witness); + self.add_data(AcirVarData::Expr(expr)) + } + (AcirVarData::Expr(lhs_expr), AcirVarData::Expr(rhs_expr)) => { + let degree_one = if lhs_expr.is_linear() && rhs_expr.is_degree_one_univariate() { + Some((lhs_expr, rhs_expr)) + } else if rhs_expr.is_linear() && lhs_expr.is_degree_one_univariate() { + Some((rhs_expr, lhs_expr)) + } else { + None + }; + if let Some((lin, univariate)) = degree_one { + let mut expr = Expression::default(); + let rhs_term = univariate.linear_combinations[0]; + for term in lin.linear_combinations.iter() { + expr.push_multiplication_term(term.0 * rhs_term.0, term.1, rhs_term.1); + } + expr.push_addition_term(lin.q_c * rhs_term.0, rhs_term.1); + expr.sort(); + expr = expr.add_mul(univariate.q_c, &lin); + self.add_data(AcirVarData::Expr(expr)) + } else { + let lhs = self.get_or_create_witness_var(lhs)?; + let rhs = self.get_or_create_witness_var(rhs)?; + self.mul_var(lhs, rhs)? + } + } + _ => { let lhs = self.get_or_create_witness_var(lhs)?; let rhs = self.get_or_create_witness_var(rhs)?; - self.mul_var(lhs, rhs)? } }; diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index f136d3c5fb2..93ea703721f 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -65,6 +65,7 @@ pub(crate) enum Intrinsic { FromField, AsField, AsWitness, + IsUnconstrained, } impl std::fmt::Display for Intrinsic { @@ -89,6 +90,7 @@ impl std::fmt::Display for Intrinsic { Intrinsic::FromField => write!(f, "from_field"), Intrinsic::AsField => write!(f, "as_field"), Intrinsic::AsWitness => write!(f, "as_witness"), + Intrinsic::IsUnconstrained => write!(f, "is_unconstrained"), } } } @@ -116,7 +118,8 @@ impl Intrinsic { | Intrinsic::SliceRemove | Intrinsic::StrAsBytes | Intrinsic::FromField - | Intrinsic::AsField => false, + | Intrinsic::AsField + | Intrinsic::IsUnconstrained => false, // Some black box functions have side-effects Intrinsic::BlackBox(func) => matches!(func, BlackBoxFunc::RecursiveAggregation), @@ -145,6 +148,7 @@ impl Intrinsic { "from_field" => Some(Intrinsic::FromField), "as_field" => Some(Intrinsic::AsField), "as_witness" => Some(Intrinsic::AsWitness), + "is_unconstrained" => Some(Intrinsic::IsUnconstrained), other => BlackBoxFunc::lookup(other).map(Intrinsic::BlackBox), } } diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index 8e320de9337..8f57d9de368 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -294,6 +294,7 @@ pub(super) fn simplify_call( SimplifyResult::SimplifiedToInstruction(instruction) } Intrinsic::AsWitness => SimplifyResult::None, + Intrinsic::IsUnconstrained => SimplifyResult::None, } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/mod.rs b/compiler/noirc_evaluator/src/ssa/opt/mod.rs index 27536d59ea5..f6c3f022bfc 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/mod.rs @@ -17,5 +17,6 @@ mod rc; mod remove_bit_shifts; mod remove_enable_side_effects; mod remove_if_else; +mod resolve_is_unconstrained; mod simplify_cfg; mod unrolling; diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs index 9309652d508..464faa57323 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs @@ -158,7 +158,8 @@ impl Context { | Intrinsic::FromField | Intrinsic::AsField | Intrinsic::AsSlice - | Intrinsic::AsWitness => false, + | Intrinsic::AsWitness + | Intrinsic::IsUnconstrained => false, }, // We must assume that functions contain a side effect as we cannot inspect more deeply. diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs index 3d2b5142219..91b455dbf29 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs @@ -232,6 +232,7 @@ fn slice_capacity_change( | Intrinsic::BlackBox(_) | Intrinsic::FromField | Intrinsic::AsField - | Intrinsic::AsWitness => SizeChange::None, + | Intrinsic::AsWitness + | Intrinsic::IsUnconstrained => SizeChange::None, } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/resolve_is_unconstrained.rs b/compiler/noirc_evaluator/src/ssa/opt/resolve_is_unconstrained.rs new file mode 100644 index 00000000000..2c9e33ae528 --- /dev/null +++ b/compiler/noirc_evaluator/src/ssa/opt/resolve_is_unconstrained.rs @@ -0,0 +1,56 @@ +use crate::ssa::{ + ir::{ + function::{Function, RuntimeType}, + instruction::{Instruction, Intrinsic}, + types::Type, + value::Value, + }, + ssa_gen::Ssa, +}; +use acvm::FieldElement; +use fxhash::FxHashSet as HashSet; + +impl Ssa { + /// An SSA pass to find any calls to `Intrinsic::IsUnconstrained` and replacing any uses of the result of the intrinsic + /// with the resolved boolean value. + /// Note that this pass must run after the pass that does runtime separation, since in SSA generation an ACIR function can end up targeting brillig. + #[tracing::instrument(level = "trace", skip(self))] + pub(crate) fn resolve_is_unconstrained(mut self) -> Self { + for func in self.functions.values_mut() { + replace_is_unconstrained_result(func); + } + self + } +} + +fn replace_is_unconstrained_result(func: &mut Function) { + let mut is_unconstrained_calls = HashSet::default(); + // Collect all calls to is_unconstrained + for block_id in func.reachable_blocks() { + for &instruction_id in func.dfg[block_id].instructions() { + let target_func = match &func.dfg[instruction_id] { + Instruction::Call { func, .. } => *func, + _ => continue, + }; + + if let Value::Intrinsic(Intrinsic::IsUnconstrained) = &func.dfg[target_func] { + is_unconstrained_calls.insert(instruction_id); + } + } + } + + for instruction_id in is_unconstrained_calls { + let call_returns = func.dfg.instruction_results(instruction_id); + let original_return_id = call_returns[0]; + + // We replace the result with a fresh id. This will be unused, so the DIE pass will remove the leftover intrinsic call. + func.dfg.replace_result(instruction_id, original_return_id); + + let is_within_unconstrained = func.dfg.make_constant( + FieldElement::from(matches!(func.runtime(), RuntimeType::Brillig)), + Type::bool(), + ); + // Replace all uses of the original return value with the constant + func.dfg.set_value_from_id(original_return_id, is_within_unconstrained); + } +} diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index b33bca33225..3502565c264 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -194,7 +194,7 @@ impl<'context> Elaborator<'context> { let expr_id = self.interner.push_expr(ident); self.interner.push_expr_location(expr_id, call_expr_span, self.file); let ident = old_value.ident.clone(); - let typ = self.type_check_variable(ident, expr_id); + let typ = self.type_check_variable(ident, expr_id, None); self.interner.push_expr_type(expr_id, typ.clone()); capture_types.push(typ); fmt_str_idents.push(expr_id); @@ -291,16 +291,25 @@ impl<'context> Elaborator<'context> { Some(method_ref) => { // Automatically add `&mut` if the method expects a mutable reference and // the object is not already one. - if let HirMethodReference::FuncId(func_id) = &method_ref { - if *func_id != FuncId::dummy_id() { - let function_type = self.interner.function_meta(func_id).typ.clone(); - - self.try_add_mutable_reference_to_object( - &function_type, - &mut object_type, - &mut object, - ); + let func_id = match &method_ref { + HirMethodReference::FuncId(func_id) => *func_id, + HirMethodReference::TraitMethodId(method_id, _) => { + let id = self.interner.trait_method_id(*method_id); + let definition = self.interner.definition(id); + let DefinitionKind::Function(func_id) = definition.kind else { + unreachable!("Expected trait function to be a DefinitionKind::Function") + }; + func_id } + }; + + if func_id != FuncId::dummy_id() { + let function_type = self.interner.function_meta(&func_id).typ.clone(); + self.try_add_mutable_reference_to_object( + &function_type, + &mut object_type, + &mut object, + ); } // These arguments will be given to the desugared function call. @@ -322,6 +331,7 @@ impl<'context> Elaborator<'context> { let generics = method_call.generics.map(|option_inner| { option_inner.into_iter().map(|generic| self.resolve_type(generic)).collect() }); + let turbofish_generics = generics.clone(); let method_call = HirMethodCallExpression { method, object, arguments, location, generics }; @@ -335,7 +345,8 @@ impl<'context> Elaborator<'context> { self.interner, ); - let func_type = self.type_check_variable(function_name, function_id); + let func_type = + self.type_check_variable(function_name, function_id, turbofish_generics); // Type check the new call now that it has been changed from a method call // to a function call. This way we avoid duplicating code. diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index e91009de2fb..370c5c50764 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -308,8 +308,6 @@ impl<'context> Elaborator<'context> { self.scopes.start_function(); self.current_item = Some(DependencyId::Function(id)); - // Check whether the function has globals in the local module and add them to the scope - self.resolve_local_globals(); self.trait_bounds = function.def.where_clause.clone(); let is_low_level_or_oracle = function @@ -497,18 +495,6 @@ impl<'context> Elaborator<'context> { None } - fn resolve_local_globals(&mut self) { - let globals = vecmap(self.interner.get_all_globals(), |global| { - (global.id, global.local_id, global.ident.clone()) - }); - for (id, local_module_id, name) in globals { - if local_module_id == self.local_module { - let definition = DefinitionKind::Global(id); - self.add_global_variable_decl(name, definition); - } - } - } - /// TODO: This is currently only respected for generic free functions /// there's a bunch of other places where trait constraints can pop up fn resolve_trait_constraints( @@ -517,21 +503,20 @@ impl<'context> Elaborator<'context> { ) -> Vec { where_clause .iter() - .cloned() .filter_map(|constraint| self.resolve_trait_constraint(constraint)) .collect() } pub fn resolve_trait_constraint( &mut self, - constraint: UnresolvedTraitConstraint, + constraint: &UnresolvedTraitConstraint, ) -> Option { - let typ = self.resolve_type(constraint.typ); + let typ = self.resolve_type(constraint.typ.clone()); let trait_generics = - vecmap(constraint.trait_bound.trait_generics, |typ| self.resolve_type(typ)); + vecmap(&constraint.trait_bound.trait_generics, |typ| self.resolve_type(typ.clone())); let span = constraint.trait_bound.trait_path.span(); - let the_trait = self.lookup_trait_or_error(constraint.trait_bound.trait_path)?; + let the_trait = self.lookup_trait_or_error(constraint.trait_bound.trait_path.clone())?; let trait_id = the_trait.id; let expected_generics = the_trait.generics.len(); @@ -571,9 +556,6 @@ impl<'context> Elaborator<'context> { self.scopes.start_function(); self.current_item = Some(DependencyId::Function(func_id)); - // Check whether the function has globals in the local module and add them to the scope - self.resolve_local_globals(); - let location = Location::new(func.name_ident().span(), self.file); let id = self.interner.function_definition_id(func_id); let name_ident = HirIdent::non_trait_method(id, location); @@ -894,41 +876,69 @@ impl<'context> Elaborator<'context> { self.file = trait_impl.file_id; self.local_module = trait_impl.module_id; - let unresolved_type = trait_impl.object_type; - let self_type_span = unresolved_type.span; - let old_generics_length = self.generics.len(); self.generics = trait_impl.resolved_generics; + self.current_trait_impl = trait_impl.impl_id; - let trait_generics = - vecmap(&trait_impl.trait_generics, |generic| self.resolve_type(generic.clone())); + self.elaborate_functions(trait_impl.methods); - let self_type = trait_impl.resolved_object_type.unwrap_or(Type::Error); + self.self_type = None; + self.current_trait_impl = None; + self.generics.clear(); + } - let impl_id = - trait_impl.impl_id.expect("An impl's id should be set during define_function_metas"); + fn collect_impls( + &mut self, + module: LocalModuleId, + impls: &mut [(Vec, Span, UnresolvedFunctions)], + ) { + self.local_module = module; - self.current_trait_impl = trait_impl.impl_id; + for (generics, span, unresolved) in impls { + self.file = unresolved.file_id; + let old_generic_count = self.generics.len(); + self.add_generics(generics); + self.declare_methods_on_struct(false, unresolved, *span); + self.generics.truncate(old_generic_count); + } + } - let mut methods = trait_impl.methods.function_ids(); + fn collect_trait_impl(&mut self, trait_impl: &mut UnresolvedTraitImpl) { + self.local_module = trait_impl.module_id; + self.file = trait_impl.file_id; + trait_impl.trait_id = self.resolve_trait_by_path(trait_impl.trait_path.clone()); - self.elaborate_functions(trait_impl.methods); + let self_type = trait_impl.methods.self_type.clone(); + let self_type = + self_type.expect("Expected struct type to be set before collect_trait_impl"); + + let self_type_span = trait_impl.object_type.span; if matches!(self_type, Type::MutableReference(_)) { let span = self_type_span.unwrap_or_else(|| trait_impl.trait_path.span()); self.push_err(DefCollectorErrorKind::MutableReferenceInTraitImpl { span }); } + assert!(trait_impl.trait_id.is_some()); if let Some(trait_id) = trait_impl.trait_id { + self.collect_trait_impl_methods(trait_id, trait_impl); + + let span = trait_impl.object_type.span.expect("All trait self types should have spans"); + self.generics = trait_impl.resolved_generics.clone(); + self.declare_methods_on_struct(true, &mut trait_impl.methods, span); + + let methods = trait_impl.methods.function_ids(); for func_id in &methods { self.interner.set_function_trait(*func_id, self_type.clone(), trait_id); } let where_clause = trait_impl .where_clause - .into_iter() + .iter() .flat_map(|item| self.resolve_trait_constraint(item)) .collect(); + let trait_generics = trait_impl.resolved_trait_generics.clone(); + let resolved_trait_impl = Shared::new(TraitImpl { ident: trait_impl.trait_path.last_segment().clone(), typ: self_type.clone(), @@ -945,7 +955,7 @@ impl<'context> Elaborator<'context> { self_type.clone(), trait_id, trait_generics, - impl_id, + trait_impl.impl_id.expect("impl_id should be set in define_function_metas"), generics, resolved_trait_impl, ) { @@ -962,40 +972,7 @@ impl<'context> Elaborator<'context> { } } - self.self_type = None; - self.current_trait_impl = None; - self.generics.truncate(old_generics_length); - } - - fn collect_impls( - &mut self, - module: LocalModuleId, - impls: &mut [(Vec, Span, UnresolvedFunctions)], - ) { - self.local_module = module; - - for (generics, span, unresolved) in impls { - self.file = unresolved.file_id; - let old_generic_count = self.generics.len(); - self.add_generics(generics); - self.declare_methods_on_struct(false, unresolved, *span); - self.generics.truncate(old_generic_count); - } - } - - fn collect_trait_impl(&mut self, trait_impl: &mut UnresolvedTraitImpl) { - self.local_module = trait_impl.module_id; - self.file = trait_impl.file_id; - trait_impl.trait_id = self.resolve_trait_by_path(trait_impl.trait_path.clone()); - - if let Some(trait_id) = trait_impl.trait_id { - self.collect_trait_impl_methods(trait_id, trait_impl); - - let span = trait_impl.object_type.span.expect("All trait self types should have spans"); - self.generics = trait_impl.resolved_generics.clone(); - self.declare_methods_on_struct(true, &mut trait_impl.methods, span); - self.generics.clear(); - } + self.generics.clear(); } fn get_module_mut(&mut self, module: ModuleId) -> &mut ModuleData { @@ -1009,10 +986,9 @@ impl<'context> Elaborator<'context> { functions: &mut UnresolvedFunctions, span: Span, ) { - let self_type = functions - .self_type - .as_ref() - .expect("Expected struct type to be set before declare_methods_on_struct"); + let self_type = functions.self_type.as_ref(); + let self_type = + self_type.expect("Expected struct type to be set before declare_methods_on_struct"); let function_ids = functions.function_ids(); @@ -1189,7 +1165,6 @@ impl<'context> Elaborator<'context> { self.local_module = alias.module_id; let generics = self.add_generics(&alias.type_alias_def.generics); - self.resolve_local_globals(); self.current_item = Some(DependencyId::Alias(alias_id)); let typ = self.resolve_type(alias.type_alias_def.typ); self.interner.set_type_alias(alias_id, typ, generics); @@ -1241,9 +1216,6 @@ impl<'context> Elaborator<'context> { self.recover_generics(|this| { let generics = this.add_generics(&unresolved.generics); - // Check whether the struct definition has globals in the local module and add them to the scope - this.resolve_local_globals(); - this.current_item = Some(DependencyId::Struct(struct_id)); this.resolving_ids.insert(struct_id); @@ -1276,10 +1248,11 @@ impl<'context> Elaborator<'context> { self.push_err(ResolverError::MutableGlobal { span }); } + let name = let_stmt.pattern.name_ident().clone(); let (let_statement, _typ) = self.elaborate_let(let_stmt); let statement_id = self.interner.get_global(global_id).let_statement; - self.interner.get_global_definition_mut(global_id).kind = definition_kind; + self.interner.get_global_definition_mut(global_id).kind = definition_kind.clone(); self.interner.replace_statement(statement_id, let_statement); } @@ -1311,10 +1284,13 @@ impl<'context> Elaborator<'context> { self.local_module = trait_impl.module_id; let unresolved_type = &trait_impl.object_type; - let self_type_span = unresolved_type.span; self.add_generics(&trait_impl.generics); trait_impl.resolved_generics = self.generics.clone(); + let trait_generics = + vecmap(&trait_impl.trait_generics, |generic| self.resolve_type(generic.clone())); + trait_impl.resolved_trait_generics = trait_generics; + let self_type = self.resolve_type(unresolved_type.clone()); self.self_type = Some(self_type.clone()); diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 810e1b90743..17c11b88f4a 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -5,6 +5,7 @@ use rustc_hash::FxHashSet as HashSet; use crate::{ ast::ERROR_IDENT, hir::{ + def_collector::dc_crate::CompilationError, resolution::errors::ResolverError, type_check::{Source, TypeCheckError}, }, @@ -330,9 +331,9 @@ impl<'context> Elaborator<'context> { ) -> (ExprId, Type) { let span = variable.span; let expr = self.resolve_variable(variable); - let id = self.interner.push_expr(HirExpression::Ident(expr.clone(), generics)); + let id = self.interner.push_expr(HirExpression::Ident(expr.clone(), generics.clone())); self.interner.push_expr_location(id, span, self.file); - let typ = self.type_check_variable(expr, id); + let typ = self.type_check_variable(expr, id, generics); self.interner.push_expr_type(id, typ.clone()); (id, typ) } @@ -384,14 +385,19 @@ impl<'context> Elaborator<'context> { } } - pub(super) fn type_check_variable(&mut self, ident: HirIdent, expr_id: ExprId) -> Type { + pub(super) fn type_check_variable( + &mut self, + ident: HirIdent, + expr_id: ExprId, + generics: Option>, + ) -> Type { let mut bindings = TypeBindings::new(); // Add type bindings from any constraints that were used. // We need to do this first since otherwise instantiating the type below // will replace each trait generic with a fresh type variable, rather than // the type used in the trait constraint (if it exists). See #4088. - if let ImplKind::TraitMethod(_, constraint, _) = &ident.impl_kind { + if let ImplKind::TraitMethod(_, constraint, assumed) = &ident.impl_kind { let the_trait = self.interner.get_trait(constraint.trait_id); assert_eq!(the_trait.generics.len(), constraint.trait_generics.len()); @@ -401,6 +407,16 @@ impl<'context> Elaborator<'context> { bindings.insert(param.id(), (param.clone(), arg.clone())); } } + + // If the trait impl is already assumed to exist we should add any type bindings for `Self`. + // Otherwise `self` will be replaced with a fresh type variable, which will require the user + // to specify a redundant type annotation. + if *assumed { + bindings.insert( + the_trait.self_type_typevar_id, + (the_trait.self_type_typevar.clone(), constraint.typ.clone()), + ); + } } // An identifiers type may be forall-quantified in the case of generic functions. @@ -409,10 +425,21 @@ impl<'context> Elaborator<'context> { // variable to handle generic functions. let t = self.interner.id_type_substitute_trait_as_type(ident.id); + let definition = self.interner.try_definition(ident.id); + let function_generic_count = definition.map_or(0, |definition| match &definition.kind { + DefinitionKind::Function(function) => { + self.interner.function_modifiers(function).generic_count + } + _ => 0, + }); + + let span = self.interner.expr_span(&expr_id); + let location = self.interner.expr_location(&expr_id); // This instantiates a trait's generics as well which need to be set // when the constraint below is later solved for when the function is // finished. How to link the two? - let (typ, bindings) = t.instantiate_with_bindings(bindings, self.interner); + let (typ, bindings) = + self.instantiate(t, bindings, generics, function_generic_count, span, location); // Push any trait constraints required by this definition to the context // to be checked later when the type of this variable is further constrained. @@ -447,6 +474,39 @@ impl<'context> Elaborator<'context> { typ } + fn instantiate( + &mut self, + typ: Type, + bindings: TypeBindings, + turbofish_generics: Option>, + function_generic_count: usize, + span: Span, + location: Location, + ) -> (Type, TypeBindings) { + match turbofish_generics { + Some(turbofish_generics) => { + if turbofish_generics.len() != function_generic_count { + let type_check_err = TypeCheckError::IncorrectTurbofishGenericCount { + expected_count: function_generic_count, + actual_count: turbofish_generics.len(), + span, + }; + self.errors.push((CompilationError::TypeError(type_check_err), location.file)); + typ.instantiate_with_bindings(bindings, self.interner) + } else { + // Fetch the count of any implicit generics on the function, such as + // for a method within a generic impl. + let implicit_generic_count = match &typ { + Type::Forall(generics, _) => generics.len() - function_generic_count, + _ => 0, + }; + typ.instantiate_with(turbofish_generics, self.interner, implicit_generic_count) + } + } + None => typ.instantiate_with_bindings(bindings, self.interner), + } + } + fn get_ident_from_path(&mut self, path: Path) -> (HirIdent, usize) { let location = Location::new(path.span(), self.file); diff --git a/compiler/noirc_frontend/src/elaborator/traits.rs b/compiler/noirc_frontend/src/elaborator/traits.rs index b5713e175be..c29a6871ccc 100644 --- a/compiler/noirc_frontend/src/elaborator/traits.rs +++ b/compiler/noirc_frontend/src/elaborator/traits.rs @@ -154,9 +154,6 @@ impl<'context> Elaborator<'context> { let old_generic_count = self.generics.len(); self.scopes.start_function(); - // Check whether the function has globals in the local module and add them to the scope - self.resolve_local_globals(); - self.trait_bounds = where_clause.to_vec(); let kind = FunctionKind::Normal; diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 838aac3f067..05147af5459 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -121,11 +121,15 @@ pub struct UnresolvedTraitImpl { pub generics: UnresolvedGenerics, pub where_clause: Vec, - // These fields are filled in later + // Every field after this line is filled in later in the elaborator pub trait_id: Option, pub impl_id: Option, pub resolved_object_type: Option, pub resolved_generics: Vec<(Rc, TypeVariable, Span)>, + + // The resolved generic on the trait itself. E.g. it is the `` in + // `impl Foo for Bar { ... }` + pub resolved_trait_generics: Vec, } #[derive(Clone)] diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index baea00ad23d..df8fe6dc878 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -195,6 +195,7 @@ impl<'a> ModCollector<'a> { impl_id: None, resolved_object_type: None, resolved_generics: Vec::new(), + resolved_trait_generics: Vec::new(), }; self.def_collector.items.trait_impls.push(unresolved_trait_impl); diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index abff466e1d5..336c1cedbb6 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -371,7 +371,7 @@ impl<'interner> TypeChecker<'interner> { // We need to do this first since otherwise instantiating the type below // will replace each trait generic with a fresh type variable, rather than // the type used in the trait constraint (if it exists). See #4088. - if let ImplKind::TraitMethod(_, constraint, _) = &ident.impl_kind { + if let ImplKind::TraitMethod(_, constraint, assumed) = &ident.impl_kind { let the_trait = self.interner.get_trait(constraint.trait_id); assert_eq!(the_trait.generics.len(), constraint.trait_generics.len()); @@ -381,6 +381,16 @@ impl<'interner> TypeChecker<'interner> { bindings.insert(param.id(), (param.clone(), arg.clone())); } } + + // If the trait impl is already assumed to exist we should add any type bindings for `Self`. + // Otherwise `self` will be replaced with a fresh type variable, which will require the user + // to specify a redundant type annotation. + if *assumed { + bindings.insert( + the_trait.self_type_typevar_id, + (the_trait.self_type_typevar.clone(), constraint.typ.clone()), + ); + } } // An identifiers type may be forall-quantified in the case of generic functions. @@ -389,8 +399,6 @@ impl<'interner> TypeChecker<'interner> { // variable to handle generic functions. let t = self.interner.id_type_substitute_trait_as_type(ident.id); - let span = self.interner.expr_span(expr_id); - let definition = self.interner.try_definition(ident.id); let function_generic_count = definition.map_or(0, |definition| match &definition.kind { DefinitionKind::Function(function) => { @@ -399,6 +407,7 @@ impl<'interner> TypeChecker<'interner> { _ => 0, }); + let span = self.interner.expr_span(expr_id); // This instantiates a trait's generics as well which need to be set // when the constraint below is later solved for when the function is // finished. How to link the two? diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 7bf5655486b..bd81752c046 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -1430,14 +1430,14 @@ fn specify_method_types_with_turbofish() { } impl Foo { - fn generic_method(_self: Self) where U: Default { + fn generic_method(_self: Self) -> U where U: Default { U::default() } } fn main() { let foo: Foo = Foo { inner: 1 }; - foo.generic_method::(); + let _ = foo.generic_method::(); } "#; let errors = get_program_errors(src); diff --git a/docs/docs/noir/standard_library/is_unconstrained.md b/docs/docs/noir/standard_library/is_unconstrained.md new file mode 100644 index 00000000000..bb157e719dc --- /dev/null +++ b/docs/docs/noir/standard_library/is_unconstrained.md @@ -0,0 +1,59 @@ +--- +title: Is Unconstrained Function +description: + The is_unconstrained function returns wether the context at that point of the program is unconstrained or not. +keywords: + [ + unconstrained + ] +--- + +It's very common for functions in circuits to take unconstrained hints of an expensive computation and then verify it. This is done by running the hint in an unconstrained context and then verifying the result in a constrained context. + +When a function is marked as unconstrained, any subsequent functions that it calls will also be run in an unconstrained context. However, if we are implementing a library function, other users might call it within an unconstrained context or a constrained one. Generally, in an unconstrained context we prefer just computing the result instead of taking a hint of it and verifying it, since that'd mean doing the same computation twice: + +```rust + +fn my_expensive_computation(){ + ... +} + +unconstrained fn my_expensive_computation_hint(){ + my_expensive_computation() +} + +pub fn external_interface(){ + my_expensive_computation_hint(); + // verify my_expensive_computation: If external_interface is called from unconstrained, this is redundant + ... +} + +``` + +In order to improve the performance in an unconstrained context you can use the function at `std::runtime::is_unconstrained() -> bool`: + + +```rust +use dep::std::runtime::is_unconstrained; + +fn my_expensive_computation(){ + ... +} + +unconstrained fn my_expensive_computation_hint(){ + my_expensive_computation() +} + +pub fn external_interface(){ + if is_unconstrained() { + my_expensive_computation(); + } else { + my_expensive_computation_hint(); + // verify my_expensive_computation + ... + } +} + +``` + +The is_unconstrained result is resolved at compile time, so in unconstrained contexts the compiler removes the else branch, and in constrained contexts the compiler removes the if branch, reducing the amount of compute necessary to run external_interface. \ No newline at end of file diff --git a/noir_stdlib/src/eddsa.nr b/noir_stdlib/src/eddsa.nr index 1ab0158af8f..337969be90e 100644 --- a/noir_stdlib/src/eddsa.nr +++ b/noir_stdlib/src/eddsa.nr @@ -45,7 +45,7 @@ where H: Hasher + Default { // Ensure S < Subgroup Order assert(signature_s.lt(bjj.suborder)); // Calculate the h = H(R, A, msg) - let mut hasher: H = H::default(); + let mut hasher = H::default(); hasher.write(signature_r8_x); hasher.write(signature_r8_y); hasher.write(pub_key_x); diff --git a/noir_stdlib/src/field/bn254.nr b/noir_stdlib/src/field/bn254.nr index 2e82d9e7c23..bcdc23f80dc 100644 --- a/noir_stdlib/src/field/bn254.nr +++ b/noir_stdlib/src/field/bn254.nr @@ -1,11 +1,13 @@ +use crate::runtime::is_unconstrained; + // The low and high decomposition of the field modulus global PLO: Field = 53438638232309528389504892708671455233; global PHI: Field = 64323764613183177041862057485226039389; global TWO_POW_128: Field = 0x100000000000000000000000000000000; -/// A hint for decomposing a single field into two 16 byte fields. -unconstrained fn decompose_unsafe(x: Field) -> (Field, Field) { +// Decomposes a single field into two 16 byte fields. +fn compute_decomposition(x: Field) -> (Field, Field) { let x_bytes = x.to_le_bytes(32); let mut low: Field = 0; @@ -21,37 +23,11 @@ unconstrained fn decompose_unsafe(x: Field) -> (Field, Field) { (low, high) } -// Assert that (alo > blo && ahi >= bhi) || (alo <= blo && ahi > bhi) -fn assert_gt_limbs(a: (Field, Field), b: (Field, Field)) { - let (alo, ahi) = a; - let (blo, bhi) = b; - let borrow = lte_unsafe_16(alo, blo); - - let rlo = alo - blo - 1 + (borrow as Field) * TWO_POW_128; - let rhi = ahi - bhi - (borrow as Field); - - rlo.assert_max_bit_size(128); - rhi.assert_max_bit_size(128); -} - -/// Decompose a single field into two 16 byte fields. -pub fn decompose(x: Field) -> (Field, Field) { - // Take hints of the decomposition - let (xlo, xhi) = decompose_unsafe(x); - - // Range check the limbs - xlo.assert_max_bit_size(128); - xhi.assert_max_bit_size(128); - - // Check that the decomposition is correct - assert_eq(x, xlo + TWO_POW_128 * xhi); - - // Assert that the decomposition of P is greater than the decomposition of x - assert_gt_limbs((PLO, PHI), (xlo, xhi)); - (xlo, xhi) +unconstrained fn decompose_hint(x: Field) -> (Field, Field) { + compute_decomposition(x) } -fn lt_unsafe_internal(x: Field, y: Field, num_bytes: u32) -> bool { +fn compute_lt(x: Field, y: Field, num_bytes: u32) -> bool { let x_bytes = x.to_le_radix(256, num_bytes); let y_bytes = y.to_le_radix(256, num_bytes); let mut x_is_lt = false; @@ -70,29 +46,67 @@ fn lt_unsafe_internal(x: Field, y: Field, num_bytes: u32) -> bool { x_is_lt } -fn lte_unsafe_internal(x: Field, y: Field, num_bytes: u32) -> bool { +fn compute_lte(x: Field, y: Field, num_bytes: u32) -> bool { if x == y { true } else { - lt_unsafe_internal(x, y, num_bytes) + compute_lt(x, y, num_bytes) } } -unconstrained fn lt_unsafe_32(x: Field, y: Field) -> bool { - lt_unsafe_internal(x, y, 32) +unconstrained fn lt_32_hint(x: Field, y: Field) -> bool { + compute_lt(x, y, 32) } -unconstrained fn lte_unsafe_16(x: Field, y: Field) -> bool { - lte_unsafe_internal(x, y, 16) +unconstrained fn lte_16_hint(x: Field, y: Field) -> bool { + compute_lte(x, y, 16) +} + +// Assert that (alo > blo && ahi >= bhi) || (alo <= blo && ahi > bhi) +fn assert_gt_limbs(a: (Field, Field), b: (Field, Field)) { + let (alo, ahi) = a; + let (blo, bhi) = b; + let borrow = lte_16_hint(alo, blo); + + let rlo = alo - blo - 1 + (borrow as Field) * TWO_POW_128; + let rhi = ahi - bhi - (borrow as Field); + + rlo.assert_max_bit_size(128); + rhi.assert_max_bit_size(128); +} + +/// Decompose a single field into two 16 byte fields. +pub fn decompose(x: Field) -> (Field, Field) { + if is_unconstrained() { + compute_decomposition(x) + } else { + // Take hints of the decomposition + let (xlo, xhi) = decompose_hint(x); + + // Range check the limbs + xlo.assert_max_bit_size(128); + xhi.assert_max_bit_size(128); + + // Check that the decomposition is correct + assert_eq(x, xlo + TWO_POW_128 * xhi); + + // Assert that the decomposition of P is greater than the decomposition of x + assert_gt_limbs((PLO, PHI), (xlo, xhi)); + (xlo, xhi) + } } pub fn assert_gt(a: Field, b: Field) { - // Decompose a and b - let a_limbs = decompose(a); - let b_limbs = decompose(b); + if is_unconstrained() { + assert(compute_lt(b, a, 32)); + } else { + // Decompose a and b + let a_limbs = decompose(a); + let b_limbs = decompose(b); - // Assert that a_limbs is greater than b_limbs - assert_gt_limbs(a_limbs, b_limbs) + // Assert that a_limbs is greater than b_limbs + assert_gt_limbs(a_limbs, b_limbs) + } } pub fn assert_lt(a: Field, b: Field) { @@ -100,14 +114,19 @@ pub fn assert_lt(a: Field, b: Field) { } pub fn gt(a: Field, b: Field) -> bool { - if a == b { - false - } else if lt_unsafe_32(a, b) { - assert_gt(b, a); + if is_unconstrained() { + compute_lt(b, a, 32) + } else if a == b { false - } else { - assert_gt(a, b); - true + } else { + // Take a hint of the comparison and verify it + if lt_32_hint(a, b) { + assert_gt(b, a); + false + } else { + assert_gt(a, b); + true + } } } @@ -117,44 +136,41 @@ pub fn lt(a: Field, b: Field) -> bool { mod tests { // TODO: Allow imports from "super" - use crate::field::bn254::{ - decompose_unsafe, decompose, lt_unsafe_internal, assert_gt, gt, lt, TWO_POW_128, - lte_unsafe_internal, PLO, PHI - }; + use crate::field::bn254::{decompose_hint, decompose, compute_lt, assert_gt, gt, lt, TWO_POW_128, compute_lte, PLO, PHI}; #[test] - fn check_decompose_unsafe() { - assert_eq(decompose_unsafe(TWO_POW_128), (0, 1)); - assert_eq(decompose_unsafe(TWO_POW_128 + 0x1234567890), (0x1234567890, 1)); - assert_eq(decompose_unsafe(0x1234567890), (0x1234567890, 0)); + fn check_decompose() { + assert_eq(decompose(TWO_POW_128), (0, 1)); + assert_eq(decompose(TWO_POW_128 + 0x1234567890), (0x1234567890, 1)); + assert_eq(decompose(0x1234567890), (0x1234567890, 0)); } #[test] - fn check_decompose() { + unconstrained fn check_decompose_unconstrained() { assert_eq(decompose(TWO_POW_128), (0, 1)); assert_eq(decompose(TWO_POW_128 + 0x1234567890), (0x1234567890, 1)); assert_eq(decompose(0x1234567890), (0x1234567890, 0)); } #[test] - fn check_lt_unsafe() { - assert(lt_unsafe_internal(0, 1, 16)); - assert(lt_unsafe_internal(0, 0x100, 16)); - assert(lt_unsafe_internal(0x100, TWO_POW_128 - 1, 16)); - assert(!lt_unsafe_internal(0, TWO_POW_128, 16)); + fn check_compute_lt() { + assert(compute_lt(0, 1, 16)); + assert(compute_lt(0, 0x100, 16)); + assert(compute_lt(0x100, TWO_POW_128 - 1, 16)); + assert(!compute_lt(0, TWO_POW_128, 16)); } #[test] - fn check_lte_unsafe() { - assert(lte_unsafe_internal(0, 1, 16)); - assert(lte_unsafe_internal(0, 0x100, 16)); - assert(lte_unsafe_internal(0x100, TWO_POW_128 - 1, 16)); - assert(!lte_unsafe_internal(0, TWO_POW_128, 16)); - - assert(lte_unsafe_internal(0, 0, 16)); - assert(lte_unsafe_internal(0x100, 0x100, 16)); - assert(lte_unsafe_internal(TWO_POW_128 - 1, TWO_POW_128 - 1, 16)); - assert(lte_unsafe_internal(TWO_POW_128, TWO_POW_128, 16)); + fn check_compute_lte() { + assert(compute_lte(0, 1, 16)); + assert(compute_lte(0, 0x100, 16)); + assert(compute_lte(0x100, TWO_POW_128 - 1, 16)); + assert(!compute_lte(0, TWO_POW_128, 16)); + + assert(compute_lte(0, 0, 16)); + assert(compute_lte(0x100, 0x100, 16)); + assert(compute_lte(TWO_POW_128 - 1, TWO_POW_128 - 1, 16)); + assert(compute_lte(TWO_POW_128, TWO_POW_128, 16)); } #[test] @@ -166,6 +182,15 @@ mod tests { assert_gt(0 - 1, 0); } + #[test] + unconstrained fn check_assert_gt_unconstrained() { + assert_gt(1, 0); + assert_gt(0x100, 0); + assert_gt((0 - 1), (0 - 2)); + assert_gt(TWO_POW_128, 0); + assert_gt(0 - 1, 0); + } + #[test] fn check_gt() { assert(gt(1, 0)); @@ -178,6 +203,18 @@ mod tests { assert(!gt(0 - 2, 0 - 1)); } + #[test] + unconstrained fn check_gt_unconstrained() { + assert(gt(1, 0)); + assert(gt(0x100, 0)); + assert(gt((0 - 1), (0 - 2))); + assert(gt(TWO_POW_128, 0)); + assert(!gt(0, 0)); + assert(!gt(0, 0x100)); + assert(gt(0 - 1, 0 - 2)); + assert(!gt(0 - 2, 0 - 1)); + } + #[test] fn check_plo_phi() { assert_eq(PLO + PHI * TWO_POW_128, 0); diff --git a/noir_stdlib/src/lib.nr b/noir_stdlib/src/lib.nr index f11b21003ad..ad47171fa46 100644 --- a/noir_stdlib/src/lib.nr +++ b/noir_stdlib/src/lib.nr @@ -25,6 +25,7 @@ mod default; mod prelude; mod uint128; mod bigint; +mod runtime; // Oracle calls are required to be wrapped in an unconstrained function // Thus, the only argument to the `println` oracle is expected to always be an ident @@ -70,3 +71,4 @@ pub fn wrapping_mul(x: T, y: T) -> T { #[builtin(as_witness)] pub fn as_witness(x: Field) {} + diff --git a/noir_stdlib/src/runtime.nr b/noir_stdlib/src/runtime.nr new file mode 100644 index 00000000000..c075107cd52 --- /dev/null +++ b/noir_stdlib/src/runtime.nr @@ -0,0 +1,2 @@ +#[builtin(is_unconstrained)] +pub fn is_unconstrained() -> bool {} diff --git a/test_programs/execution_success/brillig_slice_input/Nargo.toml b/test_programs/compile_success_empty/brillig_slice_input/Nargo.toml similarity index 100% rename from test_programs/execution_success/brillig_slice_input/Nargo.toml rename to test_programs/compile_success_empty/brillig_slice_input/Nargo.toml diff --git a/test_programs/execution_success/brillig_slice_input/src/main.nr b/test_programs/compile_success_empty/brillig_slice_input/src/main.nr similarity index 100% rename from test_programs/execution_success/brillig_slice_input/src/main.nr rename to test_programs/compile_success_empty/brillig_slice_input/src/main.nr diff --git a/test_programs/execution_success/regression_4383/Nargo.toml b/test_programs/compile_success_empty/regression_4383/Nargo.toml similarity index 100% rename from test_programs/execution_success/regression_4383/Nargo.toml rename to test_programs/compile_success_empty/regression_4383/Nargo.toml diff --git a/test_programs/execution_success/regression_4383/src/main.nr b/test_programs/compile_success_empty/regression_4383/src/main.nr similarity index 100% rename from test_programs/execution_success/regression_4383/src/main.nr rename to test_programs/compile_success_empty/regression_4383/src/main.nr diff --git a/test_programs/execution_success/regression_4436/Nargo.toml b/test_programs/compile_success_empty/regression_4436/Nargo.toml similarity index 100% rename from test_programs/execution_success/regression_4436/Nargo.toml rename to test_programs/compile_success_empty/regression_4436/Nargo.toml diff --git a/test_programs/execution_success/regression_4436/src/main.nr b/test_programs/compile_success_empty/regression_4436/src/main.nr similarity index 100% rename from test_programs/execution_success/regression_4436/src/main.nr rename to test_programs/compile_success_empty/regression_4436/src/main.nr diff --git a/test_programs/execution_success/slice_init_with_complex_type/Nargo.toml b/test_programs/compile_success_empty/slice_init_with_complex_type/Nargo.toml similarity index 100% rename from test_programs/execution_success/slice_init_with_complex_type/Nargo.toml rename to test_programs/compile_success_empty/slice_init_with_complex_type/Nargo.toml diff --git a/test_programs/execution_success/slice_init_with_complex_type/Prover.toml b/test_programs/compile_success_empty/slice_init_with_complex_type/Prover.toml similarity index 100% rename from test_programs/execution_success/slice_init_with_complex_type/Prover.toml rename to test_programs/compile_success_empty/slice_init_with_complex_type/Prover.toml diff --git a/test_programs/execution_success/slice_init_with_complex_type/src/main.nr b/test_programs/compile_success_empty/slice_init_with_complex_type/src/main.nr similarity index 100% rename from test_programs/execution_success/slice_init_with_complex_type/src/main.nr rename to test_programs/compile_success_empty/slice_init_with_complex_type/src/main.nr diff --git a/test_programs/execution_success/is_unconstrained/Nargo.toml b/test_programs/execution_success/is_unconstrained/Nargo.toml new file mode 100644 index 00000000000..deef68c7f72 --- /dev/null +++ b/test_programs/execution_success/is_unconstrained/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "is_unconstrained" +type = "bin" +authors = [""] + +[dependencies] diff --git a/test_programs/execution_success/is_unconstrained/Prover.toml b/test_programs/execution_success/is_unconstrained/Prover.toml new file mode 100644 index 00000000000..8b137891791 --- /dev/null +++ b/test_programs/execution_success/is_unconstrained/Prover.toml @@ -0,0 +1 @@ + diff --git a/test_programs/execution_success/is_unconstrained/src/main.nr b/test_programs/execution_success/is_unconstrained/src/main.nr new file mode 100644 index 00000000000..af40af1029f --- /dev/null +++ b/test_programs/execution_success/is_unconstrained/src/main.nr @@ -0,0 +1,14 @@ +use dep::std::runtime::is_unconstrained; + +fn check(should_be_unconstrained: bool) { + assert_eq(should_be_unconstrained, is_unconstrained()); +} + +unconstrained fn unconstrained_intermediate() { + check(true); +} + +fn main() { + unconstrained_intermediate(); + check(false); +} diff --git a/test_programs/execution_success/turbofish_call_func_diff_types/src/main.nr b/test_programs/execution_success/turbofish_call_func_diff_types/src/main.nr index 709a694e77b..b880d3ae047 100644 --- a/test_programs/execution_success/turbofish_call_func_diff_types/src/main.nr +++ b/test_programs/execution_success/turbofish_call_func_diff_types/src/main.nr @@ -23,9 +23,7 @@ fn main(x: Field, y: pub Field) { fn hash_simple_array(input: [Field; 2]) -> Field where H: Hasher + Default { // Check that we can call a trait method instead of a trait implementation - // TODO(https://github.com/noir-lang/noir/issues/5063): Need to remove the need for this type annotation - // Curently, without the annotation we will get `Expression type is ambiguous` when trying to use the `hasher` - let mut hasher: H = H::default(); + let mut hasher = H::default(); // Regression that the object is converted to a mutable reference type `&mut _`. // Otherwise will see `Expected type &mut _, found type H`. // Then we need to make sure to also auto dereference later in the type checking process diff --git a/test_programs/gates_report.sh b/test_programs/gates_report.sh index 3b0b4d9e148..b33e81b037c 100755 --- a/test_programs/gates_report.sh +++ b/test_programs/gates_report.sh @@ -1,36 +1,37 @@ #!/usr/bin/env bash set -e +BACKEND=${BACKEND:-bb} + # These tests are incompatible with gas reporting excluded_dirs=("workspace" "workspace_default_member" "double_verify_nested_proof") -# These tests cause failures in CI with a stack overflow for some reason. -ci_excluded_dirs=("eddsa") - current_dir=$(pwd) -base_path="$current_dir/execution_success" -test_dirs=$(ls $base_path) +artifacts_path="$current_dir/acir_artifacts" +test_dirs=$(ls $artifacts_path) -# We generate a Noir workspace which contains all of the test cases -# This allows us to generate a gates report using `nargo info` for all of them at once. +echo "{\"programs\": [" > gates_report.json -echo "[workspace]" > Nargo.toml -echo "members = [" >> Nargo.toml +# Bound for checking where to place last parentheses +NUM_ARTIFACTS=$(ls -1q "$artifacts_path" | wc -l) -for dir in $test_dirs; do - if [[ " ${excluded_dirs[@]} " =~ " ${dir} " ]]; then - continue - fi +ITER="1" +for pathname in $test_dirs; do + ARTIFACT_NAME=$(basename "$pathname") + + GATES_INFO=$($BACKEND gates -b "$artifacts_path/$ARTIFACT_NAME/target/program.json") + MAIN_FUNCTION_INFO=$(echo $GATES_INFO | jq -r '.functions[0] | .name = "main"') + echo "{\"package_name\": \"$ARTIFACT_NAME\", \"functions\": [$MAIN_FUNCTION_INFO]" >> gates_report.json - if [[ ${CI-false} = "true" ]] && [[ " ${ci_excluded_dirs[@]} " =~ " ${dir} " ]]; then - continue + if (($ITER == $NUM_ARTIFACTS)); then + echo "}" >> gates_report.json + else + echo "}, " >> gates_report.json fi - echo " \"execution_success/$dir\"," >> Nargo.toml + ITER=$(( $ITER + 1 )) done -echo "]" >> Nargo.toml +echo "]}" >> gates_report.json -nargo info --json > gates_report.json -rm Nargo.toml diff --git a/test_programs/rebuild.sh b/test_programs/rebuild.sh index 4733bad10c3..cfc6479febf 100755 --- a/test_programs/rebuild.sh +++ b/test_programs/rebuild.sh @@ -8,6 +8,14 @@ process_dir() { local current_dir=$2 local dir_name=$(basename "$dir") + if [[ ! -f "$dir/Nargo.toml" ]]; then + # This directory isn't a proper test but just hold some stale build artifacts + # We then delete it and carry on. + rm -rf $dir + return 0 + fi + + if [[ ! -d "$current_dir/acir_artifacts/$dir_name" ]]; then mkdir -p $current_dir/acir_artifacts/$dir_name fi diff --git a/tooling/debugger/ignored-tests.txt b/tooling/debugger/ignored-tests.txt index cda26169421..a6d3c9a3a94 100644 --- a/tooling/debugger/ignored-tests.txt +++ b/tooling/debugger/ignored-tests.txt @@ -25,3 +25,4 @@ fold_fibonacci fold_complex_outputs slice_init_with_complex_type hashmap +is_unconstrained \ No newline at end of file