diff --git a/.noir-sync-commit b/.noir-sync-commit index 336e1ccacbf..b7fb2d8b4d0 100644 --- a/.noir-sync-commit +++ b/.noir-sync-commit @@ -1 +1 @@ -cdbb940a883ae32dd84c667ec06b0d155f2d7520 +5192e537708fc9ec51f53bb6a6629c9d682532d5 diff --git a/noir/noir-repo/.github/workflows/gates_report.yml b/noir/noir-repo/.github/workflows/gates_report.yml index 0cc94a1a04d..0b0a527b69e 100644 --- a/noir/noir-repo/.github/workflows/gates_report.yml +++ b/noir/noir-repo/.github/workflows/gates_report.yml @@ -80,7 +80,7 @@ jobs: - name: Compare gates reports id: gates_diff - uses: vezenovm/noir-gates-diff@acf12797860f237117e15c0d6e08d64253af52b6 + uses: noir-lang/noir-gates-diff@1931aaaa848a1a009363d6115293f7b7fc72bb87 with: report: gates_report.json summaryQuantile: 0.9 # only display the 10% most significant circuit size diffs in the summary (defaults to 20%) diff --git a/noir/noir-repo/.github/workflows/gates_report_brillig.yml b/noir/noir-repo/.github/workflows/gates_report_brillig.yml new file mode 100644 index 00000000000..4e12a6fcbca --- /dev/null +++ b/noir/noir-repo/.github/workflows/gates_report_brillig.yml @@ -0,0 +1,92 @@ +name: Report Brillig bytecode size 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_brillig_bytecode_size_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 Brillig bytecode size report + working-directory: ./test_programs + run: | + chmod +x gates_report_brillig.sh + ./gates_report_brillig.sh + mv gates_report_brillig.json ../gates_report_brillig.json + + - name: Compare Brillig bytecode size reports + id: brillig_bytecode_diff + uses: noir-lang/noir-gates-diff@3fb844067b25d1b59727ea600b614503b33503f4 + with: + report: gates_report_brillig.json + header: | + # Changes to Brillig bytecode sizes + brillig_report: true + summaryQuantile: 0.9 # only display the 10% most significant bytecode size diffs in the summary (defaults to 20%) + + - name: Add bytecode size diff to sticky comment + if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target' + uses: marocchino/sticky-pull-request-comment@v2 + with: + header: brillig + # delete the comment in case changes no longer impact brillig bytecode sizes + delete: ${{ !steps.brillig_bytecode_diff.outputs.markdown }} + message: ${{ steps.brillig_bytecode_diff.outputs.markdown }} \ No newline at end of file diff --git a/noir/noir-repo/.gitignore b/noir/noir-repo/.gitignore index 2c877a4d02c..aeb7d8757c4 100644 --- a/noir/noir-repo/.gitignore +++ b/noir/noir-repo/.gitignore @@ -33,6 +33,7 @@ tooling/noir_js/lib !compiler/wasm/noir-script/target gates_report.json +gates_report_brillig.json # Github Actions scratch space # This gives a location to download artifacts into the repository in CI without making git dirty. diff --git a/noir/noir-repo/Cargo.lock b/noir/noir-repo/Cargo.lock index bacbf939786..e1159c71201 100644 --- a/noir/noir-repo/Cargo.lock +++ b/noir/noir-repo/Cargo.lock @@ -2722,6 +2722,7 @@ dependencies = [ "acvm", "async-lsp", "codespan-lsp", + "convert_case 0.6.0", "fm", "fxhash", "lsp-types 0.94.1", diff --git a/noir/noir-repo/acvm-repo/acir/src/circuit/opcodes.rs b/noir/noir-repo/acvm-repo/acir/src/circuit/opcodes.rs index b1fdc5e0080..d7f0f5f6f1f 100644 --- a/noir/noir-repo/acvm-repo/acir/src/circuit/opcodes.rs +++ b/noir/noir-repo/acvm-repo/acir/src/circuit/opcodes.rs @@ -2,6 +2,10 @@ use super::{ brillig::{BrilligFunctionId, BrilligInputs, BrilligOutputs}, directives::Directive, }; + +pub mod function_id; +pub use function_id::AcirFunctionId; + use crate::native_types::{Expression, Witness}; use acir_field::AcirField; use serde::{Deserialize, Serialize}; @@ -125,7 +129,7 @@ pub enum Opcode { Call { /// Id for the function being called. It is the responsibility of the executor /// to fetch the appropriate circuit from this id. - id: u32, + id: AcirFunctionId, /// Inputs to the function call inputs: Vec, /// Outputs of the function call diff --git a/noir/noir-repo/acvm-repo/acir/src/circuit/opcodes/function_id.rs b/noir/noir-repo/acvm-repo/acir/src/circuit/opcodes/function_id.rs new file mode 100644 index 00000000000..b5abb1b3942 --- /dev/null +++ b/noir/noir-repo/acvm-repo/acir/src/circuit/opcodes/function_id.rs @@ -0,0 +1,17 @@ +use serde::{Deserialize, Serialize}; + +#[derive(Clone, Copy, PartialEq, Eq, Debug, Serialize, Deserialize, Hash)] +#[serde(transparent)] +pub struct AcirFunctionId(pub u32); + +impl AcirFunctionId { + pub fn as_usize(&self) -> usize { + self.0 as usize + } +} + +impl std::fmt::Display for AcirFunctionId { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} diff --git a/noir/noir-repo/acvm-repo/acir/tests/test_program_serialization.rs b/noir/noir-repo/acvm-repo/acir/tests/test_program_serialization.rs index 1a634eeea9c..ce28d47021c 100644 --- a/noir/noir-repo/acvm-repo/acir/tests/test_program_serialization.rs +++ b/noir/noir-repo/acvm-repo/acir/tests/test_program_serialization.rs @@ -14,7 +14,7 @@ use std::collections::BTreeSet; use acir::{ circuit::{ brillig::{BrilligBytecode, BrilligFunctionId, BrilligInputs, BrilligOutputs}, - opcodes::{BlackBoxFuncCall, BlockId, FunctionInput, MemOp}, + opcodes::{AcirFunctionId, BlackBoxFuncCall, BlockId, FunctionInput, MemOp}, Circuit, Opcode, Program, PublicInputs, }, native_types::{Expression, Witness}, @@ -381,13 +381,13 @@ fn nested_acir_call_circuit() { // x // } let nested_call = Opcode::Call { - id: 1, + id: AcirFunctionId(1), inputs: vec![Witness(0), Witness(1)], outputs: vec![Witness(2)], predicate: None, }; let nested_call_two = Opcode::Call { - id: 1, + id: AcirFunctionId(1), inputs: vec![Witness(0), Witness(1)], outputs: vec![Witness(3)], predicate: None, @@ -419,7 +419,7 @@ fn nested_acir_call_circuit() { q_c: FieldElement::one() + FieldElement::one(), }); let call = Opcode::Call { - id: 2, + id: AcirFunctionId(2), inputs: vec![Witness(2), Witness(1)], outputs: vec![Witness(3)], predicate: None, diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs index 83c5aeb6296..647c11bd3c3 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs @@ -6,7 +6,7 @@ use acir::{ brillig::ForeignCallResult, circuit::{ brillig::{BrilligBytecode, BrilligFunctionId}, - opcodes::{BlockId, ConstantOrWitnessEnum, FunctionInput}, + opcodes::{AcirFunctionId, BlockId, ConstantOrWitnessEnum, FunctionInput}, AssertionPayload, ErrorSelector, ExpressionOrMemory, Opcode, OpcodeLocation, RawAssertionPayload, ResolvedAssertionPayload, STRING_ERROR_SELECTOR, }, @@ -575,7 +575,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { else { unreachable!("Not executing a Call opcode"); }; - if *id == 0 { + if *id == AcirFunctionId(0) { return Err(OpcodeResolutionError::AcirMainCallAttempted { opcode_location: ErrorLocation::Resolved(OpcodeLocation::Acir( self.instruction_pointer(), @@ -716,7 +716,7 @@ pub(crate) fn is_predicate_false( #[derive(Debug, Clone, PartialEq)] pub struct AcirCallWaitInfo { /// Index in the list of ACIR function's that should be called - pub id: u32, + pub id: AcirFunctionId, /// Initial witness for the given circuit to be called pub initial_witness: WitnessMap, } diff --git a/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs b/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs index c596dcf9614..98a0c4c3abe 100644 --- a/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs +++ b/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs @@ -250,7 +250,7 @@ impl<'a, B: BlackBoxFunctionSolver> ProgramExecutor<'a, B> { acvm.resolve_pending_foreign_call(result); } ACVMStatus::RequiresAcirCall(call_info) => { - let acir_to_call = &self.functions[call_info.id as usize]; + let acir_to_call = &self.functions[call_info.id.as_usize()]; let initial_witness = call_info.initial_witness; let call_solved_witness = self .execute_circuit(acir_to_call, initial_witness, witness_stack) @@ -267,7 +267,7 @@ impl<'a, B: BlackBoxFunctionSolver> ProgramExecutor<'a, B> { } } acvm.resolve_pending_acir_call(call_resolved_outputs); - witness_stack.push(call_info.id, call_solved_witness.clone()); + witness_stack.push(call_info.id.0, call_solved_witness.clone()); } } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index df7ae86a202..166b4ec2d49 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -8,7 +8,7 @@ use crate::ssa::ir::dfg::CallStack; use crate::ssa::ir::types::Type as SsaType; use crate::ssa::ir::{instruction::Endian, types::NumericType}; use acvm::acir::circuit::brillig::{BrilligFunctionId, BrilligInputs, BrilligOutputs}; -use acvm::acir::circuit::opcodes::{BlockId, BlockType, MemOp}; +use acvm::acir::circuit::opcodes::{AcirFunctionId, BlockId, BlockType, MemOp}; use acvm::acir::circuit::{AssertionPayload, ExpressionOrMemory, ExpressionWidth, Opcode}; use acvm::blackbox_solver; use acvm::brillig_vm::{MemoryValue, VMStatus, VM}; @@ -1983,7 +1983,7 @@ impl AcirContext { pub(crate) fn call_acir_function( &mut self, - id: u32, + id: AcirFunctionId, inputs: Vec, output_count: usize, predicate: AcirVar, diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index c4abeecd68c..346d6fcd921 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -29,7 +29,7 @@ use crate::brillig::brillig_ir::BrilligContext; use crate::brillig::{brillig_gen::brillig_fn::FunctionContext as BrilligFunctionContext, Brillig}; use crate::errors::{InternalError, InternalWarning, RuntimeError, SsaReport}; pub(crate) use acir_ir::generated_acir::GeneratedAcir; -use acvm::acir::circuit::opcodes::BlockType; +use acvm::acir::circuit::opcodes::{AcirFunctionId, BlockType}; use noirc_frontend::monomorphization::ast::InlineType; use acvm::acir::circuit::brillig::{BrilligBytecode, BrilligFunctionId}; @@ -775,7 +775,7 @@ impl<'a> Context<'a> { .get(id) .expect("ICE: should have an associated final index"); let output_vars = self.acir_context.call_acir_function( - *acir_function_id, + AcirFunctionId(*acir_function_id), inputs, output_count, self.current_side_effects_enabled_var, @@ -2867,9 +2867,13 @@ fn can_omit_element_sizes_array(array_typ: &Type) -> bool { #[cfg(test)] mod test { + use acvm::{ acir::{ - circuit::{brillig::BrilligFunctionId, ExpressionWidth, Opcode, OpcodeLocation}, + circuit::{ + brillig::BrilligFunctionId, opcodes::AcirFunctionId, ExpressionWidth, Opcode, + OpcodeLocation, + }, native_types::Witness, }, FieldElement, @@ -3020,8 +3024,18 @@ mod test { let main_opcodes = main_acir.opcodes(); assert_eq!(main_opcodes.len(), 3, "Should have two calls to `foo`"); - check_call_opcode(&main_opcodes[0], 1, vec![Witness(0), Witness(1)], vec![Witness(2)]); - check_call_opcode(&main_opcodes[1], 1, vec![Witness(0), Witness(1)], vec![Witness(3)]); + check_call_opcode( + &main_opcodes[0], + AcirFunctionId(1), + vec![Witness(0), Witness(1)], + vec![Witness(2)], + ); + check_call_opcode( + &main_opcodes[1], + AcirFunctionId(1), + vec![Witness(0), Witness(1)], + vec![Witness(3)], + ); if let Opcode::AssertZero(expr) = &main_opcodes[2] { assert_eq!(expr.linear_combinations[0].0, FieldElement::from(1u128)); @@ -3076,9 +3090,19 @@ mod test { let main_opcodes = main_acir.opcodes(); assert_eq!(main_opcodes.len(), 3, "Should have two calls to `foo` and an assert"); - check_call_opcode(&main_opcodes[0], 1, vec![Witness(0), Witness(1)], vec![Witness(2)]); + check_call_opcode( + &main_opcodes[0], + AcirFunctionId(1), + vec![Witness(0), Witness(1)], + vec![Witness(2)], + ); // The output of the first call should be the input of the second call - check_call_opcode(&main_opcodes[1], 1, vec![Witness(2), Witness(1)], vec![Witness(3)]); + check_call_opcode( + &main_opcodes[1], + AcirFunctionId(1), + vec![Witness(2), Witness(1)], + vec![Witness(3)], + ); } fn basic_nested_call(inline_type: InlineType) { @@ -3167,9 +3191,19 @@ mod test { assert_eq!(main_opcodes.len(), 3, "Should have two calls to `foo` and an assert"); // Both of these should call func_with_nested_foo_call f1 - check_call_opcode(&main_opcodes[0], 1, vec![Witness(0), Witness(1)], vec![Witness(2)]); + check_call_opcode( + &main_opcodes[0], + AcirFunctionId(1), + vec![Witness(0), Witness(1)], + vec![Witness(2)], + ); // The output of the first call should be the input of the second call - check_call_opcode(&main_opcodes[1], 1, vec![Witness(0), Witness(1)], vec![Witness(3)]); + check_call_opcode( + &main_opcodes[1], + AcirFunctionId(1), + vec![Witness(0), Witness(1)], + vec![Witness(3)], + ); let func_with_nested_call_acir = &acir_functions[1]; let func_with_nested_call_opcodes = func_with_nested_call_acir.opcodes(); @@ -3182,7 +3216,7 @@ mod test { // Should call foo f2 check_call_opcode( &func_with_nested_call_opcodes[1], - 2, + AcirFunctionId(2), vec![Witness(3), Witness(1)], vec![Witness(4)], ); @@ -3190,7 +3224,7 @@ mod test { fn check_call_opcode( opcode: &Opcode, - expected_id: u32, + expected_id: AcirFunctionId, expected_inputs: Vec, expected_outputs: Vec, ) { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs index 5b1139e5b9c..e5a25dcfef1 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs @@ -66,6 +66,8 @@ mod block; use std::collections::{BTreeMap, BTreeSet}; +use fxhash::FxHashMap as HashMap; + use crate::ssa::{ ir::{ basic_block::BasicBlockId, @@ -111,6 +113,10 @@ struct PerFunctionContext<'f> { /// We avoid removing individual instructions as we go since removing elements /// from the middle of Vecs many times will be slower than a single call to `retain`. instructions_to_remove: BTreeSet, + + /// Track a value's last load across all blocks. + /// If a value is not used in anymore loads we can remove the last store to that value. + last_loads: HashMap, } impl<'f> PerFunctionContext<'f> { @@ -124,6 +130,7 @@ impl<'f> PerFunctionContext<'f> { inserter: FunctionInserter::new(function), blocks: BTreeMap::new(), instructions_to_remove: BTreeSet::new(), + last_loads: HashMap::default(), } } @@ -140,6 +147,18 @@ impl<'f> PerFunctionContext<'f> { let references = self.find_starting_references(block); self.analyze_block(block, references); } + + // If we never load from an address within a function we can remove all stores to that address. + // This rule does not apply to reference parameters, which we must also check for before removing these stores. + for (block_id, block) in self.blocks.iter() { + let block_params = self.inserter.function.dfg.block_parameters(*block_id); + for (value, store_instruction) in block.last_stores.iter() { + let is_reference_param = block_params.contains(value); + if self.last_loads.get(value).is_none() && !is_reference_param { + self.instructions_to_remove.insert(*store_instruction); + } + } + } } /// The value of each reference at the start of the given block is the unification @@ -239,6 +258,8 @@ impl<'f> PerFunctionContext<'f> { self.instructions_to_remove.insert(instruction); } else { references.mark_value_used(address, self.inserter.function); + + self.last_loads.insert(address, instruction); } } Instruction::Store { address, value } => { @@ -594,10 +615,8 @@ mod tests { // acir fn main f0 { // b0(): // v7 = allocate - // store Field 5 at v7 // jmp b1(Field 5) // b1(v3: Field): - // store Field 6 at v7 // return v3, Field 5, Field 6 // } let ssa = ssa.mem2reg(); @@ -609,9 +628,9 @@ mod tests { assert_eq!(count_loads(main.entry_block(), &main.dfg), 0); assert_eq!(count_loads(b1, &main.dfg), 0); - // Neither store is removed since they are each the last in the block and there are multiple blocks - assert_eq!(count_stores(main.entry_block(), &main.dfg), 1); - assert_eq!(count_stores(b1, &main.dfg), 1); + // All stores are removed as there are no loads to the values being stored anywhere in the function. + assert_eq!(count_stores(main.entry_block(), &main.dfg), 0); + assert_eq!(count_stores(b1, &main.dfg), 0); // The jmp to b1 should also be a constant 5 now match main.dfg[main.entry_block()].terminator() { @@ -641,8 +660,8 @@ mod tests { // b1(): // store Field 1 at v3 // store Field 2 at v4 - // v8 = load v3 - // v9 = eq v8, Field 2 + // v7 = load v3 + // v8 = eq v7, Field 2 // return // } let main_id = Id::test_new(0); @@ -681,12 +700,9 @@ mod tests { // acir fn main f0 { // b0(): // v9 = allocate - // store Field 0 at v9 // v10 = allocate - // store v9 at v10 // jmp b1() // b1(): - // store Field 2 at v9 // return // } let ssa = ssa.mem2reg(); @@ -698,14 +714,17 @@ mod tests { assert_eq!(count_loads(main.entry_block(), &main.dfg), 0); assert_eq!(count_loads(b1, &main.dfg), 0); - // Only the first store in b1 is removed since there is another store to the same reference + // All stores should be removed. + // The first store in b1 is removed since there is another store to the same reference // in the same block, and the store is not needed before the later store. - assert_eq!(count_stores(main.entry_block(), &main.dfg), 2); - assert_eq!(count_stores(b1, &main.dfg), 1); + // The rest of the stores are also removed as no loads are done within any blocks + // to the stored values. + assert_eq!(count_stores(main.entry_block(), &main.dfg), 0); + assert_eq!(count_stores(b1, &main.dfg), 0); let b1_instructions = main.dfg[b1].instructions(); // We expect the last eq to be optimized out - assert_eq!(b1_instructions.len(), 1); + assert_eq!(b1_instructions.len(), 0); } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs index b31b8f36031..65e94c4fcf4 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -506,7 +506,7 @@ impl<'context> Elaborator<'context> { unseen_fields.remove(&field_name); seen_fields.insert(field_name.clone()); - self.unify_with_coercions(&field_type, expected_type, resolved, || { + self.unify_with_coercions(&field_type, expected_type, resolved, field_span, || { TypeCheckError::TypeMismatch { expected_typ: expected_type.to_string(), expr_typ: field_type.to_string(), diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/statements.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/statements.rs index 48380383eb0..da4492eb211 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/statements.rs @@ -78,7 +78,7 @@ impl<'context> Elaborator<'context> { let r#type = if annotated_type != Type::Error { // Now check if LHS is the same type as the RHS // Importantly, we do not coerce any types implicitly - self.unify_with_coercions(&expr_type, &annotated_type, expression, || { + self.unify_with_coercions(&expr_type, &annotated_type, expression, expr_span, || { TypeCheckError::TypeMismatch { expected_typ: annotated_type.to_string(), expr_typ: expr_type.to_string(), @@ -136,7 +136,7 @@ impl<'context> Elaborator<'context> { self.push_err(TypeCheckError::VariableMustBeMutable { name, span }); } - self.unify_with_coercions(&expr_type, &lvalue_type, expression, || { + self.unify_with_coercions(&expr_type, &lvalue_type, expression, span, || { TypeCheckError::TypeMismatchWithSource { actual: expr_type.clone(), expected: lvalue_type.clone(), diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs index 58d019c86aa..80c5aad7fd2 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs @@ -641,10 +641,18 @@ impl<'context> Elaborator<'context> { actual: &Type, expected: &Type, expression: ExprId, + span: Span, make_error: impl FnOnce() -> TypeCheckError, ) { let mut errors = Vec::new(); - actual.unify_with_coercions(expected, expression, self.interner, &mut errors, make_error); + actual.unify_with_coercions( + expected, + expression, + span, + self.interner, + &mut errors, + make_error, + ); self.errors.extend(errors.into_iter().map(|error| (error.into(), self.file))); } @@ -736,10 +744,12 @@ impl<'context> Elaborator<'context> { } for (param, (arg, arg_expr_id, arg_span)) in fn_params.iter().zip(callsite_args) { - self.unify_with_coercions(arg, param, *arg_expr_id, || TypeCheckError::TypeMismatch { - expected_typ: param.to_string(), - expr_typ: arg.to_string(), - expr_span: *arg_span, + self.unify_with_coercions(arg, param, *arg_expr_id, *arg_span, || { + TypeCheckError::TypeMismatch { + expected_typ: param.to_string(), + expr_typ: arg.to_string(), + expr_span: *arg_span, + } }); } @@ -1449,7 +1459,7 @@ impl<'context> Elaborator<'context> { }); } } else { - self.unify_with_coercions(&body_type, declared_return_type, body_id, || { + self.unify_with_coercions(&body_type, declared_return_type, body_id, func_span, || { let mut error = TypeCheckError::TypeMismatchWithSource { expected: declared_return_type.clone(), actual: body_type.clone(), diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index cec4b1fd434..f65c5ae8cdf 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -17,8 +17,8 @@ use rustc_hash::FxHashMap as HashMap; use crate::{ ast::{ - ExpressionKind, FunctionKind, FunctionReturnType, IntegerBitSize, Literal, UnaryOp, - UnresolvedType, UnresolvedTypeData, Visibility, + ArrayLiteral, ExpressionKind, FunctionKind, FunctionReturnType, IntegerBitSize, Literal, + UnaryOp, UnresolvedType, UnresolvedTypeData, Visibility, }, hir::comptime::{errors::IResult, value::add_token_spans, InterpreterError, Value}, hir_def::function::FunctionBody, @@ -47,6 +47,7 @@ impl<'local, 'context> Interpreter<'local, 'context> { "array_as_str_unchecked" => array_as_str_unchecked(interner, arguments, location), "array_len" => array_len(interner, arguments, location), "as_slice" => as_slice(interner, arguments, location), + "expr_as_array" => expr_as_array(arguments, return_type, location), "expr_as_binary_op" => expr_as_binary_op(arguments, return_type, location), "expr_as_bool" => expr_as_bool(arguments, return_type, location), "expr_as_function_call" => expr_as_function_call(arguments, return_type, location), @@ -54,8 +55,15 @@ impl<'local, 'context> Interpreter<'local, 'context> { "expr_as_index" => expr_as_index(arguments, return_type, location), "expr_as_integer" => expr_as_integer(arguments, return_type, location), "expr_as_member_access" => expr_as_member_access(arguments, return_type, location), - "expr_as_unary_op" => expr_as_unary_op(arguments, return_type, location), + "expr_as_repeated_element_array" => { + expr_as_repeated_element_array(arguments, return_type, location) + } + "expr_as_repeated_element_slice" => { + expr_as_repeated_element_slice(arguments, return_type, location) + } + "expr_as_slice" => expr_as_slice(arguments, return_type, location), "expr_as_tuple" => expr_as_tuple(arguments, return_type, location), + "expr_as_unary_op" => expr_as_unary_op(arguments, return_type, location), "is_unconstrained" => Ok(Value::Bool(true)), "function_def_name" => function_def_name(interner, arguments, location), "function_def_parameters" => function_def_parameters(interner, arguments, location), @@ -758,6 +766,56 @@ fn zeroed(return_type: Type) -> IResult { } } +// fn as_array(self) -> Option<[Expr]> +fn expr_as_array( + arguments: Vec<(Value, Location)>, + return_type: Type, + location: Location, +) -> IResult { + expr_as(arguments, return_type, location, |expr| { + if let ExpressionKind::Literal(Literal::Array(ArrayLiteral::Standard(exprs))) = expr { + let exprs = exprs.into_iter().map(|expr| Value::Expr(expr.kind)).collect(); + let typ = Type::Slice(Box::new(Type::Quoted(QuotedType::Expr))); + Some(Value::Slice(exprs, typ)) + } else { + None + } + }) +} + +// fn as_binary_op(self) -> Option<(Expr, BinaryOp, Expr)> +fn expr_as_binary_op( + arguments: Vec<(Value, Location)>, + return_type: Type, + location: Location, +) -> IResult { + expr_as(arguments, return_type.clone(), location, |expr| { + if let ExpressionKind::Infix(infix_expr) = expr { + let option_type = extract_option_generic_type(return_type); + let Type::Tuple(mut tuple_types) = option_type else { + panic!("Expected the return type option generic arg to be a tuple"); + }; + assert_eq!(tuple_types.len(), 3); + + tuple_types.pop().unwrap(); + let binary_op_type = tuple_types.pop().unwrap(); + + // For the op value we use the enum member index, which should match noir_stdlib/src/meta/op.nr + let binary_op_value = infix_expr.operator.contents as u128; + + let mut fields = HashMap::default(); + fields.insert(Rc::new("op".to_string()), Value::Field(binary_op_value.into())); + + let unary_op = Value::Struct(fields, binary_op_type); + let lhs = Value::Expr(infix_expr.lhs.kind); + let rhs = Value::Expr(infix_expr.rhs.kind); + Some(Value::Tuple(vec![lhs, unary_op, rhs])) + } else { + None + } + }) +} + // fn as_bool(self) -> Option fn expr_as_bool( arguments: Vec<(Value, Location)>, @@ -872,70 +930,55 @@ fn expr_as_member_access( }) } -// fn as_unary_op(self) -> Option<(UnaryOp, Expr)> -fn expr_as_unary_op( +// fn as_repeated_element_array(self) -> Option<(Expr, Expr)> +fn expr_as_repeated_element_array( arguments: Vec<(Value, Location)>, return_type: Type, location: Location, ) -> IResult { - expr_as(arguments, return_type.clone(), location, |expr| { - if let ExpressionKind::Prefix(prefix_expr) = expr { - let option_type = extract_option_generic_type(return_type); - let Type::Tuple(mut tuple_types) = option_type else { - panic!("Expected the return type option generic arg to be a tuple"); - }; - assert_eq!(tuple_types.len(), 2); - - tuple_types.pop().unwrap(); - let unary_op_type = tuple_types.pop().unwrap(); - - // These values should match the values used in noir_stdlib/src/meta/op.nr - let unary_op_value: u128 = match prefix_expr.operator { - UnaryOp::Minus => 0, - UnaryOp::Not => 1, - UnaryOp::MutableReference => 2, - UnaryOp::Dereference { .. } => 3, - }; - - let mut fields = HashMap::default(); - fields.insert(Rc::new("op".to_string()), Value::Field(unary_op_value.into())); - - let unary_op = Value::Struct(fields, unary_op_type); - let rhs = Value::Expr(prefix_expr.rhs.kind); - Some(Value::Tuple(vec![unary_op, rhs])) + expr_as(arguments, return_type, location, |expr| { + if let ExpressionKind::Literal(Literal::Array(ArrayLiteral::Repeated { + repeated_element, + length, + })) = expr + { + Some(Value::Tuple(vec![Value::Expr(repeated_element.kind), Value::Expr(length.kind)])) } else { None } }) } -// fn as_binary_op(self) -> Option<(Expr, BinaryOp, Expr)> -fn expr_as_binary_op( +// fn as_repeated_element_slice(self) -> Option<(Expr, Expr)> +fn expr_as_repeated_element_slice( arguments: Vec<(Value, Location)>, return_type: Type, location: Location, ) -> IResult { - expr_as(arguments, return_type.clone(), location, |expr| { - if let ExpressionKind::Infix(infix_expr) = expr { - let option_type = extract_option_generic_type(return_type); - let Type::Tuple(mut tuple_types) = option_type else { - panic!("Expected the return type option generic arg to be a tuple"); - }; - assert_eq!(tuple_types.len(), 3); - - tuple_types.pop().unwrap(); - let binary_op_type = tuple_types.pop().unwrap(); - - // For the op value we use the enum member index, which should match noir_stdlib/src/meta/op.nr - let binary_op_value = infix_expr.operator.contents as u128; - - let mut fields = HashMap::default(); - fields.insert(Rc::new("op".to_string()), Value::Field(binary_op_value.into())); + expr_as(arguments, return_type, location, |expr| { + if let ExpressionKind::Literal(Literal::Slice(ArrayLiteral::Repeated { + repeated_element, + length, + })) = expr + { + Some(Value::Tuple(vec![Value::Expr(repeated_element.kind), Value::Expr(length.kind)])) + } else { + None + } + }) +} - let unary_op = Value::Struct(fields, binary_op_type); - let lhs = Value::Expr(infix_expr.lhs.kind); - let rhs = Value::Expr(infix_expr.rhs.kind); - Some(Value::Tuple(vec![lhs, unary_op, rhs])) +// fn as_slice(self) -> Option<[Expr]> +fn expr_as_slice( + arguments: Vec<(Value, Location)>, + return_type: Type, + location: Location, +) -> IResult { + expr_as(arguments, return_type, location, |expr| { + if let ExpressionKind::Literal(Literal::Slice(ArrayLiteral::Standard(exprs))) = expr { + let exprs = exprs.into_iter().map(|expr| Value::Expr(expr.kind)).collect(); + let typ = Type::Slice(Box::new(Type::Quoted(QuotedType::Expr))); + Some(Value::Slice(exprs, typ)) } else { None } @@ -959,6 +1002,43 @@ fn expr_as_tuple( }) } +// fn as_unary_op(self) -> Option<(UnaryOp, Expr)> +fn expr_as_unary_op( + arguments: Vec<(Value, Location)>, + return_type: Type, + location: Location, +) -> IResult { + expr_as(arguments, return_type.clone(), location, |expr| { + if let ExpressionKind::Prefix(prefix_expr) = expr { + let option_type = extract_option_generic_type(return_type); + let Type::Tuple(mut tuple_types) = option_type else { + panic!("Expected the return type option generic arg to be a tuple"); + }; + assert_eq!(tuple_types.len(), 2); + + tuple_types.pop().unwrap(); + let unary_op_type = tuple_types.pop().unwrap(); + + // These values should match the values used in noir_stdlib/src/meta/op.nr + let unary_op_value: u128 = match prefix_expr.operator { + UnaryOp::Minus => 0, + UnaryOp::Not => 1, + UnaryOp::MutableReference => 2, + UnaryOp::Dereference { .. } => 3, + }; + + let mut fields = HashMap::default(); + fields.insert(Rc::new("op".to_string()), Value::Field(unary_op_value.into())); + + let unary_op = Value::Struct(fields, unary_op_type); + let rhs = Value::Expr(prefix_expr.rhs.kind); + Some(Value::Tuple(vec![unary_op, rhs])) + } else { + None + } + }) +} + // Helper function for implementing the `expr_as_...` functions. fn expr_as( arguments: Vec<(Value, Location)>, diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/type_check/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/type_check/errors.rs index 380753d8198..d51760f9d20 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -138,6 +138,8 @@ pub enum TypeCheckError { UnconstrainedSliceReturnToConstrained { span: Span }, #[error("Call to unconstrained function is unsafe and must be in an unconstrained function or unsafe block")] Unsafe { span: Span }, + #[error("Converting an unconstrained fn to a non-unconstrained fn is unsafe")] + UnsafeFn { span: Span }, #[error("Slices must have constant length")] NonConstantSliceLength { span: Span }, #[error("Only sized types may be used in the entry point to a program")] @@ -361,6 +363,9 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic { TypeCheckError::Unsafe { span } => { Diagnostic::simple_warning(error.to_string(), String::new(), *span) } + TypeCheckError::UnsafeFn { span } => { + Diagnostic::simple_warning(error.to_string(), String::new(), *span) + } } } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/types.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/types.rs index d2f499950f1..d6d114c7075 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/types.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/types.rs @@ -205,6 +205,12 @@ impl ResolvedGeneric { } } +enum FunctionCoercionResult { + NoCoercion, + Coerced(Type), + UnconstrainedMismatch(Type), +} + impl std::hash::Hash for StructType { fn hash(&self, state: &mut H) { self.id.hash(state); @@ -1776,6 +1782,7 @@ impl Type { &self, expected: &Type, expression: ExprId, + span: Span, interner: &mut NodeInterner, errors: &mut Vec, make_error: impl FnOnce() -> TypeCheckError, @@ -1792,17 +1799,24 @@ impl Type { } // Try to coerce `fn (..) -> T` to `unconstrained fn (..) -> T` - if let Some(coerced_self) = self.try_fn_to_unconstrained_fn_coercion(expected) { - coerced_self.unify_with_coercions(expected, expression, interner, errors, make_error); - return; - } + match self.try_fn_to_unconstrained_fn_coercion(expected) { + FunctionCoercionResult::NoCoercion => errors.push(make_error()), + FunctionCoercionResult::Coerced(coerced_self) => { + coerced_self + .unify_with_coercions(expected, expression, span, interner, errors, make_error); + } + FunctionCoercionResult::UnconstrainedMismatch(coerced_self) => { + errors.push(TypeCheckError::UnsafeFn { span }); - errors.push(make_error()); + coerced_self + .unify_with_coercions(expected, expression, span, interner, errors, make_error); + } + } } // If `self` and `expected` are function types, tries to coerce `self` to `expected`. // Returns None if no coercion can be applied, otherwise returns `self` coerced to `expected`. - fn try_fn_to_unconstrained_fn_coercion(&self, expected: &Type) -> Option { + fn try_fn_to_unconstrained_fn_coercion(&self, expected: &Type) -> FunctionCoercionResult { // If `self` and `expected` are function types, `self` can be coerced to `expected` // if `self` is unconstrained and `expected` is not. The other way around is an error, though. if let ( @@ -1810,10 +1824,15 @@ impl Type { Type::Function(_, _, _, unconstrained_expected), ) = (self.follow_bindings(), expected.follow_bindings()) { - (!unconstrained_self && unconstrained_expected) - .then(|| Type::Function(params, ret, env, unconstrained_expected)) + let coerced_type = Type::Function(params, ret, env, unconstrained_expected); + + match (unconstrained_self, unconstrained_expected) { + (true, true) | (false, false) => FunctionCoercionResult::NoCoercion, + (false, true) => FunctionCoercionResult::Coerced(coerced_type), + (true, false) => FunctionCoercionResult::UnconstrainedMismatch(coerced_type), + } } else { - None + FunctionCoercionResult::NoCoercion } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/errors.rs index 72719adb4f8..ebb58ddc224 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/errors.rs @@ -20,8 +20,16 @@ pub enum ParserErrorReason { ExpectedIdentifierAfterDot, #[error("expected an identifier after ::")] ExpectedIdentifierAfterColons, + #[error("expected {{ or -> after function parameters")] + ExpectedLeftBraceOrArrowAfterFunctionParameters, #[error("expected {{ after if condition")] ExpectedLeftBraceAfterIfCondition, + #[error("expected <, where or {{ after trait name")] + ExpectedLeftBracketOrWhereOrLeftBraceOrArrowAfterTraitName, + #[error("expected <, where or {{ after impl type")] + ExpectedLeftBracketOrWhereOrLeftBraceOrArrowAfterImplType, + #[error("expected <, where or {{ after trait impl for type")] + ExpectedLeftBracketOrWhereOrLeftBraceOrArrowAfterTraitImplForType, #[error("Expected a ; separating these two statements")] MissingSeparatingSemi, #[error("constrain keyword is deprecated")] diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser.rs index ed92143756c..eddf85becfe 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser.rs @@ -219,13 +219,27 @@ fn top_level_statement<'a>( /// /// implementation: 'impl' generics type '{' function_definition ... '}' fn implementation() -> impl NoirParser { + let methods_or_error = just(Token::LeftBrace) + .ignore_then(spanned(function::function_definition(true)).repeated()) + .then_ignore(just(Token::RightBrace)) + .or_not() + .validate(|methods, span, emit| { + if let Some(methods) = methods { + methods + } else { + emit(ParserError::with_reason( + ParserErrorReason::ExpectedLeftBracketOrWhereOrLeftBraceOrArrowAfterImplType, + span, + )); + vec![] + } + }); + keyword(Keyword::Impl) .ignore_then(function::generics()) .then(parse_type().map_with_span(|typ, span| (typ, span))) .then(where_clause()) - .then_ignore(just(Token::LeftBrace)) - .then(spanned(function::function_definition(true)).repeated()) - .then_ignore(just(Token::RightBrace)) + .then(methods_or_error) .map(|args| { let ((other_args, where_clause), methods) = args; let (generics, (object_type, type_span)) = other_args; @@ -1760,4 +1774,21 @@ mod test { assert_eq!(var.to_string(), "foo"); } + + #[test] + fn parse_recover_impl_without_body() { + let src = "impl Foo"; + + let (top_level_statement, errors) = parse_recover(implementation(), src); + assert_eq!(errors.len(), 1); + assert_eq!(errors[0].message, "expected <, where or { after impl type"); + + let top_level_statement = top_level_statement.unwrap(); + let TopLevelStatement::Impl(impl_) = top_level_statement else { + panic!("Expected to parse an impl"); + }; + + assert_eq!(impl_.object_type.to_string(), "Foo"); + assert!(impl_.methods.is_empty()); + } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/function.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/function.rs index 3de48d2e02a..56760898374 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/function.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/function.rs @@ -6,7 +6,10 @@ use super::{ self_parameter, where_clause, NoirParser, }; use crate::token::{Keyword, Token, TokenKind}; -use crate::{ast::IntegerBitSize, parser::spanned}; +use crate::{ + ast::{BlockExpression, IntegerBitSize}, + parser::spanned, +}; use crate::{ ast::{ FunctionDefinition, FunctionReturnType, ItemVisibility, NoirFunction, Param, Visibility, @@ -20,10 +23,24 @@ use crate::{ }; use chumsky::prelude::*; +use noirc_errors::Span; /// function_definition: attribute function_modifiers 'fn' ident generics '(' function_parameters ')' function_return_type block /// function_modifiers 'fn' ident generics '(' function_parameters ')' function_return_type block pub(super) fn function_definition(allow_self: bool) -> impl NoirParser { + let body_or_error = + spanned(block(fresh_statement()).or_not()).validate(|(body, body_span), span, emit| { + if let Some(body) = body { + (body, body_span) + } else { + emit(ParserError::with_reason( + ParserErrorReason::ExpectedLeftBraceOrArrowAfterFunctionParameters, + span, + )); + (BlockExpression { statements: vec![] }, Span::from(span.end()..span.end())) + } + }); + attributes() .then(function_modifiers()) .then_ignore(keyword(Keyword::Fn)) @@ -32,7 +49,7 @@ pub(super) fn function_definition(allow_self: bool) -> impl NoirParser after function parameters"); + + let noir_function = noir_function.unwrap(); + assert_eq!(noir_function.name(), "foo"); + assert_eq!(noir_function.parameters().len(), 1); + assert!(noir_function.def.body.statements.is_empty()); + } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/traits.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/traits.rs index 0874cadd34e..0cf5e63f5f3 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/traits.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/traits.rs @@ -23,14 +23,28 @@ use crate::{ use super::{generic_type_args, parse_type, primitives::ident}; pub(super) fn trait_definition() -> impl NoirParser { + let trait_body_or_error = just(Token::LeftBrace) + .ignore_then(trait_body()) + .then_ignore(just(Token::RightBrace)) + .or_not() + .validate(|items, span, emit| { + if let Some(items) = items { + items + } else { + emit(ParserError::with_reason( + ParserErrorReason::ExpectedLeftBracketOrWhereOrLeftBraceOrArrowAfterTraitName, + span, + )); + vec![] + } + }); + attributes() .then_ignore(keyword(Keyword::Trait)) .then(ident()) .then(function::generics()) .then(where_clause()) - .then_ignore(just(Token::LeftBrace)) - .then(trait_body()) - .then_ignore(just(Token::RightBrace)) + .then(trait_body_or_error) .validate(|((((attributes, name), generics), where_clause), items), span, emit| { let attributes = validate_secondary_attributes(attributes, span, emit); TopLevelStatement::Trait(NoirTrait { @@ -70,13 +84,26 @@ fn trait_function_declaration() -> impl NoirParser { let trait_function_body_or_semicolon = block(fresh_statement()).map(Option::from).or(just(Token::Semicolon).to(Option::None)); + let trait_function_body_or_semicolon_or_error = + trait_function_body_or_semicolon.or_not().validate(|body, span, emit| { + if let Some(body) = body { + body + } else { + emit(ParserError::with_reason( + ParserErrorReason::ExpectedLeftBraceOrArrowAfterFunctionParameters, + span, + )); + None + } + }); + keyword(Keyword::Fn) .ignore_then(ident()) .then(function::generics()) .then(parenthesized(function_declaration_parameters())) .then(function_return_type().map(|(_, typ)| typ)) .then(where_clause()) - .then(trait_function_body_or_semicolon) + .then(trait_function_body_or_semicolon_or_error) .map(|(((((name, generics), parameters), return_type), where_clause), body)| { TraitItem::Function { name, generics, parameters, return_type, where_clause, body } }) @@ -101,6 +128,24 @@ fn trait_type_declaration() -> impl NoirParser { /// /// trait_implementation: 'impl' generics ident generic_args for type '{' trait_implementation_body '}' pub(super) fn trait_implementation() -> impl NoirParser { + let body_or_error = + just(Token::LeftBrace) + .ignore_then(trait_implementation_body()) + .then_ignore(just(Token::RightBrace)) + .or_not() + .validate(|items, span, emit| { + if let Some(items) = items { + items + } else { + emit(ParserError::with_reason( + ParserErrorReason::ExpectedLeftBracketOrWhereOrLeftBraceOrArrowAfterTraitImplForType, + span, + )); + + vec![] + } + }); + keyword(Keyword::Impl) .ignore_then(function::generics()) .then(path_no_turbofish()) @@ -108,13 +153,10 @@ pub(super) fn trait_implementation() -> impl NoirParser { .then_ignore(keyword(Keyword::For)) .then(parse_type()) .then(where_clause()) - .then_ignore(just(Token::LeftBrace)) - .then(trait_implementation_body()) - .then_ignore(just(Token::RightBrace)) + .then(body_or_error) .map(|args| { let (((other_args, object_type), where_clause), items) = args; let ((impl_generics, trait_name), trait_generics) = other_args; - TopLevelStatement::TraitImpl(NoirTraitImpl { impl_generics, trait_name, @@ -227,4 +269,54 @@ mod test { vec!["trait MissingBody", "trait WrongDelimiter { fn foo() -> u8, fn bar() -> u8 }"], ); } + + #[test] + fn parse_recover_function_without_left_brace_or_semicolon() { + let src = "fn foo(x: i32)"; + + let (trait_item, errors) = parse_recover(trait_function_declaration(), src); + assert_eq!(errors.len(), 1); + assert_eq!(errors[0].message, "expected { or -> after function parameters"); + + let Some(TraitItem::Function { name, parameters, body, .. }) = trait_item else { + panic!("Expected to parser trait item as function"); + }; + + assert_eq!(name.to_string(), "foo"); + assert_eq!(parameters.len(), 1); + assert!(body.is_none()); + } + + #[test] + fn parse_recover_trait_without_body() { + let src = "trait Foo"; + + let (top_level_statement, errors) = parse_recover(trait_definition(), src); + assert_eq!(errors.len(), 1); + assert_eq!(errors[0].message, "expected <, where or { after trait name"); + + let top_level_statement = top_level_statement.unwrap(); + let TopLevelStatement::Trait(trait_) = top_level_statement else { + panic!("Expected to parse a trait"); + }; + + assert_eq!(trait_.name.to_string(), "Foo"); + assert!(trait_.items.is_empty()); + } + + #[test] + fn parse_recover_trait_impl_without_body() { + let src = "impl Foo for Bar"; + + let (top_level_statement, errors) = parse_recover(trait_implementation(), src); + assert_eq!(errors.len(), 1); + assert_eq!(errors[0].message, "expected <, where or { after trait impl for type"); + + let top_level_statement = top_level_statement.unwrap(); + let TopLevelStatement::TraitImpl(trait_impl) = top_level_statement else { + panic!("Expected to parse a trait impl"); + }; + + assert!(trait_impl.items.is_empty()); + } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs index 26a82216eee..bba596ed19f 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs @@ -2562,16 +2562,8 @@ fn cannot_pass_unconstrained_function_to_regular_function() { let errors = get_program_errors(src); assert_eq!(errors.len(), 1); - if let CompilationError::TypeError(TypeCheckError::TypeMismatch { - expected_typ, - expr_typ, - .. - }) = &errors[0].0 - { - assert_eq!(expected_typ, "fn() -> ()"); - assert_eq!(expr_typ, "unconstrained fn() -> ()"); - } else { - panic!("Expected a type mismatch error, got {:?}", errors[0].0); + let CompilationError::TypeError(TypeCheckError::UnsafeFn { .. }) = &errors[0].0 else { + panic!("Expected an UnsafeFn error, got {:?}", errors[0].0); }; } @@ -2630,8 +2622,8 @@ fn cannot_pass_unconstrained_function_to_constrained_function() { let errors = get_program_errors(src); assert_eq!(errors.len(), 1); - let CompilationError::TypeError(TypeCheckError::TypeMismatch { .. }) = &errors[0].0 else { - panic!("Expected a type mismatch error, got {:?}", errors[0].0); + let CompilationError::TypeError(TypeCheckError::UnsafeFn { .. }) = &errors[0].0 else { + panic!("Expected an UnsafeFn error, got {:?}", errors[0].0); }; } diff --git a/noir/noir-repo/noir_stdlib/src/cmp.nr b/noir/noir-repo/noir_stdlib/src/cmp.nr index 10182ca83b0..ec979d60753 100644 --- a/noir/noir-repo/noir_stdlib/src/cmp.nr +++ b/noir/noir-repo/noir_stdlib/src/cmp.nr @@ -18,10 +18,12 @@ impl Eq for Field { fn eq(self, other: Field) -> bool { self == other } } impl Eq for u64 { fn eq(self, other: u64) -> bool { self == other } } impl Eq for u32 { fn eq(self, other: u32) -> bool { self == other } } +impl Eq for u16 { fn eq(self, other: u16) -> bool { self == other } } impl Eq for u8 { fn eq(self, other: u8) -> bool { self == other } } impl Eq for u1 { fn eq(self, other: u1) -> bool { self == other } } impl Eq for i8 { fn eq(self, other: i8) -> bool { self == other } } +impl Eq for i16 { fn eq(self, other: i16) -> bool { self == other } } impl Eq for i32 { fn eq(self, other: i32) -> bool { self == other } } impl Eq for i64 { fn eq(self, other: i64) -> bool { self == other } } @@ -157,6 +159,18 @@ impl Ord for u32 { } } +impl Ord for u16 { + fn cmp(self, other: u16) -> Ordering { + if self < other { + Ordering::less() + } else if self > other { + Ordering::greater() + } else { + Ordering::equal() + } + } +} + impl Ord for u8 { fn cmp(self, other: u8) -> Ordering { if self < other { @@ -181,6 +195,18 @@ impl Ord for i8 { } } +impl Ord for i16 { + fn cmp(self, other: i16) -> Ordering { + if self < other { + Ordering::less() + } else if self > other { + Ordering::greater() + } else { + Ordering::equal() + } + } +} + impl Ord for i32 { fn cmp(self, other: i32) -> Ordering { if self < other { diff --git a/noir/noir-repo/noir_stdlib/src/default.nr b/noir/noir-repo/noir_stdlib/src/default.nr index f9399bfb865..3ac5fbb394e 100644 --- a/noir/noir-repo/noir_stdlib/src/default.nr +++ b/noir/noir-repo/noir_stdlib/src/default.nr @@ -17,11 +17,14 @@ comptime fn derive_default(s: StructDefinition) -> Quoted { impl Default for Field { fn default() -> Field { 0 } } +impl Default for u1 { fn default() -> u1 { 0 } } impl Default for u8 { fn default() -> u8 { 0 } } +impl Default for u16 { fn default() -> u16 { 0 } } impl Default for u32 { fn default() -> u32 { 0 } } impl Default for u64 { fn default() -> u64 { 0 } } impl Default for i8 { fn default() -> i8 { 0 } } +impl Default for i16 { fn default() -> i16 { 0 } } impl Default for i32 { fn default() -> i32 { 0 } } impl Default for i64 { fn default() -> i64 { 0 } } diff --git a/noir/noir-repo/noir_stdlib/src/hash/mod.nr b/noir/noir-repo/noir_stdlib/src/hash/mod.nr index 8a546e64309..3abc630ab10 100644 --- a/noir/noir-repo/noir_stdlib/src/hash/mod.nr +++ b/noir/noir-repo/noir_stdlib/src/hash/mod.nr @@ -195,12 +195,24 @@ impl Hash for Field { } } +impl Hash for u1 { + fn hash(self, state: &mut H) where H: Hasher{ + H::write(state, self as Field); + } +} + impl Hash for u8 { fn hash(self, state: &mut H) where H: Hasher{ H::write(state, self as Field); } } +impl Hash for u16 { + fn hash(self, state: &mut H) where H: Hasher{ + H::write(state, self as Field); + } +} + impl Hash for u32 { fn hash(self, state: &mut H) where H: Hasher{ H::write(state, self as Field); @@ -219,6 +231,12 @@ impl Hash for i8 { } } +impl Hash for i16 { + fn hash(self, state: &mut H) where H: Hasher{ + H::write(state, self as Field); + } +} + impl Hash for i32 { fn hash(self, state: &mut H) where H: Hasher{ H::write(state, self as Field); diff --git a/noir/noir-repo/noir_stdlib/src/meta/expr.nr b/noir/noir-repo/noir_stdlib/src/meta/expr.nr index 55c49531017..94889b7c3dd 100644 --- a/noir/noir-repo/noir_stdlib/src/meta/expr.nr +++ b/noir/noir-repo/noir_stdlib/src/meta/expr.nr @@ -3,6 +3,9 @@ use crate::meta::op::UnaryOp; use crate::meta::op::BinaryOp; impl Expr { + #[builtin(expr_as_array)] + fn as_array(self) -> Option<[Expr]> {} + #[builtin(expr_as_integer)] fn as_integer(self) -> Option<(Field, bool)> {} @@ -24,9 +27,202 @@ impl Expr { #[builtin(expr_as_member_access)] fn as_member_access(self) -> Option<(Expr, Quoted)> {} + #[builtin(expr_as_repeated_element_array)] + fn as_repeated_element_array(self) -> Option<(Expr, Expr)> {} + + #[builtin(expr_as_repeated_element_slice)] + fn as_repeated_element_slice(self) -> Option<(Expr, Expr)> {} + + #[builtin(expr_as_slice)] + fn as_slice(self) -> Option<[Expr]> {} + #[builtin(expr_as_tuple)] fn as_tuple(self) -> Option<[Expr]> {} #[builtin(expr_as_unary_op)] fn as_unary_op(self) -> Option<(UnaryOp, Expr)> {} } + +mod tests { + use crate::meta::op::UnaryOp; + use crate::meta::op::BinaryOp; + + #[test] + fn test_expr_as_array() { + comptime + { + let expr = quote { [1, 2, 4] }.as_expr().unwrap(); + let elems = expr.as_array().unwrap(); + assert_eq(elems.len(), 3); + assert_eq(elems[0].as_integer().unwrap(), (1, false)); + assert_eq(elems[1].as_integer().unwrap(), (2, false)); + assert_eq(elems[2].as_integer().unwrap(), (4, false)); + } + } + + #[test] + fn test_expr_as_integer() { + comptime + { + let expr = quote { 1 }.as_expr().unwrap(); + assert_eq((1, false), expr.as_integer().unwrap()); + + let expr = quote { -2 }.as_expr().unwrap(); + assert_eq((2, true), expr.as_integer().unwrap()); + } + } + + #[test] + fn test_expr_as_binary_op() { + comptime + { + assert(get_binary_op(quote { x + y }).is_add()); + assert(get_binary_op(quote { x - y }).is_subtract()); + assert(get_binary_op(quote { x * y }).is_multiply()); + assert(get_binary_op(quote { x / y }).is_divide()); + assert(get_binary_op(quote { x == y }).is_equal()); + assert(get_binary_op(quote { x != y }).is_not_equal()); + assert(get_binary_op(quote { x > y }).is_greater()); + assert(get_binary_op(quote { x >= y }).is_greater_or_equal()); + assert(get_binary_op(quote { x & y }).is_and()); + assert(get_binary_op(quote { x | y }).is_or()); + assert(get_binary_op(quote { x ^ y }).is_xor()); + assert(get_binary_op(quote { x >> y }).is_shift_right()); + assert(get_binary_op(quote { x << y }).is_shift_left()); + assert(get_binary_op(quote { x % y }).is_modulo()); + } + } + + #[test] + fn test_expr_as_bool() { + comptime + { + let expr = quote { false }.as_expr().unwrap(); + assert(expr.as_bool().unwrap() == false); + + let expr = quote { true }.as_expr().unwrap(); + assert_eq(expr.as_bool().unwrap(), true); + } + } + + #[test] + fn test_expr_as_function_call() { + comptime + { + let expr = quote { foo(42) }.as_expr().unwrap(); + let (_function, args) = expr.as_function_call().unwrap(); + assert_eq(args.len(), 1); + assert_eq(args[0].as_integer().unwrap(), (42, false)); + } + } + + #[test] + fn test_expr_as_if() { + comptime + { + let expr = quote { if 1 { 2 } }.as_expr().unwrap(); + let (_condition, _consequence, alternative) = expr.as_if().unwrap(); + assert(alternative.is_none()); + + let expr = quote { if 1 { 2 } else { 3 } }.as_expr().unwrap(); + let (_condition, _consequence, alternative) = expr.as_if().unwrap(); + assert(alternative.is_some()); + } + } + + #[test] + fn test_expr_as_index() { + comptime + { + let expr = quote { foo[bar] }.as_expr().unwrap(); + assert(expr.as_index().is_some()); + } + } + + #[test] + fn test_expr_as_member_access() { + comptime + { + let expr = quote { foo.bar }.as_expr().unwrap(); + let (_, name) = expr.as_member_access().unwrap(); + assert_eq(name, quote { bar }); + } + } + + #[test] + fn test_expr_as_repeated_element_array() { + comptime + { + let expr = quote { [1; 3] }.as_expr().unwrap(); + let (expr, length) = expr.as_repeated_element_array().unwrap(); + assert_eq(expr.as_integer().unwrap(), (1, false)); + assert_eq(length.as_integer().unwrap(), (3, false)); + } + } + + #[test] + fn test_expr_as_repeated_element_slice() { + comptime + { + let expr = quote { &[1; 3] }.as_expr().unwrap(); + let (expr, length) = expr.as_repeated_element_slice().unwrap(); + assert_eq(expr.as_integer().unwrap(), (1, false)); + assert_eq(length.as_integer().unwrap(), (3, false)); + } + } + + #[test] + fn test_expr_as_slice() { + comptime + { + let expr = quote { &[1, 3, 5] }.as_expr().unwrap(); + let elems = expr.as_slice().unwrap(); + assert_eq(elems.len(), 3); + assert_eq(elems[0].as_integer().unwrap(), (1, false)); + assert_eq(elems[1].as_integer().unwrap(), (3, false)); + assert_eq(elems[2].as_integer().unwrap(), (5, false)); + } + } + + #[test] + fn test_expr_as_tuple() { + comptime + { + let expr = quote { (1, 2) }.as_expr().unwrap(); + let tuple_exprs = expr.as_tuple().unwrap(); + assert_eq(tuple_exprs.len(), 2); + } + } + + #[test] + fn test_expr_as_unary_op() { + comptime + { + assert(get_unary_op(quote { -x }).is_minus()); + assert(get_unary_op(quote { !x }).is_not()); + assert(get_unary_op(quote { &mut x }).is_mutable_reference()); + assert(get_unary_op(quote { *x }).is_dereference()); + } + } + + #[test] + fn test_automatically_unwraps_parenthesized_expression() { + comptime + { + let expr = quote { ((if 1 { 2 })) }.as_expr().unwrap(); + assert(expr.as_if().is_some()); + } + } + + comptime fn get_unary_op(quoted: Quoted) -> UnaryOp { + let expr = quoted.as_expr().unwrap(); + let (op, _) = expr.as_unary_op().unwrap(); + op + } + + comptime fn get_binary_op(quoted: Quoted) -> BinaryOp { + let expr = quoted.as_expr().unwrap(); + let (_, op, _) = expr.as_binary_op().unwrap(); + op + } +} diff --git a/noir/noir-repo/noir_stdlib/src/ops/arith.nr b/noir/noir-repo/noir_stdlib/src/ops/arith.nr index df0ff978a7c..918c8e9bc28 100644 --- a/noir/noir-repo/noir_stdlib/src/ops/arith.nr +++ b/noir/noir-repo/noir_stdlib/src/ops/arith.nr @@ -10,6 +10,7 @@ impl Add for u64 { fn add(self, other: u64) -> u64 { self + other } } impl Add for u32 { fn add(self, other: u32) -> u32 { self + other } } impl Add for u16 { fn add(self, other: u16) -> u16 { self + other } } impl Add for u8 { fn add(self, other: u8) -> u8 { self + other } } +impl Add for u1 { fn add(self, other: u1) -> u1 { self + other } } impl Add for i8 { fn add(self, other: i8) -> i8 { self + other } } impl Add for i16 { fn add(self, other: i16) -> i16 { self + other } } @@ -28,6 +29,7 @@ impl Sub for u64 { fn sub(self, other: u64) -> u64 { self - other } } impl Sub for u32 { fn sub(self, other: u32) -> u32 { self - other } } impl Sub for u16 { fn sub(self, other: u16) -> u16 { self - other } } impl Sub for u8 { fn sub(self, other: u8) -> u8 { self - other } } +impl Sub for u1 { fn sub(self, other: u1) -> u1 { self - other } } impl Sub for i8 { fn sub(self, other: i8) -> i8 { self - other } } impl Sub for i16 { fn sub(self, other: i16) -> i16 { self - other } } @@ -46,6 +48,7 @@ impl Mul for u64 { fn mul(self, other: u64) -> u64 { self * other } } impl Mul for u32 { fn mul(self, other: u32) -> u32 { self * other } } impl Mul for u16 { fn mul(self, other: u16) -> u16 { self * other } } impl Mul for u8 { fn mul(self, other: u8) -> u8 { self * other } } +impl Mul for u1 { fn mul(self, other: u1) -> u1 { self * other } } impl Mul for i8 { fn mul(self, other: i8) -> i8 { self * other } } impl Mul for i16 { fn mul(self, other: i16) -> i16 { self * other } } @@ -64,6 +67,7 @@ impl Div for u64 { fn div(self, other: u64) -> u64 { self / other } } impl Div for u32 { fn div(self, other: u32) -> u32 { self / other } } impl Div for u16 { fn div(self, other: u16) -> u16 { self / other } } impl Div for u8 { fn div(self, other: u8) -> u8 { self / other } } +impl Div for u1 { fn div(self, other: u1) -> u1 { self / other } } impl Div for i8 { fn div(self, other: i8) -> i8 { self / other } } impl Div for i16 { fn div(self, other: i16) -> i16 { self / other } } @@ -80,6 +84,7 @@ impl Rem for u64 { fn rem(self, other: u64) -> u64 { self % other } } impl Rem for u32 { fn rem(self, other: u32) -> u32 { self % other } } impl Rem for u16 { fn rem(self, other: u16) -> u16 { self % other } } impl Rem for u8 { fn rem(self, other: u8) -> u8 { self % other } } +impl Rem for u1 { fn rem(self, other: u1) -> u1 { self % other } } impl Rem for i8 { fn rem(self, other: i8) -> i8 { self % other } } impl Rem for i16 { fn rem(self, other: i16) -> i16 { self % other } } diff --git a/noir/noir-repo/noir_stdlib/src/ops/bit.nr b/noir/noir-repo/noir_stdlib/src/ops/bit.nr index a31cfee878c..015d0008e7a 100644 --- a/noir/noir-repo/noir_stdlib/src/ops/bit.nr +++ b/noir/noir-repo/noir_stdlib/src/ops/bit.nr @@ -31,6 +31,7 @@ impl BitOr for u64 { fn bitor(self, other: u64) -> u64 { self | other } } impl BitOr for u32 { fn bitor(self, other: u32) -> u32 { self | other } } impl BitOr for u16 { fn bitor(self, other: u16) -> u16 { self | other } } impl BitOr for u8 { fn bitor(self, other: u8) -> u8 { self | other } } +impl BitOr for u1 { fn bitor(self, other: u1) -> u1 { self | other } } impl BitOr for i8 { fn bitor(self, other: i8) -> i8 { self | other } } impl BitOr for i16 { fn bitor(self, other: i16) -> i16 { self | other } } @@ -49,6 +50,7 @@ impl BitAnd for u64 { fn bitand(self, other: u64) -> u64 { self & other } } impl BitAnd for u32 { fn bitand(self, other: u32) -> u32 { self & other } } impl BitAnd for u16 { fn bitand(self, other: u16) -> u16 { self & other } } impl BitAnd for u8 { fn bitand(self, other: u8) -> u8 { self & other } } +impl BitAnd for u1 { fn bitand(self, other: u1) -> u1 { self & other } } impl BitAnd for i8 { fn bitand(self, other: i8) -> i8 { self & other } } impl BitAnd for i16 { fn bitand(self, other: i16) -> i16 { self & other } } @@ -67,6 +69,7 @@ impl BitXor for u64 { fn bitxor(self, other: u64) -> u64 { self ^ other } } impl BitXor for u32 { fn bitxor(self, other: u32) -> u32 { self ^ other } } impl BitXor for u16 { fn bitxor(self, other: u16) -> u16 { self ^ other } } impl BitXor for u8 { fn bitxor(self, other: u8) -> u8 { self ^ other } } +impl BitXor for u1 { fn bitxor(self, other: u1) -> u1 { self ^ other } } impl BitXor for i8 { fn bitxor(self, other: i8) -> i8 { self ^ other } } impl BitXor for i16 { fn bitxor(self, other: i16) -> i16 { self ^ other } } diff --git a/noir/noir-repo/test_programs/compile_success_empty/comptime_exp/Nargo.toml b/noir/noir-repo/test_programs/compile_success_empty/comptime_exp/Nargo.toml deleted file mode 100644 index df36e0e05b0..00000000000 --- a/noir/noir-repo/test_programs/compile_success_empty/comptime_exp/Nargo.toml +++ /dev/null @@ -1,7 +0,0 @@ -[package] -name = "comptime_exp" -type = "bin" -authors = [""] -compiler_version = ">=0.31.0" - -[dependencies] \ No newline at end of file diff --git a/noir/noir-repo/test_programs/compile_success_empty/comptime_exp/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/comptime_exp/src/main.nr deleted file mode 100644 index 7b25dcb5097..00000000000 --- a/noir/noir-repo/test_programs/compile_success_empty/comptime_exp/src/main.nr +++ /dev/null @@ -1,87 +0,0 @@ -use std::meta::op::UnaryOp; -use std::meta::op::BinaryOp; - -fn main() { - comptime - { - // Check Expr::as_function_call - let expr = quote { foo(bar) }.as_expr().unwrap(); - let (_function, args) = expr.as_function_call().unwrap(); - assert_eq(args.len(), 1); - - // Check Expr::as_index - let expr = quote { foo[bar] }.as_expr().unwrap(); - let _ = expr.as_index().unwrap(); - - // Check Expr::as_tuple - let expr = quote { (1, 2) }.as_expr().unwrap(); - let tuple_exprs = expr.as_tuple().unwrap(); - assert_eq(tuple_exprs.len(), 2); - - // Check Expr::as_if - let expr = quote { if 1 { 2 } }.as_expr().unwrap(); - let (_condition, _consequence, alternative) = expr.as_if().unwrap(); - assert(alternative.is_none()); - - let expr = quote { if 1 { 2 } else { 3 } }.as_expr().unwrap(); - let (_condition, _consequence, alternative) = expr.as_if().unwrap(); - assert(alternative.is_some()); - - // Check parenthesized expression is automatically unwrapped - let expr = quote { ((if 1 { 2 })) }.as_expr().unwrap(); - assert(expr.as_if().is_some()); - - // Check Expr::as_bool - let expr = quote { false }.as_expr().unwrap(); - assert(expr.as_bool().unwrap() == false); - - let expr = quote { true }.as_expr().unwrap(); - assert_eq(expr.as_bool().unwrap(), true); - - // Check Expr::as_unary_op - assert(get_unary_op(quote { -x }).is_minus()); - assert(get_unary_op(quote { !x }).is_not()); - assert(get_unary_op(quote { &mut x }).is_mutable_reference()); - assert(get_unary_op(quote { *x }).is_dereference()); - - // Check Expr::as_binary_op - assert(get_binary_op(quote { x + y }).is_add()); - assert(get_binary_op(quote { x - y }).is_subtract()); - assert(get_binary_op(quote { x * y }).is_multiply()); - assert(get_binary_op(quote { x / y }).is_divide()); - assert(get_binary_op(quote { x == y }).is_equal()); - assert(get_binary_op(quote { x != y }).is_not_equal()); - assert(get_binary_op(quote { x > y }).is_greater()); - assert(get_binary_op(quote { x >= y }).is_greater_or_equal()); - assert(get_binary_op(quote { x & y }).is_and()); - assert(get_binary_op(quote { x | y }).is_or()); - assert(get_binary_op(quote { x ^ y }).is_xor()); - assert(get_binary_op(quote { x >> y }).is_shift_right()); - assert(get_binary_op(quote { x << y }).is_shift_left()); - assert(get_binary_op(quote { x % y }).is_modulo()); - - // Check Expr::as_integer - let expr = quote { 1 }.as_expr().unwrap(); - assert_eq((1, false), expr.as_integer().unwrap()); - - let expr = quote { -2 }.as_expr().unwrap(); - assert_eq((2, true), expr.as_integer().unwrap()); - - // Check Expr::as_member_access - let expr = quote { foo.bar }.as_expr().unwrap(); - let (_, name) = expr.as_member_access().unwrap(); - assert_eq(name, quote { bar }); - } -} - -comptime fn get_unary_op(quoted: Quoted) -> UnaryOp { - let expr = quoted.as_expr().unwrap(); - let (op, _) = expr.as_unary_op().unwrap(); - op -} - -comptime fn get_binary_op(quoted: Quoted) -> BinaryOp { - let expr = quoted.as_expr().unwrap(); - let (_, op, _) = expr.as_binary_op().unwrap(); - op -} diff --git a/noir/noir-repo/test_programs/execution_success/brillig_loop_size_regression/Nargo.toml b/noir/noir-repo/test_programs/execution_success/brillig_loop_size_regression/Nargo.toml new file mode 100644 index 00000000000..d2a98e19742 --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/brillig_loop_size_regression/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "brillig_loop_size_regression" +type = "bin" +authors = [""] +compiler_version = ">=0.33.0" + +[dependencies] \ No newline at end of file diff --git a/noir/noir-repo/test_programs/execution_success/brillig_loop_size_regression/Prover.toml b/noir/noir-repo/test_programs/execution_success/brillig_loop_size_regression/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/noir/noir-repo/test_programs/execution_success/brillig_loop_size_regression/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_loop_size_regression/src/main.nr new file mode 100644 index 00000000000..488304114b9 --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/brillig_loop_size_regression/src/main.nr @@ -0,0 +1,16 @@ +struct EnumEmulation { + a: Option, + b: Option, + c: Option, +} + +unconstrained fn main() -> pub Field { + let mut emulated_enum = EnumEmulation { a: Option::some(1), b: Option::none(), c: Option::none() }; + + for _ in 0..1 { + assert_eq(emulated_enum.a.unwrap(), 1); + } + + emulated_enum.a = Option::some(2); + emulated_enum.a.unwrap() +} diff --git a/noir/noir-repo/test_programs/execution_success/verify_honk_proof/src/main.nr b/noir/noir-repo/test_programs/execution_success/verify_honk_proof/src/main.nr index d785dabffb0..667ed25f70b 100644 --- a/noir/noir-repo/test_programs/execution_success/verify_honk_proof/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/verify_honk_proof/src/main.nr @@ -13,5 +13,10 @@ fn main( // I believe we want to eventually make it public too though. key_hash: Field ) { - std::verify_proof(verification_key, proof, public_inputs, key_hash); + std::verify_proof( + verification_key, + proof, + public_inputs, + key_hash + ); } diff --git a/noir/noir-repo/test_programs/gates_report_brillig.sh b/noir/noir-repo/test_programs/gates_report_brillig.sh new file mode 100644 index 00000000000..d3f6344dbf4 --- /dev/null +++ b/noir/noir-repo/test_programs/gates_report_brillig.sh @@ -0,0 +1,33 @@ +#!/usr/bin/env bash +set -e + +# These tests are incompatible with gas reporting +excluded_dirs=("workspace" "workspace_default_member" "double_verify_nested_proof" "overlapping_dep_and_mod" "comptime_println") + +current_dir=$(pwd) +base_path="$current_dir/execution_success" +test_dirs=$(ls $base_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 "[workspace]" > Nargo.toml +echo "members = [" >> Nargo.toml + +for dir in $test_dirs; do + if [[ " ${excluded_dirs[@]} " =~ " ${dir} " ]]; then + continue + fi + + if [[ ${CI-false} = "true" ]] && [[ " ${ci_excluded_dirs[@]} " =~ " ${dir} " ]]; then + continue + fi + + echo " \"execution_success/$dir\"," >> Nargo.toml +done + +echo "]" >> Nargo.toml + +nargo info --force-brillig --json > gates_report_brillig.json + +rm Nargo.toml diff --git a/noir/noir-repo/tooling/debugger/src/context.rs b/noir/noir-repo/tooling/debugger/src/context.rs index 6ec1aff8325..890732b579c 100644 --- a/noir/noir-repo/tooling/debugger/src/context.rs +++ b/noir/noir-repo/tooling/debugger/src/context.rs @@ -566,7 +566,7 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { &mut self, call_info: AcirCallWaitInfo, ) -> DebugCommandResult { - let callee_circuit = &self.circuits[call_info.id as usize]; + let callee_circuit = &self.circuits[call_info.id.as_usize()]; let callee_witness_map = call_info.initial_witness; let callee_acvm = ACVM::new( self.backend, @@ -578,7 +578,7 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { let caller_acvm = std::mem::replace(&mut self.acvm, callee_acvm); self.acvm_stack .push(ExecutionFrame { circuit_id: self.current_circuit_id, acvm: caller_acvm }); - self.current_circuit_id = call_info.id; + self.current_circuit_id = call_info.id.0; // Explicitly handling the new ACVM status here handles two edge cases: // 1. there is a breakpoint set at the beginning of a circuit @@ -596,7 +596,7 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { let ACVMStatus::RequiresAcirCall(call_info) = self.acvm.get_status() else { unreachable!("Resolving an ACIR call, the caller is in an invalid state"); }; - let acir_to_call = &self.circuits[call_info.id as usize]; + let acir_to_call = &self.circuits[call_info.id.as_usize()]; let mut call_resolved_outputs = Vec::new(); for return_witness_index in acir_to_call.return_values.indices() { @@ -946,7 +946,7 @@ mod tests { brillig::IntegerBitSize, circuit::{ brillig::{BrilligFunctionId, BrilligInputs, BrilligOutputs}, - opcodes::{BlockId, BlockType}, + opcodes::{AcirFunctionId, BlockId, BlockType}, }, native_types::Expression, AcirField, @@ -1210,7 +1210,12 @@ mod tests { outputs: vec![], predicate: None, }, - Opcode::Call { id: 1, inputs: vec![], outputs: vec![], predicate: None }, + Opcode::Call { + id: AcirFunctionId(1), + inputs: vec![], + outputs: vec![], + predicate: None, + }, Opcode::AssertZero(Expression::default()), ], ..Circuit::default() diff --git a/noir/noir-repo/tooling/lsp/Cargo.toml b/noir/noir-repo/tooling/lsp/Cargo.toml index c5203478f5a..353a6ade904 100644 --- a/noir/noir-repo/tooling/lsp/Cargo.toml +++ b/noir/noir-repo/tooling/lsp/Cargo.toml @@ -33,6 +33,7 @@ thiserror.workspace = true fm.workspace = true rayon = "1.8.0" fxhash.workspace = true +convert_case = "0.6.0" [target.'cfg(all(target_arch = "wasm32", not(target_os = "wasi")))'.dependencies] wasm-bindgen.workspace = true diff --git a/noir/noir-repo/tooling/lsp/src/requests/completion.rs b/noir/noir-repo/tooling/lsp/src/requests/completion.rs index ad0cf0c5b9b..302152f0829 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/completion.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/completion.rs @@ -8,6 +8,7 @@ use completion_items::{ crate_completion_item, field_completion_item, simple_completion_item, struct_field_completion_item, }; +use convert_case::{Case, Casing}; use fm::{FileId, FileMap, PathString}; use kinds::{FunctionCompletionKind, FunctionKind, ModuleCompletionKind, RequestedItems}; use lsp_types::{CompletionItem, CompletionItemKind, CompletionParams, CompletionResponse}; @@ -251,6 +252,9 @@ impl<'a> NodeFinder<'a> { } fn find_in_noir_trait_impl(&mut self, noir_trait_impl: &NoirTraitImpl) { + self.find_in_path(&noir_trait_impl.trait_name, RequestedItems::OnlyTypes); + self.find_in_unresolved_type(&noir_trait_impl.object_type); + self.type_parameters.clear(); self.collect_type_parameters_in_generics(&noir_trait_impl.impl_generics); @@ -262,6 +266,8 @@ impl<'a> NodeFinder<'a> { } fn find_in_type_impl(&mut self, type_impl: &TypeImpl) { + self.find_in_unresolved_type(&type_impl.object_type); + self.type_parameters.clear(); self.collect_type_parameters_in_generics(&type_impl.generics); @@ -563,7 +569,7 @@ impl<'a> NodeFinder<'a> { let location = Location::new(member_access_expression.lhs.span, self.file); if let Some(typ) = self.interner.type_at_location(location) { let typ = typ.follow_bindings(); - let prefix = ident.to_string(); + let prefix = ident.to_string().to_case(Case::Snake); self.complete_type_fields_and_methods(&typ, &prefix); return; } @@ -679,6 +685,8 @@ impl<'a> NodeFinder<'a> { at_root = idents.is_empty(); } + let prefix = prefix.to_case(Case::Snake); + let is_single_segment = !after_colons && idents.is_empty() && path.kind == PathKind::Plain; let module_id; @@ -841,11 +849,11 @@ impl<'a> NodeFinder<'a> { segments.push(ident.clone()); if let Some(module_id) = self.resolve_module(segments) { - let prefix = String::new(); + let prefix = ""; let at_root = false; self.complete_in_module( module_id, - &prefix, + prefix, path_kind, at_root, module_completion_kind, @@ -855,7 +863,7 @@ impl<'a> NodeFinder<'a> { }; } else { // We are right after the last segment - let prefix = ident.to_string(); + let prefix = ident.to_string().to_case(Case::Snake); if segments.is_empty() { let at_root = true; self.complete_in_module( @@ -1155,8 +1163,47 @@ impl<'a> NodeFinder<'a> { } } +/// Returns true if name matches a prefix written in code. +/// `prefix` must already be in snake case. +/// This method splits both name and prefix by underscore, +/// then checks that every part of name starts with a part of +/// prefix, in order. +/// +/// For example: +/// +/// // "merk" and "ro" match "merkle" and "root" and are in order +/// name_matches("compute_merkle_root", "merk_ro") == true +/// +/// // "ro" matches "root", but "merkle" comes before it, so no match +/// name_matches("compute_merkle_root", "ro_mer") == false +/// +/// // neither "compute" nor "merkle" nor "root" start with "oot" +/// name_matches("compute_merkle_root", "oot") == false fn name_matches(name: &str, prefix: &str) -> bool { - name.starts_with(prefix) + let name = name.to_case(Case::Snake); + let name_parts: Vec<&str> = name.split('_').collect(); + + let mut last_index: i32 = -1; + for prefix_part in prefix.split('_') { + // Look past parts we already matched + let offset = if last_index >= 0 { last_index as usize + 1 } else { 0 }; + + if let Some(mut name_part_index) = + name_parts.iter().skip(offset).position(|name_part| name_part.starts_with(prefix_part)) + { + // Need to adjust the index if we skipped some segments + name_part_index += offset; + + if last_index >= name_part_index as i32 { + return false; + } + last_index = name_part_index as i32; + } else { + return false; + } + } + + true } fn module_def_id_from_reference_id(reference_id: ReferenceId) -> Option { @@ -1172,3 +1219,20 @@ fn module_def_id_from_reference_id(reference_id: ReferenceId) -> Option None, } } + +#[cfg(test)] +mod completion_name_matches_tests { + use crate::requests::completion::name_matches; + + #[test] + fn test_name_matches() { + assert!(name_matches("foo", "foo")); + assert!(name_matches("foo_bar", "bar")); + assert!(name_matches("FooBar", "foo")); + assert!(name_matches("FooBar", "bar")); + assert!(name_matches("FooBar", "foo_bar")); + assert!(name_matches("bar_baz", "bar_b")); + + assert!(!name_matches("foo_bar", "o_b")); + } +} diff --git a/noir/noir-repo/tooling/lsp/src/requests/completion/auto_import.rs b/noir/noir-repo/tooling/lsp/src/requests/completion/auto_import.rs index c5710d30b5c..8d7824502c1 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/completion/auto_import.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/completion/auto_import.rs @@ -1,8 +1,10 @@ +use std::collections::BTreeMap; + use lsp_types::{Position, Range, TextEdit}; use noirc_frontend::{ ast::ItemVisibility, graph::{CrateId, Dependency}, - hir::def_map::ModuleId, + hir::def_map::{CrateDefMap, ModuleId}, macros_api::{ModuleDefId, NodeInterner}, node_interner::ReferenceId, }; @@ -16,6 +18,8 @@ use super::{ impl<'a> NodeFinder<'a> { pub(super) fn complete_auto_imports(&mut self, prefix: &str, requested_items: RequestedItems) { + let current_module_parent_id = get_parent_module_id(self.def_maps, self.module_id); + for (name, entries) in self.interner.get_auto_import_names() { if !name_matches(name, prefix) { continue; @@ -39,8 +43,9 @@ impl<'a> NodeFinder<'a> { let module_full_path; if let ModuleDefId::ModuleId(module_id) = module_def_id { module_full_path = module_id_path( - module_id, + *module_id, &self.module_id, + current_module_parent_id, self.interner, self.dependencies, ); @@ -67,6 +72,7 @@ impl<'a> NodeFinder<'a> { module_full_path = module_id_path( parent_module, &self.module_id, + current_module_parent_id, self.interner, self.dependencies, ); @@ -111,9 +117,18 @@ impl<'a> NodeFinder<'a> { } } -fn get_parent_module(interner: &NodeInterner, module_def_id: ModuleDefId) -> Option<&ModuleId> { +fn get_parent_module(interner: &NodeInterner, module_def_id: ModuleDefId) -> Option { let reference_id = module_def_id_to_reference_id(module_def_id); - interner.reference_module(reference_id) + interner.reference_module(reference_id).copied() +} + +fn get_parent_module_id( + def_maps: &BTreeMap, + module_id: ModuleId, +) -> Option { + let crate_def_map = &def_maps[&module_id.krate]; + let module_data = &crate_def_map.modules()[module_id.local_id.0]; + module_data.parent.map(|parent| ModuleId { krate: module_id.krate, local_id: parent }) } fn module_def_id_to_reference_id(module_def_id: ModuleDefId) -> ReferenceId { @@ -127,69 +142,69 @@ fn module_def_id_to_reference_id(module_def_id: ModuleDefId) -> ReferenceId { } } -/// Computes the path of `module_id` relative to `current_module_id`. -/// If it's not relative, the full path is returned. +/// Returns the path to reach an item inside `target_module_id` from inside `current_module_id`. +/// Returns a relative path if possible. fn module_id_path( - module_id: &ModuleId, + target_module_id: ModuleId, current_module_id: &ModuleId, + current_module_parent_id: Option, interner: &NodeInterner, dependencies: &[Dependency], ) -> String { - let mut string = String::new(); - - let crate_id = module_id.krate; - let crate_name = match crate_id { - CrateId::Root(_) => Some("crate".to_string()), - CrateId::Crate(_) => dependencies - .iter() - .find(|dep| dep.crate_id == crate_id) - .map(|dep| format!("{}", dep.name)), - CrateId::Stdlib(_) => Some("std".to_string()), - CrateId::Dummy => None, - }; + if Some(target_module_id) == current_module_parent_id { + return "super".to_string(); + } - let wrote_crate = if let Some(crate_name) = crate_name { - string.push_str(&crate_name); - true - } else { - false - }; + let mut segments: Vec<&str> = Vec::new(); + let mut is_relative = false; - let Some(module_attributes) = interner.try_module_attributes(module_id) else { - return string; - }; + if let Some(module_attributes) = interner.try_module_attributes(&target_module_id) { + segments.push(&module_attributes.name); - if wrote_crate { - string.push_str("::"); - } + let mut current_attributes = module_attributes; + loop { + let parent_module_id = + &ModuleId { krate: target_module_id.krate, local_id: current_attributes.parent }; - let mut segments = Vec::new(); - let mut current_attributes = module_attributes; - loop { - let parent_module_id = - &ModuleId { krate: module_id.krate, local_id: current_attributes.parent }; - - // If the parent module is the current module we stop because we want a relative path to the module - if current_module_id == parent_module_id { - // When the path is relative we don't want the "crate::" prefix anymore - string = string.strip_prefix("crate::").unwrap_or(&string).to_string(); - break; - } + if current_module_id == parent_module_id { + is_relative = true; + break; + } - let Some(parent_attributes) = interner.try_module_attributes(parent_module_id) else { - break; - }; + if current_module_parent_id == Some(*parent_module_id) { + segments.push("super"); + is_relative = true; + break; + } - segments.push(&parent_attributes.name); - current_attributes = parent_attributes; - } + let Some(parent_attributes) = interner.try_module_attributes(parent_module_id) else { + break; + }; - for segment in segments.iter().rev() { - string.push_str(segment); - string.push_str("::"); + segments.push(&parent_attributes.name); + current_attributes = parent_attributes; + } } - string.push_str(&module_attributes.name); + let crate_id = target_module_id.krate; + let crate_name = if is_relative { + None + } else { + match crate_id { + CrateId::Root(_) => Some("crate".to_string()), + CrateId::Stdlib(_) => Some("std".to_string()), + CrateId::Crate(_) => dependencies + .iter() + .find(|dep| dep.crate_id == crate_id) + .map(|dep| dep.name.to_string()), + CrateId::Dummy => unreachable!("ICE: A dummy CrateId should not be accessible"), + } + }; + + if let Some(crate_name) = &crate_name { + segments.push(crate_name); + }; - string + segments.reverse(); + segments.join("::") } diff --git a/noir/noir-repo/tooling/lsp/src/requests/completion/completion_items.rs b/noir/noir-repo/tooling/lsp/src/requests/completion/completion_items.rs index 70afc43fe55..d4190f5241c 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/completion/completion_items.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/completion/completion_items.rs @@ -9,7 +9,10 @@ use noirc_frontend::{ }; use super::{ - sort_text::{default_sort_text, new_sort_text, operator_sort_text, self_mismatch_sort_text}, + sort_text::{ + crate_or_module_sort_text, default_sort_text, new_sort_text, operator_sort_text, + self_mismatch_sort_text, + }, FunctionCompletionKind, FunctionKind, NodeFinder, RequestedItems, }; @@ -246,11 +249,17 @@ impl<'a> NodeFinder<'a> { } pub(super) fn module_completion_item(name: impl Into) -> CompletionItem { - simple_completion_item(name, CompletionItemKind::MODULE, None) + completion_item_with_sort_text( + simple_completion_item(name, CompletionItemKind::MODULE, None), + crate_or_module_sort_text(), + ) } pub(super) fn crate_completion_item(name: impl Into) -> CompletionItem { - simple_completion_item(name, CompletionItemKind::MODULE, None) + completion_item_with_sort_text( + simple_completion_item(name, CompletionItemKind::MODULE, None), + crate_or_module_sort_text(), + ) } fn func_meta_type_to_string(func_meta: &FuncMeta, has_self_type: bool) -> String { diff --git a/noir/noir-repo/tooling/lsp/src/requests/completion/sort_text.rs b/noir/noir-repo/tooling/lsp/src/requests/completion/sort_text.rs index a1dd0ba5e94..9bdc603660f 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/completion/sort_text.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/completion/sort_text.rs @@ -1,33 +1,39 @@ /// Sort text for "new" methods: we want these to show up before anything else, /// if we are completing at something like `Foo::` pub(super) fn new_sort_text() -> String { - "3".to_string() + "a".to_string() } /// This is the default sort text. pub(super) fn default_sort_text() -> String { - "5".to_string() + "b".to_string() +} + +/// We want crates and modules to show up after other things (for example +/// local variables, functions or types) +pub(super) fn crate_or_module_sort_text() -> String { + "c".to_string() } /// Sort text for auto-import items. We want these to show up after local definitions. pub(super) fn auto_import_sort_text() -> String { - "6".to_string() + "d".to_string() } /// When completing something like `Foo::`, we want to show methods that take /// self after the other ones. pub(super) fn self_mismatch_sort_text() -> String { - "7".to_string() + "e".to_string() } /// We want to show operator methods last. pub(super) fn operator_sort_text() -> String { - "8".to_string() + "f".to_string() } /// If a name begins with underscore it's likely something that's meant to /// be private (but visibility doesn't exist everywhere yet, so for now /// we assume that) pub(super) fn underscore_sort_text() -> String { - "9".to_string() + "g".to_string() } diff --git a/noir/noir-repo/tooling/lsp/src/requests/completion/tests.rs b/noir/noir-repo/tooling/lsp/src/requests/completion/tests.rs index 825a7dd0081..8cf06adc027 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/completion/tests.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/completion/tests.rs @@ -356,14 +356,14 @@ mod completion_tests { async fn test_complete_path_after_colons_and_letter_shows_submodule() { let src = r#" mod foo { - mod bar {} + mod qux {} } fn main() { - foo::b>|< + foo::q>|< } "#; - assert_completion(src, vec![module_completion_item("bar")]).await; + assert_completion(src, vec![module_completion_item("qux")]).await; } #[test] @@ -494,7 +494,7 @@ mod completion_tests { impl SomeStruct { fn foo() { - S>|< + So>|< } } "#; @@ -517,7 +517,7 @@ mod completion_tests { impl Trait for SomeStruct { fn foo() { - S>|< + So>|< } } "#; @@ -537,7 +537,7 @@ mod completion_tests { let src = r#" fn main() { for index in 0..10 { - i>|< + ind>|< } } "#; @@ -558,13 +558,13 @@ mod completion_tests { fn lambda(f: fn(i32)) { } fn main() { - lambda(|var| v>|<) + lambda(|lambda_var| lambda_v>|<) } "#; assert_completion_excluding_auto_import( src, vec![simple_completion_item( - "var", + "lambda_var", CompletionItemKind::VARIABLE, Some("_".to_string()), )], @@ -733,16 +733,19 @@ mod completion_tests { let src = r#" fn foo(x: i>|<) {} "#; - assert_completion( - src, + + let items = get_completions(src).await; + let items = items.into_iter().filter(|item| item.label.starts_with('i')).collect(); + + assert_items_match( + items, vec![ simple_completion_item("i8", CompletionItemKind::STRUCT, Some("i8".to_string())), simple_completion_item("i16", CompletionItemKind::STRUCT, Some("i16".to_string())), simple_completion_item("i32", CompletionItemKind::STRUCT, Some("i32".to_string())), simple_completion_item("i64", CompletionItemKind::STRUCT, Some("i64".to_string())), ], - ) - .await; + ); } #[test] @@ -895,7 +898,7 @@ mod completion_tests { async fn test_suggest_struct_type_parameter() { let src = r#" struct Foo { - context: C>|< + context: Cont>|< } "#; assert_completion_excluding_auto_import( @@ -962,7 +965,7 @@ mod completion_tests { #[test] async fn test_suggest_function_type_parameters() { let src = r#" - fn foo(x: C>|<) {} + fn foo(x: Cont>|<) {} "#; assert_completion_excluding_auto_import( src, @@ -1443,7 +1446,7 @@ mod completion_tests { assert_eq!( item.label_details, Some(CompletionItemLabelDetails { - detail: Some("(use crate::foo::bar::hello_world)".to_string()), + detail: Some("(use super::bar::hello_world)".to_string()), description: Some("fn()".to_string()) }) ); @@ -1455,7 +1458,7 @@ mod completion_tests { start: Position { line: 7, character: 8 }, end: Position { line: 7, character: 8 }, }, - new_text: "use crate::foo::bar::hello_world;\n\n ".to_string(), + new_text: "use super::bar::hello_world;\n\n ".to_string(), }]) ); } @@ -1556,4 +1559,83 @@ mod completion_tests { }) ); } + + #[test] + async fn test_completes_matching_any_part_of_an_identifier_by_underscore() { + let src = r#" + struct Foo { + some_property: i32, + } + + fn foo(f: Foo) { + f.prop>|< + } + "#; + assert_completion(src, vec![field_completion_item("some_property", "i32")]).await; + } + + #[test] + async fn test_completes_in_impl_type() { + let src = r#" + struct FooBar { + } + + impl FooB>|< + "#; + + assert_completion( + src, + vec![simple_completion_item( + "FooBar", + CompletionItemKind::STRUCT, + Some("FooBar".to_string()), + )], + ) + .await; + } + + #[test] + async fn test_completes_in_impl_for_type() { + let src = r#" + struct FooBar { + } + + impl Default for FooB>|< + "#; + + assert_completion( + src, + vec![simple_completion_item( + "FooBar", + CompletionItemKind::STRUCT, + Some("FooBar".to_string()), + )], + ) + .await; + } + + #[test] + async fn test_auto_import_with_super() { + let src = r#" + pub fn bar_baz() {} + + mod tests { + fn foo() { + bar_b>|< + } + } + "#; + let items = get_completions(src).await; + assert_eq!(items.len(), 1); + + let item = &items[0]; + assert_eq!(item.label, "bar_baz()"); + assert_eq!( + item.label_details, + Some(CompletionItemLabelDetails { + detail: Some("(use super::bar_baz)".to_string()), + description: Some("fn()".to_string()) + }) + ); + } } diff --git a/noir/noir-repo/tooling/lsp/src/requests/hover.rs b/noir/noir-repo/tooling/lsp/src/requests/hover.rs index aa97def8274..95d8b82f84f 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/hover.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/hover.rs @@ -319,6 +319,21 @@ fn format_parent_module_from_module_id( args: &ProcessRequestCallbackArgs, string: &mut String, ) -> bool { + let mut segments: Vec<&str> = Vec::new(); + + if let Some(module_attributes) = args.interner.try_module_attributes(module) { + segments.push(&module_attributes.name); + + let mut current_attributes = module_attributes; + while let Some(parent_attributes) = args.interner.try_module_attributes(&ModuleId { + krate: module.krate, + local_id: current_attributes.parent, + }) { + segments.push(&parent_attributes.name); + current_attributes = parent_attributes; + } + } + let crate_id = module.krate; let crate_name = match crate_id { CrateId::Root(_) => Some(args.crate_name.clone()), @@ -328,44 +343,21 @@ fn format_parent_module_from_module_id( .find(|dep| dep.crate_id == crate_id) .map(|dep| format!("{}", dep.name)), CrateId::Stdlib(_) => Some("std".to_string()), - CrateId::Dummy => None, + CrateId::Dummy => unreachable!("ICE: A dummy CrateId should not be accessible"), }; - let wrote_crate = if let Some(crate_name) = crate_name { - string.push_str(" "); - string.push_str(&crate_name); - true - } else { - false - }; - - let Some(module_attributes) = args.interner.try_module_attributes(module) else { - return wrote_crate; + if let Some(crate_name) = &crate_name { + segments.push(crate_name); }; - if wrote_crate { - string.push_str("::"); - } else { - string.push_str(" "); - } - - let mut segments = Vec::new(); - let mut current_attributes = module_attributes; - while let Some(parent_attributes) = args.interner.try_module_attributes(&ModuleId { - krate: module.krate, - local_id: current_attributes.parent, - }) { - segments.push(&parent_attributes.name); - current_attributes = parent_attributes; - } - - for segment in segments.iter().rev() { - string.push_str(segment); - string.push_str("::"); + if segments.is_empty() { + return false; } - string.push_str(&module_attributes.name); + segments.reverse(); + string.push_str(" "); + string.push_str(&segments.join("::")); true } diff --git a/noir/noir-repo/tooling/nargo/src/ops/execute.rs b/noir/noir-repo/tooling/nargo/src/ops/execute.rs index c9cc60d03d9..2e214c4e425 100644 --- a/noir/noir-repo/tooling/nargo/src/ops/execute.rs +++ b/noir/noir-repo/tooling/nargo/src/ops/execute.rs @@ -139,9 +139,9 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> }); // Set current function to the circuit we are about to execute - self.current_function_index = call_info.id as usize; + self.current_function_index = call_info.id.as_usize(); // Execute the ACIR call - let acir_to_call = &self.functions[call_info.id as usize]; + let acir_to_call = &self.functions[call_info.id.as_usize()]; let initial_witness = call_info.initial_witness; let call_solved_witness = self.execute_circuit(initial_witness)?; @@ -163,7 +163,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> } } acvm.resolve_pending_acir_call(call_resolved_outputs); - self.witness_stack.push(call_info.id, call_solved_witness); + self.witness_stack.push(call_info.id.0, call_solved_witness); } } } diff --git a/noir/noir-repo/tooling/nargo_cli/build.rs b/noir/noir-repo/tooling/nargo_cli/build.rs index 3f8cd055569..4dcfccdf085 100644 --- a/noir/noir-repo/tooling/nargo_cli/build.rs +++ b/noir/noir-repo/tooling/nargo_cli/build.rs @@ -207,7 +207,7 @@ fn generate_compile_success_empty_tests(test_file: &mut File, test_data_dir: &Pa let json: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap_or_else(|e| {{ panic!("JSON was not well-formatted {:?}\n\n{:?}", e, std::str::from_utf8(&output.stdout)) }}); - let num_opcodes = &json["programs"][0]["functions"][0]["acir_opcodes"]; + let num_opcodes = &json["programs"][0]["functions"][0]["opcodes"]; assert_eq!(num_opcodes.as_u64().expect("number of opcodes should fit in a u64"), 0); "#; diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/info_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/info_cmd.rs index 2fd189172d8..7e4a8b4117d 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/info_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/info_cmd.rs @@ -175,6 +175,7 @@ struct ProgramInfo { #[serde(skip)] expression_width: ExpressionWidth, functions: Vec, + #[serde(skip)] unconstrained_functions_opcodes: usize, unconstrained_functions: Vec, } @@ -186,7 +187,7 @@ impl From for Vec { Fm->format!("{}", program_info.package_name), Fc->format!("{}", function.name), format!("{:?}", program_info.expression_width), - Fc->format!("{}", function.acir_opcodes), + Fc->format!("{}", function.opcodes), Fc->format!("{}", program_info.unconstrained_functions_opcodes), ] }); @@ -196,7 +197,7 @@ impl From for Vec { Fc->format!("{}", function.name), format!("N/A", ), Fc->format!("N/A"), - Fc->format!("{}", function.acir_opcodes), + Fc->format!("{}", function.opcodes), ] })); main @@ -215,7 +216,7 @@ struct ContractInfo { #[derive(Debug, Serialize)] struct FunctionInfo { name: String, - acir_opcodes: usize, + opcodes: usize, } impl From for Vec { @@ -225,7 +226,7 @@ impl From for Vec { Fm->format!("{}", contract_info.name), Fc->format!("{}", function.name), format!("{:?}", contract_info.expression_width), - Fc->format!("{}", function.acir_opcodes), + Fc->format!("{}", function.opcodes), ] }) } @@ -243,7 +244,7 @@ fn count_opcodes_and_gates_in_program( .enumerate() .map(|(i, function)| FunctionInfo { name: compiled_program.names[i].clone(), - acir_opcodes: function.opcodes.len(), + opcodes: function.opcodes.len(), }) .collect(); @@ -264,7 +265,7 @@ fn count_opcodes_and_gates_in_program( .clone() .iter() .zip(opcodes_len) - .map(|(name, len)| FunctionInfo { name: name.clone(), acir_opcodes: len }) + .map(|(name, len)| FunctionInfo { name: name.clone(), opcodes: len }) .collect(); ProgramInfo {