diff --git a/.noir-sync-commit b/.noir-sync-commit index bd020a65c3ec..7040dbf0e9d3 100644 --- a/.noir-sync-commit +++ b/.noir-sync-commit @@ -1 +1 @@ -42b4ba3fa2f1dfdb92f197bfbe25884078256ae2 +826b18a10630471c19c25ab745f9bfe045813e69 diff --git a/avm-transpiler/Cargo.toml b/avm-transpiler/Cargo.toml index 1d6d76a3db07..63c160ed1118 100644 --- a/avm-transpiler/Cargo.toml +++ b/avm-transpiler/Cargo.toml @@ -2,7 +2,7 @@ name = "avm-transpiler" version = "0.1.0" authors = ["The Aztec Team "] -edition = "2021" +edition = "2024" license = "MIT OR Apache-2.0" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html diff --git a/noir/noir-repo/.github/benchmark_projects.yml b/noir/noir-repo/.github/benchmark_projects.yml index 3503a439eb75..3a67bf99e8d2 100644 --- a/noir/noir-repo/.github/benchmark_projects.yml +++ b/noir/noir-repo/.github/benchmark_projects.yml @@ -1,4 +1,4 @@ -define: &AZ_COMMIT 17e79f42c0a1895dc87a74e4155591e38c45f09b +define: &AZ_COMMIT a90f08e245add379fa0257c81f8e2819beb190cb projects: private-kernel-inner: repo: AztecProtocol/aztec-packages diff --git a/noir/noir-repo/.github/scripts/wasm-bindgen-install.sh b/noir/noir-repo/.github/scripts/wasm-bindgen-install.sh index 209080036934..bb48062cb43f 100755 --- a/noir/noir-repo/.github/scripts/wasm-bindgen-install.sh +++ b/noir/noir-repo/.github/scripts/wasm-bindgen-install.sh @@ -6,8 +6,8 @@ cd $(dirname "$0") ./cargo-binstall-install.sh # Install wasm-bindgen-cli. -if [ "$(wasm-bindgen --version &> /dev/null | cut -d' ' -f2)" != "0.2.86" ]; then +if [ "$(wasm-bindgen --version &> /dev/null | cut -d' ' -f2)" != "0.2.100" ]; then echo "Building wasm-bindgen..." - cargo binstall wasm-bindgen-cli@0.2.86 --force --no-confirm + cargo binstall wasm-bindgen-cli@0.2.100 --force --no-confirm fi diff --git a/noir/noir-repo/.github/workflows/docs-pr.yml b/noir/noir-repo/.github/workflows/docs-pr.yml index 663142091d3f..d60bc8b841eb 100644 --- a/noir/noir-repo/.github/workflows/docs-pr.yml +++ b/noir/noir-repo/.github/workflows/docs-pr.yml @@ -71,7 +71,7 @@ jobs: - name: Install wasm-bindgen-cli uses: taiki-e/install-action@v2 with: - tool: wasm-bindgen-cli@0.2.86 + tool: wasm-bindgen-cli@0.2.100 - name: Install wasm-opt run: | diff --git a/noir/noir-repo/.github/workflows/publish-docs.yml b/noir/noir-repo/.github/workflows/publish-docs.yml index 16959256d2aa..f949576900f2 100644 --- a/noir/noir-repo/.github/workflows/publish-docs.yml +++ b/noir/noir-repo/.github/workflows/publish-docs.yml @@ -22,12 +22,12 @@ jobs: - name: Install wasm-bindgen-cli uses: taiki-e/install-action@v2 with: - tool: wasm-bindgen-cli@0.2.86 + tool: wasm-bindgen-cli@0.2.100 - name: Install wasm-opt run: | npm i wasm-opt -g - + - name: Query active docs versions run: yarn workspace docs version::stables diff --git a/noir/noir-repo/.github/workflows/release.yml b/noir/noir-repo/.github/workflows/release.yml index bbe9a7fff620..ea1f1eba6197 100644 --- a/noir/noir-repo/.github/workflows/release.yml +++ b/noir/noir-repo/.github/workflows/release.yml @@ -47,10 +47,10 @@ jobs: node-version: 18.19.0 cache: 'yarn' cache-dependency-path: 'yarn.lock' - + - name: Update yarn.lock run: yarn - + - name: Configure git run: | git config user.name noirwhal @@ -61,13 +61,13 @@ jobs: git add . git commit -m 'chore: Update root workspace acvm versions and lockfile' git push - + update-docs: name: Update docs needs: [release-please, update-acvm-workspace-package-versions] if: ${{ needs.release-please.outputs.release-pr }} runs-on: ubuntu-22.04 - + steps: - name: Checkout release branch uses: actions/checkout@v4 @@ -81,7 +81,7 @@ jobs: - name: Install wasm-bindgen-cli uses: taiki-e/install-action@v2 with: - tool: wasm-bindgen-cli@0.2.86 + tool: wasm-bindgen-cli@0.2.100 - name: Install wasm-opt run: | @@ -113,7 +113,7 @@ jobs: runs-on: ubuntu-22.04 # We want this job to always run (even if the dependant jobs fail) as we need apply changes to the sticky comment. if: ${{ always() }} - + needs: - release-please - update-acvm-workspace-package-versions @@ -123,7 +123,7 @@ jobs: # We treat any skipped or failing jobs as a failure for the workflow as a whole. FAIL: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') || contains(needs.*.result, 'skipped') }} - steps: + steps: - name: Add warning to sticky comment uses: marocchino/sticky-pull-request-comment@v2 with: @@ -175,7 +175,7 @@ jobs: needs: [release-please] if: ${{ needs.release-please.outputs.tag-name }} runs-on: ubuntu-22.04 - + steps: - name: Dispatch to publish-acvm uses: benc-uk/workflow-dispatch@v1 diff --git a/noir/noir-repo/Cargo.lock b/noir/noir-repo/Cargo.lock index 3ee2f754bdad..7961f8ecc801 100644 --- a/noir/noir-repo/Cargo.lock +++ b/noir/noir-repo/Cargo.lock @@ -2605,10 +2605,11 @@ checksum = "8eaf4bc02d17cbdd7ff4c7438cafcdf7fb9a4613313ad11b4f8fefe7d3fa0130" [[package]] name = "js-sys" -version = "0.3.63" +version = "0.3.77" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2f37a4a5928311ac501dee68b3c7613a1037d0edb30c8e5427bd832d55d1b790" +checksum = "1cfaf33c695fc6e08064efbc1f72ec937429614f25eef83af942d0e227c3a28f" dependencies = [ + "once_cell", "wasm-bindgen", ] @@ -2948,6 +2949,16 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3d97bbf43eb4f088f8ca469930cde17fa036207c9a5e02ccc5107c4e8b17c964" +[[package]] +name = "minicov" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f27fe9f1cc3c22e1687f9446c2083c4c5fc7f0bcf1c7a86bdbded14985895b4b" +dependencies = [ + "cc", + "walkdir", +] + [[package]] name = "miniz_oxide" version = "0.7.4" @@ -3045,6 +3056,7 @@ dependencies = [ "nargo", "nargo_fmt", "nargo_toml", + "noir_artifact_cli", "noir_debugger", "noir_lsp", "noirc_abi", @@ -3226,6 +3238,7 @@ dependencies = [ "clap", "color-eyre", "const_format", + "noir_artifact_cli", "noirc_artifacts", "noirc_artifacts_info", "serde", @@ -3275,6 +3288,7 @@ dependencies = [ "im", "inferno", "nargo", + "noir_artifact_cli", "noirc_abi", "noirc_artifacts", "noirc_driver", @@ -4548,12 +4562,6 @@ dependencies = [ "windows-sys 0.59.0", ] -[[package]] -name = "scoped-tls" -version = "1.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e1cf6437eb19a8f4a6cc0f7dca544973b0b78843adbfeb3683d1a94a0024a294" - [[package]] name = "scopeguard" version = "1.2.0" @@ -5660,11 +5668,13 @@ dependencies = [ [[package]] name = "wasm-bindgen" -version = "0.2.86" +version = "0.2.100" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5bba0e8cb82ba49ff4e229459ff22a191bbe9a1cb3a341610c9c33efc27ddf73" +checksum = "1edc8929d7499fc4e8f0be2262a241556cfc54a0bea223790e71446f2aab1ef5" dependencies = [ "cfg-if", + "once_cell", + "rustversion", "serde", "serde_json", "wasm-bindgen-macro", @@ -5672,13 +5682,12 @@ dependencies = [ [[package]] name = "wasm-bindgen-backend" -version = "0.2.86" +version = "0.2.100" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "19b04bc93f9d6bdee709f6bd2118f57dd6679cf1176a1af464fca3ab0d66d8fb" +checksum = "2f0a0651a5c2bc21487bde11ee802ccaf4c51935d0d3d42a6101f98161700bc6" dependencies = [ "bumpalo", "log", - "once_cell", "proc-macro2", "quote", "syn 2.0.96", @@ -5687,21 +5696,22 @@ dependencies = [ [[package]] name = "wasm-bindgen-futures" -version = "0.4.36" +version = "0.4.50" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2d1985d03709c53167ce907ff394f5316aa22cb4e12761295c5dc57dacb6297e" +checksum = "555d470ec0bc3bb57890405e5d4322cc9ea83cebb085523ced7be4144dac1e61" dependencies = [ "cfg-if", "js-sys", + "once_cell", "wasm-bindgen", "web-sys", ] [[package]] name = "wasm-bindgen-macro" -version = "0.2.86" +version = "0.2.100" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "14d6b024f1a526bb0234f52840389927257beb670610081360e5a03c5df9c258" +checksum = "7fe63fc6d09ed3792bd0897b314f53de8e16568c2b3f7982f468c0bf9bd0b407" dependencies = [ "quote", "wasm-bindgen-macro-support", @@ -5709,9 +5719,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro-support" -version = "0.2.86" +version = "0.2.100" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e128beba882dd1eb6200e1dc92ae6c5dbaa4311aa7bb211ca035779e5efc39f8" +checksum = "8ae87ea40c9f689fc23f209965b6fb8a99ad69aeeb0231408be24920604395de" dependencies = [ "proc-macro2", "quote", @@ -5722,19 +5732,21 @@ dependencies = [ [[package]] name = "wasm-bindgen-shared" -version = "0.2.86" +version = "0.2.100" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ed9d5b4305409d1fc9482fee2d7f9bcbf24b3972bf59817ef757e23982242a93" +checksum = "1a05d73b933a847d6cccdda8f838a22ff101ad9bf93e33684f39c1f5f0eece3d" +dependencies = [ + "unicode-ident", +] [[package]] name = "wasm-bindgen-test" -version = "0.3.36" +version = "0.3.50" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c9e636f3a428ff62b3742ebc3c70e254dfe12b8c2b469d688ea59cdd4abcf502" +checksum = "66c8d5e33ca3b6d9fa3b4676d774c5778031d27a578c2b007f905acf816152c3" dependencies = [ - "console_error_panic_hook", "js-sys", - "scoped-tls", + "minicov", "wasm-bindgen", "wasm-bindgen-futures", "wasm-bindgen-test-macro", @@ -5742,19 +5754,20 @@ dependencies = [ [[package]] name = "wasm-bindgen-test-macro" -version = "0.3.36" +version = "0.3.50" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f18c1fad2f7c4958e7bcce014fa212f59a65d5e3721d0f77e6c0b27ede936ba3" +checksum = "17d5042cc5fa009658f9a7333ef24291b1291a25b6382dd68862a7f3b969f69b" dependencies = [ "proc-macro2", "quote", + "syn 2.0.96", ] [[package]] name = "web-sys" -version = "0.3.63" +version = "0.3.77" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3bdd9ef4e984da1187bf8110c5cf5b845fbc87a23602cdf912386a76fcd3a7c2" +checksum = "33b6dd2ef9186f1f2072e409e99cd22a975331a6b3591b12c764e0e55c60d5d2" dependencies = [ "js-sys", "wasm-bindgen", diff --git a/noir/noir-repo/Cargo.toml b/noir/noir-repo/Cargo.toml index 2fda54652d62..9c5bf1351d1c 100644 --- a/noir/noir-repo/Cargo.toml +++ b/noir/noir-repo/Cargo.toml @@ -52,7 +52,7 @@ resolver = "2" version = "1.0.0-beta.3" # x-release-please-end authors = ["The Noir Team "] -edition = "2021" +edition = "2024" rust-version = "1.85.0" license = "MIT OR Apache-2.0" repository = "https://github.com/noir-lang/noir/" @@ -116,9 +116,9 @@ lsp-types = "0.94.1" tower = "0.4" # Wasm -wasm-bindgen = { version = "=0.2.86", features = ["serde-serialize"] } -wasm-bindgen-test = "0.3.36" -wasm-bindgen-futures = "0.4.36" +wasm-bindgen = { version = "=0.2.100", features = ["serde-serialize"] } +wasm-bindgen-test = "0.3.50" +wasm-bindgen-futures = "0.4.50" console_error_panic_hook = "0.1.7" gloo-utils = { version = "0.1", features = ["serde"] } js-sys = "0.3.62" diff --git a/noir/noir-repo/EXTERNAL_NOIR_LIBRARIES.yml b/noir/noir-repo/EXTERNAL_NOIR_LIBRARIES.yml index ddc7a27ee140..0481c5398051 100644 --- a/noir/noir-repo/EXTERNAL_NOIR_LIBRARIES.yml +++ b/noir/noir-repo/EXTERNAL_NOIR_LIBRARIES.yml @@ -1,4 +1,4 @@ -define: &AZ_COMMIT 17e79f42c0a1895dc87a74e4155591e38c45f09b +define: &AZ_COMMIT a90f08e245add379fa0257c81f8e2819beb190cb libraries: noir_check_shuffle: repo: noir-lang/noir_check_shuffle diff --git a/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs b/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs index c5668c418b87..68c3c832b5cd 100644 --- a/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs +++ b/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs @@ -252,7 +252,7 @@ impl Program { program_bytes } - // Serialize and base64 encode program + /// Serialize and base64 encode program pub fn serialize_program_base64(program: &Self, s: S) -> Result where S: Serializer, @@ -277,7 +277,7 @@ impl Deserialize<'a>> Program { Program::read(serialized_circuit) } - // Deserialize and base64 decode program + /// Deserialize and base64 decode program pub fn deserialize_program_base64<'de, D>(deserializer: D) -> Result where D: Deserializer<'de>, diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/mod.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/mod.rs index d88488177422..ad064fefd7b8 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/mod.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/mod.rs @@ -37,7 +37,7 @@ fn first_missing_assignment( inputs: &[FunctionInput], ) -> Option { inputs.iter().find_map(|input| { - if let ConstantOrWitnessEnum::Witness(ref witness) = input.input_ref() { + if let ConstantOrWitnessEnum::Witness(witness) = input.input_ref() { if witness_assignments.contains_key(witness) { None } else { Some(*witness) } } else { None diff --git a/noir/noir-repo/acvm-repo/acvm_js/Cargo.toml b/noir/noir-repo/acvm-repo/acvm_js/Cargo.toml index 71fdecd42f1e..d8416b5d89f8 100644 --- a/noir/noir-repo/acvm-repo/acvm_js/Cargo.toml +++ b/noir/noir-repo/acvm-repo/acvm_js/Cargo.toml @@ -40,7 +40,7 @@ getrandom = { workspace = true, features = ["js"] } build-data.workspace = true pkg-config = "0.3" -[dev-dependencies] +[target.'cfg(all(any(target_arch = "wasm32", target_arch = "wasm64"), target_os = "unknown"))'.dev-dependencies] wasm-bindgen-test.workspace = true [features] diff --git a/noir/noir-repo/acvm-repo/acvm_js/src/js_witness_map.rs b/noir/noir-repo/acvm-repo/acvm_js/src/js_witness_map.rs index bfdf02879ab7..a09e8e1ade7a 100644 --- a/noir/noir-repo/acvm-repo/acvm_js/src/js_witness_map.rs +++ b/noir/noir-repo/acvm-repo/acvm_js/src/js_witness_map.rs @@ -106,9 +106,9 @@ pub(crate) fn field_element_to_js_string(field_element: &FieldElement) -> JsStri format!("0x{}", field_element.to_hex()).into() } -#[cfg(test)] +#[cfg(all(test, any(target_arch = "wasm32", target_arch = "wasm64"), target_os = "unknown"))] mod test { - use wasm_bindgen_test::wasm_bindgen_test as test; + use wasm_bindgen_test::*; use std::collections::BTreeMap; @@ -120,7 +120,7 @@ mod test { use crate::JsWitnessMap; - #[test] + #[wasm_bindgen_test] fn test_witness_map_to_js() { let witness_map = BTreeMap::from([ (Witness(1), FieldElement::one()), diff --git a/noir/noir-repo/acvm-repo/brillig_vm/src/arithmetic.rs b/noir/noir-repo/acvm-repo/brillig_vm/src/arithmetic.rs index 12691424c26d..24013d1fde7a 100644 --- a/noir/noir-repo/acvm-repo/brillig_vm/src/arithmetic.rs +++ b/noir/noir-repo/acvm-repo/brillig_vm/src/arithmetic.rs @@ -231,13 +231,11 @@ fn evaluate_binary_int_op_shifts + Zero + Shl + Shr { let rhs_usize: usize = rhs as usize; - #[allow(unused_qualifications)] - if rhs_usize >= 8 * std::mem::size_of::() { T::zero() } else { lhs << rhs.into() } + if rhs_usize >= 8 * size_of::() { T::zero() } else { lhs << rhs.into() } } BinaryIntOp::Shr => { let rhs_usize: usize = rhs as usize; - #[allow(unused_qualifications)] - if rhs_usize >= 8 * std::mem::size_of::() { T::zero() } else { lhs >> rhs.into() } + if rhs_usize >= 8 * size_of::() { T::zero() } else { lhs >> rhs.into() } } _ => unreachable!("Operator not handled by this function: {op:?}"), } diff --git a/noir/noir-repo/compiler/noirc_driver/src/contract.rs b/noir/noir-repo/compiler/noirc_driver/src/contract.rs index 47f0412b7477..b4c5e9a3fcae 100644 --- a/noir/noir-repo/compiler/noirc_driver/src/contract.rs +++ b/noir/noir-repo/compiler/noirc_driver/src/contract.rs @@ -41,6 +41,8 @@ pub struct CompiledContract { pub struct ContractFunction { pub name: String, + pub hash: u64, + pub is_unconstrained: bool, pub custom_attributes: Vec, diff --git a/noir/noir-repo/compiler/noirc_driver/src/lib.rs b/noir/noir-repo/compiler/noirc_driver/src/lib.rs index 79d18cd6de28..a70d9fc72b62 100644 --- a/noir/noir-repo/compiler/noirc_driver/src/lib.rs +++ b/noir/noir-repo/compiler/noirc_driver/src/lib.rs @@ -545,6 +545,7 @@ fn compile_contract_inner( functions.push(ContractFunction { name, + hash: function.hash, custom_attributes, abi: function.abi, bytecode: function.program, diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 50ce5d4a78d7..742ccb4c9de8 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -771,21 +771,28 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> { // Slice access checks are generated separately against the slice's dynamic length field. if matches!(dfg.type_of_value(*array), Type::Array(..)) - && !dfg.is_safe_index(*index, *array) + && !dfg.is_safe_brillig_index(*index, *array) { self.validate_array_index(array_variable, index_variable); } - let items_pointer = - self.brillig_context.codegen_make_array_or_vector_items_pointer(array_variable); - - self.brillig_context.codegen_load_with_offset( - items_pointer, - index_variable, - destination_variable.extract_register(), - ); - - self.brillig_context.deallocate_register(items_pointer); + if dfg.is_constant(*index) { + self.brillig_context.codegen_load_with_offset( + array_variable.extract_register(), + index_variable, + destination_variable.extract_register(), + ); + } else { + let items_pointer = self + .brillig_context + .codegen_make_array_or_vector_items_pointer(array_variable); + self.brillig_context.codegen_load_with_offset( + items_pointer, + index_variable, + destination_variable.extract_register(), + ); + self.brillig_context.deallocate_register(items_pointer); + } } Instruction::ArraySet { array, index, value, mutable } => { let source_variable = self.convert_ssa_value(*array, dfg); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_globals.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_globals.rs index 2498569aaf21..317f763078e1 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_globals.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_globals.rs @@ -430,6 +430,8 @@ mod tests { "; let ssa = Ssa::from_str(src).unwrap(); + // Need to run SSA pass that sets up Brillig array gets + let ssa = ssa.brillig_array_gets(); // Need to run DIE to generate the used globals map, which is necessary for Brillig globals generation. let mut ssa = ssa.dead_instruction_elimination(); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs index d12c8fd7672e..b4219ce278f0 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs @@ -223,6 +223,13 @@ fn optimize_all(builder: SsaBuilder, options: &SsaEvaluatorOptions) -> Result false, } } + + /// Arrays are represented as `[RC, ...items]` where RC stands for reference count. + /// By the time of Brillig generation we expect all constant indices + /// to already account for the extra offset from the RC. + pub(crate) fn is_safe_brillig_index(&self, index: ValueId, array: ValueId) -> bool { + #[allow(clippy::match_like_matches_macro)] + match (self.type_of_value(array), self.get_numeric_constant(index)) { + (Type::Array(elements, len), Some(index)) => { + (index.to_u128() - 1) < (len as u128 * elements.len() as u128) + } + _ => false, + } + } + /// Sets the terminator instruction for the given basic block pub(crate) fn set_block_terminator( &mut self, diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/brillig_array_gets.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/brillig_array_gets.rs new file mode 100644 index 000000000000..c5d485dd1613 --- /dev/null +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/brillig_array_gets.rs @@ -0,0 +1,168 @@ +//! In the Brillig runtime arrays are represented as [RC, ...items], +//! Certain operations such as array gets only utilize the items pointer. +//! Without handling the items pointer offset in SSA, it is left to Brillig generation +//! to offset the array pointer. +//! +//! Slices are represented as Brillig vectors, where the items pointer instead starts at three rather than one. +//! A Brillig vector is represented as [RC, Size, Capacity, ...items]. +//! +//! For array operations with constant indices adding an instruction to offset the pointer +//! is unnecessary as we already know the index. This pass looks for such array operations +//! with constant indices and replaces their index with the appropriate offset. + +use fxhash::FxHashMap as HashMap; + +use crate::{ + brillig::brillig_ir::BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, + ssa::{ + Ssa, + ir::{ + function::Function, + instruction::Instruction, + types::{NumericType, Type}, + }, + }, +}; + +impl Ssa { + #[tracing::instrument(level = "trace", skip(self))] + pub(crate) fn brillig_array_gets(mut self) -> Ssa { + let brillig_functions = + self.functions.values_mut().filter(|function| function.runtime().is_brillig()); + for function in brillig_functions { + function.brillig_array_gets(); + } + + self + } +} + +impl Function { + pub(super) fn brillig_array_gets(&mut self) { + let reachable_blocks = self.reachable_blocks(); + + let mut instructions_to_update = HashMap::default(); + for block_id in reachable_blocks.into_iter() { + for instruction_id in self.dfg[block_id].instructions() { + if let Instruction::ArrayGet { array, index } = self.dfg[*instruction_id] { + if self.dfg.is_constant(index) { + instructions_to_update.insert( + *instruction_id, + (Instruction::ArrayGet { array, index }, block_id), + ); + } + } + } + } + + for (instruction_id, _) in instructions_to_update { + let new_instruction = match self.dfg[instruction_id] { + Instruction::ArrayGet { array, index } => { + let index_constant = + self.dfg.get_numeric_constant(index).expect("ICE: Expected constant index"); + let offset = if matches!(self.dfg.type_of_value(array), Type::Array(..)) { + // Brillig arrays are [RC, ...items] + 1u128 + } else { + // Brillig vectors are [RC, Size, Capacity, ...items] + 3u128 + }; + let index = self.dfg.make_constant( + index_constant + offset.into(), + NumericType::unsigned(BRILLIG_MEMORY_ADDRESSING_BIT_SIZE), + ); + Instruction::ArrayGet { array, index } + } + _ => { + continue; + } + }; + self.dfg[instruction_id] = new_instruction; + } + } +} + +#[cfg(test)] +mod tests { + use crate::ssa::opt::assert_normalized_ssa_equals; + + use super::Ssa; + + #[test] + fn offset_array_get_constant_index() { + let src = " + brillig(inline) fn main f0 { + b0(v0: [Field; 3]): + v2 = array_get v0, index u32 0 -> Field + return v2 + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.brillig_array_gets(); + + let expected = " + brillig(inline) fn main f0 { + b0(v0: [Field; 3]): + v2 = array_get v0, index u32 1 -> Field + return v2 + } + "; + + assert_normalized_ssa_equals(ssa, expected); + } + + #[test] + fn do_not_offset_dynamic_array_get() { + let src = " + brillig(inline) fn main f0 { + b0(v0: [Field; 3], v1: u32): + v2 = array_get v0, index v1 -> Field + return v2 + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.brillig_array_gets(); + assert_normalized_ssa_equals(ssa, src); + } + + #[test] + fn do_not_offset_array_get_in_acir() { + let src = " + acir(inline) fn main f0 { + b0(v0: [Field; 3]): + v2 = array_get v0, index u32 0 -> Field + return v2 + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.brillig_array_gets(); + assert_normalized_ssa_equals(ssa, src); + } + + #[test] + fn offset_slice_array_get_constant_index() { + let src = " + brillig(inline) fn main f0 { + b0(v0: [Field]): + v2 = array_get v0, index u32 0 -> Field + return v2 + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.brillig_array_gets(); + + let expected = " + brillig(inline) fn main f0 { + b0(v0: [Field]): + v2 = array_get v0, index u32 3 -> Field + return v2 + } + "; + + assert_normalized_ssa_equals(ssa, expected); + } +} diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index d038a56c0f19..3bde1d7530f0 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -1451,6 +1451,8 @@ mod test { } "; let ssa = Ssa::from_str(src).unwrap(); + // Need to run SSA pass that sets up Brillig array gets + let ssa = ssa.brillig_array_gets(); let brillig = ssa.to_brillig(&BrilligOptions::default()); let expected = " diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs index 7dfd47d997fd..38004cdf1511 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs @@ -7,6 +7,7 @@ mod array_set; mod as_slice_length; mod assert_constant; +mod brillig_array_gets; pub(crate) mod brillig_entry_points; mod check_u128_mul_overflow; mod constant_folding; diff --git a/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs b/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs index 1a9848633795..78b29b8c1fdd 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs @@ -505,7 +505,11 @@ impl PathSegment { /// /// Returns an empty span at the end of `foo` if there's no turbofish. pub fn turbofish_span(&self) -> Span { - Span::from(self.ident.span().end()..self.location.span.end()) + if self.ident.location().file == self.location.file { + Span::from(self.ident.span().end()..self.location.span.end()) + } else { + self.location.span + } } pub fn turbofish_location(&self) -> Location { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/debug/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/debug/mod.rs index a8bcb6ca9cbd..48bed1c41997 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/debug/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/debug/mod.rs @@ -382,7 +382,7 @@ impl DebugInstrumenter { fn walk_expr(&mut self, expr: &mut ast::Expression) { match &mut expr.kind { - ast::ExpressionKind::Block(ast::BlockExpression { ref mut statements, .. }) => { + ast::ExpressionKind::Block(ast::BlockExpression { statements, .. }) => { self.scope.push(HashMap::default()); self.walk_scope(statements, expr.location); } @@ -396,19 +396,19 @@ impl DebugInstrumenter { ast::ExpressionKind::Call(call_expr) => { // TODO: push a stack frame or something here? self.walk_expr(&mut call_expr.func); - call_expr.arguments.iter_mut().for_each(|ref mut expr| { + call_expr.arguments.iter_mut().for_each(|expr| { self.walk_expr(expr); }); } ast::ExpressionKind::MethodCall(mc_expr) => { // TODO: also push a stack frame here self.walk_expr(&mut mc_expr.object); - mc_expr.arguments.iter_mut().for_each(|ref mut expr| { + mc_expr.arguments.iter_mut().for_each(|expr| { self.walk_expr(expr); }); } ast::ExpressionKind::Constructor(c_expr) => { - c_expr.fields.iter_mut().for_each(|(_id, ref mut expr)| { + c_expr.fields.iter_mut().for_each(|(_id, expr)| { self.walk_expr(expr); }); } @@ -492,7 +492,7 @@ impl DebugInstrumenter { ast::StatementKind::Semi(expr) => { self.walk_expr(expr); } - ast::StatementKind::For(ref mut for_stmt) => { + ast::StatementKind::For(for_stmt) => { self.walk_for(for_stmt); } _ => {} // Constrain, Error diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/enums.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/enums.rs index 2ebdc8c1be05..90849a750d56 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/enums.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/enums.rs @@ -334,44 +334,30 @@ impl Elaborator<'_> { // - Possible diagnostics improvement: warn if `a` is defined as a variable // when there is a matching enum variant with name `Foo::a` which can // be imported. The user likely intended to reference the enum variant. - let path_len = path.segments.len(); let location = path.location; let last_ident = path.last_ident(); + // Setting this to `Some` allows us to shadow globals with the same name. + // We should avoid this if there is a `::` in the path since that means the + // user is trying to resolve to a non-local item. + let shadow_existing = path.is_ident().then_some(last_ident); + match self.resolve_path_or_error(path) { Ok(resolution) => self.path_resolution_to_constructor( resolution, + shadow_existing, Vec::new(), expected_type, location, variables_defined, ), - Err(_) if path_len == 1 => { - // Define the variable - let kind = DefinitionKind::Local(None); - - if let Some(existing) = - variables_defined.iter().find(|elem| *elem == &last_ident) - { - // Allow redefinition of `_` only, to ignore variables - if last_ident.0.contents != "_" { - let error = ResolverError::VariableAlreadyDefinedInPattern { - existing: existing.clone(), - new_location: last_ident.location(), - }; - self.push_err(error); - } + Err(error) => { + if let Some(name) = shadow_existing { + self.define_pattern_variable(name, expected_type, variables_defined) } else { - variables_defined.push(last_ident.clone()); + self.push_err(error); + Pattern::Error } - - let id = self.add_variable_decl(last_ident, false, true, true, kind).id; - self.interner.push_definition_type(id, expected_type.clone()); - Pattern::Binding(id) - } - Err(error) => { - self.push_err(error); - Pattern::Error } } } @@ -436,6 +422,32 @@ impl Elaborator<'_> { } } + fn define_pattern_variable( + &mut self, + name: Ident, + expected_type: &Type, + variables_defined: &mut Vec, + ) -> Pattern { + // Define the variable + let kind = DefinitionKind::Local(None); + + if let Some(existing) = variables_defined.iter().find(|elem| *elem == &name) { + // Allow redefinition of `_` only, to ignore variables + if name.0.contents != "_" { + self.push_err(ResolverError::VariableAlreadyDefinedInPattern { + existing: existing.clone(), + new_location: name.location(), + }); + } + } else { + variables_defined.push(name.clone()); + } + + let id = self.add_variable_decl(name, false, true, true, kind).id; + self.interner.push_definition_type(id, expected_type.clone()); + Pattern::Binding(id) + } + fn constructor_to_pattern( &mut self, constructor: ConstructorExpression, @@ -490,13 +502,21 @@ impl Elaborator<'_> { expected_type: &Type, variables_defined: &mut Vec, ) -> Pattern { + let syntax_error = |this: &mut Self| { + this.push_err(ResolverError::InvalidSyntaxInPattern { location: name.location }); + Pattern::Error + }; + match name.kind { ExpressionKind::Variable(path) => { let location = path.location; match self.resolve_path_or_error(path) { + // Use None for `name` here - we don't want to define a variable if this + // resolves to an existing item. Ok(resolution) => self.path_resolution_to_constructor( resolution, + None, args, expected_type, location, @@ -526,28 +546,47 @@ impl Elaborator<'_> { variables_defined, ) } else { - panic!("Invalid expr kind {name}") + syntax_error(self) } } - other => todo!("invalid constructor `{other}`"), + _ => syntax_error(self), } } + /// Convert a PathResolutionItem - usually an enum variant or global - to a Constructor. + /// If `name` is `Some`, we'll define a Pattern::Binding instead of erroring if the + /// item doesn't resolve to a variant or global. This would shadow an existing + /// value such as a free function. Generally this is desired unless the variable was + /// a path with multiple components such as `foo::bar` which should always be treated as + /// a path to an existing item. fn path_resolution_to_constructor( &mut self, - name: PathResolutionItem, + resolution: PathResolutionItem, + name: Option, args: Vec, expected_type: &Type, location: Location, variables_defined: &mut Vec, ) -> Pattern { - let (actual_type, expected_arg_types, variant_index) = match name { + let (actual_type, expected_arg_types, variant_index) = match &resolution { PathResolutionItem::Global(id) => { // variant constant - let global = self.interner.get_global(id); - let variant_index = match global.value { - GlobalValue::Resolved(Value::Enum(tag, ..)) => tag, - _ => todo!("Value is not an enum constant"), + self.elaborate_global_if_unresolved(id); + let global = self.interner.get_global(*id); + let variant_index = match &global.value { + GlobalValue::Resolved(Value::Enum(tag, ..)) => *tag, + // This may be a global constant. Treat it like a normal constant + GlobalValue::Resolved(value) => { + let value = value.clone(); + return self.global_constant_to_integer_constructor( + value, + expected_type, + location, + ); + } + // We tried to resolve this value above so there must have been an error + // in doing so. Avoid reporting an additional error. + _ => return Pattern::Error, }; let global_type = self.interner.definition_type(global.definition_id); @@ -556,8 +595,12 @@ impl Elaborator<'_> { } PathResolutionItem::Method(_type_id, _type_turbofish, func_id) => { // TODO(#7430): Take type_turbofish into account when instantiating the function's type - let meta = self.interner.function_meta(&func_id); - let Some(variant_index) = meta.enum_variant_index else { todo!("not a variant") }; + let meta = self.interner.function_meta(func_id); + let Some(variant_index) = meta.enum_variant_index else { + let item = resolution.description(); + self.push_err(ResolverError::UnexpectedItemInPattern { location, item }); + return Pattern::Error; + }; let (actual_type, expected_arg_types) = match meta.typ.instantiate(self.interner).0 { @@ -567,18 +610,22 @@ impl Elaborator<'_> { (actual_type, expected_arg_types, variant_index) } - PathResolutionItem::Module(_) => todo!("path_resolution_to_constructor {name:?}"), - PathResolutionItem::Type(_) => todo!("path_resolution_to_constructor {name:?}"), - PathResolutionItem::TypeAlias(_) => todo!("path_resolution_to_constructor {name:?}"), - PathResolutionItem::Trait(_) => todo!("path_resolution_to_constructor {name:?}"), - PathResolutionItem::ModuleFunction(_) => { - todo!("path_resolution_to_constructor {name:?}") - } - PathResolutionItem::TypeAliasFunction(_, _, _) => { - todo!("path_resolution_to_constructor {name:?}") - } - PathResolutionItem::TraitFunction(_, _, _) => { - todo!("path_resolution_to_constructor {name:?}") + PathResolutionItem::Module(_) + | PathResolutionItem::Type(_) + | PathResolutionItem::TypeAlias(_) + | PathResolutionItem::Trait(_) + | PathResolutionItem::ModuleFunction(_) + | PathResolutionItem::TypeAliasFunction(_, _, _) + | PathResolutionItem::TraitFunction(_, _, _) => { + // This variable refers to an existing item + if let Some(name) = name { + // If name is set, shadow the existing item + return self.define_pattern_variable(name, expected_type, variables_defined); + } else { + let item = resolution.description(); + self.push_err(ResolverError::UnexpectedItemInPattern { location, item }); + return Pattern::Error; + } } }; @@ -591,7 +638,10 @@ impl Elaborator<'_> { }); if args.len() != expected_arg_types.len() { - // error expected N args, found M? + let expected = expected_arg_types.len(); + let found = args.len(); + self.push_err(TypeCheckError::ArityMisMatch { expected, found, location }); + return Pattern::Error; } let args = args.into_iter().zip(expected_arg_types); @@ -602,6 +652,55 @@ impl Elaborator<'_> { Pattern::Constructor(constructor, args) } + fn global_constant_to_integer_constructor( + &mut self, + constant: Value, + expected_type: &Type, + location: Location, + ) -> Pattern { + let actual_type = constant.get_type(); + self.unify(&actual_type, expected_type, || TypeCheckError::TypeMismatch { + expected_typ: expected_type.to_string(), + expr_typ: actual_type.to_string(), + expr_location: location, + }); + + // Convert a signed integer type like i32 to SignedField + macro_rules! signed_to_signed_field { + ($value:expr) => {{ + let negative = $value < 0; + // Widen the value so that SignedType::MIN does not wrap to 0 when negated below + let mut widened = $value as i128; + if negative { + widened = -widened; + } + SignedField::new(widened.into(), negative) + }}; + } + + let value = match constant { + Value::Bool(value) => SignedField::positive(value), + Value::Field(value) => SignedField::positive(value), + Value::I8(value) => signed_to_signed_field!(value), + Value::I16(value) => signed_to_signed_field!(value), + Value::I32(value) => signed_to_signed_field!(value), + Value::I64(value) => signed_to_signed_field!(value), + Value::U1(value) => SignedField::positive(value), + Value::U8(value) => SignedField::positive(value as u128), + Value::U16(value) => SignedField::positive(value as u128), + Value::U32(value) => SignedField::positive(value), + Value::U64(value) => SignedField::positive(value), + Value::U128(value) => SignedField::positive(value), + Value::Zeroed(_) => SignedField::positive(0u32), + _ => { + self.push_err(ResolverError::NonIntegerGlobalUsedInPattern { location }); + return Pattern::Error; + } + }; + + Pattern::Int(value) + } + fn struct_name_and_field_types( &mut self, typ: &Type, @@ -663,8 +762,6 @@ impl Elaborator<'_> { Ok(HirMatch::Switch(branch_var, cases, Some(fallback))) } - Type::Array(_, _) => todo!(), - Type::Slice(_) => todo!(), Type::Bool => { let cases = vec![ (Constructor::False, Vec::new(), Vec::new()), @@ -715,12 +812,16 @@ impl Elaborator<'_> { } else { drop(def); let typ = Type::DataType(type_def, generics); - todo!("Cannot match on type {typ}") + Err(ResolverError::TypeUnsupportedInMatch { typ, location }) } } - typ @ (Type::Alias(_, _) - | Type::TypeVariable(_) + // We could match on these types in the future + typ @ (Type::Array(_, _) + | Type::Slice(_) | Type::String(_) + // But we'll never be able to match on these + | Type::Alias(_, _) + | Type::TypeVariable(_) | Type::FmtString(_, _) | Type::TraitAsType(_, _, _) | Type::NamedGeneric(_, _) @@ -731,7 +832,9 @@ impl Elaborator<'_> { | Type::Constant(_, _) | Type::Quoted(_) | Type::InfixExpr(_, _, _, _) - | Type::Error) => todo!("Cannot match on type {typ:?}"), + | Type::Error) => { + Err(ResolverError::TypeUnsupportedInMatch { typ, location }) + }, } } @@ -769,11 +872,9 @@ impl Elaborator<'_> { let (key, cons) = match col.pattern { Pattern::Int(val) => ((val, val), Constructor::Int(val)), Pattern::Range(start, stop) => ((start, stop), Constructor::Range(start, stop)), - Pattern::Error => continue, - pattern => { - eprintln!("Unexpected pattern for integer type: {pattern:?}"); - continue; - } + // Any other pattern shouldn't have an integer type and we expect a type + // check error to already have been issued. + _ => continue, }; if let Some(index) = tested.get(&key) { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs index eeb79afacf9a..ea784812aafb 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs @@ -209,6 +209,7 @@ pub enum ElaborateReason { /// Evaluating `Module::add_item` AddingItemToModule, } + impl ElaborateReason { fn to_macro_error(self, error: CompilationError, location: Location) -> ComptimeError { match self { 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 b61b734fba26..0610155a7981 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs @@ -1051,7 +1051,7 @@ impl Elaborator<'_> { // Matches on TypeVariable must be first to follow any type // bindings. (TypeVariable(var), other) | (other, TypeVariable(var)) => { - if let TypeBinding::Bound(ref binding) = &*var.borrow() { + if let TypeBinding::Bound(binding) = &*var.borrow() { return self.comparator_operand_type_rules(other, binding, op, location); } @@ -1178,7 +1178,7 @@ impl Elaborator<'_> { }; return Ok((lhs_type.clone(), use_impl)); } - if let TypeBinding::Bound(ref binding) = &*int.borrow() { + if let TypeBinding::Bound(binding) = &*int.borrow() { return self.infix_operand_type_rules(binding, op, other, location); } let use_impl = self.bind_type_variables_for_infix(lhs_type, op, rhs_type, location); @@ -1263,7 +1263,7 @@ impl Elaborator<'_> { // Matches on TypeVariable must be first so that we follow any type // bindings. TypeVariable(int) => { - if let TypeBinding::Bound(ref binding) = &*int.borrow() { + if let TypeBinding::Bound(binding) = &*int.borrow() { return self.prefix_operand_type_rules(op, binding, location); } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs index 96dbf61533d4..7bd5c79dc905 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -186,6 +186,12 @@ pub enum ResolverError { InvalidSyntaxInPattern { location: Location }, #[error("Variable '{existing}' was already defined in the same match pattern")] VariableAlreadyDefinedInPattern { existing: Ident, new_location: Location }, + #[error("Only integer globals can be used in match patterns")] + NonIntegerGlobalUsedInPattern { location: Location }, + #[error("Cannot match on values of type `{typ}`")] + TypeUnsupportedInMatch { typ: Type, location: Location }, + #[error("Expected a struct, enum, or literal value in pattern, but found a {item}")] + UnexpectedItemInPattern { location: Location, item: &'static str }, } impl ResolverError { @@ -252,6 +258,9 @@ impl ResolverError { | ResolverError::MutatingComptimeInNonComptimeContext { location, .. } | ResolverError::InvalidInternedStatementInExpr { location, .. } | ResolverError::InvalidSyntaxInPattern { location } + | ResolverError::NonIntegerGlobalUsedInPattern { location, .. } + | ResolverError::TypeUnsupportedInMatch { location, .. } + | ResolverError::UnexpectedItemInPattern { location, .. } | ResolverError::VariableAlreadyDefinedInPattern { new_location: location, .. } => { *location } @@ -298,7 +307,7 @@ impl<'a> From<&'a ResolverError> for Diagnostic { let mut diagnostic = Diagnostic::simple_warning( format!("unused variable {name}"), - "unused variable ".to_string(), + "unused variable".to_string(), ident.location(), ); diagnostic.unnecessary = true; @@ -794,6 +803,25 @@ impl<'a> From<&'a ResolverError> for Diagnostic { error.add_secondary(format!("`{existing}` was previously defined here"), existing.location()); error }, + ResolverError::NonIntegerGlobalUsedInPattern { location } => { + let message = "Only integer or boolean globals can be used in match patterns".to_string(); + let secondary = "This global is not an integer or boolean".to_string(); + Diagnostic::simple_error(message, secondary, *location) + }, + ResolverError::TypeUnsupportedInMatch { typ, location } => { + Diagnostic::simple_error( + format!("Cannot match on values of type `{typ}`"), + String::new(), + *location, + ) + }, + ResolverError::UnexpectedItemInPattern { item, location } => { + Diagnostic::simple_error( + format!("Expected a struct, enum, or literal pattern, but found a {item}"), + String::new(), + *location, + ) + }, } } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/import.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/import.rs index ceb1dee9cb35..16e25b804651 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -115,7 +115,8 @@ impl<'a> From<&'a PathResolutionError> for CustomDiagnostic { CustomDiagnostic::simple_warning(error.to_string(), String::new(), ident.location()) } PathResolutionError::UnresolvedWithPossibleTraitsToImport { ident, traits } => { - let traits = vecmap(traits, |trait_name| format!("`{}`", trait_name)); + let mut traits = vecmap(traits, |trait_name| format!("`{}`", trait_name)); + traits.sort(); CustomDiagnostic::simple_error( error.to_string(), format!( @@ -126,7 +127,8 @@ impl<'a> From<&'a PathResolutionError> for CustomDiagnostic { ) } PathResolutionError::MultipleTraitsInScope { ident, traits } => { - let traits = vecmap(traits, |trait_name| format!("`{}`", trait_name)); + let mut traits = vecmap(traits, |trait_name| format!("`{}`", trait_name)); + traits.sort(); CustomDiagnostic::simple_error( error.to_string(), format!( 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 1fdc0b30f103..f3c2c635b041 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 @@ -540,7 +540,7 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic { Diagnostic::simple_error(message, String::new(), *location) } - TypeCheckError::CallDeprecated { location, ref note, .. } => { + TypeCheckError::CallDeprecated { location, note, .. } => { let primary_message = error.to_string(); let secondary_message = note.clone().unwrap_or_default(); 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 01d726386a16..4fd5b46657ec 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 @@ -1454,8 +1454,8 @@ impl Type { Type::NamedGeneric(var, _) => var.kind(), Type::Constant(_, kind) => kind.clone(), Type::TypeVariable(var) => match &*var.borrow() { - TypeBinding::Bound(ref typ) => typ.kind(), - TypeBinding::Unbound(_, ref type_var_kind) => type_var_kind.clone(), + TypeBinding::Bound(typ) => typ.kind(), + TypeBinding::Unbound(_, type_var_kind) => type_var_kind.clone(), }, Type::InfixExpr(lhs, _op, rhs, _) => lhs.infix_kind(rhs), Type::Alias(def, generics) => def.borrow().get_type(generics).kind(), @@ -1651,8 +1651,8 @@ impl Type { typ.try_bind_to_polymorphic_int(var, bindings, only_integer) } // Avoid infinitely recursive bindings - TypeBinding::Unbound(ref id, _) if *id == target_id => Ok(()), - TypeBinding::Unbound(ref new_target_id, Kind::IntegerOrField) => { + TypeBinding::Unbound(id, _) if *id == target_id => Ok(()), + TypeBinding::Unbound(new_target_id, Kind::IntegerOrField) => { let type_var_kind = Kind::IntegerOrField; if only_integer { let var_clone = var.clone(); @@ -1675,7 +1675,7 @@ impl Type { bindings.insert(target_id, (var.clone(), Kind::Integer, this.clone())); Ok(()) } - TypeBinding::Unbound(new_target_id, ref type_var_kind) => { + TypeBinding::Unbound(new_target_id, type_var_kind) => { let var_clone = var.clone(); // Bind to the most specific type variable kind let clone_kind = @@ -2913,7 +2913,7 @@ impl From<&Type> for PrintableType { Type::Error => unreachable!(), Type::Unit => PrintableType::Unit, Type::Constant(_, _) => unreachable!(), - Type::DataType(def, ref args) => { + Type::DataType(def, args) => { let data_type = def.borrow(); let name = data_type.name.to_string(); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/lexer/token.rs b/noir/noir-repo/compiler/noirc_frontend/src/lexer/token.rs index 26b17295d798..7367489f6251 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/lexer/token.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/lexer/token.rs @@ -255,18 +255,18 @@ pub enum Token { pub fn token_to_borrowed_token(token: &Token) -> BorrowedToken<'_> { match token { - Token::Ident(ref s) => BorrowedToken::Ident(s), + Token::Ident(s) => BorrowedToken::Ident(s), Token::Int(n) => BorrowedToken::Int(*n), Token::Bool(b) => BorrowedToken::Bool(*b), - Token::Str(ref b) => BorrowedToken::Str(b), - Token::FmtStr(ref b, length) => BorrowedToken::FmtStr(b, *length), - Token::RawStr(ref b, hashes) => BorrowedToken::RawStr(b, *hashes), + Token::Str(b) => BorrowedToken::Str(b), + Token::FmtStr(b, length) => BorrowedToken::FmtStr(b, *length), + Token::RawStr(b, hashes) => BorrowedToken::RawStr(b, *hashes), Token::Keyword(k) => BorrowedToken::Keyword(*k), Token::AttributeStart { is_inner, is_tag } => { BorrowedToken::AttributeStart { is_inner: *is_inner, is_tag: *is_tag } } - Token::LineComment(ref s, _style) => BorrowedToken::LineComment(s, *_style), - Token::BlockComment(ref s, _style) => BorrowedToken::BlockComment(s, *_style), + Token::LineComment(s, _style) => BorrowedToken::LineComment(s, *_style), + Token::BlockComment(s, _style) => BorrowedToken::BlockComment(s, *_style), Token::Quote(stream) => BorrowedToken::Quote(stream), Token::QuotedType(id) => BorrowedToken::QuotedType(*id), Token::InternedExpr(id) => BorrowedToken::InternedExpression(*id), @@ -274,7 +274,7 @@ pub fn token_to_borrowed_token(token: &Token) -> BorrowedToken<'_> { Token::InternedLValue(id) => BorrowedToken::InternedLValue(*id), Token::InternedUnresolvedTypeData(id) => BorrowedToken::InternedUnresolvedTypeData(*id), Token::InternedPattern(id) => BorrowedToken::InternedPattern(*id), - Token::IntType(ref i) => BorrowedToken::IntType(i.clone()), + Token::IntType(i) => BorrowedToken::IntType(i.clone()), Token::Less => BorrowedToken::Less, Token::LessEqual => BorrowedToken::LessEqual, Token::Greater => BorrowedToken::Greater, @@ -312,7 +312,7 @@ pub fn token_to_borrowed_token(token: &Token) -> BorrowedToken<'_> { Token::DollarSign => BorrowedToken::DollarSign, Token::EOF => BorrowedToken::EOF, Token::Invalid(c) => BorrowedToken::Invalid(*c), - Token::Whitespace(ref s) => BorrowedToken::Whitespace(s), + Token::Whitespace(s) => BorrowedToken::Whitespace(s), Token::UnquoteMarker(id) => BorrowedToken::UnquoteMarker(*id), } } @@ -578,7 +578,7 @@ pub enum TokenKind { impl fmt::Display for TokenKind { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - TokenKind::Token(ref tok) => write!(f, "{tok}"), + TokenKind::Token(tok) => write!(f, "{tok}"), TokenKind::Ident => write!(f, "identifier"), TokenKind::Literal => write!(f, "literal"), TokenKind::Keyword => write!(f, "keyword"), @@ -926,9 +926,9 @@ impl fmt::Display for FunctionAttribute { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { FunctionAttribute::Test(scope) => write!(f, "#[test{scope}]"), - FunctionAttribute::Foreign(ref k) => write!(f, "#[foreign({k})]"), - FunctionAttribute::Builtin(ref k) => write!(f, "#[builtin({k})]"), - FunctionAttribute::Oracle(ref k) => write!(f, "#[oracle({k})]"), + FunctionAttribute::Foreign(k) => write!(f, "#[foreign({k})]"), + FunctionAttribute::Builtin(k) => write!(f, "#[builtin({k})]"), + FunctionAttribute::Oracle(k) => write!(f, "#[oracle({k})]"), FunctionAttribute::Fold => write!(f, "#[fold]"), FunctionAttribute::NoPredicates => write!(f, "#[no_predicates]"), FunctionAttribute::InlineAlways => write!(f, "#[inline_always]"), @@ -1001,18 +1001,18 @@ impl SecondaryAttribute { pub(crate) fn contents(&self) -> String { match self { SecondaryAttribute::Deprecated(None) => "deprecated".to_string(), - SecondaryAttribute::Deprecated(Some(ref note)) => { + SecondaryAttribute::Deprecated(Some(note)) => { format!("deprecated({note:?})") } - SecondaryAttribute::Tag(ref attribute) => format!("'{}", attribute.contents), - SecondaryAttribute::Meta(ref meta) => meta.to_string(), + SecondaryAttribute::Tag(attribute) => format!("'{}", attribute.contents), + SecondaryAttribute::Meta(meta) => meta.to_string(), SecondaryAttribute::ContractLibraryMethod => "contract_library_method".to_string(), SecondaryAttribute::Export => "export".to_string(), - SecondaryAttribute::Field(ref k) => format!("field({k})"), - SecondaryAttribute::Abi(ref k) => format!("abi({k})"), + SecondaryAttribute::Field(k) => format!("field({k})"), + SecondaryAttribute::Abi(k) => format!("abi({k})"), SecondaryAttribute::Varargs => "varargs".to_string(), SecondaryAttribute::UseCallersScope => "use_callers_scope".to_string(), - SecondaryAttribute::Allow(ref k) => format!("allow({k})"), + SecondaryAttribute::Allow(k) => format!("allow({k})"), } } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs index 1e14f497ac5c..62ed1ef2e684 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -1182,7 +1182,7 @@ impl<'interner> Monomorphizer<'interner> { unreachable!("All TraitAsType should be replaced before calling convert_type"); } HirType::NamedGeneric(binding, _) => { - if let TypeBinding::Bound(ref binding) = &*binding.borrow() { + if let TypeBinding::Bound(binding) = &*binding.borrow() { return Self::convert_type(binding, location); } @@ -1198,12 +1198,12 @@ impl<'interner> Monomorphizer<'interner> { Self::convert_type(to, location)? } - HirType::TypeVariable(ref binding) => { + HirType::TypeVariable(binding) => { let type_var_kind = match &*binding.borrow() { - TypeBinding::Bound(ref binding) => { + TypeBinding::Bound(binding) => { return Self::convert_type(binding, location); } - TypeBinding::Unbound(_, ref type_var_kind) => type_var_kind.clone(), + TypeBinding::Unbound(_, type_var_kind) => type_var_kind.clone(), }; // Default any remaining unbound type variables. @@ -1328,18 +1328,18 @@ impl<'interner> Monomorphizer<'interner> { HirType::Array(_length, element) => Self::check_type(element.as_ref(), location), HirType::Slice(element) => Self::check_type(element.as_ref(), location), HirType::NamedGeneric(binding, _) => { - if let TypeBinding::Bound(ref binding) = &*binding.borrow() { + if let TypeBinding::Bound(binding) = &*binding.borrow() { return Self::check_type(binding, location); } Ok(()) } - HirType::TypeVariable(ref binding) => { + HirType::TypeVariable(binding) => { let type_var_kind = match &*binding.borrow() { TypeBinding::Bound(binding) => { return Self::check_type(binding, location); } - TypeBinding::Unbound(_, ref type_var_kind) => type_var_kind.clone(), + TypeBinding::Unbound(_, type_var_kind) => type_var_kind.clone(), }; // Default any remaining unbound type variables. diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/path.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/path.rs index 239ee8a4d6db..a58bd9e1bb18 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/path.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/path.rs @@ -121,7 +121,11 @@ impl Parser<'_> { None }; - segments.push(PathSegment { ident, generics, location }); + segments.push(PathSegment { + ident, + generics, + location: self.location_since(location), + }); if self.at(Token::DoubleColon) && matches!(self.next_token.token(), Token::Ident(..)) diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs index ca52a2a9d3de..7bbe1f60873e 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs @@ -3,6 +3,7 @@ mod aliases; mod arithmetic_generics; mod bound_checks; +mod enums; mod imports; mod metaprogramming; mod name_shadowing; @@ -15,23 +16,17 @@ mod visibility; // XXX: These tests repeat a lot of code // what we should do is have test cases which are passed to a test harness // A test harness will allow for more expressive and readable tests -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashMap}; use crate::elaborator::{FrontendOptions, UnstableFeature}; use fm::FileId; use iter_extended::vecmap; -use noirc_errors::Location; +use noirc_errors::{CustomDiagnostic, Location, Span}; -use crate::ast::IntegerBitSize; use crate::hir::Context; -use crate::hir::comptime::InterpreterError; use crate::hir::def_collector::dc_crate::CompilationError; -use crate::hir::def_collector::errors::{DefCollectorErrorKind, DuplicateType}; use crate::hir::def_map::ModuleData; -use crate::hir::resolution::errors::ResolverError; -use crate::hir::resolution::import::PathResolutionError; -use crate::hir::type_check::TypeCheckError; use crate::node_interner::{NodeInterner, StmtId}; use crate::hir::def_collector::dc_crate::DefCollector; @@ -62,7 +57,7 @@ pub(crate) fn remove_experimental_warnings(errors: &mut Vec) { pub(crate) fn get_program(src: &str) -> (ParsedModule, Context, Vec) { let allow_parser_errors = false; - get_program_with_maybe_parser_errors(src, allow_parser_errors, FrontendOptions::test_default()) + get_program_with_options(src, allow_parser_errors, FrontendOptions::test_default()) } pub(crate) fn get_program_using_features( @@ -72,13 +67,13 @@ pub(crate) fn get_program_using_features( let allow_parser_errors = false; let mut options = FrontendOptions::test_default(); options.enabled_unstable_features = features; - get_program_with_maybe_parser_errors(src, allow_parser_errors, options) + get_program_with_options(src, allow_parser_errors, options) } /// Compile a program. /// /// The stdlib is not available for these snippets. -pub(crate) fn get_program_with_maybe_parser_errors( +pub(crate) fn get_program_with_options( src: &str, allow_parser_errors: bool, options: FrontendOptions, @@ -150,6 +145,161 @@ fn assert_no_errors(src: &str) { } } +/// Given a source file with annotated errors, like this +/// +/// fn main() -> pub i32 { +/// ^^^ expected i32 because of return type +/// true +/// ~~~~ bool returned here +/// } +/// +/// where: +/// - lines with "^^^" are primary errors +/// - lines with "~~~" are secondary errors +/// +/// this method will check that compiling the program without those error markers +/// will produce errors at those locations and with/ those messages. +fn check_errors(src: &str) { + let allow_parser_errors = false; + check_errors_with_options(src, allow_parser_errors, FrontendOptions::test_default()); +} + +fn check_errors_using_features(src: &str, features: &[UnstableFeature]) { + let allow_parser_errors = false; + let mut options = FrontendOptions::test_default(); + options.enabled_unstable_features = features; + check_errors_with_options(src, allow_parser_errors, options); +} + +fn check_errors_with_options(src: &str, allow_parser_errors: bool, options: FrontendOptions) { + let lines = src.lines().collect::>(); + + // Here we'll hold just the lines that are code + let mut code_lines = Vec::new(); + // Here we'll capture lines that are primary error spans, like: + // + // ^^^ error message + let mut primary_spans_with_errors: Vec<(Span, String)> = Vec::new(); + // Here we'll capture lines that are secondary error spans, like: + // + // ~~~ error message + let mut secondary_spans_with_errors: Vec<(Span, String)> = Vec::new(); + // The byte at the start of this line + let mut byte = 0; + // The length of the last line, needed to go back to the byte at the beginning of the last line + let mut last_line_length = 0; + for line in lines { + if let Some((span, message)) = + get_error_line_span_and_message(line, '^', byte, last_line_length) + { + primary_spans_with_errors.push((span, message)); + continue; + } + + if let Some((span, message)) = + get_error_line_span_and_message(line, '~', byte, last_line_length) + { + secondary_spans_with_errors.push((span, message)); + continue; + } + + code_lines.push(line); + + byte += line.len() + 1; // For '\n' + last_line_length = line.len(); + } + + let mut primary_spans_with_errors: HashMap = + primary_spans_with_errors.into_iter().collect(); + + let mut secondary_spans_with_errors: HashMap = + secondary_spans_with_errors.into_iter().collect(); + + let src = code_lines.join("\n"); + let (_, _, errors) = get_program_with_options(&src, allow_parser_errors, options); + if errors.is_empty() && !primary_spans_with_errors.is_empty() { + panic!("Expected some errors but got none"); + } + + let errors = errors.iter().map(CustomDiagnostic::from).collect::>(); + for error in &errors { + let secondary = error + .secondaries + .first() + .unwrap_or_else(|| panic!("Expected {:?} to have a secondary label", error)); + let span = secondary.location.span; + let message = &error.message; + + let Some(expected_message) = primary_spans_with_errors.remove(&span) else { + if let Some(message) = secondary_spans_with_errors.get(&span) { + panic!( + "Error at {span:?} with message {message:?} is annotated as secondary but should be primary" + ); + } else { + panic!( + "Couldn't find primary error at {span:?} with message {message:?}.\nAll errors: {errors:?}" + ); + } + }; + + assert_eq!(message, &expected_message, "Primary error at {span:?} has unexpected message"); + + for secondary in &error.secondaries { + let message = &secondary.message; + if message.is_empty() { + continue; + } + + let span = secondary.location.span; + let Some(expected_message) = secondary_spans_with_errors.remove(&span) else { + if let Some(message) = primary_spans_with_errors.get(&span) { + panic!( + "Error at {span:?} with message {message:?} is annotated as primary but should be secondary" + ); + } else { + panic!( + "Couldn't find secondary error at {span:?} with message {message:?}.\nAll errors: {errors:?}" + ); + }; + }; + + assert_eq!( + message, &expected_message, + "Secondary error at {span:?} has unexpected message" + ); + } + } + + if !primary_spans_with_errors.is_empty() { + panic!("These primary errors didn't happen: {primary_spans_with_errors:?}"); + } + + if !secondary_spans_with_errors.is_empty() { + panic!("These secondary errors didn't happen: {secondary_spans_with_errors:?}"); + } +} + +/// Helper function for `check_errors` that returns the span that +/// `^^^^` or `~~~~` occupy, together with the message that follows it. +fn get_error_line_span_and_message( + line: &str, + char: char, + byte: usize, + last_line_length: usize, +) -> Option<(Span, String)> { + if !line.trim().starts_with(char) { + return None; + } + + let chars = line.chars().collect::>(); + let first_caret = chars.iter().position(|c| *c == char).unwrap(); + let last_caret = chars.iter().rposition(|c| *c == char).unwrap(); + let start = byte - last_line_length; + let span = Span::from((start + first_caret - 1) as u32..(start + last_caret) as u32); + let error = line.trim().trim_start_matches(char).trim().to_string(); + Some((span, error)) +} + #[test] fn check_trait_implemented_for_all_t() { let src = " @@ -212,10 +362,13 @@ fn check_trait_implementation_duplicate_method() { impl Default for Foo { // Duplicate trait methods should not compile fn default(x: Field, y: Field) -> Field { + ^^^^^^^ Duplicate definitions of trait associated function with name default found + ~~~~~~~ First trait associated function found here y + 2 * x } // Duplicate trait methods should not compile fn default(x: Field, y: Field) -> Field { + ~~~~~~~ Second trait associated function found here x + 2 * y } } @@ -223,31 +376,12 @@ fn check_trait_implementation_duplicate_method() { fn main() { let _ = Foo { bar: 1, array: [2, 3] }; // silence Foo never constructed warning }"; - - let errors = get_program_errors(src); - assert!(!has_parser_error(&errors)); - assert!(errors.len() == 1, "Expected 1 error, got: {:?}", errors); - - for err in errors { - match &err { - CompilationError::DefinitionError(DefCollectorErrorKind::Duplicate { - typ, - first_def, - second_def, - }) => { - assert_eq!(typ, &DuplicateType::TraitAssociatedFunction); - assert_eq!(first_def, "default"); - assert_eq!(second_def, "default"); - } - _ => { - panic!("No other errors are expected! Found = {:?}", err); - } - }; - } + check_errors(src); } #[test] fn check_trait_wrong_method_return_type() { + // TODO: improve the error location let src = " trait Default { fn default() -> Self; @@ -258,6 +392,7 @@ fn check_trait_wrong_method_return_type() { impl Default for Foo { fn default() -> Field { + ^^^^^^^ Expected type Foo, found type Field 0 } } @@ -266,29 +401,12 @@ fn check_trait_wrong_method_return_type() { let _ = Foo {}; // silence Foo never constructed warning } "; - let errors = get_program_errors(src); - assert!(!has_parser_error(&errors)); - assert!(errors.len() == 1, "Expected 1 error, got: {:?}", errors); - - for err in errors { - match &err { - CompilationError::TypeError(TypeCheckError::TypeMismatch { - expected_typ, - expr_typ, - expr_location: _, - }) => { - assert_eq!(expected_typ, "Foo"); - assert_eq!(expr_typ, "Field"); - } - _ => { - panic!("No other errors are expected! Found = {:?}", err); - } - }; - } + check_errors(src); } #[test] fn check_trait_wrong_method_return_type2() { + // TODO: improve the error location let src = " trait Default { fn default(x: Field, y: Field) -> Self; @@ -301,6 +419,7 @@ fn check_trait_wrong_method_return_type2() { impl Default for Foo { fn default(x: Field, _y: Field) -> Field { + ^^^^^^^ Expected type Foo, found type Field x } } @@ -308,25 +427,7 @@ fn check_trait_wrong_method_return_type2() { fn main() { let _ = Foo { bar: 1, array: [2, 3] }; // silence Foo never constructed warning }"; - let errors = get_program_errors(src); - assert!(!has_parser_error(&errors)); - assert!(errors.len() == 1, "Expected 1 error, got: {:?}", errors); - - for err in errors { - match &err { - CompilationError::TypeError(TypeCheckError::TypeMismatch { - expected_typ, - expr_typ, - expr_location: _, - }) => { - assert_eq!(expected_typ, "Foo"); - assert_eq!(expr_typ, "Field"); - } - _ => { - panic!("No other errors are expected! Found = {:?}", err); - } - }; - } + check_errors(src); } #[test] @@ -345,6 +446,8 @@ fn check_trait_missing_implementation() { } impl Default for Foo { + ^^^ Method `method2` from trait `Default` is not implemented + ~~~ Please implement method2 here fn default(x: Field, y: Field) -> Self { Self { bar: x, array: [x,y] } } @@ -353,25 +456,7 @@ fn check_trait_missing_implementation() { fn main() { } "; - let errors = get_program_errors(src); - assert!(!has_parser_error(&errors)); - assert!(errors.len() == 1, "Expected 1 error, got: {:?}", errors); - - for err in errors { - match &err { - CompilationError::DefinitionError(DefCollectorErrorKind::TraitMissingMethod { - trait_name, - method_name, - trait_impl_location: _, - }) => { - assert_eq!(trait_name, "Default"); - assert_eq!(method_name, "method2"); - } - _ => { - panic!("No other errors are expected! Found = {:?}", err); - } - }; - } + check_errors(src); } #[test] @@ -382,8 +467,8 @@ fn check_trait_not_in_scope() { array: [Field; 2], } - // Default trait does not exist impl Default for Foo { + ^^^^^^^ Trait Default not found fn default(x: Field, y: Field) -> Self { Self { bar: x, array: [x,y] } } @@ -391,23 +476,8 @@ fn check_trait_not_in_scope() { fn main() { } - "; - let errors = get_program_errors(src); - assert!(!has_parser_error(&errors)); - assert!(errors.len() == 1, "Expected 1 error, got: {:?}", errors); - for err in errors { - match &err { - CompilationError::DefinitionError(DefCollectorErrorKind::TraitNotFound { - trait_path, - }) => { - assert_eq!(trait_path.as_string(), "Default"); - } - _ => { - panic!("No other errors are expected! Found = {:?}", err); - } - }; - } + check_errors(src); } #[test] @@ -421,9 +491,9 @@ fn check_trait_wrong_method_name() { array: [Field; 2], } - // wrong trait name method should not compile impl Default for Foo { fn does_not_exist(x: Field, y: Field) -> Self { + ^^^^^^^^^^^^^^ Method with name `does_not_exist` is not part of trait `Default`, therefore it can't be implemented Self { bar: x, array: [x,y] } } } @@ -431,32 +501,12 @@ fn check_trait_wrong_method_name() { fn main() { let _ = Foo { bar: 1, array: [2, 3] }; // silence Foo never constructed warning }"; - let compilation_errors = get_program_errors(src); - assert!(!has_parser_error(&compilation_errors)); - assert!( - compilation_errors.len() == 1, - "Expected 1 compilation error, got: {:?}", - compilation_errors - ); - - for err in compilation_errors { - match &err { - CompilationError::DefinitionError(DefCollectorErrorKind::MethodNotInTrait { - trait_name, - impl_method, - }) => { - assert_eq!(trait_name, "Default"); - assert_eq!(impl_method, "does_not_exist"); - } - _ => { - panic!("No other errors are expected! Found = {:?}", err); - } - }; - } + check_errors(src); } #[test] fn check_trait_wrong_parameter() { + // TODO: improve the error location let src = " trait Default { fn default(x: Field) -> Self; @@ -468,6 +518,7 @@ fn check_trait_wrong_parameter() { impl Default for Foo { fn default(x: u32) -> Self { + ^ Parameter #1 of method `default` must be of type Field, not u32 Foo {bar: x} } } @@ -475,27 +526,7 @@ fn check_trait_wrong_parameter() { fn main() { } "; - let errors = get_program_errors(src); - assert!(!has_parser_error(&errors)); - assert!(errors.len() == 1, "Expected 1 error, got: {:?}", errors); - - for err in errors { - match &err { - CompilationError::TypeError(TypeCheckError::TraitMethodParameterTypeMismatch { - method_name, - expected_typ, - actual_typ, - .. - }) => { - assert_eq!(method_name, "default"); - assert_eq!(expected_typ, "Field"); - assert_eq!(actual_typ, "u32"); - } - _ => { - panic!("No other errors are expected! Found = {:?}", err); - } - }; - } + check_errors(src); } #[test] @@ -512,34 +543,15 @@ fn check_trait_wrong_parameter2() { impl Default for Foo { fn default(x: Field, y: Foo) -> Self { + ^ Parameter #2 of method `default` must be of type Field, not Foo Self { bar: x, array: [x, y.bar] } } } fn main() { - }"; - - let errors = get_program_errors(src); - assert!(!has_parser_error(&errors)); - assert!(errors.len() == 1, "Expected 1 error, got: {:?}", errors); - - for err in errors { - match &err { - CompilationError::TypeError(TypeCheckError::TraitMethodParameterTypeMismatch { - method_name, - expected_typ, - actual_typ, - .. - }) => { - assert_eq!(method_name, "default"); - assert_eq!(expected_typ, "Field"); - assert_eq!(actual_typ, "Foo"); - } - _ => { - panic!("No other errors are expected! Found = {:?}", err); - } - }; } + "; + check_errors(src); } #[test] @@ -547,30 +559,14 @@ fn check_trait_wrong_parameter_type() { let src = " pub trait Default { fn default(x: Field, y: NotAType) -> Field; + ^^^^^^^^ Could not resolve 'NotAType' in path } fn main(x: Field, y: Field) { assert(y == x); - }"; - let errors = get_program_errors(src); - assert!(!has_parser_error(&errors)); - - // This is a duplicate error in the name resolver & type checker. - // In the elaborator there is no duplicate and only 1 error is issued - assert!(errors.len() <= 2, "Expected 1 or 2 errors, got: {:?}", errors); - - for err in errors { - match &err { - CompilationError::ResolverError(ResolverError::PathResolutionError( - PathResolutionError::Unresolved(ident), - )) => { - assert_eq!(ident, "NotAType"); - } - _ => { - panic!("No other errors are expected! Found = {:?}", err); - } - }; } + "; + check_errors(src); } #[test] @@ -587,6 +583,7 @@ fn check_trait_wrong_parameters_count() { impl Default for Foo { fn default(x: Field) -> Self { + ^^^^^^^ `Default::default` expects 2 parameters, but this method has 1 Self { bar: x, array: [x, x] } } } @@ -594,28 +591,7 @@ fn check_trait_wrong_parameters_count() { fn main() { } "; - let errors = get_program_errors(src); - assert!(!has_parser_error(&errors)); - assert!(errors.len() == 1, "Expected 1 error, got: {:?}", errors); - for err in errors { - match &err { - CompilationError::TypeError(TypeCheckError::MismatchTraitImplNumParameters { - actual_num_parameters, - expected_num_parameters, - trait_name, - method_name, - .. - }) => { - assert_eq!(actual_num_parameters, &1_usize); - assert_eq!(expected_num_parameters, &2_usize); - assert_eq!(method_name, "default"); - assert_eq!(trait_name, "Default"); - } - _ => { - panic!("No other errors are expected in this test case! Found = {:?}", err); - } - }; - } + check_errors(src); } #[test] @@ -626,6 +602,7 @@ fn check_trait_impl_for_non_type() { } impl Default for main { + ^^^^ expected type got function fn default(x: Field, y: Field) -> Field { x + y } @@ -633,20 +610,7 @@ fn check_trait_impl_for_non_type() { fn main() {} "; - let errors = get_program_errors(src); - assert!(!has_parser_error(&errors)); - assert!(errors.len() == 1, "Expected 1 error, got: {:?}", errors); - for err in errors { - match &err { - CompilationError::ResolverError(ResolverError::Expected { expected, got, .. }) => { - assert_eq!(*expected, "type"); - assert_eq!(*got, "function"); - } - _ => { - panic!("No other errors are expected! Found = {:?}", err); - } - }; - } + check_errors(src); } #[test] @@ -662,8 +626,8 @@ fn check_impl_struct_not_trait() { z: Field, } - // Default is a struct not a trait impl Default for Foo { + ^^^^^^^ Default is not a trait, therefore it can't be implemented fn default(x: Field, y: Field) -> Self { Self { bar: x, array: [x,y] } } @@ -673,27 +637,15 @@ fn check_impl_struct_not_trait() { let _ = Default { x: 1, z: 1 }; // silence Default never constructed warning } "; - let errors = get_program_errors(src); - assert!(!has_parser_error(&errors)); - assert!(errors.len() == 1, "Expected 1 error, got: {:?}", errors); - for err in errors { - match &err { - CompilationError::DefinitionError(DefCollectorErrorKind::NotATrait { - not_a_trait_name, - }) => { - assert_eq!(not_a_trait_name.to_string(), "Default"); - } - _ => { - panic!("No other errors are expected! Found = {:?}", err); - } - }; - } + check_errors(src); } #[test] fn check_trait_duplicate_declaration() { let src = " trait Default { + ^^^^^^^ Duplicate definitions of trait definition with name Default found + ~~~~~~~ First trait definition found here fn default(x: Field, y: Field) -> Self; } @@ -708,32 +660,15 @@ fn check_trait_duplicate_declaration() { } } - trait Default { + ~~~~~~~ Second trait definition found here fn default(x: Field) -> Self; } fn main() { - }"; - let errors = get_program_errors(src); - assert!(!has_parser_error(&errors)); - assert!(errors.len() == 1, "Expected 1 error, got: {:?}", errors); - for err in errors { - match &err { - CompilationError::DefinitionError(DefCollectorErrorKind::Duplicate { - typ, - first_def, - second_def, - }) => { - assert_eq!(typ, &DuplicateType::Trait); - assert_eq!(first_def, "Default"); - assert_eq!(second_def, "Default"); - } - _ => { - panic!("No other errors are expected! Found = {:?}", err); - } - }; } + "; + check_errors(src); } #[test] @@ -746,20 +681,17 @@ fn check_trait_duplicate_implementation() { } impl Default for Foo { + ~~~~~~~ Previous impl defined here } impl Default for Foo { + ^^^ Impl for type `Foo` overlaps with existing impl + ~~~ Overlapping impl } fn main() { let _ = Foo { bar: 1 }; // silence Foo never constructed warning } "; - let errors = get_program_errors(src); - assert!(!has_parser_error(&errors)); - assert!(errors.len() == 1, "Expected 1 errors, got: {:?}", errors); - assert!(matches!( - errors[0], - CompilationError::DefinitionError(DefCollectorErrorKind::OverlappingImpl { .. }) - )); + check_errors(src); } #[test] @@ -774,22 +706,19 @@ fn check_trait_duplicate_implementation_with_alias() { type MyType = MyStruct; impl Default for MyStruct { + ~~~~~~~ Previous impl defined here } impl Default for MyType { + ^^^^^^ Impl for type `MyType` overlaps with existing impl + ~~~~~~ Overlapping impl } fn main() { let _ = MyStruct {}; // silence MyStruct never constructed warning } "; - let errors = get_program_errors(src); - assert!(!has_parser_error(&errors)); - assert!(errors.len() == 1, "Expected 2 errors, got: {:?}", errors); - assert!(matches!( - errors[0], - CompilationError::DefinitionError(DefCollectorErrorKind::OverlappingImpl { .. }) - )); + check_errors(src); } #[test] @@ -807,7 +736,8 @@ fn test_impl_self_within_default_def() { fn ok(self) -> Self { self } - }"; + } + "; assert_no_errors(src); } @@ -934,6 +864,7 @@ fn resolve_empty_function() { "; assert_no_errors(src); } + #[test] fn resolve_basic_function() { let src = r#" @@ -944,24 +875,18 @@ fn resolve_basic_function() { "#; assert_no_errors(src); } + #[test] fn resolve_unused_var() { let src = r#" fn main(x : Field) { let y = x + x; + ^ unused variable y + ~ unused variable assert(x == x); } "#; - - let errors = get_program_errors(src); - assert!(errors.len() == 1, "Expected 1 error, got: {:?}", errors); - // It should be regarding the unused variable - match &errors[0] { - CompilationError::ResolverError(ResolverError::UnusedVariable { ident }) => { - assert_eq!(&ident.0.contents, "y"); - } - _ => unreachable!("we should only have an unused var error"), - } + check_errors(src); } #[test] @@ -970,20 +895,11 @@ fn resolve_unresolved_var() { fn main(x : Field) { let y = x + x; assert(y == z); + ^ cannot find `z` in this scope + ~ not found in this scope } "#; - let errors = get_program_errors(src); - assert!(errors.len() == 1, "Expected 1 error, got: {:?}", errors); - // It should be regarding the unresolved var `z` (Maybe change to undeclared and special case) - match &errors[0] { - CompilationError::ResolverError(ResolverError::VariableNotDeclared { - name, - location: _, - }) => { - assert_eq!(name, "z"); - } - _ => unimplemented!("we should only have an unresolved variable"), - } + check_errors(src); } #[test] @@ -991,23 +907,10 @@ fn unresolved_path() { let src = " fn main(x : Field) { let _z = some::path::to::a::func(x); + ^^^^ Could not resolve 'some' in path } "; - let errors = get_program_errors(src); - assert!(errors.len() == 1, "Expected 1 error, got: {:?}", errors); - for compilation_error in errors { - match compilation_error { - CompilationError::ResolverError(err) => { - match err { - ResolverError::PathResolutionError(PathResolutionError::Unresolved(name)) => { - assert_eq!(name.to_string(), "some"); - } - _ => unimplemented!("we should only have an unresolved function"), - }; - } - _ => unimplemented!(), - } - } + check_errors(src); } #[test] @@ -1026,36 +929,16 @@ fn multiple_resolution_errors() { let src = r#" fn main(x : Field) { let y = foo::bar(x); + ^^^ Could not resolve 'foo' in path let z = y + a; + ^ cannot find `a` in this scope + ~ not found in this scope + ^ unused variable z + ~ unused variable + } "#; - - let errors = get_program_errors(src); - assert!(errors.len() == 3, "Expected 3 errors, got: {:?}", errors); - - // Errors are: - // `a` is undeclared - // `z` is unused - // `foo::bar` does not exist - for compilation_error in errors { - match compilation_error { - CompilationError::ResolverError(err) => { - match err { - ResolverError::UnusedVariable { ident } => { - assert_eq!(&ident.0.contents, "z"); - } - ResolverError::VariableNotDeclared { name, .. } => { - assert_eq!(name, "a"); - } - ResolverError::PathResolutionError(PathResolutionError::Unresolved(name)) => { - assert_eq!(name.to_string(), "foo"); - } - _ => unimplemented!(), - }; - } - _ => unimplemented!(), - } - } + check_errors(src); } #[test] @@ -1137,8 +1020,8 @@ fn resolve_basic_closure() { #[test] fn resolve_simplified_closure() { // based on bug https://github.com/noir-lang/noir/issues/1088 - - let src = r#"fn do_closure(x: Field) -> Field { + let src = r#" + fn do_closure(x: Field) -> Field { let y = x; let ret_capture = || { y @@ -1203,40 +1086,20 @@ fn resolve_fmt_strings() { let src = r#" fn main() { let string = f"this is i: {i}"; + ^ cannot find `i` in this scope + ~ not found in this scope println(string); + ^^^^^^^^^^^^^^^ Unused expression result of type fmtstr<14, ()> let new_val = 10; println(f"random_string{new_val}{new_val}"); + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Unused expression result of type fmtstr<31, (Field, Field)> } fn println(x : T) -> T { x } "#; - - let errors = get_program_errors(src); - assert!(errors.len() == 3, "Expected 5 errors, got: {:?}", errors); - - for err in errors { - match &err { - CompilationError::ResolverError(ResolverError::VariableNotDeclared { - name, .. - }) => { - assert_eq!(name, "i"); - } - CompilationError::TypeError(TypeCheckError::UnusedResultError { - expr_type: _, - expr_location, - }) => { - let a = src - .get(expr_location.span.start() as usize..expr_location.span.end() as usize) - .unwrap(); - assert!( - a == "println(string)" || a == "println(f\"random_string{new_val}{new_val}\")" - ); - } - _ => unimplemented!(), - }; - } + check_errors(src); } fn monomorphize_program(src: &str) -> Result { @@ -1285,24 +1148,20 @@ fn lambda$f1(mut env$l1: (Field)) -> Field { check_rewrite(src, expected_rewrite); } -// TODO(https://github.com/noir-lang/noir/issues/6780): currently failing -// with a stack overflow #[test] -#[ignore] fn deny_cyclic_globals() { let src = r#" global A: u32 = B; + ^ This global recursively depends on itself + ^ Dependency cycle found + ~ 'A' recursively depends on itself: A -> B -> A global B: u32 = A; + ^ Variable not in scope + ~ Could not find variable fn main() {} "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::ResolverError(ResolverError::DependencyCycle { .. }) - )); + check_errors(src); } #[test] @@ -1310,9 +1169,11 @@ fn deny_cyclic_type_aliases() { let src = r#" type A = B; type B = A; + ^^^^^^^^^^ Dependency cycle found + ~~~~~~~~~~ 'B' recursively depends on itself: B -> A -> B fn main() {} "#; - assert_eq!(get_program_errors(src).len(), 1); + check_errors(src); } #[test] @@ -1322,9 +1183,10 @@ fn ensure_nested_type_aliases_type_check() { type B = u8; fn main() { let _a: A = 0 as u16; + ^^^^^^^^ Expected type A, found type u16 } "#; - assert_eq!(get_program_errors(src).len(), 1); + check_errors(src); } #[test] @@ -1333,7 +1195,7 @@ fn type_aliases_in_entry_point() { type Foo = u8; fn main(_x: Foo) {} "#; - assert_eq!(get_program_errors(src).len(), 0); + assert_no_errors(src); } #[test] @@ -1345,7 +1207,7 @@ fn operators_in_global_used_in_type() { let _array: [Field; COUNT] = [1, 2, 3]; } "#; - assert_eq!(get_program_errors(src).len(), 0); + assert_no_errors(src); } #[test] @@ -1355,14 +1217,18 @@ fn break_and_continue_in_constrained_fn() { for i in 0 .. 10 { if i == 2 { continue; + ^^^^^^^^^ continue is only allowed in unconstrained functions + ~~~~~~~~~ Constrained code must always have a known number of loop iterations } if i == 5 { break; + ^^^^^^ break is only allowed in unconstrained functions + ~~~~~~ Constrained code must always have a known number of loop iterations } } } "#; - assert_eq!(get_program_errors(src).len(), 2); + check_errors(src); } #[test] @@ -1370,10 +1236,12 @@ fn break_and_continue_outside_loop() { let src = r#" unconstrained fn main() { continue; + ^^^^^^^^^ continue is only allowed within loops break; + ^^^^^^ break is only allowed within loops } "#; - assert_eq!(get_program_errors(src).len(), 2); + check_errors(src); } // Regression for #2540 @@ -1389,8 +1257,7 @@ fn for_loop_over_array() { hello(array); } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 0); + assert_no_errors(src); } // Regression for #4545 @@ -1400,51 +1267,47 @@ fn type_aliases_in_main() { type Outer = [u8; N]; fn main(_arg: Outer<1>) {} "#; - assert_eq!(get_program_errors(src).len(), 0); + assert_no_errors(src); } #[test] fn ban_mutable_globals() { - // Mutable globals are only allowed in a comptime context let src = r#" mut global FOO: Field = 0; + ^^^ Only `comptime` globals may be mutable fn main() { let _ = FOO; // silence FOO never used warning } "#; - assert_eq!(get_program_errors(src).len(), 1); + check_errors(src); } #[test] fn deny_inline_attribute_on_unconstrained() { + // TODO: improve the error location let src = r#" #[no_predicates] unconstrained pub fn foo(x: Field, y: Field) { + ^^^ misplaced #[no_predicates] attribute on unconstrained function foo. Only allowed on constrained functions + ~~~ misplaced #[no_predicates] attribute assert(x != y); } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::ResolverError(ResolverError::NoPredicatesAttributeOnUnconstrained { .. }) - )); + check_errors(src); } #[test] fn deny_fold_attribute_on_unconstrained() { + // TODO: improve the error location let src = r#" #[fold] unconstrained pub fn foo(x: Field, y: Field) { + ^^^ misplaced #[fold] attribute on unconstrained function foo. Only allowed on constrained functions + ~~~ misplaced #[fold] attribute assert(x != y); } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::ResolverError(ResolverError::FoldAttributeOnUnconstrained { .. }) - )); + check_errors(src); } #[test] @@ -1474,8 +1337,7 @@ fn specify_function_types_with_turbofish() { let _ = generic_func::(); } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 0); + assert_no_errors(src); } #[test] @@ -1508,8 +1370,7 @@ fn specify_method_types_with_turbofish() { let _ = foo.generic_method::(); } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 0); + assert_no_errors(src); } #[test] @@ -1537,14 +1398,10 @@ fn incorrect_turbofish_count_function_call() { fn main() { let _ = generic_func::(); + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Expected 2 generics from this function, but 3 were provided } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::TypeError(TypeCheckError::IncorrectTurbofishGenericCount { .. }), - )); + check_errors(src); } #[test] @@ -1575,14 +1432,10 @@ fn incorrect_turbofish_count_method_call() { fn main() { let foo: Foo = Foo { inner: 1 }; let _ = foo.generic_method::(); + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Expected 1 generic from this function, but 2 were provided } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::TypeError(TypeCheckError::IncorrectTurbofishGenericCount { .. }), - )); + check_errors(src); } #[test] @@ -1593,15 +1446,12 @@ fn struct_numeric_generic_in_function() { } pub fn bar() { + ^ N has a type of Foo. The only supported numeric generic types are `u1`, `u8`, `u16`, and `u32`. + ~ Unsupported numeric generic type let _ = Foo { inner: 1 }; // silence Foo never constructed warning } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::ResolverError(ResolverError::UnsupportedNumericGenericType { .. }), - )); + check_errors(src); } #[test] @@ -1612,19 +1462,18 @@ fn struct_numeric_generic_in_struct() { } pub struct Bar { } + ^ N has a type of Foo. The only supported numeric generic types are `u1`, `u8`, `u16`, and `u32`. + ~ Unsupported numeric generic type "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::ResolverError(ResolverError::UnsupportedNumericGenericType(_)), - )); + check_errors(src); } #[test] fn bool_numeric_generic() { let src = r#" pub fn read() -> Field { + ^ N has a type of bool. The only supported numeric generic types are `u1`, `u8`, `u16`, and `u32`. + ~ Unsupported numeric generic type if N { 0 } else { @@ -1632,12 +1481,7 @@ fn bool_numeric_generic() { } } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::ResolverError(ResolverError::UnsupportedNumericGenericType { .. }), - )); + check_errors(src); } #[test] @@ -1646,49 +1490,30 @@ fn numeric_generic_binary_operation_type_mismatch() { pub fn foo() -> bool { let mut check: bool = true; check = N; + ^ Cannot assign an expression of type Field to a value of type bool check } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::TypeError(TypeCheckError::TypeMismatchWithSource { .. }), - )); + check_errors(src); } #[test] fn bool_generic_as_loop_bound() { - let src = r#" - pub fn read() { // error here - let mut fields = [0; N]; // error here - for i in 0..N { // error here + // TODO: improve the error location of the last error (should be just on N) + let src = r#" + pub fn read() { + ^ N has a type of bool. The only supported numeric generic types are `u1`, `u8`, `u16`, and `u32`. + ~ Unsupported numeric generic type + let mut fields = [0; N]; + ^ Expected kind numeric u32, found kind numeric bool + for i in 0..N { + ^^^^ Expected type Field, found type bool fields[i] = i + 1; } assert(fields[0] == 1); } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 3); - assert!(matches!( - errors[0], - CompilationError::ResolverError(ResolverError::UnsupportedNumericGenericType { .. }), - )); - - assert!(matches!( - errors[1], - CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }), - )); - - let CompilationError::TypeError(TypeCheckError::TypeMismatch { - expected_typ, expr_typ, .. - }) = &errors[2] - else { - panic!("Got an error other than a type mismatch"); - }; - - assert_eq!(expected_typ, "Field"); - assert_eq!(expr_typ, "bool"); + check_errors(src); } #[test] @@ -1701,98 +1526,73 @@ fn numeric_generic_in_function_signature() { #[test] fn numeric_generic_as_struct_field_type_fails() { + // TODO: improve error message, in Rust it says "expected type, found const parameter `N`" + // which might be more understandable let src = r#" pub struct Foo { a: Field, b: N, + ^ Expected kind normal, found kind numeric u32 } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }), - )); + check_errors(src); } #[test] fn normal_generic_as_array_length() { + // TODO: improve error location, should be just on N let src = r#" pub struct Foo { a: Field, b: [Field; N], + ^^^^^^^^^^ Expected kind numeric u32, found kind normal } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }), - )); + check_errors(src); } #[test] fn numeric_generic_as_param_type() { + // TODO: improve the error message, see what Rust does let src = r#" pub fn foo(x: I) -> I { + ^ Expected kind normal, found kind numeric u32 + ^ Expected kind normal, found kind numeric u32 let _q: I = 5; + ^ Expected kind normal, found kind numeric u32 x } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 3); - - // Error from the parameter type - assert!(matches!( - errors[0], - CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }), - )); - // Error from the let statement annotated type - assert!(matches!( - errors[1], - CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }), - )); - // Error from the return type - assert!(matches!( - errors[2], - CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }), - )); + check_errors(src); } #[test] fn numeric_generic_as_unused_param_type() { + // TODO: improve the error message let src = r#" pub fn foo(_x: I) { } + ^ Expected kind normal, found kind numeric u32 "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }), - )); + check_errors(src); } #[test] fn numeric_generic_as_unused_trait_fn_param_type() { + // TODO: improve the error message let src = r#" trait Foo { + ^^^ unused trait Foo + ~~~ unused trait fn foo(_x: I) { } + ^ Expected kind normal, found kind numeric u32 } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 2); - assert!(matches!( - errors[0], - CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }), - )); - // Foo is unused - assert!( - matches!(errors[1], CompilationError::ResolverError(ResolverError::UnusedItem { .. }),) - ); + check_errors(src); } #[test] fn numeric_generic_as_return_type() { + // TODO: improve the error message let src = r#" // std::mem::zeroed() without stdlib trait Zeroed { @@ -1800,27 +1600,20 @@ fn numeric_generic_as_return_type() { } fn foo(x: T) -> I where T: Zeroed { + ^ Expected kind normal, found kind numeric Field + ^^^ unused function foo + ~~~ unused function x.zeroed() } fn main() {} "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 2); - - // Error from the return type - assert!(matches!( - errors[0], - CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }), - )); - // foo is unused - assert!( - matches!(errors[1], CompilationError::ResolverError(ResolverError::UnusedItem { .. }),) - ); + check_errors(src); } #[test] fn numeric_generic_used_in_nested_type_fails() { + // TODO: improve the error message let src = r#" pub struct Foo { a: Field, @@ -1828,33 +1621,26 @@ fn numeric_generic_used_in_nested_type_fails() { } pub struct Bar { inner: N + ^ Expected kind normal, found kind numeric u32 } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }), - )); + check_errors(src); } #[test] fn normal_generic_used_in_nested_array_length_fail() { + // TODO: improve the error message let src = r#" pub struct Foo { a: Field, b: Bar, + ^ Expected kind numeric u32, found kind normal } pub struct Bar { inner: [Field; N] } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }), - )); + check_errors(src); } #[test] @@ -1969,6 +1755,7 @@ fn numeric_generic_used_in_turbofish() { // allow u16 to be used as an array size #[test] fn numeric_generic_u16_array_size() { + // TODO: improve the error location (and maybe the message) let src = r#" fn len(_arr: [Field; N]) -> u32 { N @@ -1976,19 +1763,12 @@ fn numeric_generic_u16_array_size() { pub fn foo() -> u32 { let fields: [Field; N] = [0; N]; + ^^^^^^^^^^ Expected kind numeric u32, found kind numeric u16 + ^ Expected kind numeric u32, found kind numeric u16 len(fields) } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 2); - assert!(matches!( - errors[0], - CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }), - )); - assert!(matches!( - errors[1], - CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }), - )); + check_errors(src); } #[test] @@ -2002,8 +1782,7 @@ fn numeric_generic_field_larger_than_u32() { let _ = foo::(); } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 0); + assert_no_errors(src); } #[test] @@ -2026,8 +1805,7 @@ fn numeric_generic_field_arithmetic_larger_than_u32() { let _ = size(foo::()); } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 0); + assert_no_errors(src); } #[test] @@ -2035,14 +1813,11 @@ fn cast_256_to_u8_size_checks() { let src = r#" fn main() { assert(256 as u8 == 0); + ^^^^^^^^^ Casting value of type Field to a smaller type (u8) + ~~~~~~~~~ casting untyped value (256) to a type with a maximum size (255) that's smaller than it } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::TypeError(TypeCheckError::DownsizingCast { .. }), - )); + check_errors(src); } // TODO(https://github.com/noir-lang/noir/issues/6247): @@ -2054,8 +1829,7 @@ fn cast_negative_one_to_u8_size_checks() { assert((-1) as u8 != 0); } "#; - let errors = get_program_errors(src); - assert!(errors.is_empty()); + assert_no_errors(src); } #[test] @@ -2090,48 +1864,32 @@ fn normal_generic_used_when_numeric_expected_in_where_clause() { } pub fn read() -> T where T: Deserialize { + ^ Expected kind numeric u32, found kind normal T::deserialize([0, 1]) } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }), - )); + check_errors(src); + // TODO: improve the error location for the array (should be on N) let src = r#" trait Deserialize { fn deserialize(fields: [Field; N]) -> Self; } pub fn read() -> T where T: Deserialize { + ^ Expected kind numeric u32, found kind normal let mut fields: [Field; N] = [0; N]; + ^ Expected kind numeric u32, found kind normal + ^^^^^^^^^^ Expected kind numeric u32, found kind normal for i in 0..N { + ^ cannot find `N` in this scope + ~ not found in this scope fields[i] = i as Field + 1; } T::deserialize(fields) } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 4); - assert!(matches!( - errors[0], - CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }), - )); - assert!(matches!( - errors[1], - CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }), - )); - assert!(matches!( - errors[2], - CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }), - )); - // N - assert!(matches!( - errors[3], - CompilationError::ResolverError(ResolverError::VariableNotDeclared { .. }), - )); + check_errors(src); } #[test] @@ -2145,6 +1903,7 @@ fn numeric_generics_type_kind_mismatch() { fn bar() -> u16 { foo::() + ^ Expected kind numeric u32, found kind numeric u16 } global M: u16 = 3; @@ -2153,12 +1912,7 @@ fn numeric_generics_type_kind_mismatch() { let _ = bar::(); } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }), - )); + check_errors(src); } #[test] @@ -2180,6 +1934,7 @@ fn numeric_generics_value_kind_mismatch_u32_u64() { pub fn push(&mut self, elem: T) { assert(self.len < MaxLen, "push out of bounds"); + ^^^^^^^^^^^^^^^^^ Integers must have the same bit width LHS is 64, RHS is 32 self.storage[self.len] = elem; self.len += 1; } @@ -2189,20 +1944,12 @@ fn numeric_generics_value_kind_mismatch_u32_u64() { let _ = BoundedVec { storage: [1], len: 1 }; // silence never constructed warning } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::TypeError(TypeCheckError::IntegerBitWidth { - bit_width_x: IntegerBitSize::SixtyFour, - bit_width_y: IntegerBitSize::ThirtyTwo, - .. - }), - )); + check_errors(src); } #[test] fn quote_code_fragments() { + // TODO: have the error also point to `contact!` as a secondary // This test ensures we can quote (and unquote/splice) code fragments // which by themselves are not valid code. They only need to be valid // by the time they are unquoted into the macro's call site. @@ -2210,6 +1957,7 @@ fn quote_code_fragments() { fn main() { comptime { concat!(quote { assert( }, quote { false); }); + ^^^^^ Assertion failed } } @@ -2217,11 +1965,7 @@ fn quote_code_fragments() { quote { $a $b } } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - use InterpreterError::FailingConstraint; - assert!(matches!(&errors[0], CompilationError::InterpreterError(FailingConstraint { .. }))); + check_errors(src); } #[test] @@ -2233,6 +1977,7 @@ fn impl_stricter_than_trait_no_trait_method_constraints() { // We want to make sure we trigger the error when override a trait method // which itself has no trait constraints. fn serialize(self) -> [Field; N]; + ~~~~~~~~~ definition of `serialize` from trait } trait ToField { @@ -2254,6 +1999,8 @@ fn impl_stricter_than_trait_no_trait_method_constraints() { impl Serialize<2> for MyType { fn serialize(self) -> [Field; 2] where T: ToField { + ^^^^^^^ impl has stricter requirements than trait + ~~~~~~~ impl has extra requirement `T: ToField` [ self.a.to_field(), self.b.to_field() ] } } @@ -2268,13 +2015,7 @@ fn impl_stricter_than_trait_no_trait_method_constraints() { let _ = MyType { a: 1, b: 1 }; // silence MyType never constructed warning } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - &errors[0], - CompilationError::DefinitionError(DefCollectorErrorKind::ImplIsStricterThanTrait { .. }) - )); + check_errors(src); } #[test] @@ -2287,26 +2028,18 @@ fn impl_stricter_than_trait_different_generics() { fn foo_good() where T: Default; fn foo_bad() where T: Default; + ~~~~~~~ definition of `foo_bad` from trait } impl Foo for () { fn foo_good() where A: Default {} fn foo_bad() where B: Default {} + ^^^^^^^ impl has stricter requirements than trait + ~~~~~~~ impl has extra requirement `B: Default` } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - if let CompilationError::DefinitionError(DefCollectorErrorKind::ImplIsStricterThanTrait { - constraint_typ, - .. - }) = &errors[0] - { - assert!(matches!(constraint_typ.to_string().as_str(), "B")); - } else { - panic!("Expected DefCollectorErrorKind::ImplIsStricterThanTrait but got {:?}", errors[0]); - } + check_errors(src); } #[test] @@ -2328,14 +2061,17 @@ fn impl_stricter_than_trait_different_object_generics() { fn bar_good() where Option: MyTrait, OtherOption>: OtherTrait; fn bar_bad() where Option: MyTrait, OtherOption>: OtherTrait; + ~~~~~~~ definition of `bar_bad` from trait fn array_good() where [T; 8]: MyTrait; fn array_bad() where [T; 8]: MyTrait; + ~~~~~~~~~ definition of `array_bad` from trait fn tuple_good() where (Option, Option): MyTrait; fn tuple_bad() where (Option, Option): MyTrait; + ~~~~~~~~~ definition of `tuple_bad` from trait } impl Bar for () { @@ -2348,58 +2084,27 @@ fn impl_stricter_than_trait_different_object_generics() { where OtherOption>: OtherTrait, Option: MyTrait { } + ^^^^^^^ impl has stricter requirements than trait + ~~~~~~~ impl has extra requirement `Option: MyTrait` fn array_good() where [A; 8]: MyTrait { } fn array_bad() where [B; 8]: MyTrait { } + ^^^^^^^ impl has stricter requirements than trait + ~~~~~~~ impl has extra requirement `[B; 8]: MyTrait` fn tuple_good() where (Option, Option): MyTrait { } fn tuple_bad() where (Option, Option): MyTrait { } + ^^^^^^^ impl has stricter requirements than trait + ~~~~~~~ impl has extra requirement `(Option, Option): MyTrait` } fn main() { let _ = OtherOption { inner: Option { inner: 1 } }; // silence unused warnings } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 3); - if let CompilationError::DefinitionError(DefCollectorErrorKind::ImplIsStricterThanTrait { - constraint_typ, - constraint_name, - .. - }) = &errors[0] - { - assert!(matches!(constraint_typ.to_string().as_str(), "Option")); - assert!(matches!(constraint_name.as_str(), "MyTrait")); - } else { - panic!("Expected DefCollectorErrorKind::ImplIsStricterThanTrait but got {:?}", errors[0]); - } - - if let CompilationError::DefinitionError(DefCollectorErrorKind::ImplIsStricterThanTrait { - constraint_typ, - constraint_name, - .. - }) = &errors[1] - { - assert!(matches!(constraint_typ.to_string().as_str(), "[B; 8]")); - assert!(matches!(constraint_name.as_str(), "MyTrait")); - } else { - panic!("Expected DefCollectorErrorKind::ImplIsStricterThanTrait but got {:?}", errors[0]); - } - - if let CompilationError::DefinitionError(DefCollectorErrorKind::ImplIsStricterThanTrait { - constraint_typ, - constraint_name, - .. - }) = &errors[2] - { - assert!(matches!(constraint_typ.to_string().as_str(), "(Option, Option)")); - assert!(matches!(constraint_name.as_str(), "MyTrait")); - } else { - panic!("Expected DefCollectorErrorKind::ImplIsStricterThanTrait but got {:?}", errors[0]); - } + check_errors(src); } #[test] @@ -2415,32 +2120,22 @@ fn impl_stricter_than_trait_different_trait() { trait Bar { fn bar() where Option: Default; + ~~~ definition of `bar` from trait } impl Bar for () { // Trait constraint differs due to the trait even though the constraint // types are the same. fn bar() where Option: OtherDefault {} + ^^^^^^^^^^^^ impl has stricter requirements than trait + ~~~~~~~~~~~~ impl has extra requirement `Option: OtherDefault` } fn main() { let _ = Option { inner: 1 }; // silence Option never constructed warning } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - if let CompilationError::DefinitionError(DefCollectorErrorKind::ImplIsStricterThanTrait { - constraint_typ, - constraint_name, - .. - }) = &errors[0] - { - assert!(matches!(constraint_typ.to_string().as_str(), "Option")); - assert!(matches!(constraint_name.as_str(), "OtherDefault")); - } else { - panic!("Expected DefCollectorErrorKind::ImplIsStricterThanTrait but got {:?}", errors[0]); - } + check_errors(src); } #[test] @@ -2450,6 +2145,7 @@ fn trait_impl_where_clause_stricter_pass() { fn good_foo() where H: OtherTrait; fn bad_foo() where H: OtherTrait; + ~~~~~~~ definition of `bad_foo` from trait } trait OtherTrait {} @@ -2462,26 +2158,15 @@ fn trait_impl_where_clause_stricter_pass() { fn good_foo() where B: OtherTrait { } fn bad_foo() where A: OtherTrait { } + ^^^^^^^^^^ impl has stricter requirements than trait + ~~~~~~~~~~ impl has extra requirement `A: OtherTrait` } fn main() { let _ = Option { inner: 1 }; // silence Option never constructed warning } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - if let CompilationError::DefinitionError(DefCollectorErrorKind::ImplIsStricterThanTrait { - constraint_typ, - constraint_name, - .. - }) = &errors[0] - { - assert!(matches!(constraint_typ.to_string().as_str(), "A")); - assert!(matches!(constraint_name.as_str(), "OtherTrait")); - } else { - panic!("Expected DefCollectorErrorKind::ImplIsStricterThanTrait but got {:?}", errors[0]); - } + check_errors(src); } #[test] @@ -2489,31 +2174,19 @@ fn impl_stricter_than_trait_different_trait_generics() { let src = r#" trait Foo { fn foo() where T: T2; + ~~~ definition of `foo` from trait } impl Foo for () { // Should be A: T2 fn foo() where A: T2 {} + ^^ impl has stricter requirements than trait + ~~ impl has extra requirement `A: T2` } trait T2 {} "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - if let CompilationError::DefinitionError(DefCollectorErrorKind::ImplIsStricterThanTrait { - constraint_typ, - constraint_name, - constraint_generics, - .. - }) = &errors[0] - { - assert!(matches!(constraint_typ.to_string().as_str(), "A")); - assert!(matches!(constraint_name.as_str(), "T2")); - assert!(matches!(constraint_generics.ordered[0].to_string().as_str(), "B")); - } else { - panic!("Expected DefCollectorErrorKind::ImplIsStricterThanTrait but got {:?}", errors[0]); - } + check_errors(src); } #[test] @@ -2550,6 +2223,8 @@ fn impl_not_found_for_inner_impl() { impl MyType { fn do_thing_with_serialization_with_extra_steps(self) -> Field { process_array(serialize_thing(self)) + ^^^^^^^^^^^^^^^ No matching impl found for `T: ToField` + ~~~~~~~~~~~~~~~ No impl for `T: ToField` } } @@ -2557,13 +2232,7 @@ fn impl_not_found_for_inner_impl() { let _ = MyType { a: 1, b: 1 }; // silence MyType never constructed warning } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - &errors[0], - CompilationError::TypeError(TypeCheckError::NoMatchingImplFound { .. }) - )); + check_errors(src); } #[test] @@ -2571,16 +2240,12 @@ fn cannot_call_unconstrained_function_outside_of_unsafe() { let src = r#" fn main() { foo(); + ^^^^^ Call to unconstrained function is unsafe and must be in an unconstrained function or unsafe block } unconstrained fn foo() {} "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::Unsafe { .. }) = &errors[0] else { - panic!("Expected an 'unsafe' error, got {:?}", errors[0]); - }; + check_errors(src); } #[test] @@ -2588,26 +2253,19 @@ fn cannot_call_unconstrained_first_class_function_outside_of_unsafe() { let src = r#" fn main() { let func = foo; - // Warning should trigger here func(); + ^^^^^^ Call to unconstrained function is unsafe and must be in an unconstrained function or unsafe block inner(func); } fn inner(x: unconstrained fn() -> ()) { - // Warning should trigger here x(); + ^^^ Call to unconstrained function is unsafe and must be in an unconstrained function or unsafe block } unconstrained fn foo() {} "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 2); - - for error in &errors { - let CompilationError::TypeError(TypeCheckError::Unsafe { .. }) = error else { - panic!("Expected an 'unsafe' error, got {:?}", errors[0]); - }; - } + check_errors(src); } #[test] @@ -2641,15 +2299,11 @@ fn missing_unsafe_block_when_needing_type_annotations() { impl BigNumTrait for BigNum { fn __is_zero(self) -> bool { self.__is_zero_impl() + ^^^^^^^^^^^^^^^^^^^ Call to unconstrained function is unsafe and must be in an unconstrained function or unsafe block } } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::Unsafe { .. }) = &errors[0] else { - panic!("Expected an 'unsafe' error, got {:?}", errors[0]); - }; + check_errors(src); } #[test] @@ -2658,6 +2312,7 @@ fn cannot_pass_unconstrained_function_to_regular_function() { fn main() { let func = foo; expect_regular(func); + ^^^^ Converting an unconstrained fn to a non-unconstrained fn is unsafe } unconstrained fn foo() {} @@ -2665,12 +2320,7 @@ fn cannot_pass_unconstrained_function_to_regular_function() { fn expect_regular(_func: fn() -> ()) { } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::UnsafeFn { .. }) = &errors[0] else { - panic!("Expected an UnsafeFn error, got {:?}", errors[0]); - }; + check_errors(src); } #[test] @@ -2678,24 +2328,13 @@ fn cannot_assign_unconstrained_and_regular_fn_to_variable() { let src = r#" fn main() { let _func = if true { foo } else { bar }; + ^^^ Expected type fn() -> (), found type unconstrained fn() -> () } fn foo() {} unconstrained fn bar() {} "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::Context { err, .. }) = &errors[0] else { - panic!("Expected a context error, got {:?}", errors[0]); - }; - - if let TypeCheckError::TypeMismatch { expected_typ, expr_typ, .. } = err.as_ref() { - assert_eq!(expected_typ, "fn() -> ()"); - assert_eq!(expr_typ, "unconstrained fn() -> ()"); - } else { - panic!("Expected a type mismatch error, got {:?}", errors[0]); - }; + check_errors(src); } #[test] @@ -2719,18 +2358,14 @@ fn cannot_pass_unconstrained_function_to_constrained_function() { fn main() { let func = foo; expect_regular(func); + ^^^^ Converting an unconstrained fn to a non-unconstrained fn is unsafe } unconstrained fn foo() {} fn expect_regular(_func: fn() -> ()) {} "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::UnsafeFn { .. }) = &errors[0] else { - panic!("Expected an UnsafeFn error, got {:?}", errors[0]); - }; + check_errors(src); } #[test] @@ -2767,24 +2402,11 @@ fn trait_impl_generics_count_mismatch() { trait Foo {} impl Foo<()> for Field {} + ^^^ Foo expects 0 generics but 1 was given - fn main() {}"#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::GenericCountMismatch { - item, - expected, - found, - .. - }) = &errors[0] - else { - panic!("Expected a generic count mismatch error, got {:?}", errors[0]); - }; - - assert_eq!(item, "Foo"); - assert_eq!(*expected, 0); - assert_eq!(*found, 1); + fn main() {} + "#; + check_errors(src); } #[test] @@ -2799,31 +2421,19 @@ fn bit_not_on_untyped_integer() { #[test] fn duplicate_struct_field() { + // TODO: the primary error location should be on the second field let src = r#" pub struct Foo { x: i32, + ^ Duplicate definitions of struct field with name x found + ~ First struct field found here x: i32, + ~ Second struct field found here } fn main() {} "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::DefinitionError(DefCollectorErrorKind::Duplicate { - typ: _, - first_def, - second_def, - }) = &errors[0] - else { - panic!("Expected a 'duplicate' error, got {:?}", errors[0]); - }; - - assert_eq!(first_def.to_string(), "x"); - assert_eq!(second_def.to_string(), "x"); - - assert_eq!(first_def.span().start(), 30); - assert_eq!(second_def.span().start(), 46); + check_errors(src); } #[test] @@ -2861,23 +2471,12 @@ fn incorrect_generic_count_on_struct_impl() { let src = r#" struct Foo {} impl Foo {} + ^^^ Foo expects 0 generics but 1 was given fn main() { let _ = Foo {}; // silence Foo never constructed warning } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::GenericCountMismatch { - found, expected, .. - }) = errors[0] - else { - panic!("Expected an incorrect generic count mismatch error, got {:?}", errors[0]); - }; - - assert_eq!(found, 1); - assert_eq!(expected, 0); + check_errors(src); } #[test] @@ -2885,23 +2484,12 @@ fn incorrect_generic_count_on_type_alias() { let src = r#" pub struct Foo {} pub type Bar = Foo; + ^^^ Foo expects 0 generics but 1 was given fn main() { let _ = Foo {}; // silence Foo never constructed warning } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::GenericCountMismatch { - found, expected, .. - }) = errors[0] - else { - panic!("Expected an incorrect generic count mismatch error, got {:?}", errors[0]); - }; - - assert_eq!(found, 1); - assert_eq!(expected, 0); + check_errors(src); } #[test] @@ -2958,14 +2546,18 @@ fn uses_self_type_in_trait_where_clause() { } pub trait Foo where Self: Trait { + ~~~~~ required by this bound in `Foo fn foo(self) -> bool { self.trait_func() + ^^^^^^^^^^^^^^^^^ No method named 'trait_func' found for type 'Bar' } } struct Bar {} impl Foo for Bar { + ^^^ The trait bound `_: Trait` is not satisfied + ~~~ The trait `Trait` is not implemented for `_ } @@ -2973,22 +2565,7 @@ fn uses_self_type_in_trait_where_clause() { let _ = Bar {}; // silence Bar never constructed warning } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 2); - - let CompilationError::ResolverError(ResolverError::TraitNotImplemented { .. }) = &errors[0] - else { - panic!("Expected a trait not implemented error, got {:?}", errors[0]); - }; - - let CompilationError::TypeError(TypeCheckError::UnresolvedMethodCall { method_name, .. }) = - &errors[1] - else { - panic!("Expected an unresolved method call error, got {:?}", errors[1]); - }; - - assert_eq!(method_name, "trait_func"); + check_errors(src); } #[test] @@ -3016,13 +2593,10 @@ fn error_on_cast_over_type_variable() { fn main() { let x = "a"; let _: Field = foo(|x| x as Field, x); + ^ Expected type Field, found type str<1> } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - assert!(matches!(errors[0], CompilationError::TypeError(TypeCheckError::TypeMismatch { .. }))); + check_errors(src); } #[test] @@ -3105,15 +2679,9 @@ fn impl_missing_associated_type() { } impl Foo for () {} + ^^^ `Foo` is missing the associated type `Assoc` "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - assert!(matches!( - &errors[0], - CompilationError::TypeError(TypeCheckError::MissingNamedTypeArg { .. }) - )); + check_errors(src); } #[test] @@ -3133,22 +2701,12 @@ fn as_trait_path_syntax_resolves_outside_impl() { // AsTraitPath syntax is a bit silly when associated types // are explicitly specified let _: i64 = 1 as >::Assoc; + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Expected type i64, found type i32 let _ = Bar {}; // silence Bar never constructed warning } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - use CompilationError::TypeError; - use TypeCheckError::TypeMismatch; - let TypeError(TypeMismatch { expected_typ, expr_typ, .. }) = errors[0].clone() else { - panic!("Expected TypeMismatch error, found {:?}", errors[0]); - }; - - assert_eq!(expected_typ, "i64".to_string()); - assert_eq!(expr_typ, "i32".to_string()); + check_errors(src); } #[test] @@ -3166,70 +2724,69 @@ fn as_trait_path_syntax_no_impl() { fn main() { let _: i64 = 1 as >::Assoc; + ^^^ No matching impl found for `Bar: Foo` + ~~~ No impl for `Bar: Foo` let _ = Bar {}; // silence Bar never constructed warning } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - use CompilationError::TypeError; - assert!(matches!(&errors[0], TypeError(TypeCheckError::NoMatchingImplFound { .. }))); + check_errors(src); } #[test] -fn dont_infer_globals_to_u32_from_type_use() { +fn do_not_infer_globals_to_u32_from_type_use() { + // TODO: improve the error location (maybe it should be on the global name) let src = r#" global ARRAY_LEN = 3; + ^ Globals must have a specified type + ~ Inferred type is `Field` global STR_LEN: _ = 2; + ^ Globals must have a specified type + ~ Inferred type is `Field` global FMT_STR_LEN = 2; + ^ Globals must have a specified type + ~ Inferred type is `Field` fn main() { let _a: [u32; ARRAY_LEN] = [1, 2, 3]; + ^^^^^^^^^^^^^^^^ Expected kind numeric u32, found kind numeric Field let _b: str = "hi"; + ^^^^^^^^^^^^ Expected kind numeric u32, found kind numeric Field let _c: fmtstr = f"hi"; + ^^^^^^^^^^^^^^^^^^^^^^ Expected kind numeric u32, found kind numeric Field } "#; - - let mut errors = get_program_errors(src); - assert_eq!(errors.len(), 6); - for error in errors.drain(0..3) { - assert!(matches!( - error, - CompilationError::ResolverError(ResolverError::UnspecifiedGlobalType { .. }) - )); - } - for error in errors { - assert!(matches!( - error, - CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }) - )); - } + check_errors(src); } #[test] -fn dont_infer_partial_global_types() { +fn do_not_infer_partial_global_types() { let src = r#" pub global ARRAY: [Field; _] = [0; 3]; + ^^^^^^ Globals must have a specified type + ~~~~~~ Inferred type is `[Field; 3]` pub global NESTED_ARRAY: [[Field; _]; 3] = [[]; 3]; + ^^^^^^^ Globals must have a specified type + ~~~~~~~ Inferred type is `[[Field; 0]; 3]` pub global STR: str<_> = "hi"; + ^^^^ Globals must have a specified type + ~~~~ Inferred type is `str<2>` + pub global NESTED_STR: [str<_>] = &["hi"]; + ^^^^^^^ Globals must have a specified type + ~~~~~~~ Inferred type is `[str<2>]` pub global FORMATTED_VALUE: str<5> = "there"; pub global FMT_STR: fmtstr<_, _> = f"hi {FORMATTED_VALUE}"; - pub global TUPLE_WITH_MULTIPLE: ([str<_>], [[Field; _]; 3]) = (&["hi"], [[]; 3]); + ^^^^^^^^^^^^^^^^^^^^^^^ Globals must have a specified type + ~~~~~~~~~~~~~~~~~~~~~~~ Inferred type is `fmtstr<20, (str<5>)>` + pub global TUPLE_WITH_MULTIPLE: ([str<_>], [[Field; _]; 3]) = + (&["hi"], [[]; 3]); + ^^^^^^^^^^^^^^^^^^ Globals must have a specified type + ~~~~~~~~~~~~~~~~~~ Inferred type is `([str<2>], [[Field; 0]; 3])` fn main() { } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 6); - for error in errors { - assert!(matches!( - error, - CompilationError::ResolverError(ResolverError::UnspecifiedGlobalType { .. }) - )); - } + check_errors(src); } #[test] @@ -3245,9 +2802,7 @@ fn u32_globals_as_sizes_in_types() { let _c: fmtstr = f"hi"; } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 0); + assert_no_errors(src); } #[test] @@ -3259,6 +2814,8 @@ fn struct_array_len() { impl Array { pub fn len(self) -> u32 { + ^^^^ unused variable self + ~~~~ unused variable N as u32 } } @@ -3270,17 +2827,11 @@ fn struct_array_len() { assert(ys.len() == 2); } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::ResolverError(ResolverError::UnusedVariable { .. }) - )); + check_errors(src); } // TODO(https://github.com/noir-lang/noir/issues/6245): -// support u16 as an array size +// support u8 as an array size #[test] fn non_u32_as_array_length() { let src = r#" @@ -3288,15 +2839,10 @@ fn non_u32_as_array_length() { fn main() { let _a: [u32; ARRAY_LEN] = [1, 2, 3]; + ^^^^^^^^^^^^^^^^ Expected kind numeric u32, found kind numeric u8 } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }) - )); + check_errors(src); } #[test] @@ -3308,9 +2854,7 @@ fn use_non_u32_generic_in_struct() { let _: S<3> = S {}; } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 0); + assert_no_errors(src); } #[test] @@ -3333,10 +2877,7 @@ fn use_numeric_generic_in_trait_method() { let _ = Bar{}.foo(bytes); } "#; - - let errors = get_program_errors(src); - println!("{errors:?}"); - assert_eq!(errors.len(), 0); + assert_no_errors(src); } #[test] @@ -3362,26 +2903,17 @@ fn trait_unconstrained_methods_typechecked_correctly() { assert_eq(2.foo(), 2.identity() as Field); } "#; - - let errors = get_program_errors(src); - println!("{errors:?}"); - assert_eq!(errors.len(), 0); + assert_no_errors(src); } #[test] fn error_if_attribute_not_in_scope() { let src = r#" #[not_in_scope] + ^^^^^^^^^^^^^^^ Attribute function `not_in_scope` is not in scope fn main() {} "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - assert!(matches!( - errors[0], - CompilationError::ResolverError(ResolverError::AttributeFunctionNotInScope { .. }) - )); + check_errors(src); } #[test] @@ -3394,9 +2926,7 @@ fn arithmetic_generics_rounding_pass() { fn round(_x: [Field; N / M * M]) {} "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 0); + assert_no_errors(src); } #[test] @@ -3406,14 +2936,12 @@ fn arithmetic_generics_rounding_fail() { // Do not simplify N/M*M to just N // This should be 3/2*2 = 2, not 3 round::<3, 2>([1, 2, 3]); + ^^^^^^^^^ Expected type [Field; 2], found type [Field; 3] } fn round(_x: [Field; N / M * M]) {} "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!(errors[0], CompilationError::TypeError(TypeCheckError::TypeMismatch { .. }))); + check_errors(src); } #[test] @@ -3431,12 +2959,10 @@ fn arithmetic_generics_rounding_fail_on_struct() { // Do not simplify N/M*M to just N // This should be 3/2*2 = 2, not 3 let _: W<3> = foo(w_3, w_2); + ^^^^^^^^^^^^^ Expected type W<3>, found type W<2> } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!(errors[0], CompilationError::TypeError(TypeCheckError::TypeMismatch { .. }))); + check_errors(src); } #[test] @@ -3449,42 +2975,58 @@ fn unconditional_recursion_fail() { let srcs = vec![ r#" fn main() { + ^^^^ function `main` cannot return without recursing + ~~~~ function cannot return without recursing main() } "#, r#" fn main() -> pub bool { + ^^^^ function `main` cannot return without recursing + ~~~~ function cannot return without recursing if main() { true } else { false } } "#, r#" fn main() -> pub bool { + ^^^^ function `main` cannot return without recursing + ~~~~ function cannot return without recursing if true { main() } else { main() } } "#, r#" fn main() -> pub u64 { + ^^^^ function `main` cannot return without recursing + ~~~~ function cannot return without recursing main() + main() } "#, r#" fn main() -> pub u64 { + ^^^^ function `main` cannot return without recursing + ~~~~ function cannot return without recursing 1 + main() } "#, r#" fn main() -> pub bool { + ^^^^ function `main` cannot return without recursing + ~~~~ function cannot return without recursing let _ = main(); true } "#, r#" fn main(a: u64, b: u64) -> pub u64 { + ^^^^ function `main` cannot return without recursing + ~~~~ function cannot return without recursing main(a + b, main(a, b)) } "#, r#" fn main() -> pub u64 { + ^^^^ function `main` cannot return without recursing + ~~~~ function cannot return without recursing foo(1, main()) } fn foo(a: u64, b: u64) -> u64 { @@ -3493,12 +3035,16 @@ fn unconditional_recursion_fail() { "#, r#" fn main() -> pub u64 { + ^^^^ function `main` cannot return without recursing + ~~~~ function cannot return without recursing let (a, b) = (main(), main()); a + b } "#, r#" fn main() -> pub u64 { + ^^^^ function `main` cannot return without recursing + ~~~~ function cannot return without recursing let mut sum = 0; for i in 0 .. main() { sum += i; @@ -3509,19 +3055,7 @@ fn unconditional_recursion_fail() { ]; for src in srcs { - let errors = get_program_errors(src); - assert!( - !errors.is_empty(), - "expected 'unconditional recursion' error, got nothing; src = {src}" - ); - - for error in errors { - let CompilationError::ResolverError(ResolverError::UnconditionalRecursion { .. }) = - error - else { - panic!("Expected an 'unconditional recursion' error, got {:?}; src = {src}", error); - }; - } + check_errors(src); } } @@ -3721,117 +3255,89 @@ fn errors_with_better_message_when_trying_to_invoke_struct_field_that_is_a_funct impl Foo { fn call(self) -> bool { self.wrapped(1) + ^^^^^^^^^^^^^^^ Cannot invoke function field 'wrapped' on type 'Foo' as a method + ~~~~~~~~~~~~~~~ to call the function stored in 'wrapped', surround the field access with parentheses: '(', ')' } } fn main() {} "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::CannotInvokeStructFieldFunctionType { - method_name, - .. - }) = &errors[0] - else { - panic!("Expected a 'CannotInvokeStructFieldFunctionType' error, got {:?}", errors[0]); - }; - - assert_eq!(method_name, "wrapped"); -} - -fn test_disallows_attribute_on_impl_method( - attr: &str, - check_error: impl FnOnce(&CompilationError), -) { - let src = format!( - " - pub struct Foo {{ }} - - impl Foo {{ - #[{attr}] - fn foo() {{ }} - }} - - fn main() {{ }} - " - ); - let errors = get_program_errors(&src); - assert_eq!(errors.len(), 1); - check_error(&errors[0]); -} - -fn test_disallows_attribute_on_trait_impl_method( - attr: &str, - check_error: impl FnOnce(&CompilationError), -) { - let src = format!( - " - pub trait Trait {{ - fn foo() {{ }} - }} - - pub struct Foo {{ }} - - impl Trait for Foo {{ - #[{attr}] - fn foo() {{ }} - }} - - fn main() {{ }} - " - ); - let errors = get_program_errors(&src); - assert_eq!(errors.len(), 1); - check_error(&errors[0]); + check_errors(src); } #[test] fn disallows_test_attribute_on_impl_method() { - test_disallows_attribute_on_impl_method("test", |error| { - assert!(matches!( - error, - CompilationError::DefinitionError( - DefCollectorErrorKind::TestOnAssociatedFunction { .. } - ) - )); - }); + // TODO: improve the error location + let src = " + pub struct Foo { } + + impl Foo { + #[test] + fn foo() { } + ^^^ The `#[test]` attribute is disallowed on `impl` methods + } + + fn main() { } + "; + check_errors(src); } #[test] fn disallows_test_attribute_on_trait_impl_method() { - test_disallows_attribute_on_trait_impl_method("test", |error| { - assert!(matches!( - error, - CompilationError::DefinitionError( - DefCollectorErrorKind::TestOnAssociatedFunction { .. } - ) - )); - }); + let src = " + pub trait Trait { + fn foo() { } + } + + pub struct Foo { } + + impl Trait for Foo { + #[test] + fn foo() { } + ^^^ The `#[test]` attribute is disallowed on `impl` methods + } + + fn main() { } + "; + check_errors(src); } #[test] fn disallows_export_attribute_on_impl_method() { - test_disallows_attribute_on_impl_method("export", |error| { - assert!(matches!( - error, - CompilationError::DefinitionError( - DefCollectorErrorKind::ExportOnAssociatedFunction { .. } - ) - )); - }); + // TODO: improve the error location + let src = " + pub struct Foo { } + + impl Foo { + #[export] + fn foo() { } + ^^^ The `#[export]` attribute is disallowed on `impl` methods + } + + fn main() { } + "; + check_errors(src); } #[test] fn disallows_export_attribute_on_trait_impl_method() { - test_disallows_attribute_on_trait_impl_method("export", |error| { - assert!(matches!( - error, - CompilationError::DefinitionError( - DefCollectorErrorKind::ExportOnAssociatedFunction { .. } - ) - )); - }); + // TODO: improve the error location + let src = " + pub trait Trait { + fn foo() { } + } + + pub struct Foo { } + + impl Trait for Foo { + #[export] + fn foo() { } + ^^^ The `#[export]` attribute is disallowed on `impl` methods + } + + fn main() { } + "; + check_errors(src); } #[test] @@ -3850,38 +3356,27 @@ fn disallows_underscore_on_right_hand_side() { fn main() { let _ = 1; let _x = _; + ^ in expressions, `_` can only be used on the left-hand side of an assignment + ~ `_` not allowed here } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::VariableNotDeclared { name, .. }) = - &errors[0] - else { - panic!("Expected a VariableNotDeclared error, got {:?}", errors[0]); - }; - - assert_eq!(name, "_"); + check_errors(src); } #[test] fn errors_on_cyclic_globals() { let src = r#" pub comptime global A: u32 = B; + ^ This global recursively depends on itself + ^ Dependency cycle found + ~ 'A' recursively depends on itself: A -> B -> A pub comptime global B: u32 = A; + ^ Variable not in scope + ~ Could not find variable fn main() { } "#; - let errors = get_program_errors(src); - - assert!(errors.iter().any(|error| matches!( - error, - CompilationError::InterpreterError(InterpreterError::GlobalsDependencyCycle { .. }) - ))); - assert!(errors.iter().any(|error| matches!( - error, - CompilationError::ResolverError(ResolverError::DependencyCycle { .. }) - ))); + check_errors(src); } #[test] @@ -3890,18 +3385,14 @@ fn warns_on_unneeded_unsafe() { fn main() { // Safety: test unsafe { + ^^^^^^ Unnecessary `unsafe` block foo() } } fn foo() {} "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - &errors[0], - CompilationError::TypeError(TypeCheckError::UnnecessaryUnsafeBlock { .. }) - )); + check_errors(src); } #[test] @@ -3912,6 +3403,8 @@ fn warns_on_nested_unsafe() { unsafe { // Safety: test unsafe { + ^^^^^^ Unnecessary `unsafe` block + ~~~~~~ Because it's nested inside another `unsafe` block foo() } } @@ -3919,14 +3412,7 @@ fn warns_on_nested_unsafe() { unconstrained fn foo() {} "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - let CompilationError::TypeError(TypeCheckError::NestedUnsafeBlock { location }) = &errors[0] - else { - panic!("Expected NestedUnsafeBlock"); - }; - - assert_eq!(&src[location.span.start() as usize..location.span.end() as usize], "unsafe"); + check_errors(src); } #[test] @@ -4213,28 +3699,6 @@ fn regression_7088() { assert_no_errors(src); } -#[test] -fn error_with_duplicate_enum_variant() { - let src = r#" - enum Foo { - Bar(i32), - Bar(u8), - } - - fn main() {} - "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 2); - assert!(matches!( - &errors[0], - CompilationError::DefinitionError(DefCollectorErrorKind::Duplicate { .. }) - )); - assert!(matches!( - &errors[1], - CompilationError::ResolverError(ResolverError::UnusedItem { .. }) - )); -} - #[test] fn errors_on_empty_loop_no_break() { let src = r#" @@ -4247,14 +3711,11 @@ fn errors_on_empty_loop_no_break() { unconstrained fn foo() { loop {} + ^^^^ `loop` must have at least one `break` in it + ~~~~ Infinite loops are disallowed } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - &errors[0], - CompilationError::ResolverError(ResolverError::LoopWithoutBreak { .. }) - )); + check_errors(src); } #[test] @@ -4270,6 +3731,8 @@ fn errors_on_loop_without_break() { unconstrained fn foo() { let mut x = 1; loop { + ^^^^ `loop` must have at least one `break` in it + ~~~~ Infinite loops are disallowed x += 1; bar(x); } @@ -4277,12 +3740,7 @@ fn errors_on_loop_without_break() { fn bar(_: Field) {} "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - &errors[0], - CompilationError::ResolverError(ResolverError::LoopWithoutBreak { .. }) - )); + check_errors(src); } #[test] @@ -4298,6 +3756,8 @@ fn errors_on_loop_without_break_with_nested_loop() { unconstrained fn foo() { let mut x = 1; loop { + ^^^^ `loop` must have at least one `break` in it + ~~~~ Infinite loops are disallowed x += 1; bar(x); loop { @@ -4309,12 +3769,7 @@ fn errors_on_loop_without_break_with_nested_loop() { fn bar(_: Field) {} "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - &errors[0], - CompilationError::ResolverError(ResolverError::LoopWithoutBreak { .. }) - )); + check_errors(src); } #[test] @@ -4339,16 +3794,11 @@ fn errors_on_if_without_else_type_mismatch() { fn main() { if true { 1 + ^ Expected type Field, found type () } } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::Context { err, .. }) = &errors[0] else { - panic!("Expected a Context error"); - }; - assert!(matches!(**err, TypeCheckError::TypeMismatch { .. })); + check_errors(src); } #[test] @@ -4363,15 +3813,11 @@ fn errors_if_for_body_type_is_not_unit() { fn main() { for _ in 0..1 { 1 + ^ Expected type (), found type Field } } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::TypeMismatch { .. }) = &errors[0] else { - panic!("Expected a TypeMismatch error"); - }; + check_errors(src); } #[test] @@ -4382,15 +3828,11 @@ fn errors_if_loop_body_type_is_not_unit() { if false { break; } 1 + ^ Expected type (), found type Field } } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::TypeMismatch { .. }) = &errors[0] else { - panic!("Expected a TypeMismatch error"); - }; + check_errors(src); } #[test] @@ -4399,174 +3841,30 @@ fn errors_if_while_body_type_is_not_unit() { unconstrained fn main() { while 1 == 1 { 1 + ^ Expected type (), found type Field } } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::TypeMismatch { .. }) = &errors[0] else { - panic!("Expected a TypeMismatch error"); - }; -} - -#[test] -fn errors_on_unspecified_unstable_enum() { - // Enums are experimental - this will need to be updated when they are stabilized - let src = r#" - enum Foo { Bar } - - fn main() { - let _x = Foo::Bar; - } - "#; - - let no_features = &[]; - let errors = get_program_using_features(src, no_features).2; - assert_eq!(errors.len(), 1); - - let CompilationError::ParseError(error) = &errors[0] else { - panic!("Expected a ParseError experimental feature error"); - }; - - assert!(matches!(error.reason(), Some(ParserErrorReason::ExperimentalFeature(_)))); -} - -#[test] -fn errors_on_unspecified_unstable_match() { - // Enums are experimental - this will need to be updated when they are stabilized - let src = r#" - fn main() { - match 3 { - _ => (), - } - } - "#; - - let no_features = &[]; - let errors = get_program_using_features(src, no_features).2; - assert_eq!(errors.len(), 1); - - let CompilationError::ParseError(error) = &errors[0] else { - panic!("Expected a ParseError experimental feature error"); - }; - - assert!(matches!(error.reason(), Some(ParserErrorReason::ExperimentalFeature(_)))); -} - -#[test] -fn errors_on_repeated_match_variables_in_pattern() { - let src = r#" - fn main() { - match (1, 2) { - (_x, _x) => (), - } - } - "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - assert!(matches!( - &errors[0], - CompilationError::ResolverError(ResolverError::VariableAlreadyDefinedInPattern { .. }) - )); + check_errors(src); } #[test] fn check_impl_duplicate_method_without_self() { + // TODO: the primary error location should be n the second `foo` let src = " pub struct Foo {} impl Foo { fn foo() {} + ^^^ duplicate definitions of foo found + ~~~ first definition found here fn foo() {} + ~~~ second definition found here } fn main() {} "; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - assert!(matches!( - errors[0], - CompilationError::ResolverError(ResolverError::DuplicateDefinition { .. }) - )); -} - -#[test] -fn duplicate_field_in_match_struct_pattern() { - let src = r#" -fn main() { - let foo = Foo { x: 10, y: 20 }; - match foo { - Foo { x: _, x: _, y: _ } => {} - } -} - -struct Foo { - x: i32, - y: Field, -} - "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - assert!(matches!( - &errors[0], - CompilationError::ResolverError(ResolverError::DuplicateField { .. }) - )); -} - -#[test] -fn missing_field_in_match_struct_pattern() { - let src = r#" -fn main() { - let foo = Foo { x: 10, y: 20 }; - match foo { - Foo { x: _ } => {} - } -} - -struct Foo { - x: i32, - y: Field, -} - "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - assert!(matches!( - &errors[0], - CompilationError::ResolverError(ResolverError::MissingFields { .. }) - )); -} - -#[test] -fn no_such_field_in_match_struct_pattern() { - let src = r#" -fn main() { - let foo = Foo { x: 10, y: 20 }; - match foo { - Foo { x: _, y: _, z: _ } => {} - } -} - -struct Foo { - x: i32, - y: Field, -} - "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - assert!(matches!( - &errors[0], - CompilationError::ResolverError(ResolverError::NoSuchField { .. }) - )); + check_errors(src); } #[test] diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/arithmetic_generics.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/arithmetic_generics.rs index 83de9c077ab3..bcbdbdd6211e 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/arithmetic_generics.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/arithmetic_generics.rs @@ -2,11 +2,10 @@ use acvm::{AcirField, FieldElement}; -use super::get_program_errors; use crate::hir::type_check::TypeCheckError; use crate::hir_def::types::{BinaryTypeOperator, Type}; use crate::monomorphization::errors::MonomorphizationError; -use crate::tests::get_monomorphization_error; +use crate::tests::{assert_no_errors, get_monomorphization_error}; #[test] fn arithmetic_generics_canonicalization_deduplication_regression() { @@ -23,8 +22,7 @@ fn arithmetic_generics_canonicalization_deduplication_regression() { }; } "#; - let errors = get_program_errors(source); - assert_eq!(errors.len(), 0); + assert_no_errors(source); } #[test] @@ -52,9 +50,7 @@ fn checked_casts_do_not_prevent_canonicalization() { } } "#; - let errors = get_program_errors(source); - println!("{:?}", errors); - assert_eq!(errors.len(), 0); + assert_no_errors(source); } #[test] @@ -76,9 +72,7 @@ fn arithmetic_generics_checked_cast_zeros() { bar(w) } "#; - - let errors = get_program_errors(source); - assert_eq!(errors.len(), 0); + assert_no_errors(source); let monomorphization_error = get_monomorphization_error(source); assert!(monomorphization_error.is_some()); @@ -123,9 +117,7 @@ fn arithmetic_generics_checked_cast_indirect_zeros() { let _ = bar(w); } "#; - - let errors = get_program_errors(source); - assert_eq!(errors.len(), 0); + assert_no_errors(source); let monomorphization_error = get_monomorphization_error(source); assert!(monomorphization_error.is_some()); @@ -166,8 +158,7 @@ fn global_numeric_generic_larger_than_u32() { let _ = foo::(); } "#; - let errors = get_program_errors(source); - assert_eq!(errors.len(), 0); + assert_no_errors(source); } #[test] @@ -196,6 +187,5 @@ fn global_arithmetic_generic_larger_than_u32() { let _ = foo::().size(); } "#; - let errors = get_program_errors(source); - assert_eq!(errors.len(), 0); + assert_no_errors(source); } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/bound_checks.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/bound_checks.rs index 3a9efd716dea..6a084e68c7e3 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/bound_checks.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/bound_checks.rs @@ -1,24 +1,14 @@ -use crate::hir::def_collector::dc_crate::CompilationError; - -use super::get_program_errors; +use crate::tests::check_errors; #[test] fn overflowing_u8() { let src = r#" fn main() { let _: u8 = 256; - }"#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - if let CompilationError::TypeError(error) = &errors[0] { - assert_eq!( - error.to_string(), - "The value `256` cannot fit into `u8` which has range `0..=255`" - ); - } else { - panic!("Expected OverflowingAssignment error, got {:?}", errors[0]); - } + ^^^ The value `256` cannot fit into `u8` which has range `0..=255` + } + "#; + check_errors(src); } #[test] @@ -26,18 +16,10 @@ fn underflowing_u8() { let src = r#" fn main() { let _: u8 = -1; - }"#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - if let CompilationError::TypeError(error) = &errors[0] { - assert_eq!( - error.to_string(), - "The value `-1` cannot fit into `u8` which has range `0..=255`" - ); - } else { - panic!("Expected OverflowingAssignment error, got {:?}", errors[0]); - } + ^^ The value `-1` cannot fit into `u8` which has range `0..=255` + } + "#; + check_errors(src); } #[test] @@ -45,18 +27,10 @@ fn overflowing_i8() { let src = r#" fn main() { let _: i8 = 128; - }"#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - if let CompilationError::TypeError(error) = &errors[0] { - assert_eq!( - error.to_string(), - "The value `128` cannot fit into `i8` which has range `-128..=127`" - ); - } else { - panic!("Expected OverflowingAssignment error, got {:?}", errors[0]); - } + ^^^ The value `128` cannot fit into `i8` which has range `-128..=127` + } + "#; + check_errors(src); } #[test] @@ -64,16 +38,8 @@ fn underflowing_i8() { let src = r#" fn main() { let _: i8 = -129; - }"#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - if let CompilationError::TypeError(error) = &errors[0] { - assert_eq!( - error.to_string(), - "The value `-129` cannot fit into `i8` which has range `-128..=127`" - ); - } else { - panic!("Expected OverflowingAssignment error, got {:?}", errors[0]); - } + ^^^^ The value `-129` cannot fit into `i8` which has range `-128..=127` + } + "#; + check_errors(src); } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/enums.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/enums.rs new file mode 100644 index 000000000000..78f0442bc9fd --- /dev/null +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/enums.rs @@ -0,0 +1,202 @@ +use crate::{ + hir::def_collector::dc_crate::CompilationError, + parser::ParserErrorReason, + tests::{assert_no_errors, get_program_using_features}, +}; + +use super::{check_errors, check_errors_using_features}; + +#[test] +fn error_with_duplicate_enum_variant() { + // TODO: the primary error should be on the second `Bar` + let src = r#" + pub enum Foo { + Bar(i32), + ^^^ Duplicate definitions of enum variant with name Bar found + ~~~ First enum variant found here + Bar(u8), + ~~~ Second enum variant found here + } + + fn main() {} + "#; + check_errors(src); +} + +#[test] +fn errors_on_unspecified_unstable_enum() { + // Enums are experimental - this will need to be updated when they are stabilized + let src = r#" + enum Foo { Bar } + ^^^ This requires the unstable feature 'enums' which is not enabled + ~~~ Pass -Zenums to nargo to enable this feature at your own risk. + + fn main() { + let _x = Foo::Bar; + } + "#; + let no_features = &[]; + check_errors_using_features(src, no_features); +} + +#[test] +fn errors_on_unspecified_unstable_match() { + // TODO: update this test. Right now it's hard to test because the span happens in the entire + // `match` node but ideally it would be nice if it only happened in the `match` keyword. + // Enums are experimental - this will need to be updated when they are stabilized + let src = r#" + fn main() { + match 3 { + _ => (), + } + } + "#; + + let no_features = &[]; + let errors = get_program_using_features(src, no_features).2; + assert_eq!(errors.len(), 1); + + let CompilationError::ParseError(error) = &errors[0] else { + panic!("Expected a ParseError experimental feature error"); + }; + + assert!(matches!(error.reason(), Some(ParserErrorReason::ExperimentalFeature(_)))); +} + +#[test] +fn errors_on_repeated_match_variables_in_pattern() { + let src = r#" + fn main() { + match (1, 2) { + (_x, _x) => (), + ^^ Variable `_x` was already defined in the same match pattern + ~~ `_x` redefined here + ~~ `_x` was previously defined here + } + } + "#; + check_errors(src); +} + +#[test] +fn duplicate_field_in_match_struct_pattern() { + let src = r#" + fn main() { + let foo = Foo { x: 10, y: 20 }; + match foo { + Foo { x: _, x: _, y: _ } => {} + ^ duplicate field x + } + } + + struct Foo { + x: i32, + y: Field, + } + "#; + check_errors(src); +} + +#[test] +fn missing_field_in_match_struct_pattern() { + let src = r#" + fn main() { + let foo = Foo { x: 10, y: 20 }; + match foo { + Foo { x: _ } => {} + ^^^ missing field y in struct Foo + } + } + + struct Foo { + x: i32, + y: Field, + } + "#; + check_errors(src); +} + +#[test] +fn no_such_field_in_match_struct_pattern() { + let src = r#" + fn main() { + let foo = Foo { x: 10, y: 20 }; + match foo { + Foo { x: _, y: _, z: _ } => {} + ^ no such field z defined in struct Foo + } + } + + struct Foo { + x: i32, + y: Field, + } + "#; + check_errors(src); +} + +#[test] +fn match_integer_type_mismatch_in_pattern() { + let src = r#" + fn main() { + match 2 { + Foo::One(_) => (), + ^^^^^^^^ Expected type Field, found type Foo + } + } + + enum Foo { + One(i32), + } + "#; + check_errors(src); +} + +#[test] +fn match_shadow_global() { + let src = r#" + fn main() { + match 2 { + foo => assert_eq(foo, 2), + } + } + + fn foo() {} + "#; + assert_no_errors(src); +} + +#[test] +fn match_no_shadow_global() { + let src = r#" + fn main() { + match 2 { + crate::foo => (), + ^^^^^^^^^^ Expected a struct, enum, or literal pattern, but found a function + } + } + + fn foo() {} + "#; + check_errors(src); +} + +#[test] +fn constructor_arg_arity_mismatch_in_pattern() { + let src = r#" + fn main() { + match Foo::One(1) { + Foo::One(_, _) => (), + ^^^^^^^^ Expected 1 argument, but found 2 + Foo::Two(_) => (), + ^^^^^^^^ Expected 2 arguments, but found 1 + } + } + + enum Foo { + One(i32), + Two(i32, i32), + } + "#; + check_errors(src); +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/imports.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/imports.rs index dafc5b098de8..ba2dad285d6a 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/imports.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/imports.rs @@ -1,9 +1,6 @@ -use crate::hir::{ - def_collector::{dc_crate::CompilationError, errors::DefCollectorErrorKind}, - resolution::{errors::ResolverError, import::PathResolutionError}, -}; +use crate::tests::check_errors; -use super::{assert_no_errors, get_program_errors}; +use super::assert_no_errors; #[test] fn use_super() { @@ -23,19 +20,11 @@ fn use_super() { #[test] fn no_super() { - let src = "use super::some_func;"; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::DefinitionError(DefCollectorErrorKind::PathResolutionError( - PathResolutionError::NoSuper(location), - )) = &errors[0] - else { - panic!("Expected a 'no super' error, got {:?}", errors[0]); - }; - - assert_eq!(location.span.start(), 4); - assert_eq!(location.span.end(), 9); + let src = " + use super::some_func; + ^^^^^ There is no super module + "; + check_errors(src); } #[test] @@ -69,18 +58,11 @@ fn warns_on_use_of_private_exported_item() { fn main() { foo::baz(); + ^^^ baz is private and not visible from the current module + ~~~ baz is private } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - assert!(matches!( - &errors[0], - CompilationError::ResolverError(ResolverError::PathResolutionError( - PathResolutionError::Private(..), - )) - )); + check_errors(src); } #[test] @@ -110,22 +92,15 @@ fn warns_on_re_export_of_item_with_less_visibility() { } pub use bar::baz; + ^^^ cannot re-export baz because it has less visibility than this use statement + ~~~ consider marking baz as pub } fn main() { foo::baz(); } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - assert!(matches!( - &errors[0], - CompilationError::DefinitionError( - DefCollectorErrorKind::CannotReexportItemWithLessVisibility { .. } - ) - )); + check_errors(src); } #[test] @@ -136,21 +111,10 @@ fn errors_if_using_alias_in_import() { } use foo::bar::baz; + ^^^ bar is a type alias, not a module fn main() { } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::DefinitionError(DefCollectorErrorKind::PathResolutionError( - PathResolutionError::NotAModule { ident, kind }, - )) = &errors[0] - else { - panic!("Expected a 'not a module' error, got {:?}", errors[0]); - }; - - assert_eq!(ident.to_string(), "bar"); - assert_eq!(*kind, "type alias"); + check_errors(src); } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/metaprogramming.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/metaprogramming.rs index 1ea23b4301c7..a19ef17d8353 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/metaprogramming.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/metaprogramming.rs @@ -3,15 +3,13 @@ use noirc_errors::Located; use crate::{ ast::Ident, hir::{ - comptime::{ComptimeError, InterpreterError}, + comptime::ComptimeError, def_collector::{ dc_crate::CompilationError, errors::{DefCollectorErrorKind, DuplicateType}, }, - resolution::errors::ResolverError, - type_check::TypeCheckError, }, - parser::ParserErrorReason, + tests::check_errors, }; use super::{assert_no_errors, get_program_errors}; @@ -23,39 +21,30 @@ fn comptime_let() { comptime let my_var = 2; assert_eq(my_var, 2); }"#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 0); + assert_no_errors(src); } #[test] fn comptime_code_rejects_dynamic_variable() { - let src = r#"fn main(x: Field) { + let src = r#" + fn main(x: Field) { comptime let my_var = (x - x) + 2; + ^ Non-comptime variable `x` referenced in comptime code + ~ Non-comptime variables can't be used in comptime code assert_eq(my_var, 2); - }"#; - let errors = get_program_errors(src); - - assert_eq!(errors.len(), 1); - match &errors[0] { - CompilationError::InterpreterError(InterpreterError::NonComptimeVarReferenced { - name, - .. - }) => { - assert_eq!(name, "x"); - } - _ => panic!("expected an InterpreterError"), } + "#; + check_errors(src); } #[test] fn comptime_type_in_runtime_code() { - let source = "pub fn foo(_f: FunctionDefinition) {}"; - let errors = get_program_errors(source); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::ResolverError(ResolverError::ComptimeTypeInRuntimeCode { .. }) - )); + let source = " + pub fn foo(_f: FunctionDefinition) {} + ^^^^^^^^^^^^^^^^^^ Comptime-only type `FunctionDefinition` cannot be used in runtime code + ~~~~~~~~~~~~~~~~~~ Comptime-only type used here + "; + check_errors(source); } #[test] @@ -64,6 +53,7 @@ fn macro_result_type_mismatch() { fn main() { comptime { let x = unquote!(quote { "test" }); + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Expected type Field, found type str<4> let _: Field = x; } } @@ -72,10 +62,7 @@ fn macro_result_type_mismatch() { q } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!(errors[0], CompilationError::TypeError(TypeCheckError::TypeMismatch { .. }))); + check_errors(src); } #[test] @@ -127,6 +114,8 @@ fn allows_references_to_structs_generated_by_macros() { #[test] fn errors_if_macros_inject_functions_with_name_collisions() { + // This can't be tested using `check_errors` right now because the two secondary + // errors land on the same span. let src = r#" comptime fn make_colliding_functions(_s: StructDefinition) -> Quoted { quote { @@ -198,6 +187,8 @@ fn does_not_fail_to_parse_macro_on_parser_warning() { quote { pub fn bar() { unsafe { + ^^^^^^ Unsafe block must have a safety comment above it + ~~~~~~ The comment must start with the "Safety: " word foo(); } } @@ -208,12 +199,5 @@ fn does_not_fail_to_parse_macro_on_parser_warning() { bar() } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ParseError(parser_error) = &errors[0] else { - panic!("Expected a ParseError, got {:?}", errors[0]); - }; - - assert!(matches!(parser_error.reason(), Some(ParserErrorReason::MissingSafetyComment))); + check_errors(src); } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/references.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/references.rs index 236b22b9e900..a11cd0eea538 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/references.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/references.rs @@ -1,9 +1,4 @@ -use crate::hir::{ - def_collector::dc_crate::CompilationError, resolution::errors::ResolverError, - type_check::TypeCheckError, -}; - -use super::get_program_errors; +use crate::tests::check_errors; #[test] fn cannot_mutate_immutable_variable() { @@ -11,21 +6,12 @@ fn cannot_mutate_immutable_variable() { fn main() { let array = [1]; mutate(&mut array); + ^^^^^ Cannot mutate immutable variable `array` } fn mutate(_: &mut [Field; 1]) {} "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::CannotMutateImmutableVariable { name, .. }) = - &errors[0] - else { - panic!("Expected a CannotMutateImmutableVariable error"); - }; - - assert_eq!(name, "array"); + check_errors(src); } #[test] @@ -38,23 +24,14 @@ fn cannot_mutate_immutable_variable_on_member_access() { fn main() { let foo = Foo { x: 0 }; mutate(&mut foo.x); + ^^^^^ Cannot mutate immutable variable `foo` } fn mutate(foo: &mut Field) { *foo = 1; } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::CannotMutateImmutableVariable { name, .. }) = - &errors[0] - else { - panic!("Expected a CannotMutateImmutableVariable error"); - }; - - assert_eq!(name, "foo"); + check_errors(src); } #[test] @@ -62,23 +39,15 @@ fn does_not_crash_when_passing_mutable_undefined_variable() { let src = r#" fn main() { mutate(&mut undefined); + ^^^^^^^^^ cannot find `undefined` in this scope + ~~~~~~~~~ not found in this scope } fn mutate(foo: &mut Field) { *foo = 1; } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::VariableNotDeclared { name, .. }) = - &errors[0] - else { - panic!("Expected a VariableNotDeclared error"); - }; - - assert_eq!(name, "undefined"); + check_errors(src); } #[test] @@ -90,6 +59,7 @@ fn constrained_reference_to_unconstrained() { // Safety: test context unsafe { mut_ref_input(x_ref, y); + ^^^^^ Cannot pass a mutable reference from a constrained runtime to an unconstrained runtime } } @@ -100,13 +70,5 @@ fn constrained_reference_to_unconstrained() { *x = y; } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::ConstrainedReferenceToUnconstrained { .. }) = - &errors[0] - else { - panic!("Expected an error about passing a constrained reference to unconstrained"); - }; + check_errors(src); } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/traits.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/traits.rs index e5b19e695d43..5ba63bc6a29d 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/traits.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/traits.rs @@ -1,11 +1,6 @@ use crate::elaborator::FrontendOptions; -use crate::hir::def_collector::dc_crate::CompilationError; -use crate::hir::def_collector::errors::DefCollectorErrorKind; -use crate::hir::resolution::errors::ResolverError; -use crate::hir::resolution::import::PathResolutionError; -use crate::hir::type_check::TypeCheckError; -use crate::tests::{get_program_errors, get_program_with_maybe_parser_errors}; +use crate::tests::{check_errors, get_program_with_options}; use super::assert_no_errors; @@ -107,63 +102,49 @@ fn trait_inheritance_with_generics_4() { #[test] fn trait_inheritance_dependency_cycle() { + // TODO: maybe the error location should be just on Foo let src = r#" trait Foo: Bar {} + ^^^^^^^^^^^^^^^^^ Dependency cycle found + ~~~~~~~~~~~~~~~~~ 'Foo' recursively depends on itself: Foo -> Bar -> Foo trait Bar: Foo {} fn main() {} "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - assert!(matches!( - errors[0], - CompilationError::ResolverError(ResolverError::DependencyCycle { .. }) - )); + check_errors(src); } #[test] fn trait_inheritance_missing_parent_implementation() { + // TODO: the secondary errors are missing a closing backtick let src = r#" pub trait Foo {} pub trait Bar: Foo {} + ~~~ required by this bound in `Bar pub struct Struct {} impl Bar for Struct {} + ^^^^^^ The trait bound `Struct: Foo` is not satisfied + ~~~~~~ The trait `Foo` is not implemented for `Struct fn main() { let _ = Struct {}; // silence Struct never constructed warning } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::TraitNotImplemented { - impl_trait, - missing_trait: the_trait, - type_missing_trait: typ, - .. - }) = &errors[0] - else { - panic!("Expected a TraitNotImplemented error, got {:?}", &errors[0]); - }; - - assert_eq!(the_trait, "Foo"); - assert_eq!(typ, "Struct"); - assert_eq!(impl_trait, "Bar"); + check_errors(src); } #[test] fn errors_on_unknown_type_in_trait_where_clause() { let src = r#" pub trait Foo where T: Unknown {} + ^^^^^^^ Could not resolve 'Unknown' in path fn main() { } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); + check_errors(src); } #[test] @@ -222,6 +203,7 @@ fn does_not_error_if_impl_trait_constraint_is_satisfied_for_type_variable() { "#; assert_no_errors(src); } + #[test] fn errors_if_impl_trait_constraint_is_not_satisfied() { let src = r#" @@ -232,6 +214,7 @@ fn errors_if_impl_trait_constraint_is_not_satisfied() { pub trait Foo where T: Greeter, + ~~~~~~~ required by this bound in `Foo { fn greet(object: U) where @@ -246,25 +229,12 @@ fn errors_if_impl_trait_constraint_is_not_satisfied() { pub struct Bar; impl Foo for Bar {} + ^^^ The trait bound `SomeGreeter: Greeter` is not satisfied + ~~~ The trait `Greeter` is not implemented for `SomeGreeter fn main() {} "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::TraitNotImplemented { - impl_trait, - missing_trait: the_trait, - type_missing_trait: typ, - .. - }) = &errors[0] - else { - panic!("Expected a TraitNotImplemented error, got {:?}", &errors[0]); - }; - - assert_eq!(the_trait, "Greeter"); - assert_eq!(typ, "SomeGreeter"); - assert_eq!(impl_trait, "Foo"); + check_errors(src); } #[test] @@ -443,6 +413,7 @@ fn trait_alias_polymorphic_where_clause() { fn qux(x: T) -> bool where T: Qux { x.foo().bar().baz() + ^^^^^^^^^^^^^^^^^^^ No method named 'baz' found for type 'U' } impl Foo for Field { @@ -465,35 +436,14 @@ fn trait_alias_polymorphic_where_clause() { fn main() { assert(0.foo().bar().baz() == qux(0)); + ^^^ No matching impl found for `T: Baz` + ~~~ No impl for `T: Baz` } "#; // TODO(https://github.com/noir-lang/noir/issues/6467) // assert_no_errors(src); - let errors = get_program_errors(src); - assert_eq!(errors.len(), 2); - - match &errors[0] { - CompilationError::TypeError(TypeCheckError::UnresolvedMethodCall { - method_name, .. - }) => { - assert_eq!(method_name, "baz"); - } - other => { - panic!("expected UnresolvedMethodCall, but found {:?}", other); - } - } - - match &errors[1] { - CompilationError::TypeError(TypeCheckError::NoMatchingImplFound(err)) => { - assert_eq!(err.constraints.len(), 2); - assert_eq!(err.constraints[0].1, "Baz"); - assert_eq!(err.constraints[1].1, "Qux<_>"); - } - other => { - panic!("expected NoMatchingImplFound, but found {:?}", other); - } - } + check_errors(src); } // TODO(https://github.com/noir-lang/noir/issues/6467): currently failing, so @@ -519,10 +469,12 @@ fn trait_alias_with_where_clause_has_equivalent_errors() { pub fn qux(x: T, _: U) -> bool where U: Qux { x.baz() + ^^^^^^^ No method named 'baz' found for type 'T' } fn main() {} "#; + check_errors(src); let alias_src = r#" trait Bar { @@ -537,37 +489,12 @@ fn trait_alias_with_where_clause_has_equivalent_errors() { pub fn qux(x: T, _: U) -> bool where U: Qux { x.baz() + ^^^^^^^ No method named 'baz' found for type 'T' } fn main() {} "#; - - let errors = get_program_errors(src); - let alias_errors = get_program_errors(alias_src); - - assert_eq!(errors.len(), 1); - assert_eq!(alias_errors.len(), 1); - - match (&errors[0], &alias_errors[0]) { - ( - CompilationError::TypeError(TypeCheckError::UnresolvedMethodCall { - method_name, - object_type, - .. - }), - CompilationError::TypeError(TypeCheckError::UnresolvedMethodCall { - method_name: alias_method_name, - object_type: alias_object_type, - .. - }), - ) => { - assert_eq!(method_name, alias_method_name); - assert_eq!(object_type, alias_object_type); - } - other => { - panic!("expected UnresolvedMethodCall, but found {:?}", other); - } - } + check_errors(alias_src); } #[test] @@ -654,7 +581,7 @@ fn does_not_crash_on_as_trait_path_with_empty_path() { let allow_parser_errors = true; let options = FrontendOptions::test_default(); - let (_, _, errors) = get_program_with_maybe_parser_errors(src, allow_parser_errors, options); + let (_, _, errors) = get_program_with_options(src, allow_parser_errors, options); assert!(!errors.is_empty()); } @@ -663,6 +590,7 @@ fn warns_if_trait_is_not_in_scope_for_function_call_and_there_is_only_one_trait_ let src = r#" fn main() { let _ = Bar::foo(); + ^^^ trait `private_mod::Foo` which provides `foo` is implemented but not in scope, please import it } pub struct Bar { @@ -680,17 +608,7 @@ fn warns_if_trait_is_not_in_scope_for_function_call_and_there_is_only_one_trait_ } } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::PathResolutionError( - PathResolutionError::TraitMethodNotInScope { ident, trait_name }, - )) = &errors[0] - else { - panic!("Expected a 'trait method not in scope' error"); - }; - assert_eq!(ident.to_string(), "foo"); - assert_eq!(trait_name, "private_mod::Foo"); + check_errors(src); } #[test] @@ -792,6 +710,8 @@ fn errors_if_trait_is_not_in_scope_for_function_call_and_there_are_multiple_cand let src = r#" fn main() { let _ = Bar::foo(); + ^^^ Could not resolve 'foo' in path + ~~~ The following traits which provide `foo` are implemented but not in scope: `private_mod::Foo2`, `private_mod::Foo` } pub struct Bar { @@ -819,18 +739,7 @@ fn errors_if_trait_is_not_in_scope_for_function_call_and_there_are_multiple_cand } } "#; - let mut errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::PathResolutionError( - PathResolutionError::UnresolvedWithPossibleTraitsToImport { ident, mut traits }, - )) = errors.remove(0) - else { - panic!("Expected a 'trait method not in scope' error"); - }; - assert_eq!(ident.to_string(), "foo"); - traits.sort(); - assert_eq!(traits, vec!["private_mod::Foo", "private_mod::Foo2"]); + check_errors(src); } #[test] @@ -841,6 +750,8 @@ fn errors_if_multiple_trait_methods_are_in_scope_for_function_call() { fn main() { let _ = Bar::foo(); + ^^^ Multiple applicable items in scope + ~~~ All these trait which provide `foo` are implemented and in scope: `private_mod::Foo2`, `private_mod::Foo` } pub struct Bar { @@ -868,18 +779,7 @@ fn errors_if_multiple_trait_methods_are_in_scope_for_function_call() { } } "#; - let mut errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::PathResolutionError( - PathResolutionError::MultipleTraitsInScope { ident, mut traits }, - )) = errors.remove(0) - else { - panic!("Expected a 'trait method not in scope' error"); - }; - assert_eq!(ident.to_string(), "foo"); - traits.sort(); - assert_eq!(traits, vec!["private_mod::Foo", "private_mod::Foo2"]); + check_errors(src); } #[test] @@ -888,6 +788,7 @@ fn warns_if_trait_is_not_in_scope_for_method_call_and_there_is_only_one_trait_me fn main() { let bar = Bar { x: 42 }; let _ = bar.foo(); + ^^^^^^^^^ trait `private_mod::Foo` which provides `foo` is implemented but not in scope, please import it } pub struct Bar { @@ -906,17 +807,7 @@ fn warns_if_trait_is_not_in_scope_for_method_call_and_there_is_only_one_trait_me } } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::PathResolutionError( - PathResolutionError::TraitMethodNotInScope { ident, trait_name }, - )) = &errors[0] - else { - panic!("Expected a 'trait method not in scope' error"); - }; - assert_eq!(ident.to_string(), "foo"); - assert_eq!(trait_name, "private_mod::Foo"); + check_errors(src); } #[test] @@ -954,6 +845,8 @@ fn errors_if_trait_is_not_in_scope_for_method_call_and_there_are_multiple_candid fn main() { let bar = Bar { x: 42 }; let _ = bar.foo(); + ^^^^^^^^^ Could not resolve 'foo' in path + ~~~~~~~~~ The following traits which provide `foo` are implemented but not in scope: `private_mod::Foo2`, `private_mod::Foo` } pub struct Bar { @@ -982,18 +875,7 @@ fn errors_if_trait_is_not_in_scope_for_method_call_and_there_are_multiple_candid } } "#; - let mut errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::PathResolutionError( - PathResolutionError::UnresolvedWithPossibleTraitsToImport { ident, mut traits }, - )) = errors.remove(0) - else { - panic!("Expected a 'trait method not in scope' error"); - }; - assert_eq!(ident.to_string(), "foo"); - traits.sort(); - assert_eq!(traits, vec!["private_mod::Foo", "private_mod::Foo2"]); + check_errors(src); } #[test] @@ -1005,6 +887,8 @@ fn errors_if_multiple_trait_methods_are_in_scope_for_method_call() { fn main() { let bar = Bar { x : 42 }; let _ = bar.foo(); + ^^^^^^^^^ Multiple applicable items in scope + ~~~~~~~~~ All these trait which provide `foo` are implemented and in scope: `private_mod::Foo2`, `private_mod::Foo` } pub struct Bar { @@ -1033,18 +917,7 @@ fn errors_if_multiple_trait_methods_are_in_scope_for_method_call() { } } "#; - let mut errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::PathResolutionError( - PathResolutionError::MultipleTraitsInScope { ident, mut traits }, - )) = errors.remove(0) - else { - panic!("Expected a 'trait method not in scope' error"); - }; - assert_eq!(ident.to_string(), "foo"); - traits.sort(); - assert_eq!(traits, vec!["private_mod::Foo", "private_mod::Foo2"]); + check_errors(src); } #[test] @@ -1084,28 +957,17 @@ fn type_checks_trait_default_method_and_errors() { let src = r#" pub trait Foo { fn foo(self) -> i32 { + ^^^ expected type i32, found type bool + ~~~ expected i32 because of return type let _ = self; true + ~~~~ bool returned here } } fn main() {} "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::TypeMismatchWithSource { - expected, - actual, - .. - }) = &errors[0] - else { - panic!("Expected a type mismatch error, got {:?}", errors[0]); - }; - - assert_eq!(expected.to_string(), "i32"); - assert_eq!(actual.to_string(), "bool"); + check_errors(src); } #[test] @@ -1147,6 +1009,7 @@ fn warns_if_trait_is_not_in_scope_for_primitive_function_call_and_there_is_only_ let src = r#" fn main() { let _ = Field::foo(); + ^^^ trait `private_mod::Foo` which provides `foo` is implemented but not in scope, please import it } mod private_mod { @@ -1161,17 +1024,7 @@ fn warns_if_trait_is_not_in_scope_for_primitive_function_call_and_there_is_only_ } } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::PathResolutionError( - PathResolutionError::TraitMethodNotInScope { ident, trait_name }, - )) = &errors[0] - else { - panic!("Expected a 'trait method not in scope' error"); - }; - assert_eq!(ident.to_string(), "foo"); - assert_eq!(trait_name, "private_mod::Foo"); + check_errors(src); } #[test] @@ -1180,6 +1033,7 @@ fn warns_if_trait_is_not_in_scope_for_primitive_method_call_and_there_is_only_on fn main() { let x: Field = 1; let _ = x.foo(); + ^^^^^^^ trait `private_mod::Foo` which provides `foo` is implemented but not in scope, please import it } mod private_mod { @@ -1194,17 +1048,7 @@ fn warns_if_trait_is_not_in_scope_for_primitive_method_call_and_there_is_only_on } } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::PathResolutionError( - PathResolutionError::TraitMethodNotInScope { ident, trait_name }, - )) = &errors[0] - else { - panic!("Expected a 'trait method not in scope' error"); - }; - assert_eq!(ident.to_string(), "foo"); - assert_eq!(trait_name, "private_mod::Foo"); + check_errors(src); } #[test] @@ -1213,6 +1057,7 @@ fn warns_if_trait_is_not_in_scope_for_generic_function_call_and_there_is_only_on fn main() { let x: i32 = 1; let _ = x.foo(); + ^^^^^^^ trait `private_mod::Foo` which provides `foo` is implemented but not in scope, please import it } mod private_mod { @@ -1227,17 +1072,7 @@ fn warns_if_trait_is_not_in_scope_for_generic_function_call_and_there_is_only_on } } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::PathResolutionError( - PathResolutionError::TraitMethodNotInScope { ident, trait_name }, - )) = &errors[0] - else { - panic!("Expected a 'trait method not in scope' error"); - }; - assert_eq!(ident.to_string(), "foo"); - assert_eq!(trait_name, "private_mod::Foo"); + check_errors(src); } #[test] @@ -1248,23 +1083,19 @@ fn error_on_duplicate_impl_with_associated_type() { } impl Foo for i32 { + ~~~ Previous impl defined here type Bar = u32; } impl Foo for i32 { + ^^^ Impl for type `i32` overlaps with existing impl + ~~~ Overlapping impl type Bar = u8; } fn main() {} "#; - - // Expect "Impl for type `i32` overlaps with existing impl" - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - use CompilationError::DefinitionError; - use DefCollectorErrorKind::*; - assert!(matches!(&errors[0], DefinitionError(OverlappingImpl { .. }))); + check_errors(src); } #[test] @@ -1275,23 +1106,19 @@ fn error_on_duplicate_impl_with_associated_constant() { } impl Foo for i32 { + ~~~ Previous impl defined here let Bar = 5; } impl Foo for i32 { + ^^^ Impl for type `i32` overlaps with existing impl + ~~~ Overlapping impl let Bar = 6; } fn main() {} "#; - - // Expect "Impl for type `i32` overlaps with existing impl" - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - use CompilationError::DefinitionError; - use DefCollectorErrorKind::*; - assert!(matches!(&errors[0], DefinitionError(OverlappingImpl { .. }))); + check_errors(src); } // See https://github.com/noir-lang/noir/issues/6530 @@ -1335,8 +1162,7 @@ fn regression_6530() { let _: Field = foo.into(); } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 0); + assert_no_errors(src); } #[test] @@ -1383,9 +1209,8 @@ fn calls_trait_method_using_struct_name_when_multiple_impls_exist_and_errors_tur } fn main() { let _ = U60Repr::::from2([1, 2, 3]); + ^^^^^^^^^ Expected type Field, found type [Field; 3] } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!(errors[0], CompilationError::TypeError(TypeCheckError::TypeMismatch { .. }))); + check_errors(src); } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/turbofish.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/turbofish.rs index bda590533cf3..d1bf9002ab74 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/turbofish.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/turbofish.rs @@ -1,10 +1,6 @@ -use crate::hir::{ - def_collector::dc_crate::CompilationError, - resolution::{errors::ResolverError, import::PathResolutionError}, - type_check::TypeCheckError, -}; +use crate::tests::check_errors; -use super::{assert_no_errors, get_program_errors}; +use super::assert_no_errors; #[test] fn turbofish_numeric_generic_nested_call() { @@ -66,15 +62,10 @@ fn turbofish_in_constructor_generics_mismatch() { fn main() { let _ = Foo:: { x: 1 }; + ^^^^^^^^^^^^ struct Foo expects 1 generic but 2 were given } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::TypeError(TypeCheckError::GenericCountMismatch { .. }), - )); + check_errors(src); } #[test] @@ -87,21 +78,10 @@ fn turbofish_in_constructor() { fn main() { let x: Field = 0; let _ = Foo:: { x: x }; + ^ Expected type i32, found type Field } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::TypeMismatch { - expected_typ, expr_typ, .. - }) = &errors[0] - else { - panic!("Expected a type mismatch error, got {:?}", errors[0]); - }; - - assert_eq!(expected_typ, "i32"); - assert_eq!(expr_typ, "Field"); + check_errors(src); } #[test] @@ -122,6 +102,7 @@ fn turbofish_in_struct_pattern() { #[test] fn turbofish_in_struct_pattern_errors_if_type_mismatch() { + // TODO: maybe the error should be on the expression let src = r#" struct Foo { x: T @@ -130,17 +111,11 @@ fn turbofish_in_struct_pattern_errors_if_type_mismatch() { fn main() { let value: Field = 0; let Foo:: { x } = Foo { x: value }; + ^^^^^^^^^^^^^^^^ Cannot assign an expression of type Foo to a value of type Foo let _ = x; } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::TypeMismatchWithSource { .. }) = &errors[0] - else { - panic!("Expected a type mismatch error, got {:?}", errors[0]); - }; + check_errors(src); } #[test] @@ -153,26 +128,11 @@ fn turbofish_in_struct_pattern_generic_count_mismatch() { fn main() { let value = 0; let Foo:: { x } = Foo { x: value }; + ^^^^^^^^^^^^ struct Foo expects 1 generic but 2 were given let _ = x; } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::GenericCountMismatch { - item, - expected, - found, - .. - }) = &errors[0] - else { - panic!("Expected a generic count mismatch error, got {:?}", errors[0]); - }; - - assert_eq!(item, "struct Foo"); - assert_eq!(*expected, 1); - assert_eq!(*found, 2); + check_errors(src); } #[test] @@ -202,19 +162,10 @@ fn errors_if_turbofish_after_module() { fn main() { moo::::foo(); + ^^^^^^^ turbofish (`::<_>`) not allowed on module `moo` } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::PathResolutionError( - PathResolutionError::TurbofishNotAllowedOnItem { item, .. }, - )) = &errors[0] - else { - panic!("Expected a turbofish not allowed on item error, got {:?}", errors[0]); - }; - assert_eq!(item, "module `moo`"); + check_errors(src); } #[test] @@ -252,22 +203,10 @@ fn turbofish_in_type_before_call_errors() { fn main() { let _ = Foo::::new(true); + ^^^^ Expected type i32, found type bool } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::TypeMismatch { - expected_typ, - expr_typ, - expr_location: _, - }) = &errors[0] - else { - panic!("Expected a type mismatch error, got {:?}", errors[0]); - }; - - assert_eq!(expected_typ, "i32"); - assert_eq!(expr_typ, "bool"); + check_errors(src); } #[test] @@ -312,22 +251,10 @@ fn use_generic_type_alias_with_turbofish_in_method_call_errors() { fn main() { let _ = Bar::::new(true); + ^^^^ Expected type i32, found type bool } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::TypeMismatch { - expected_typ, - expr_typ, - expr_location: _, - }) = &errors[0] - else { - panic!("Expected a type mismatch error, got {:?}", errors[0]); - }; - - assert_eq!(expected_typ, "i32"); - assert_eq!(expr_typ, "bool"); + check_errors(src); } #[test] @@ -371,22 +298,10 @@ fn use_generic_type_alias_with_partial_generics_with_turbofish_in_method_call_er fn main() { let _ = Bar::::new(1, 1); + ^ Expected type bool, found type Field } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::TypeMismatch { - expected_typ, - expr_typ, - expr_location: _, - }) = &errors[0] - else { - panic!("Expected a type mismatch error, got {:?}", errors[0]); - }; - - assert_eq!(expected_typ, "bool"); - assert_eq!(expr_typ, "Field"); + check_errors(src); } #[test] @@ -407,22 +322,10 @@ fn use_generic_type_alias_with_partial_generics_with_turbofish_in_method_call_er fn main() { let _ = Bar::::new(true, true); + ^^^^ Expected type i32, found type bool } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::TypeMismatch { - expected_typ, - expr_typ, - expr_location: _, - }) = &errors[0] - else { - panic!("Expected a type mismatch error, got {:?}", errors[0]); - }; - - assert_eq!(expected_typ, "i32"); - assert_eq!(expr_typ, "bool"); + check_errors(src); } #[test] @@ -440,20 +343,8 @@ fn trait_function_with_turbofish_on_trait_gives_error() { fn main() { let _: i32 = Foo::::foo(1); + ^ Expected type bool, found type Field } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::TypeError(TypeCheckError::TypeMismatch { - expected_typ, - expr_typ, - expr_location: _, - }) = &errors[0] - else { - panic!("Expected a type mismatch error, got {:?}", errors[0]); - }; - - assert_eq!(expected_typ, "bool"); - assert_eq!(expr_typ, "Field"); + check_errors(src); } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/unused_items.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/unused_items.rs index 0840eb18a388..a3c501142444 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/unused_items.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/unused_items.rs @@ -1,9 +1,4 @@ -use crate::{ - hir::{def_collector::dc_crate::CompilationError, resolution::errors::ResolverError}, - tests::assert_no_errors, -}; - -use super::get_program_errors; +use crate::tests::{assert_no_errors, check_errors}; #[test] fn errors_on_unused_private_import() { @@ -17,6 +12,8 @@ fn errors_on_unused_private_import() { } use foo::bar; + ^^^ unused import bar + ~~~ unused import use foo::baz; use foo::Foo; @@ -27,17 +24,7 @@ fn errors_on_unused_private_import() { baz(); } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item }) = &errors[0] - else { - panic!("Expected an unused item error"); - }; - - assert_eq!(ident.to_string(), "bar"); - assert_eq!(item.item_type(), "import"); + check_errors(src); } #[test] @@ -52,6 +39,8 @@ fn errors_on_unused_pub_crate_import() { } pub(crate) use foo::bar; + ^^^ unused import bar + ~~~ unused import use foo::baz; use foo::Foo; @@ -62,17 +51,7 @@ fn errors_on_unused_pub_crate_import() { baz(); } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item }) = &errors[0] - else { - panic!("Expected an unused item error"); - }; - - assert_eq!(ident.to_string(), "bar"); - assert_eq!(item.item_type(), "import"); + check_errors(src); } #[test] @@ -88,51 +67,37 @@ fn errors_on_unused_function() { fn foo() { + ^^^ unused function foo + ~~~ unused function bar(); } fn bar() {} "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item }) = &errors[0] - else { - panic!("Expected an unused item error"); - }; - - assert_eq!(ident.to_string(), "foo"); - assert_eq!(item.item_type(), "function"); + check_errors(src); } #[test] fn errors_on_unused_struct() { let src = r#" struct Foo {} + ^^^ struct `Foo` is never constructed + ~~~ struct is never constructed struct Bar {} fn main() { let _ = Bar {}; } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item }) = &errors[0] - else { - panic!("Expected an unused item error"); - }; - - assert_eq!(ident.to_string(), "Foo"); - assert_eq!(item.item_type(), "struct"); + check_errors(src); } #[test] fn errors_on_unused_trait() { let src = r#" trait Foo {} + ^^^ unused trait Foo + ~~~ unused trait trait Bar {} pub struct Baz { @@ -143,17 +108,7 @@ fn errors_on_unused_trait() { fn main() { } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item }) = &errors[0] - else { - panic!("Expected an unused item error"); - }; - - assert_eq!(ident.to_string(), "Foo"); - assert_eq!(item.item_type(), "trait"); + check_errors(src); } #[test] @@ -171,44 +126,28 @@ fn silences_unused_variable_warning() { fn errors_on_unused_type_alias() { let src = r#" type Foo = Field; + ^^^ unused type alias Foo + ~~~ unused type alias type Bar = Field; pub fn bar(_: Bar) {} fn main() {} "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item }) = &errors[0] - else { - panic!("Expected an unused item error"); - }; - - assert_eq!(ident.to_string(), "Foo"); - assert_eq!(item.item_type(), "type alias"); + check_errors(src); } #[test] fn warns_on_unused_global() { let src = r#" global foo: u32 = 1; + ^^^ unused global foo + ~~~ unused global global bar: Field = 1; fn main() { let _ = bar; } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item }) = &errors[0] - else { - panic!("Expected an unused item warning"); - }; - - assert_eq!(ident.to_string(), "foo"); - assert_eq!(item.item_type(), "global"); + check_errors(src); } #[test] @@ -262,9 +201,7 @@ fn no_warning_on_inner_struct_when_parent_is_used() { assert_eq(foos[0].a, 10); } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 0); + assert_no_errors(src); } #[test] diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/visibility.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/visibility.rs index ce41141b8d80..ee53dcbc84ff 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/visibility.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/visibility.rs @@ -1,11 +1,4 @@ -use crate::{ - hir::{ - comptime::ComptimeError, - def_collector::{dc_crate::CompilationError, errors::DefCollectorErrorKind}, - resolution::{errors::ResolverError, import::PathResolutionError}, - }, - tests::{assert_no_errors, get_program_errors}, -}; +use crate::tests::{assert_no_errors, check_errors}; #[test] fn errors_once_on_unused_import_that_is_not_accessible() { @@ -15,39 +8,13 @@ fn errors_once_on_unused_import_that_is_not_accessible() { struct Foo {} } use moo::Foo; + ^^^ Foo is private and not visible from the current module + ~~~ Foo is private fn main() { let _ = Foo {}; } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0], - CompilationError::DefinitionError(DefCollectorErrorKind::PathResolutionError( - PathResolutionError::Private { .. } - )) - )); -} - -fn assert_type_is_more_private_than_item_error(src: &str, private_typ: &str, public_item: &str) { - let errors = get_program_errors(src); - - assert!(!errors.is_empty(), "expected visibility error, got nothing"); - for error in &errors { - let CompilationError::ResolverError(ResolverError::TypeIsMorePrivateThenItem { - typ, - item, - .. - }) = error - else { - panic!("Expected a type vs item visibility error, got {}", error); - }; - - assert_eq!(typ, private_typ); - assert_eq!(item, public_item); - } - assert_eq!(errors.len(), 1, "only expected one error"); + check_errors(src); } #[test] @@ -55,12 +22,13 @@ fn errors_if_type_alias_aliases_more_private_type() { let src = r#" struct Foo {} pub type Bar = Foo; + ^^^ Type `Foo` is more private than item `Bar` pub fn no_unused_warnings() { let _: Bar = Foo {}; } fn main() {} "#; - assert_type_is_more_private_than_item_error(src, "Foo", "Bar"); + check_errors(src); } #[test] @@ -69,13 +37,14 @@ fn errors_if_type_alias_aliases_more_private_type_in_generic() { pub struct Generic { value: T } struct Foo {} pub type Bar = Generic; + ^^^^^^^^^^^^ Type `Foo` is more private than item `Bar` pub fn no_unused_warnings() { let _ = Foo {}; let _: Bar = Generic { value: Foo {} }; } fn main() {} "#; - assert_type_is_more_private_than_item_error(src, "Foo", "Bar"); + check_errors(src); } #[test] @@ -85,6 +54,7 @@ fn errors_if_pub_type_alias_leaks_private_type_in_generic() { struct Bar {} pub struct Foo { pub value: T } pub type FooBar = Foo; + ^^^^^^^^ Type `Bar` is more private than item `FooBar` pub fn no_unused_warnings() { let _: FooBar = Foo { value: Bar {} }; @@ -92,7 +62,7 @@ fn errors_if_pub_type_alias_leaks_private_type_in_generic() { } fn main() {} "#; - assert_type_is_more_private_than_item_error(src, "Bar", "FooBar"); + check_errors(src); } #[test] @@ -102,6 +72,7 @@ fn errors_if_pub_struct_field_leaks_private_type_in_generic() { struct Bar {} pub struct Foo { pub value: T } pub struct FooBar { pub value: Foo } + ^^^^^ Type `Bar` is more private than item `FooBar::value` pub fn no_unused_warnings() { let _ = FooBar { value: Foo { value: Bar {} } }; @@ -109,7 +80,7 @@ fn errors_if_pub_struct_field_leaks_private_type_in_generic() { } fn main() {} "#; - assert_type_is_more_private_than_item_error(src, "Bar", "FooBar::value"); + check_errors(src); } #[test] @@ -119,12 +90,13 @@ fn errors_if_pub_function_leaks_private_type_in_return() { struct Bar {} pub fn bar() -> Bar { + ^^^ Type `Bar` is more private than item `bar` Bar {} } } fn main() {} "#; - assert_type_is_more_private_than_item_error(src, "Bar", "bar"); + check_errors(src); } #[test] @@ -133,6 +105,7 @@ fn errors_if_pub_function_leaks_private_type_in_arg() { pub mod moo { struct Bar {} pub fn bar(_bar: Bar) {} + ^^^ Type `Bar` is more private than item `bar` pub fn no_unused_warnings() { let _ = Bar {}; @@ -140,7 +113,7 @@ fn errors_if_pub_function_leaks_private_type_in_arg() { } fn main() {} "#; - assert_type_is_more_private_than_item_error(src, "Bar", "bar"); + check_errors(src); } #[test] @@ -173,6 +146,7 @@ fn errors_if_pub_function_on_pub_struct_returns_private() { impl Foo { pub fn bar() -> Bar { + ^^^ Type `Bar` is more private than item `bar` Bar {} } } @@ -183,7 +157,7 @@ fn errors_if_pub_function_on_pub_struct_returns_private() { } fn main() {} "#; - assert_type_is_more_private_than_item_error(src, "Bar", "bar"); + check_errors(src); } #[test] @@ -219,6 +193,7 @@ fn errors_if_pub_trait_returns_private_struct() { pub trait Foo { fn foo() -> Bar; + ^^^ Type `Bar` is more private than item `foo` } pub fn no_unused_warnings() { @@ -227,7 +202,7 @@ fn errors_if_pub_trait_returns_private_struct() { } fn main() {} "#; - assert_type_is_more_private_than_item_error(src, "Bar", "foo"); + check_errors(src); } #[test] @@ -263,20 +238,11 @@ fn errors_if_trying_to_access_public_function_inside_private_module() { } fn main() { foo::bar::baz() + ^^^ bar is private and not visible from the current module + ~~~ bar is private } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::PathResolutionError( - PathResolutionError::Private(ident), - )) = &errors[0] - else { - panic!("Expected a private error"); - }; - - assert_eq!(ident.to_string(), "bar"); + check_errors(src); } #[test] @@ -294,22 +260,13 @@ fn warns_if_calling_private_struct_method() { pub fn method(foo: moo::Foo) { foo.bar() + ^^^ bar is private and not visible from the current module + ~~~ bar is private } fn main() {} "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::PathResolutionError( - PathResolutionError::Private(ident), - )) = &errors[0] - else { - panic!("Expected a private error"); - }; - - assert_eq!(ident.to_string(), "bar"); + check_errors(src); } #[test] @@ -386,22 +343,13 @@ fn error_when_accessing_private_struct_field() { fn foo(foo: moo::Foo) -> Field { foo.x + ^ x is private and not visible from the current module + ~ x is private } fn main() {} "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::PathResolutionError( - PathResolutionError::Private(ident), - )) = &errors[0] - else { - panic!("Expected a private error"); - }; - - assert_eq!(ident.to_string(), "x"); + check_errors(src); } #[test] @@ -455,20 +403,11 @@ fn error_when_using_private_struct_field_in_constructor() { fn main() { let _ = moo::Foo { x: 1 }; + ^ x is private and not visible from the current module + ~ x is private } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::PathResolutionError( - PathResolutionError::Private(ident), - )) = &errors[0] - else { - panic!("Expected a private error"); - }; - - assert_eq!(ident.to_string(), "x"); + check_errors(src); } #[test] @@ -482,24 +421,15 @@ fn error_when_using_private_struct_field_in_struct_pattern() { fn foo(foo: moo::Foo) -> Field { let moo::Foo { x } = foo; + ^ x is private and not visible from the current module + ~ x is private x } fn main() { } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::PathResolutionError( - PathResolutionError::Private(ident), - )) = &errors[0] - else { - panic!("Expected a private error"); - }; - - assert_eq!(ident.to_string(), "x"); + check_errors(src); } #[test] @@ -564,21 +494,12 @@ fn errors_if_accessing_private_struct_member_inside_comptime_context() { comptime { let foo = Foo::new(5); let _ = foo.inner; + ^^^^^ inner is private and not visible from the current module + ~~~~~ inner is private }; } "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::PathResolutionError( - PathResolutionError::Private(ident), - )) = &errors[0] - else { - panic!("Expected a private error"); - }; - - assert_eq!(ident.to_string(), "inner"); + check_errors(src); } #[test] @@ -593,6 +514,7 @@ fn errors_if_accessing_private_struct_member_inside_function_generated_at_compti use foo::Foo; #[generate_inner_accessor] + ~~~~~~~~~~~~~~~~~~~~~~~~~~ While running this function attribute struct Bar { bar_inner: Foo, } @@ -601,6 +523,8 @@ fn errors_if_accessing_private_struct_member_inside_function_generated_at_compti quote { fn bar_get_foo_inner(x: Bar) -> Field { x.bar_inner.foo_inner + ^^^^^^^^^ foo_inner is private and not visible from the current module + ~~~~~~~~~ foo_inner is private } } } @@ -609,22 +533,5 @@ fn errors_if_accessing_private_struct_member_inside_function_generated_at_compti let _ = bar_get_foo_inner(x); } "#; - - let mut errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ComptimeError(ComptimeError::ErrorRunningAttribute { error, .. }) = - errors.remove(0) - else { - panic!("Expected a ComptimeError, got {:?}", errors[0]); - }; - - let CompilationError::ResolverError(ResolverError::PathResolutionError( - PathResolutionError::Private(ident), - )) = *error - else { - panic!("Expected a private error"); - }; - - assert_eq!(ident.to_string(), "foo_inner"); + check_errors(src); } diff --git a/noir/noir-repo/compiler/wasm/src/compile.rs b/noir/noir-repo/compiler/wasm/src/compile.rs index ff9488322edc..021462d9f46e 100644 --- a/noir/noir-repo/compiler/wasm/src/compile.rs +++ b/noir/noir-repo/compiler/wasm/src/compile.rs @@ -13,7 +13,7 @@ use noirc_frontend::{ hir::Context, }; use serde::Deserialize; -use std::{collections::HashMap, path::Path}; +use std::{collections::BTreeMap, path::Path}; use wasm_bindgen::prelude::*; use crate::errors::{CompileError, JsCompileError}; @@ -128,16 +128,21 @@ impl JsCompileContractResult { #[derive(Deserialize, Default)] pub(crate) struct DependencyGraph { pub(crate) root_dependencies: Vec, - pub(crate) library_dependencies: HashMap>, + pub(crate) library_dependencies: BTreeMap>, } +/// This is map contains the paths of all of the files in the entry-point crate and +/// the transitive dependencies of the entry-point crate. +/// +/// This is for all intents and purposes the file system that the compiler will use to resolve/compile +/// files in the crate being compiled and its dependencies. +/// +/// Using a `BTreeMap` to add files to the [FileManager] in a deterministic order, +/// which affects the `FileId` in the `Location`s in the AST on which the `hash` is based. +/// Note that we cannot expect to match the IDs assigned by the `FileManager` used by `nargo`, +/// because there the order is determined by the dependency graph as well as the file name. #[wasm_bindgen] -// This is a map containing the paths of all of the files in the entry-point crate and -// the transitive dependencies of the entry-point crate. -// -// This is for all intents and purposes the file system that the compiler will use to resolve/compile -// files in the crate being compiled and its dependencies. #[derive(Deserialize, Default)] -pub struct PathToFileSourceMap(pub(crate) HashMap); +pub struct PathToFileSourceMap(pub(crate) BTreeMap); #[wasm_bindgen] impl PathToFileSourceMap { @@ -207,7 +212,7 @@ pub fn compile_contract( let compiled_contract = noirc_driver::compile_contract(&mut context, crate_id, &compile_options) - .map_err(|errs| { + .map_err(|errs: Vec| { CompileError::with_file_diagnostics( "Failed to compile contract", errs, @@ -231,7 +236,7 @@ fn prepare_context( ::into_serde(&JsValue::from(dependency_graph)) .map_err(|err| err.to_string())? } else { - DependencyGraph { root_dependencies: vec![], library_dependencies: HashMap::new() } + DependencyGraph { root_dependencies: vec![], library_dependencies: BTreeMap::new() } }; let fm = file_manager_with_source_map(file_source_map); @@ -272,7 +277,7 @@ pub(crate) fn file_manager_with_source_map(source_map: PathToFileSourceMap) -> F // upon some library `lib1`. Then the packages that `lib1` depend upon will be placed in the // `library_dependencies` list and the `lib1` will be placed in the `root_dependencies` list. fn process_dependency_graph(context: &mut Context, dependency_graph: DependencyGraph) { - let mut crate_names: HashMap<&CrateName, CrateId> = HashMap::new(); + let mut crate_names: BTreeMap<&CrateName, CrateId> = BTreeMap::new(); for lib in &dependency_graph.root_dependencies { let crate_id = add_noir_lib(context, lib); @@ -312,7 +317,7 @@ mod test { use crate::compile::PathToFileSourceMap; use super::{DependencyGraph, file_manager_with_source_map, process_dependency_graph}; - use std::{collections::HashMap, path::Path}; + use std::{collections::BTreeMap, path::Path}; fn setup_test_context(source_map: PathToFileSourceMap) -> Context<'static, 'static> { let mut fm = file_manager_with_source_map(source_map); @@ -333,7 +338,7 @@ mod test { #[test] fn test_works_with_empty_dependency_graph() { let dependency_graph = - DependencyGraph { root_dependencies: vec![], library_dependencies: HashMap::new() }; + DependencyGraph { root_dependencies: vec![], library_dependencies: BTreeMap::new() }; let source_map = PathToFileSourceMap::default(); let mut context = setup_test_context(source_map); @@ -348,7 +353,7 @@ mod test { fn test_works_with_root_dependencies() { let dependency_graph = DependencyGraph { root_dependencies: vec![crate_name("lib1")], - library_dependencies: HashMap::new(), + library_dependencies: BTreeMap::new(), }; let source_map = PathToFileSourceMap( @@ -368,7 +373,7 @@ mod test { fn test_works_with_duplicate_root_dependencies() { let dependency_graph = DependencyGraph { root_dependencies: vec![crate_name("lib1"), crate_name("lib1")], - library_dependencies: HashMap::new(), + library_dependencies: BTreeMap::new(), }; let source_map = PathToFileSourceMap( @@ -387,7 +392,7 @@ mod test { fn test_works_with_transitive_dependencies() { let dependency_graph = DependencyGraph { root_dependencies: vec![crate_name("lib1")], - library_dependencies: HashMap::from([ + library_dependencies: BTreeMap::from([ (crate_name("lib1"), vec![crate_name("lib2")]), (crate_name("lib2"), vec![crate_name("lib3")]), ]), @@ -413,7 +418,7 @@ mod test { fn test_works_with_missing_dependencies() { let dependency_graph = DependencyGraph { root_dependencies: vec![crate_name("lib1")], - library_dependencies: HashMap::from([(crate_name("lib2"), vec![crate_name("lib3")])]), + library_dependencies: BTreeMap::from([(crate_name("lib2"), vec![crate_name("lib3")])]), }; let source_map = PathToFileSourceMap( diff --git a/noir/noir-repo/compiler/wasm/src/types/noir_artifact.ts b/noir/noir-repo/compiler/wasm/src/types/noir_artifact.ts index b7b42186c1a1..2abdea63c516 100644 --- a/noir/noir-repo/compiler/wasm/src/types/noir_artifact.ts +++ b/noir/noir-repo/compiler/wasm/src/types/noir_artifact.ts @@ -58,6 +58,8 @@ export interface ABIVariable { export interface NoirFunctionEntry { /** The name of the function. */ name: string; + /** The hash of the circuit. */ + hash?: string; /** Whether the function is unconstrained. */ is_unconstrained: boolean; /** The custom attributes applied to the function. */ @@ -96,7 +98,7 @@ export interface ProgramArtifact { /** Version of noir used for the build. */ noir_version: string; /** The hash of the circuit. */ - hash?: number; + hash?: string; /** * The ABI of the function. */ abi: Abi; /** The bytecode of the circuit in base64. */ diff --git a/noir/noir-repo/compiler/wasm/test/compiler/shared/compile.test.ts b/noir/noir-repo/compiler/wasm/test/compiler/shared/compile.test.ts index f9e37530cbcd..8c9af58fa0ae 100644 --- a/noir/noir-repo/compiler/wasm/test/compiler/shared/compile.test.ts +++ b/noir/noir-repo/compiler/wasm/test/compiler/shared/compile.test.ts @@ -27,10 +27,11 @@ export function shouldCompileProgramIdentically( // Prepare noir-wasm artifact const noirWasmProgram = noirWasmArtifact.program; expect(noirWasmProgram).not.to.be.undefined; - const [_noirWasmDebugInfos, norWasmFileMap] = deleteProgramDebugMetadata(noirWasmProgram); + const [_noirWasmDebugInfos, noirWasmFileMap] = deleteProgramDebugMetadata(noirWasmProgram); normalizeVersion(noirWasmProgram); - // We first compare both contracts without considering debug info + // We first compare both contracts without considering debug info. + // We can't expect hashes to match `nargo` because of the different order in which dependencies are visited. delete (noirWasmProgram as Partial).hash; delete (nargoArtifact as Partial).hash; expect(nargoArtifact).to.deep.eq(noirWasmProgram); @@ -38,7 +39,7 @@ export function shouldCompileProgramIdentically( // Compare the file maps, ignoring keys, since those depend in the order in which files are visited, // which may change depending on the file manager implementation. Also ignores paths, since the base // path is reported differently between nargo and noir-wasm. - expect(getSources(nargoFileMap)).to.have.members(getSources(norWasmFileMap)); + expect(getSources(nargoFileMap)).to.have.members(getSources(noirWasmFileMap)); // Compare the debug symbol information, ignoring the actual ids used for file identifiers. // Debug symbol info looks like the following, what we need is to ignore the 'file' identifiers @@ -63,16 +64,23 @@ export function shouldCompileContractIdentically( // Prepare noir-wasm artifact const noirWasmContract = noirWasmArtifact.contract; expect(noirWasmContract).not.to.be.undefined; - const [noirWasmDebugInfos, norWasmFileMap] = deleteContractDebugMetadata(noirWasmContract); + const [noirWasmDebugInfos, noirWasmFileMap] = deleteContractDebugMetadata(noirWasmContract); normalizeVersion(noirWasmContract); // We first compare both contracts without considering debug info + // We can't expect hashes to match `nargo` because of the different order in which dependencies are visited. + nargoArtifact.functions.forEach(function (f) { + delete (f as Partial).hash; + }); + noirWasmContract.functions.forEach(function (f) { + delete (f as Partial).hash; + }); expect(nargoArtifact).to.deep.eq(noirWasmContract); // Compare the file maps, ignoring keys, since those depend in the order in which files are visited, // which may change depending on the file manager implementation. Also ignores paths, since the base // path is reported differently between nargo and noir-wasm. - expect(getSources(nargoFileMap)).to.have.members(getSources(norWasmFileMap)); + expect(getSources(nargoFileMap)).to.have.members(getSources(noirWasmFileMap)); // Compare the debug symbol information, ignoring the actual ids used for file identifiers. // Debug symbol info looks like the following, what we need is to ignore the 'file' identifiers diff --git a/noir/noir-repo/test_programs/compile_success_empty/enums/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/enums/src/main.nr index e862c2bc54f0..3b12af1c15d5 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/enums/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/enums/src/main.nr @@ -20,8 +20,20 @@ fn primitive_tests() { false => fail(), true => (), } + + // Ensure we can match on MIN and use globals as constants + let i64_min = I64_MIN; + match i64_min { + 9_223_372_036_854_775_807 => fail(), + -9_223_372_036_854_775_807 => fail(), + 0 => fail(), + I64_MIN => (), + _ => fail(), + } } +global I64_MIN: i64 = -9_223_372_036_854_775_808; + enum Foo { A(Field, Field), B(u32), diff --git a/noir/noir-repo/tooling/artifact_cli/src/fs/artifact.rs b/noir/noir-repo/tooling/artifact_cli/src/fs/artifact.rs index 98823599d5cb..1a55764a9fed 100644 --- a/noir/noir-repo/tooling/artifact_cli/src/fs/artifact.rs +++ b/noir/noir-repo/tooling/artifact_cli/src/fs/artifact.rs @@ -1,4 +1,4 @@ -use std::path::Path; +use std::path::{Path, PathBuf}; use crate::{ Artifact, @@ -6,11 +6,13 @@ use crate::{ }; use noirc_artifacts::contract::ContractArtifact; use noirc_artifacts::program::ProgramArtifact; +use noirc_driver::CrateName; +use serde::de::Error; impl Artifact { /// Try to parse an artifact as a binary program or a contract pub fn read_from_file(path: &Path) -> Result { - let json = std::fs::read(path).map_err(FilesystemError::from)?; + let json = std::fs::read(path.with_extension("json")).map_err(FilesystemError::from)?; let as_program = || serde_json::from_slice::(&json).map(Artifact::Program); let as_contract = @@ -35,3 +37,55 @@ pub fn read_bytecode_from_file( .map_err(|e| FilesystemError::InvalidBytecodeFile(file_path, e.to_string()))?; Ok(bytecode) } + +/// Read a `ProgramArtifact`. Returns error if it turns out to be a `ContractArtifact`. +pub fn read_program_from_file(path: &Path) -> Result { + match Artifact::read_from_file(path)? { + Artifact::Program(program) => Ok(program), + Artifact::Contract(contract) => { + let msg = format!( + "expected a program artifact but found a contract in {}: {}", + path.display(), + contract.name + ); + Err(CliError::ArtifactDeserializationError(serde_json::Error::custom(msg))) + } + } +} + +pub fn save_program_to_file( + program_artifact: &ProgramArtifact, + crate_name: &CrateName, + output_dir: &Path, +) -> Result { + let circuit_name: String = crate_name.into(); + save_build_artifact_to_file(program_artifact, &circuit_name, output_dir) +} + +pub fn save_contract_to_file( + compiled_contract: &ContractArtifact, + circuit_name: &str, + output_dir: &Path, +) -> Result { + save_build_artifact_to_file(compiled_contract, circuit_name, output_dir) +} + +fn save_build_artifact_to_file( + build_artifact: &T, + artifact_name: &str, + output_dir: &Path, +) -> Result { + let artifact_path = output_dir.join(artifact_name).with_extension("json"); + let bytes = serde_json::to_vec(build_artifact)?; + write_to_file(&bytes, &artifact_path)?; + Ok(artifact_path) +} + +/// Create the parent directory if needed and write the bytes to a file. +pub fn write_to_file(bytes: &[u8], path: &Path) -> Result<(), FilesystemError> { + if let Some(dir) = path.parent() { + std::fs::create_dir_all(dir)?; + } + std::fs::write(path, bytes)?; + Ok(()) +} diff --git a/noir/noir-repo/tooling/artifact_cli/src/fs/witness.rs b/noir/noir-repo/tooling/artifact_cli/src/fs/witness.rs index ce36266149d2..cf1fe75aad75 100644 --- a/noir/noir-repo/tooling/artifact_cli/src/fs/witness.rs +++ b/noir/noir-repo/tooling/artifact_cli/src/fs/witness.rs @@ -3,14 +3,14 @@ use std::path::{Path, PathBuf}; use acir::{FieldElement, native_types::WitnessStackError}; use acvm::acir::native_types::WitnessStack; -use crate::errors::FilesystemError; +use crate::errors::{CliError, FilesystemError}; /// Write `witness.gz` to the output directory. pub fn save_witness_to_dir( witnesses: WitnessStack, witness_name: &str, witness_dir: &Path, -) -> Result { +) -> Result { std::fs::create_dir_all(witness_dir)?; let witness_path = witness_dir.join(witness_name).with_extension("gz"); diff --git a/noir/noir-repo/tooling/debugger/src/dap.rs b/noir/noir-repo/tooling/debugger/src/dap.rs index 91556aef24c7..1df27d8ea6fd 100644 --- a/noir/noir-repo/tooling/debugger/src/dap.rs +++ b/noir/noir-repo/tooling/debugger/src/dap.rs @@ -466,14 +466,14 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession< } fn map_source_breakpoints(&mut self, args: &SetBreakpointsArguments) -> Vec { - let Some(ref source) = &args.source.path else { + let Some(source) = &args.source.path else { return vec![]; }; let Some(file_id) = self.find_file_id(source) else { eprintln!("WARN: file ID for source {source} not found"); return vec![]; }; - let Some(ref breakpoints) = &args.breakpoints else { + let Some(breakpoints) = &args.breakpoints else { return vec![]; }; let mut breakpoints_to_set: Vec<(DebugLocation, i64)> = vec![]; diff --git a/noir/noir-repo/tooling/debugger/src/foreign_calls.rs b/noir/noir-repo/tooling/debugger/src/foreign_calls.rs index 9c1c481c93f8..efae3df407a2 100644 --- a/noir/noir-repo/tooling/debugger/src/foreign_calls.rs +++ b/noir/noir-repo/tooling/debugger/src/foreign_calls.rs @@ -66,7 +66,7 @@ impl DefaultDebugForeignCallExecutor { pub fn from_artifact<'a>( output: PrintOutput<'a>, artifact: &DebugArtifact, - ) -> impl DebugForeignCallExecutor + 'a { + ) -> impl DebugForeignCallExecutor + use<'a> { let mut ex = Self::default(); ex.load_artifact(artifact); Self::make(output, ex) diff --git a/noir/noir-repo/tooling/inspector/Cargo.toml b/noir/noir-repo/tooling/inspector/Cargo.toml index 2124f7e9a282..d7ab641e4837 100644 --- a/noir/noir-repo/tooling/inspector/Cargo.toml +++ b/noir/noir-repo/tooling/inspector/Cargo.toml @@ -26,3 +26,4 @@ const_format.workspace = true acir.workspace = true noirc_artifacts.workspace = true noirc_artifacts_info.workspace = true +noir_artifact_cli.workspace = true diff --git a/noir/noir-repo/tooling/inspector/src/cli/info_cmd.rs b/noir/noir-repo/tooling/inspector/src/cli/info_cmd.rs index 9780b56efcad..b221042c79ff 100644 --- a/noir/noir-repo/tooling/inspector/src/cli/info_cmd.rs +++ b/noir/noir-repo/tooling/inspector/src/cli/info_cmd.rs @@ -2,6 +2,7 @@ use std::path::PathBuf; use clap::Args; use color_eyre::eyre; +use noir_artifact_cli::Artifact; use noirc_artifacts::program::ProgramArtifact; use noirc_artifacts_info::{InfoReport, count_opcodes_and_gates_in_program, show_info_report}; @@ -13,22 +14,49 @@ pub(crate) struct InfoCommand { /// Output a JSON formatted report. Changes to this format are not currently considered breaking. #[clap(long, hide = true)] json: bool, + + /// Name of the function to print, if the artifact is a contract. + #[clap(long)] + contract_fn: Option, } pub(crate) fn run(args: InfoCommand) -> eyre::Result<()> { - let file = std::fs::File::open(args.artifact.clone())?; - let artifact: ProgramArtifact = serde_json::from_reader(file)?; - - let package_name = args - .artifact - .with_extension("") - .file_name() - .map(|s| s.to_string_lossy().to_string()) - .unwrap_or_else(|| "artifact".to_string()); - - let program_info = count_opcodes_and_gates_in_program(artifact, package_name.to_string(), None); - - let info_report = InfoReport { programs: vec![program_info] }; + let artifact = Artifact::read_from_file(&args.artifact)?; + + let programs = match artifact { + Artifact::Program(program) => { + let package_name = args + .artifact + .with_extension("") + .file_name() + .map(|s| s.to_string_lossy().to_string()) + .unwrap_or_else(|| "artifact".to_string()); + + vec![count_opcodes_and_gates_in_program(program, package_name, None)] + } + Artifact::Contract(contract) => contract + .functions + .into_iter() + .filter(|f| args.contract_fn.as_ref().map(|n| *n == f.name).unwrap_or(true)) + .map(|f| { + // We have to cheat to be able to call `count_opcodes_and_gates_in_program`. + let package_name = format!("{}::{}", contract.name, f.name); + let program = ProgramArtifact { + noir_version: contract.noir_version.clone(), + hash: f.hash, + abi: f.abi, + bytecode: f.bytecode, + debug_symbols: f.debug_symbols, + file_map: contract.file_map.clone(), + names: f.names, + brillig_names: f.brillig_names, + }; + count_opcodes_and_gates_in_program(program, package_name, None) + }) + .collect::>(), + }; + + let info_report = InfoReport { programs }; show_info_report(info_report, args.json); Ok(()) diff --git a/noir/noir-repo/tooling/inspector/src/cli/print_acir_cmd.rs b/noir/noir-repo/tooling/inspector/src/cli/print_acir_cmd.rs index f3dfe528973f..c3580a715d88 100644 --- a/noir/noir-repo/tooling/inspector/src/cli/print_acir_cmd.rs +++ b/noir/noir-repo/tooling/inspector/src/cli/print_acir_cmd.rs @@ -2,20 +2,38 @@ use std::path::PathBuf; use clap::Args; use color_eyre::eyre; -use noirc_artifacts::program::ProgramArtifact; +use noir_artifact_cli::Artifact; #[derive(Debug, Clone, Args)] pub(crate) struct PrintAcirCommand { /// The artifact to print artifact: PathBuf, + + /// Name of the function to print, if the artifact is a contract. + #[clap(long)] + contract_fn: Option, } pub(crate) fn run(args: PrintAcirCommand) -> eyre::Result<()> { - let file = std::fs::File::open(args.artifact.clone())?; - let artifact: ProgramArtifact = serde_json::from_reader(file)?; + let artifact = Artifact::read_from_file(&args.artifact)?; - println!("Compiled ACIR for main:"); - println!("{}", artifact.bytecode); + match artifact { + Artifact::Program(program) => { + println!("Compiled ACIR for main:"); + println!("{}", program.bytecode); + } + Artifact::Contract(contract) => { + println!("Compiled circuits for contract '{}':", contract.name); + for function in contract + .functions + .into_iter() + .filter(|f| args.contract_fn.as_ref().map(|n| *n == f.name).unwrap_or(true)) + { + println!("Compiled ACIR for function '{}':", function.name); + println!("{}", function.bytecode); + } + } + } Ok(()) } diff --git a/noir/noir-repo/tooling/lsp/src/requests/code_action.rs b/noir/noir-repo/tooling/lsp/src/requests/code_action.rs index 53844b4b3535..6d74ce2d1f96 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/code_action.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/code_action.rs @@ -44,7 +44,7 @@ mod tests; pub(crate) fn on_code_action_request( state: &mut LspState, params: CodeActionParams, -) -> impl Future, ResponseError>> { +) -> impl Future, ResponseError>> + use<> { let uri = params.text_document.clone().uri; let position = params.range.start; let text_document_position_params = diff --git a/noir/noir-repo/tooling/lsp/src/requests/code_lens_request.rs b/noir/noir-repo/tooling/lsp/src/requests/code_lens_request.rs index 09117a653013..1870e8e06024 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/code_lens_request.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/code_lens_request.rs @@ -39,7 +39,7 @@ fn package_selection_args(workspace: &Workspace, package: &Package) -> Vec impl Future> { +) -> impl Future> + use<> { future::ready(on_code_lens_request_inner(state, params)) } diff --git a/noir/noir-repo/tooling/lsp/src/requests/completion.rs b/noir/noir-repo/tooling/lsp/src/requests/completion.rs index f07cb20303e9..eab8522693f8 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/completion.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/completion.rs @@ -57,7 +57,7 @@ mod tests; pub(crate) fn on_completion_request( state: &mut LspState, params: CompletionParams, -) -> impl Future, ResponseError>> { +) -> impl Future, ResponseError>> + use<> { let uri = params.text_document_position.clone().text_document.uri; let result = process_request(state, params.text_document_position.clone(), |args| { @@ -608,7 +608,7 @@ impl<'a> NodeFinder<'a> { self.complete_tuple_fields(types, self_prefix); } Type::TypeVariable(var) | Type::NamedGeneric(var, _) => { - if let TypeBinding::Bound(ref typ) = &*var.borrow() { + if let TypeBinding::Bound(typ) = &*var.borrow() { return self.complete_type_fields_and_methods( typ, prefix, @@ -1906,13 +1906,10 @@ fn get_field_type(typ: &Type, name: &str) -> Option { } } Type::Alias(alias_type, generics) => Some(alias_type.borrow().get_type(generics)), - Type::TypeVariable(var) | Type::NamedGeneric(var, _) => { - if let TypeBinding::Bound(ref typ) = &*var.borrow() { - get_field_type(typ, name) - } else { - None - } - } + Type::TypeVariable(var) | Type::NamedGeneric(var, _) => match &*var.borrow() { + TypeBinding::Bound(typ) => get_field_type(typ, name), + _ => None, + }, _ => None, } } @@ -1924,13 +1921,10 @@ fn get_array_element_type(typ: Type) -> Option { let typ = alias_type.borrow().get_type(&generics); get_array_element_type(typ) } - Type::TypeVariable(var) | Type::NamedGeneric(var, _) => { - if let TypeBinding::Bound(typ) = &*var.borrow() { - get_array_element_type(typ.clone()) - } else { - None - } - } + Type::TypeVariable(var) | Type::NamedGeneric(var, _) => match &*var.borrow() { + TypeBinding::Bound(typ) => get_array_element_type(typ.clone()), + _ => None, + }, _ => None, } } diff --git a/noir/noir-repo/tooling/lsp/src/requests/document_symbol.rs b/noir/noir-repo/tooling/lsp/src/requests/document_symbol.rs index b44c3ef32041..1d36aabdbe74 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/document_symbol.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/document_symbol.rs @@ -23,7 +23,7 @@ use super::process_request; pub(crate) fn on_document_symbol_request( state: &mut LspState, params: DocumentSymbolParams, -) -> impl Future, ResponseError>> { +) -> impl Future, ResponseError>> + use<> { let Ok(file_path) = params.text_document.uri.to_file_path() else { return future::ready(Ok(None)); }; diff --git a/noir/noir-repo/tooling/lsp/src/requests/goto_declaration.rs b/noir/noir-repo/tooling/lsp/src/requests/goto_declaration.rs index eb5ec5fcea53..9df6a25f8cf1 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/goto_declaration.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/goto_declaration.rs @@ -11,7 +11,7 @@ use super::{process_request, to_lsp_location}; pub(crate) fn on_goto_declaration_request( state: &mut LspState, params: GotoDeclarationParams, -) -> impl Future> { +) -> impl Future> + use<> { let result = on_goto_definition_inner(state, params); future::ready(result) } diff --git a/noir/noir-repo/tooling/lsp/src/requests/goto_definition.rs b/noir/noir-repo/tooling/lsp/src/requests/goto_definition.rs index bd6fb1e75cbd..4e015e948610 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/goto_definition.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/goto_definition.rs @@ -14,7 +14,7 @@ use super::{process_request, to_lsp_location}; pub(crate) fn on_goto_definition_request( state: &mut LspState, params: GotoDefinitionParams, -) -> impl Future> { +) -> impl Future> + use<> { let result = on_goto_definition_inner(state, params, false); future::ready(result) } @@ -22,7 +22,7 @@ pub(crate) fn on_goto_definition_request( pub(crate) fn on_goto_type_definition_request( state: &mut LspState, params: GotoTypeDefinitionParams, -) -> impl Future> { +) -> impl Future> + use<> { let result = on_goto_definition_inner(state, params, true); future::ready(result) } diff --git a/noir/noir-repo/tooling/lsp/src/requests/hover.rs b/noir/noir-repo/tooling/lsp/src/requests/hover.rs index cbb7dafd3c51..0b53b56eaf91 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/hover.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/hover.rs @@ -16,7 +16,7 @@ mod from_visitor; pub(crate) fn on_hover_request( state: &mut LspState, params: HoverParams, -) -> impl Future, ResponseError>> { +) -> impl Future, ResponseError>> + use<> { let uri = params.text_document_position_params.text_document.uri.clone(); let position = params.text_document_position_params.position; let result = process_request(state, params.text_document_position_params, |args| { diff --git a/noir/noir-repo/tooling/lsp/src/requests/inlay_hint.rs b/noir/noir-repo/tooling/lsp/src/requests/inlay_hint.rs index 755cd3cbf177..be7722e744d2 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/inlay_hint.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/inlay_hint.rs @@ -26,7 +26,7 @@ use super::{InlayHintsOptions, process_request, to_lsp_location}; pub(crate) fn on_inlay_hint_request( state: &mut LspState, params: InlayHintParams, -) -> impl Future>, ResponseError>> { +) -> impl Future>, ResponseError>> + use<> { let text_document_position_params = TextDocumentPositionParams { text_document: params.text_document.clone(), position: Position { line: 0, character: 0 }, @@ -515,18 +515,17 @@ fn push_type_parts(typ: &Type, parts: &mut Vec, files: &File parts.push(string_part("&mut ")); push_type_parts(typ, parts, files); } - Type::TypeVariable(binding) => { - if let TypeBinding::Unbound(_, kind) = &*binding.borrow() { - match kind { - Kind::Any | Kind::Normal => push_type_variable_parts(binding, parts, files), - Kind::Integer => push_type_parts(&Type::default_int_type(), parts, files), - Kind::IntegerOrField => parts.push(string_part("Field")), - Kind::Numeric(ref typ) => push_type_parts(typ, parts, files), - } - } else { + Type::TypeVariable(binding) => match &*binding.borrow() { + TypeBinding::Unbound(_, kind) => match kind { + Kind::Any | Kind::Normal => push_type_variable_parts(binding, parts, files), + Kind::Integer => push_type_parts(&Type::default_int_type(), parts, files), + Kind::IntegerOrField => parts.push(string_part("Field")), + Kind::Numeric(typ) => push_type_parts(typ, parts, files), + }, + _ => { push_type_variable_parts(binding, parts, files); } - } + }, Type::CheckedCast { to, .. } => push_type_parts(to, parts, files), Type::FieldElement diff --git a/noir/noir-repo/tooling/lsp/src/requests/mod.rs b/noir/noir-repo/tooling/lsp/src/requests/mod.rs index 80146d5fe132..9126ab38e103 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/mod.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/mod.rs @@ -191,7 +191,7 @@ impl Default for LspInitializationOptions { pub(crate) fn on_initialize( state: &mut LspState, params: InitializeParams, -) -> impl Future> { +) -> impl Future> + use<> { state.root_path = params.root_uri.and_then(|root_uri| root_uri.to_file_path().ok()); let initialization_options: LspInitializationOptions = params .initialization_options @@ -293,7 +293,7 @@ pub(crate) fn on_initialize( pub(crate) fn on_formatting( state: &mut LspState, params: lsp_types::DocumentFormattingParams, -) -> impl Future>, ResponseError>> { +) -> impl Future>, ResponseError>> + use<> { std::future::ready(on_formatting_inner(state, params)) } @@ -441,7 +441,7 @@ where pub(crate) fn on_shutdown( _state: &mut LspState, _params: (), -) -> impl Future> { +) -> impl Future> + use<> { async { Ok(()) } } diff --git a/noir/noir-repo/tooling/lsp/src/requests/references.rs b/noir/noir-repo/tooling/lsp/src/requests/references.rs index 3b4ef12b5b74..fbe69c99871d 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/references.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/references.rs @@ -10,7 +10,7 @@ use super::{find_all_references_in_workspace, process_request}; pub(crate) fn on_references_request( state: &mut LspState, params: ReferenceParams, -) -> impl Future>, ResponseError>> { +) -> impl Future>, ResponseError>> + use<> { let include_declaration = params.context.include_declaration; let result = process_request(state, params.text_document_position, |args| { find_all_references_in_workspace( diff --git a/noir/noir-repo/tooling/lsp/src/requests/rename.rs b/noir/noir-repo/tooling/lsp/src/requests/rename.rs index 01181fc60f4c..00e39451caea 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/rename.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/rename.rs @@ -16,7 +16,7 @@ use super::{find_all_references_in_workspace, process_request}; pub(crate) fn on_prepare_rename_request( state: &mut LspState, params: TextDocumentPositionParams, -) -> impl Future, ResponseError>> { +) -> impl Future, ResponseError>> + use<> { let result = process_request(state, params, |args| { let reference_id = args.interner.reference_at_location(args.location); let rename_possible = match reference_id { @@ -33,7 +33,7 @@ pub(crate) fn on_prepare_rename_request( pub(crate) fn on_rename_request( state: &mut LspState, params: RenameParams, -) -> impl Future, ResponseError>> { +) -> impl Future, ResponseError>> + use<> { let result = process_request(state, params.text_document_position, |args| { let rename_changes = find_all_references_in_workspace( args.location, diff --git a/noir/noir-repo/tooling/lsp/src/requests/signature_help.rs b/noir/noir-repo/tooling/lsp/src/requests/signature_help.rs index 2c901beb675e..1709106e1218 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/signature_help.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/signature_help.rs @@ -26,7 +26,7 @@ mod tests; pub(crate) fn on_signature_help_request( state: &mut LspState, params: SignatureHelpParams, -) -> impl Future, ResponseError>> { +) -> impl Future, ResponseError>> + use<> { let uri = params.text_document_position_params.clone().text_document.uri; let result = process_request(state, params.text_document_position_params.clone(), |args| { diff --git a/noir/noir-repo/tooling/lsp/src/requests/test_run.rs b/noir/noir-repo/tooling/lsp/src/requests/test_run.rs index 30a9d87350c2..f0d49c4d8640 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/test_run.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/test_run.rs @@ -19,7 +19,7 @@ use crate::{ pub(crate) fn on_test_run_request( state: &mut LspState, params: NargoTestRunParams, -) -> impl Future> { +) -> impl Future> + use<> { future::ready(on_test_run_request_inner(state, params)) } diff --git a/noir/noir-repo/tooling/lsp/src/requests/tests.rs b/noir/noir-repo/tooling/lsp/src/requests/tests.rs index 4590d1e602c0..234bd971eb5d 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/tests.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/tests.rs @@ -14,7 +14,7 @@ use crate::{ pub(crate) fn on_tests_request( state: &mut LspState, params: NargoTestsParams, -) -> impl Future> { +) -> impl Future> + use<> { future::ready(on_tests_request_inner(state, params)) } diff --git a/noir/noir-repo/tooling/lsp/src/trait_impl_method_stub_generator.rs b/noir/noir-repo/tooling/lsp/src/trait_impl_method_stub_generator.rs index c57de92b7a35..3a60a882aeab 100644 --- a/noir/noir-repo/tooling/lsp/src/trait_impl_method_stub_generator.rs +++ b/noir/noir-repo/tooling/lsp/src/trait_impl_method_stub_generator.rs @@ -447,7 +447,7 @@ impl<'a> TraitImplMethodStubGenerator<'a> { Kind::Any | Kind::Normal | Kind::Integer | Kind::IntegerOrField => { self.string.push_str(&generic.name); } - Kind::Numeric(ref typ) => { + Kind::Numeric(typ) => { self.string.push_str("let "); self.string.push_str(&generic.name); self.string.push_str(": "); diff --git a/noir/noir-repo/tooling/nargo/src/foreign_calls/default.rs b/noir/noir-repo/tooling/nargo/src/foreign_calls/default.rs index 1007aacf897b..c6053a1175e1 100644 --- a/noir/noir-repo/tooling/nargo/src/foreign_calls/default.rs +++ b/noir/noir-repo/tooling/nargo/src/foreign_calls/default.rs @@ -80,7 +80,7 @@ impl<'a> DefaultForeignCallBuilder<'a> { use rand::Rng; base.add_layer(self.resolver_url.map(|resolver_url| { - let id = rand::thread_rng().gen(); + let id = rand::thread_rng().r#gen(); RPCForeignCallExecutor::new( &resolver_url, id, @@ -136,7 +136,7 @@ impl DefaultForeignCallExecutor { resolver_url: Option<&str>, root_path: Option, package_name: Option, - ) -> impl ForeignCallExecutor + 'a + ) -> impl ForeignCallExecutor + 'a + use<'a, F> where F: AcirField + Serialize + for<'de> Deserialize<'de> + 'a, { diff --git a/noir/noir-repo/tooling/nargo_cli/Cargo.toml b/noir/noir-repo/tooling/nargo_cli/Cargo.toml index 92eeed1b3911..5aa37cffcda0 100644 --- a/noir/noir-repo/tooling/nargo_cli/Cargo.toml +++ b/noir/noir-repo/tooling/nargo_cli/Cargo.toml @@ -37,6 +37,7 @@ nargo = { workspace = true, features = ["rpc"] } nargo_fmt.workspace = true nargo_toml.workspace = true noir_lsp.workspace = true +noir_artifact_cli.workspace = true noir_debugger.workspace = true noirc_driver = { workspace = true, features = ["bn254"] } noirc_frontend = { workspace = true, features = ["bn254"] } diff --git a/noir/noir-repo/tooling/nargo_cli/benches/criterion.rs b/noir/noir-repo/tooling/nargo_cli/benches/criterion.rs index b42d71a263e7..22c1107c71ca 100644 --- a/noir/noir-repo/tooling/nargo_cli/benches/criterion.rs +++ b/noir/noir-repo/tooling/nargo_cli/benches/criterion.rs @@ -3,16 +3,12 @@ use acvm::{FieldElement, acir::native_types::WitnessMap}; use assert_cmd::prelude::{CommandCargoExt, OutputAssertExt}; use criterion::{Criterion, criterion_group, criterion_main}; -use noirc_abi::{ - Abi, InputMap, - input_parser::{Format, InputValue}, -}; -use noirc_artifacts::program::ProgramArtifact; +use noir_artifact_cli::fs::{artifact::read_program_from_file, inputs::read_inputs_from_file}; use noirc_driver::CompiledProgram; use pprof::criterion::{Output, PProfProfiler}; +use std::cell::RefCell; use std::hint::black_box; use std::path::Path; -use std::{cell::RefCell, collections::BTreeMap}; use std::{process::Command, time::Duration}; include!("./utils.rs"); @@ -50,14 +46,14 @@ fn read_compiled_programs_and_inputs( for package in binary_packages { let program_artifact_path = workspace.package_build_path(package); - let program: CompiledProgram = read_program_from_file(&program_artifact_path).into(); + let program: CompiledProgram = + read_program_from_file(&program_artifact_path).unwrap().into(); let (inputs, _) = read_inputs_from_file( - &package.root_dir, - nargo::constants::PROVER_INPUT_FILE, - Format::Toml, + &package.root_dir.join(nargo::constants::PROVER_INPUT_FILE).with_extension("toml"), &program.abi, - ); + ) + .expect("failed to read input"); let initial_witness = program.abi.encode(&inputs, None).expect("failed to encode input witness"); @@ -67,40 +63,6 @@ fn read_compiled_programs_and_inputs( programs } -/// Read the bytecode and ABI from the compilation output -fn read_program_from_file(circuit_path: &Path) -> ProgramArtifact { - let file_path = circuit_path.with_extension("json"); - let input_string = std::fs::read(file_path).expect("failed to read artifact file"); - serde_json::from_slice(&input_string).expect("failed to deserialize artifact") -} - -/// Read the inputs from Prover.toml -fn read_inputs_from_file( - path: &Path, - file_name: &str, - format: Format, - abi: &Abi, -) -> (InputMap, Option) { - if abi.is_empty() { - return (BTreeMap::new(), None); - } - - let file_path = path.join(file_name).with_extension(format.ext()); - if !file_path.exists() { - if abi.parameters.is_empty() { - return (BTreeMap::new(), None); - } else { - panic!("input file doesn't exist: {}", file_path.display()); - } - } - - let input_string = std::fs::read_to_string(file_path).expect("failed to read input file"); - let mut input_map = format.parse(&input_string, abi).expect("failed to parse input"); - let return_value = input_map.remove(noirc_abi::MAIN_RETURN_NAME); - - (input_map, return_value) -} - /// Use the nargo CLI to compile a test program, then benchmark its execution /// by executing the command directly from the benchmark, so that we can have /// meaningful flamegraphs about the ACVM. diff --git a/noir/noir-repo/tooling/nargo_cli/benches/iai.rs b/noir/noir-repo/tooling/nargo_cli/benches/iai.rs index bcd60111ccf6..c0526e9a7e2f 100644 --- a/noir/noir-repo/tooling/nargo_cli/benches/iai.rs +++ b/noir/noir-repo/tooling/nargo_cli/benches/iai.rs @@ -6,7 +6,7 @@ use std::process::Command; include!("./utils.rs"); macro_rules! iai_command { - ($command_name:tt, $command_string:expr) => { + ($command_name:tt, $command_string:expr_2021) => { paste! { fn []() { let test_program_dirs = get_selected_tests(); diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/check_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/check_cmd.rs index 27a0e6d86ec6..2247f40f1817 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/check_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/check_cmd.rs @@ -8,6 +8,7 @@ use nargo::{ package::Package, parse_all, prepare_package, workspace::Workspace, }; use nargo_toml::PackageSelection; +use noir_artifact_cli::fs::artifact::write_to_file; use noirc_abi::{AbiParameter, AbiType, MAIN_RETURN_NAME}; use noirc_driver::{CompileOptions, CrateId, check_crate, compute_function_abi}; use noirc_frontend::{ @@ -15,8 +16,7 @@ use noirc_frontend::{ monomorphization::monomorphize, }; -use super::{LockType, WorkspaceCommand}; -use super::{PackageOptions, fs::write_to_file}; +use super::{LockType, PackageOptions, WorkspaceCommand}; /// Check a local package and all of its dependencies for errors #[derive(Debug, Clone, Args)] @@ -103,7 +103,8 @@ fn check_package( if should_write_prover { let prover_toml = create_input_toml_template(parameters.clone(), None); - write_to_file(prover_toml.as_bytes(), &path_to_prover_input); + write_to_file(prover_toml.as_bytes(), &path_to_prover_input) + .expect("failed to write template"); } else { eprintln!("Note: Prover.toml already exists. Use --overwrite to force overwrite."); } diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/compile_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/compile_cmd.rs index af07c2d5fcdc..2ee19f5edc01 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -9,6 +9,9 @@ use nargo::package::Package; use nargo::workspace::Workspace; use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; use nargo_toml::PackageSelection; +use noir_artifact_cli::fs::artifact::{ + read_program_from_file, save_contract_to_file, save_program_to_file, +}; use noirc_driver::DEFAULT_EXPRESSION_WIDTH; use noirc_driver::NOIR_ARTIFACT_VERSION_STRING; use noirc_driver::{CompilationResult, CompileOptions, CompiledContract}; @@ -20,7 +23,6 @@ use notify_debouncer_full::new_debouncer; use crate::errors::CliError; -use super::fs::program::{read_program_from_file, save_contract_to_file, save_program_to_file}; use super::{LockType, PackageOptions, WorkspaceCommand}; use rayon::prelude::*; @@ -181,7 +183,7 @@ fn compile_programs( // The loaded circuit includes backend specific transformations, which might be different from the current target. let load_cached_program = |package| { let program_artifact_path = workspace.package_build_path(package); - read_program_from_file(program_artifact_path) + read_program_from_file(&program_artifact_path) .ok() .filter(|p| p.noir_version == NOIR_ARTIFACT_VERSION_STRING) .map(|p| p.into()) @@ -241,7 +243,8 @@ fn compile_programs( // Check solvability. nargo::ops::check_program(&program)?; // Overwrite the build artifacts with the final circuit, which includes the backend specific transformations. - save_program_to_file(&program.into(), &package.name, workspace.target_directory_path()); + save_program_to_file(&program.into(), &package.name, &workspace.target_directory_path()) + .expect("failed to save program"); Ok(((), warnings)) }; @@ -292,7 +295,8 @@ fn save_contract( &contract.into(), &format!("{}-{}", package.name, contract_name), target_dir, - ); + ) + .expect("failed to save contract"); if show_artifact_paths { println!("Saved contract artifact to: {}", artifact_path.display()); } @@ -321,6 +325,7 @@ mod tests { use nargo::ops::compile_program; use nargo_toml::PackageSelection; use noirc_driver::{CompileOptions, CrateName}; + use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; use crate::cli::test_cmd::formatters::diagnostic_to_string; use crate::cli::{ @@ -343,7 +348,7 @@ mod tests { fn read_test_program_dirs( test_programs_dir: &Path, test_sub_dir: &str, - ) -> impl Iterator { + ) -> impl Iterator + use<> { let test_case_dir = test_programs_dir.join(test_sub_dir); std::fs::read_dir(test_case_dir) .unwrap() @@ -393,7 +398,7 @@ mod tests { assert!(!test_workspaces.is_empty(), "should find some test workspaces"); - test_workspaces.iter().for_each(|workspace| { + test_workspaces.par_iter().for_each(|workspace| { let (file_manager, parsed_files) = parse_workspace(workspace); let binary_packages = workspace.into_iter().filter(|package| package.is_binary()); diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/dap_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/dap_cmd.rs index 853558532c4c..8987ed80d3e6 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/dap_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/dap_cmd.rs @@ -6,7 +6,7 @@ use clap::Args; use nargo::constants::PROVER_INPUT_FILE; use nargo::workspace::Workspace; use nargo_toml::{PackageSelection, get_package_manifest, resolve_workspace_from_toml}; -use noirc_abi::input_parser::Format; +use noir_artifact_cli::fs::inputs::read_inputs_from_file; use noirc_driver::{CompileOptions, CompiledProgram, NOIR_ARTIFACT_VERSION_STRING}; use noirc_frontend::graph::CrateName; @@ -20,7 +20,6 @@ use dap::types::Capabilities; use serde_json::Value; use super::debug_cmd::compile_bin_package_for_debugging; -use super::fs::inputs::read_inputs_from_file; use crate::errors::CliError; use noir_debugger::errors::{DapError, LoadError}; @@ -125,11 +124,13 @@ fn load_and_compile_project( let compiled_program = nargo::ops::transform_program(compiled_program, expression_width); - let (inputs_map, _) = - read_inputs_from_file(&package.root_dir, prover_name, Format::Toml, &compiled_program.abi) - .map_err(|_| { - LoadError::Generic(format!("Failed to read program inputs from {}", prover_name)) - })?; + let (inputs_map, _) = read_inputs_from_file( + &package.root_dir.join(prover_name).with_extension("toml"), + &compiled_program.abi, + ) + .map_err(|e| { + LoadError::Generic(format!("Failed to read program inputs from {prover_name}: {e}")) + })?; let initial_witness = compiled_program .abi .encode(&inputs_map, None) @@ -160,7 +161,7 @@ fn loop_uninitialized_dap( server.respond(req.error("Missing launch arguments"))?; continue; }; - let Some(Value::String(ref project_folder)) = additional_data.get("projectFolder") + let Some(Value::String(project_folder)) = additional_data.get("projectFolder") else { server.respond(req.error("Missing project folder argument"))?; continue; diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/debug_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/debug_cmd.rs index 2b40c33b1bce..f12e83297e66 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/debug_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/debug_cmd.rs @@ -1,4 +1,4 @@ -use std::path::PathBuf; +use std::path::Path; use acvm::FieldElement; use acvm::acir::native_types::WitnessStack; @@ -13,14 +13,15 @@ use nargo::package::{CrateName, Package}; use nargo::workspace::Workspace; use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; use nargo_toml::PackageSelection; +use noir_artifact_cli::fs::inputs::read_inputs_from_file; +use noir_artifact_cli::fs::witness::save_witness_to_dir; use noirc_abi::InputMap; -use noirc_abi::input_parser::{Format, InputValue}; +use noirc_abi::input_parser::InputValue; use noirc_driver::{CompileOptions, CompiledProgram, file_manager_with_stdlib}; use noirc_frontend::debug::DebugInstrumenter; use noirc_frontend::hir::ParsedFiles; use super::compile_cmd::get_target_width; -use super::fs::{inputs::read_inputs_from_file, witness::save_witness_to_dir}; use super::{LockType, WorkspaceCommand}; use crate::errors::CliError; @@ -112,7 +113,7 @@ pub(crate) fn compile_bin_package_for_debugging( skip_instrumentation: bool, compile_options: CompileOptions, ) -> Result { - let mut workspace_file_manager = file_manager_with_stdlib(std::path::Path::new("")); + let mut workspace_file_manager = file_manager_with_stdlib(Path::new("")); insert_all_files_for_workspace_into_file_manager(workspace, &mut workspace_file_manager); let mut parsed_files = parse_all(&workspace_file_manager); @@ -188,7 +189,7 @@ fn run_async( program: CompiledProgram, prover_name: &str, witness_name: &Option, - target_dir: &PathBuf, + target_dir: &Path, pedantic_solving: bool, raw_source_printing: bool, ) -> Result<(), CliError> { @@ -234,8 +235,10 @@ fn debug_program_and_decode( raw_source_printing: bool, ) -> Result<(Option, Option>), CliError> { // Parse the initial witness values from Prover.toml - let (inputs_map, _) = - read_inputs_from_file(&package.root_dir, prover_name, Format::Toml, &program.abi)?; + let (inputs_map, _) = read_inputs_from_file( + &package.root_dir.join(prover_name).with_extension("toml"), + &program.abi, + )?; let program_abi = program.abi.clone(); let witness_stack = debug_program(program, &inputs_map, pedantic_solving, raw_source_printing)?; diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/execute_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/execute_cmd.rs index d0001b4d0043..c7c3227795de 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -12,15 +12,16 @@ use nargo::foreign_calls::DefaultForeignCallBuilder; use nargo::package::Package; use nargo::workspace::Workspace; use nargo_toml::PackageSelection; +use noir_artifact_cli::fs::artifact::read_program_from_file; +use noir_artifact_cli::fs::inputs::read_inputs_from_file; +use noir_artifact_cli::fs::witness::save_witness_to_dir; use noirc_abi::InputMap; -use noirc_abi::input_parser::{Format, InputValue}; +use noirc_abi::input_parser::InputValue; use noirc_artifacts::debug::DebugArtifact; use noirc_driver::{CompileOptions, CompiledProgram}; use super::compile_cmd::compile_workspace_full; -use super::fs::{inputs::read_inputs_from_file, witness::save_witness_to_dir}; use super::{LockType, PackageOptions, WorkspaceCommand}; -use crate::cli::fs::program::read_program_from_file; use crate::errors::CliError; /// Executes a circuit to calculate its return value @@ -67,8 +68,7 @@ pub(crate) fn run(args: ExecuteCommand, workspace: Workspace) -> Result<(), CliE let binary_packages = workspace.into_iter().filter(|package| package.is_binary()); for package in binary_packages { let program_artifact_path = workspace.package_build_path(package); - let program: CompiledProgram = - read_program_from_file(program_artifact_path.clone())?.into(); + let program: CompiledProgram = read_program_from_file(&program_artifact_path)?.into(); let abi = program.abi.clone(); let results = execute_program_and_decode( @@ -117,8 +117,10 @@ fn execute_program_and_decode( pedantic_solving: bool, ) -> Result { // Parse the initial witness values from Prover.toml - let (inputs_map, expected_return) = - read_inputs_from_file(&package.root_dir, prover_name, Format::Toml, &program.abi)?; + let (inputs_map, expected_return) = read_inputs_from_file( + &package.root_dir.join(prover_name).with_extension("toml"), + &program.abi, + )?; let witness_stack = execute_program( &program, &inputs_map, diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/export_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/export_cmd.rs index 270f71565e0c..9fe682bef7ff 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/export_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/export_cmd.rs @@ -1,5 +1,6 @@ use nargo::errors::CompileError; use nargo::ops::report_errors; +use noir_artifact_cli::fs::artifact::save_program_to_file; use noirc_errors::FileDiagnostic; use noirc_frontend::hir::ParsedFiles; use rayon::prelude::*; @@ -19,7 +20,6 @@ use crate::errors::CliError; use super::check_cmd::check_crate_and_report_errors; -use super::fs::program::save_program_to_file; use super::{LockType, PackageOptions, WorkspaceCommand}; /// Exports functions marked with #[export] attribute @@ -97,7 +97,7 @@ fn compile_exported_functions( let export_dir = workspace.export_directory_path(); for (function_name, program) in exported_programs { - save_program_to_file(&program.into(), &function_name.parse().unwrap(), &export_dir); + save_program_to_file(&program.into(), &function_name.parse().unwrap(), &export_dir)?; } Ok(()) } diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/fs/inputs.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/fs/inputs.rs deleted file mode 100644 index 54b12ceb5f06..000000000000 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/fs/inputs.rs +++ /dev/null @@ -1,42 +0,0 @@ -use noirc_abi::{ - Abi, InputMap, MAIN_RETURN_NAME, - input_parser::{Format, InputValue}, -}; -use std::{collections::BTreeMap, path::Path}; - -use crate::errors::FilesystemError; - -/// Returns the circuit's parameters and its return value, if one exists. -/// # Examples -/// -/// ```ignore -/// let (input_map, return_value): (InputMap, Option) = -/// read_inputs_from_file(path, "Verifier", Format::Toml, &abi)?; -/// ``` -pub(crate) fn read_inputs_from_file>( - path: P, - file_name: &str, - format: Format, - abi: &Abi, -) -> Result<(InputMap, Option), FilesystemError> { - if abi.is_empty() { - return Ok((BTreeMap::new(), None)); - } - - let file_path = path.as_ref().join(file_name).with_extension(format.ext()); - if !file_path.exists() { - if abi.parameters.is_empty() { - // Reading a return value from the `Prover.toml` is optional, - // so if the ABI has no parameters we can skip reading the file if it doesn't exist. - return Ok((BTreeMap::new(), None)); - } else { - return Err(FilesystemError::MissingTomlFile(file_name.to_owned(), file_path)); - } - } - - let input_string = std::fs::read_to_string(file_path).unwrap(); - let mut input_map = format.parse(&input_string, abi)?; - let return_value = input_map.remove(MAIN_RETURN_NAME); - - Ok((input_map, return_value)) -} diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/fs/mod.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/fs/mod.rs deleted file mode 100644 index 8658bd5b2482..000000000000 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/fs/mod.rs +++ /dev/null @@ -1,30 +0,0 @@ -use std::{ - fs::File, - io::Write, - path::{Path, PathBuf}, -}; - -pub(super) mod inputs; -pub(super) mod program; -pub(super) mod witness; - -pub(super) fn create_named_dir(named_dir: &Path, name: &str) -> PathBuf { - std::fs::create_dir_all(named_dir) - .unwrap_or_else(|_| panic!("could not create the `{name}` directory")); - - PathBuf::from(named_dir) -} - -pub(super) fn write_to_file(bytes: &[u8], path: &Path) -> String { - let display = path.display(); - - let mut file = match File::create(path) { - Err(why) => panic!("couldn't create {display}: {why}"), - Ok(file) => file, - }; - - match file.write_all(bytes) { - Err(why) => panic!("couldn't write to {display}: {why}"), - Ok(_) => display.to_string(), - } -} diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/fs/program.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/fs/program.rs deleted file mode 100644 index 87783e4573a9..000000000000 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/fs/program.rs +++ /dev/null @@ -1,51 +0,0 @@ -use std::path::{Path, PathBuf}; - -use nargo::package::CrateName; -use noirc_artifacts::{contract::ContractArtifact, program::ProgramArtifact}; - -use crate::errors::FilesystemError; - -use super::{create_named_dir, write_to_file}; - -pub(crate) fn save_program_to_file>( - program_artifact: &ProgramArtifact, - crate_name: &CrateName, - circuit_dir: P, -) -> PathBuf { - let circuit_name: String = crate_name.into(); - save_build_artifact_to_file(program_artifact, &circuit_name, circuit_dir) -} - -pub(crate) fn save_contract_to_file>( - compiled_contract: &ContractArtifact, - circuit_name: &str, - circuit_dir: P, -) -> PathBuf { - save_build_artifact_to_file(compiled_contract, circuit_name, circuit_dir) -} - -fn save_build_artifact_to_file, T: ?Sized + serde::Serialize>( - build_artifact: &T, - artifact_name: &str, - circuit_dir: P, -) -> PathBuf { - create_named_dir(circuit_dir.as_ref(), "target"); - let circuit_path = circuit_dir.as_ref().join(artifact_name).with_extension("json"); - write_to_file(&serde_json::to_vec(build_artifact).unwrap(), &circuit_path); - - circuit_path -} - -#[tracing::instrument(level = "trace", skip_all)] -pub(crate) fn read_program_from_file>( - circuit_path: P, -) -> Result { - let file_path = circuit_path.as_ref().with_extension("json"); - - let input_string = - std::fs::read(&file_path).map_err(|_| FilesystemError::PathNotValid(file_path))?; - let program = serde_json::from_slice(&input_string) - .map_err(|err| FilesystemError::ProgramSerializationError(err.to_string()))?; - - Ok(program) -} diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/fs/witness.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/fs/witness.rs deleted file mode 100644 index c9905261299f..000000000000 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/fs/witness.rs +++ /dev/null @@ -1,22 +0,0 @@ -use std::path::{Path, PathBuf}; - -use acvm::{FieldElement, acir::native_types::WitnessStack}; -use nargo::constants::WITNESS_EXT; - -use super::{create_named_dir, write_to_file}; -use crate::errors::FilesystemError; - -pub(crate) fn save_witness_to_dir>( - witness_stack: WitnessStack, - witness_name: &str, - witness_dir: P, -) -> Result { - create_named_dir(witness_dir.as_ref(), "witness"); - let witness_path = witness_dir.as_ref().join(witness_name).with_extension(WITNESS_EXT); - - let buf: Vec = witness_stack.try_into()?; - - write_to_file(buf.as_slice(), &witness_path); - - Ok(witness_path) -} 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 462d014c8c94..c8a9c289fc0e 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 @@ -7,7 +7,7 @@ use nargo::{ workspace::Workspace, }; use nargo_toml::PackageSelection; -use noirc_abi::input_parser::Format; +use noir_artifact_cli::fs::{artifact::read_program_from_file, inputs::read_inputs_from_file}; use noirc_artifacts::program::ProgramArtifact; use noirc_artifacts_info::{ FunctionInfo, InfoReport, ProgramInfo, count_opcodes_and_gates_in_program, show_info_report, @@ -22,7 +22,6 @@ use crate::errors::CliError; use super::{ LockType, PackageOptions, WorkspaceCommand, compile_cmd::{compile_workspace_full, get_target_width}, - fs::{inputs::read_inputs_from_file, program::read_program_from_file}, }; /// Provides detailed information on each of a program's function (represented by a single circuit) @@ -75,7 +74,7 @@ pub(crate) fn run(mut args: InfoCommand, workspace: Workspace) -> Result<(), Cli .filter(|package| package.is_binary()) .map(|package| -> Result<(Package, ProgramArtifact), CliError> { let program_artifact_path = workspace.package_build_path(package); - let program = read_program_from_file(program_artifact_path)?; + let program = read_program_from_file(&program_artifact_path)?; Ok((package.clone(), program)) }) .collect::>()?; @@ -144,9 +143,7 @@ fn profile_brillig_execution( for (package, program_artifact) in binary_packages.iter() { // Parse the initial witness values from Prover.toml or Prover.json let (inputs_map, _) = read_inputs_from_file( - &package.root_dir, - prover_name, - Format::Toml, + &package.root_dir.join(prover_name).with_extension("toml"), &program_artifact.abi, )?; let initial_witness = program_artifact.abi.encode(&inputs_map, None)?; diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/init_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/init_cmd.rs index d821751a0eb7..bc67a9ba3a87 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/init_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/init_cmd.rs @@ -1,10 +1,10 @@ use crate::errors::CliError; use super::NargoConfig; -use super::fs::{create_named_dir, write_to_file}; use clap::Args; use nargo::constants::{PKG_FILE, SRC_DIR}; use nargo::package::{CrateName, PackageType}; +use noir_artifact_cli::fs::artifact::write_to_file; use std::path::PathBuf; /// Create a Noir project in the current directory. @@ -58,7 +58,6 @@ pub(crate) fn initialize_project( package_type: PackageType, ) { let src_dir = package_dir.join(SRC_DIR); - create_named_dir(&src_dir, "src"); let toml_contents = format!( r#"[package] @@ -69,14 +68,18 @@ authors = [""] [dependencies]"# ); - write_to_file(toml_contents.as_bytes(), &package_dir.join(PKG_FILE)); + write_to_file(toml_contents.as_bytes(), &package_dir.join(PKG_FILE)).unwrap(); // This uses the `match` syntax instead of `if` so we get a compile error when we add new package types (which likely need new template files) match package_type { - PackageType::Binary => write_to_file(BIN_EXAMPLE.as_bytes(), &src_dir.join("main.nr")), + PackageType::Binary => { + write_to_file(BIN_EXAMPLE.as_bytes(), &src_dir.join("main.nr")).unwrap(); + } PackageType::Contract => { - write_to_file(CONTRACT_EXAMPLE.as_bytes(), &src_dir.join("main.nr")) + write_to_file(CONTRACT_EXAMPLE.as_bytes(), &src_dir.join("main.nr")).unwrap(); + } + PackageType::Library => { + write_to_file(LIB_EXAMPLE.as_bytes(), &src_dir.join("lib.nr")).unwrap(); } - PackageType::Library => write_to_file(LIB_EXAMPLE.as_bytes(), &src_dir.join("lib.nr")), }; println!("Project successfully created! It is located at {}", package_dir.display()); } diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/mod.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/mod.rs index 555861900b40..0c644ae554c8 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/mod.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/mod.rs @@ -14,8 +14,6 @@ use color_eyre::eyre; use crate::errors::CliError; -mod fs; - mod check_cmd; mod compile_cmd; mod dap_cmd; @@ -213,7 +211,10 @@ where /// Lock the (selected) packages in the workspace. /// The lock taken can be shared for commands that only read the artifacts, /// or exclusive for the ones that (might) write artifacts as well. -fn lock_workspace(workspace: &Workspace, exclusive: bool) -> Result, CliError> { +fn lock_workspace( + workspace: &Workspace, + exclusive: bool, +) -> Result>, CliError> { struct LockedFile(File); impl Drop for LockedFile { diff --git a/noir/noir-repo/tooling/nargo_cli/src/errors.rs b/noir/noir-repo/tooling/nargo_cli/src/errors.rs index 9218c3a092fb..74ede00f0d07 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/errors.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/errors.rs @@ -1,37 +1,11 @@ -use acvm::{FieldElement, acir::native_types::WitnessStackError}; +use acvm::FieldElement; use nargo::{NargoError, errors::CompileError}; use nargo_toml::ManifestError; use noir_debugger::errors::DapError; -use noirc_abi::{ - AbiReturnType, - errors::{AbiError, InputParserError}, - input_parser::InputValue, -}; +use noirc_abi::{AbiReturnType, errors::AbiError, input_parser::InputValue}; use std::path::PathBuf; use thiserror::Error; -#[derive(Debug, Error)] -pub(crate) enum FilesystemError { - #[error("Error: {} is not a valid path\nRun either `nargo compile` to generate missing build artifacts or `nargo prove` to construct a proof", .0.display())] - PathNotValid(PathBuf), - - #[error( - " Error: cannot find {0}.toml file.\n Expected location: {1:?} \n Please generate this file at the expected location." - )] - MissingTomlFile(String, PathBuf), - - /// Input parsing error - #[error(transparent)] - InputParserError(#[from] InputParserError), - - /// WitnessStack serialization error - #[error(transparent)] - WitnessStackSerialization(#[from] WitnessStackError), - - #[error("Error: could not deserialize build program: {0}")] - ProgramSerializationError(String), -} - #[derive(Debug, Error)] pub(crate) enum CliError { #[error("{0}")] @@ -43,13 +17,13 @@ pub(crate) enum CliError { #[error("Invalid package name {0}. Did you mean to use `--name`?")] InvalidPackageName(String), - /// ABI encoding/decoding error + /// Artifact CLI error #[error(transparent)] - AbiError(#[from] AbiError), + ArtifactError(#[from] noir_artifact_cli::errors::CliError), - /// Filesystem errors + /// ABI encoding/decoding error #[error(transparent)] - FilesystemError(#[from] FilesystemError), + AbiError(#[from] AbiError), #[error(transparent)] LspError(#[from] async_lsp::Error), diff --git a/noir/noir-repo/tooling/nargo_fmt/src/config.rs b/noir/noir-repo/tooling/nargo_fmt/src/config.rs index f01afc87af2e..3950d267c5ea 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/config.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/config.rs @@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize}; use crate::errors::ConfigError; macro_rules! config { - ($($field_name:ident: $field_ty:ty, $default_value:expr, $description:expr );+ $(;)*) => ( + ($($field_name:ident: $field_ty:ty, $default_value:expr_2021, $description:expr_2021 );+ $(;)*) => ( pub struct Config { $( #[doc = $description] diff --git a/noir/noir-repo/tooling/noirc_abi_wasm/Cargo.toml b/noir/noir-repo/tooling/noirc_abi_wasm/Cargo.toml index b00d580515ef..1d9dc02939fc 100644 --- a/noir/noir-repo/tooling/noirc_abi_wasm/Cargo.toml +++ b/noir/noir-repo/tooling/noirc_abi_wasm/Cargo.toml @@ -31,5 +31,5 @@ getrandom = { workspace = true, features = ["js"] } [build-dependencies] build-data.workspace = true -[dev-dependencies] +[target.'cfg(all(any(target_arch = "wasm32", target_arch = "wasm64"), target_os = "unknown"))'.dev-dependencies] wasm-bindgen-test.workspace = true diff --git a/noir/noir-repo/tooling/noirc_abi_wasm/src/js_witness_map.rs b/noir/noir-repo/tooling/noirc_abi_wasm/src/js_witness_map.rs index 7b753b5584c5..8b751426ac20 100644 --- a/noir/noir-repo/tooling/noirc_abi_wasm/src/js_witness_map.rs +++ b/noir/noir-repo/tooling/noirc_abi_wasm/src/js_witness_map.rs @@ -65,9 +65,9 @@ pub(crate) fn field_element_to_js_string(field_element: &FieldElement) -> JsStri format!("0x{}", field_element.to_hex()).into() } -#[cfg(test)] +#[cfg(all(test, any(target_arch = "wasm32", target_arch = "wasm64"), target_os = "unknown"))] mod test { - use wasm_bindgen_test::wasm_bindgen_test as test; + use wasm_bindgen_test::*; use std::collections::BTreeMap; @@ -79,7 +79,7 @@ mod test { use crate::JsWitnessMap; - #[test] + #[wasm_bindgen_test] fn test_witness_map_to_js() { let witness_map = BTreeMap::from([ (Witness(1), FieldElement::one()), diff --git a/noir/noir-repo/tooling/noirc_artifacts/src/contract.rs b/noir/noir-repo/tooling/noirc_artifacts/src/contract.rs index d3d11f102fb9..c2e44e542664 100644 --- a/noir/noir-repo/tooling/noirc_artifacts/src/contract.rs +++ b/noir/noir-repo/tooling/noirc_artifacts/src/contract.rs @@ -9,6 +9,8 @@ use std::collections::{BTreeMap, HashMap}; use fm::FileId; +use super::{deserialize_hash, serialize_hash}; + #[derive(Clone, Serialize, Deserialize, Debug)] pub struct ContractOutputsArtifact { pub structs: HashMap>, @@ -55,6 +57,12 @@ impl From for ContractArtifact { pub struct ContractFunctionArtifact { pub name: String, + /// Hash of the [`Program`][noirc_frontend::monomorphization::ast::Program] from which the [`ContractFunction`] + /// was compiled. + #[serde(default)] // For backwards compatibility (it was missing). + #[serde(serialize_with = "serialize_hash", deserialize_with = "deserialize_hash")] + pub hash: u64, + pub is_unconstrained: bool, pub custom_attributes: Vec, @@ -72,7 +80,8 @@ pub struct ContractFunctionArtifact { deserialize_with = "ProgramDebugInfo::deserialize_compressed_base64_json" )] pub debug_symbols: ProgramDebugInfo, - + #[serde(default)] // For backwards compatibility (it was missing). + pub names: Vec, pub brillig_names: Vec, } @@ -80,10 +89,12 @@ impl From for ContractFunctionArtifact { fn from(func: ContractFunction) -> Self { ContractFunctionArtifact { name: func.name, + hash: func.hash, is_unconstrained: func.is_unconstrained, custom_attributes: func.custom_attributes, abi: func.abi, bytecode: func.bytecode, + names: func.names, brillig_names: func.brillig_names, debug_symbols: ProgramDebugInfo { debug_infos: func.debug }, } diff --git a/noir/noir-repo/tooling/noirc_artifacts/src/lib.rs b/noir/noir-repo/tooling/noirc_artifacts/src/lib.rs index 77873ed94098..337d5d8550da 100644 --- a/noir/noir-repo/tooling/noirc_artifacts/src/lib.rs +++ b/noir/noir-repo/tooling/noirc_artifacts/src/lib.rs @@ -9,7 +9,48 @@ //! Should any projects require/desire a different artifact format, it's expected that they will write a transformer //! to generate them using these artifacts as a starting point. +use serde::{Deserializer, Serializer, de::Visitor}; + pub mod contract; pub mod debug; mod debug_vars; pub mod program; + +/// Serialize `hash` as `String`, so that it doesn't get truncated in Javascript. +fn serialize_hash(hash: &u64, s: S) -> Result +where + S: Serializer, +{ + s.serialize_str(&hash.to_string()) +} + +/// Deserialize `hash` from `String` in JSON. +fn deserialize_hash<'de, D>(deserializer: D) -> Result +where + D: Deserializer<'de>, +{ + use serde::de::Error; + + // Backwards compatible with `hash` serialized as a number. + struct StringOrU64; + + impl Visitor<'_> for StringOrU64 { + type Value = u64; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + formatter.write_str("String or u64") + } + + fn visit_str(self, value: &str) -> Result + where + E: Error, + { + value.parse().map_err(E::custom) + } + + fn visit_u64(self, value: u64) -> Result { + Ok(value) + } + } + deserializer.deserialize_any(StringOrU64) +} diff --git a/noir/noir-repo/tooling/noirc_artifacts/src/program.rs b/noir/noir-repo/tooling/noirc_artifacts/src/program.rs index 2b498233dad8..fc452fcb2afd 100644 --- a/noir/noir-repo/tooling/noirc_artifacts/src/program.rs +++ b/noir/noir-repo/tooling/noirc_artifacts/src/program.rs @@ -9,6 +9,8 @@ use noirc_driver::DebugFile; use noirc_errors::debug_info::ProgramDebugInfo; use serde::{Deserialize, Serialize}; +use super::{deserialize_hash, serialize_hash}; + #[derive(Clone, Serialize, Deserialize, Debug)] pub struct ProgramArtifact { pub noir_version: String, @@ -17,6 +19,7 @@ pub struct ProgramArtifact { /// was compiled. /// /// Used to short-circuit compilation in the case of the source code not changing since the last compilation. + #[serde(serialize_with = "serialize_hash", deserialize_with = "deserialize_hash")] pub hash: u64, pub abi: Abi, diff --git a/noir/noir-repo/tooling/profiler/Cargo.toml b/noir/noir-repo/tooling/profiler/Cargo.toml index cdd014f6d659..b61271e03ef9 100644 --- a/noir/noir-repo/tooling/profiler/Cargo.toml +++ b/noir/noir-repo/tooling/profiler/Cargo.toml @@ -32,8 +32,8 @@ im.workspace = true acir.workspace = true nargo.workspace = true noirc_errors.workspace = true -noirc_abi.workspace = true noirc_evaluator.workspace = true +noir_artifact_cli.workspace = true thiserror.workspace = true # Logs @@ -44,4 +44,3 @@ tracing-appender = "0.2.3" noirc_abi.workspace = true noirc_driver.workspace = true tempfile.workspace = true - diff --git a/noir/noir-repo/tooling/profiler/src/cli/execution_flamegraph_cmd.rs b/noir/noir-repo/tooling/profiler/src/cli/execution_flamegraph_cmd.rs index fc4787971c69..c9d7eca50f59 100644 --- a/noir/noir-repo/tooling/profiler/src/cli/execution_flamegraph_cmd.rs +++ b/noir/noir-repo/tooling/profiler/src/cli/execution_flamegraph_cmd.rs @@ -7,14 +7,14 @@ use color_eyre::eyre::{self, Context}; use nargo::PrintOutput; use nargo::errors::try_to_diagnose_runtime_error; use nargo::foreign_calls::DefaultForeignCallBuilder; +use noir_artifact_cli::fs::artifact::read_program_from_file; +use noir_artifact_cli::fs::inputs::read_inputs_from_file; use noirc_artifacts::program::ProgramArtifact; use crate::errors::{CliError, report_error}; use crate::flamegraph::{BrilligExecutionSample, FlamegraphGenerator, InfernoFlamegraphGenerator}; -use crate::fs::{read_inputs_from_file, read_program_from_file}; use crate::opcode_formatter::format_brillig_opcode; use bn254_blackbox_solver::Bn254BlackBoxSolver; -use noirc_abi::input_parser::Format; use noirc_artifacts::debug::DebugArtifact; /// Generates a flamegraph mapping unconstrained Noir execution to source code. @@ -61,7 +61,8 @@ fn run_with_generator( ensure_brillig_entry_point(&program)?; - let (inputs_map, _) = read_inputs_from_file(prover_toml_path, Format::Toml, &program.abi)?; + let (inputs_map, _) = + read_inputs_from_file(&prover_toml_path.with_extension("toml"), &program.abi)?; let initial_witness = program.abi.encode(&inputs_map, None)?; diff --git a/noir/noir-repo/tooling/profiler/src/cli/gates_flamegraph_cmd.rs b/noir/noir-repo/tooling/profiler/src/cli/gates_flamegraph_cmd.rs index 02ba22e3e061..a82e5ea3e2a6 100644 --- a/noir/noir-repo/tooling/profiler/src/cli/gates_flamegraph_cmd.rs +++ b/noir/noir-repo/tooling/profiler/src/cli/gates_flamegraph_cmd.rs @@ -4,10 +4,10 @@ use acir::circuit::OpcodeLocation; use clap::Args; use color_eyre::eyre::{self, Context}; +use noir_artifact_cli::fs::artifact::read_program_from_file; use noirc_artifacts::debug::DebugArtifact; use crate::flamegraph::{CompilationSample, FlamegraphGenerator, InfernoFlamegraphGenerator}; -use crate::fs::read_program_from_file; use crate::gates_provider::{BackendGatesProvider, GatesProvider}; use crate::opcode_formatter::format_acir_opcode; @@ -16,11 +16,11 @@ use crate::opcode_formatter::format_acir_opcode; pub(crate) struct GatesFlamegraphCommand { /// The path to the artifact JSON file #[clap(long, short)] - artifact_path: String, + artifact_path: PathBuf, /// Path to the Noir backend binary #[clap(long, short)] - backend_path: String, + backend_path: PathBuf, /// Command to get a gates report from the backend. Defaults to "gates" #[clap(long, short = 'g', default_value = "gates")] @@ -32,7 +32,7 @@ pub(crate) struct GatesFlamegraphCommand { /// The output folder for the flamegraph svg files #[clap(long, short)] - output: String, + output: PathBuf, /// The output name for the flamegraph svg files #[clap(long, short = 'f')] @@ -41,14 +41,14 @@ pub(crate) struct GatesFlamegraphCommand { pub(crate) fn run(args: GatesFlamegraphCommand) -> eyre::Result<()> { run_with_provider( - &PathBuf::from(args.artifact_path), + &args.artifact_path, &BackendGatesProvider { - backend_path: PathBuf::from(args.backend_path), + backend_path: args.backend_path, gates_command: args.backend_gates_command, extra_args: args.backend_extra_args, }, &InfernoFlamegraphGenerator { count_name: "gates".to_string() }, - &PathBuf::from(args.output), + &args.output, args.output_filename, ) } diff --git a/noir/noir-repo/tooling/profiler/src/cli/opcodes_flamegraph_cmd.rs b/noir/noir-repo/tooling/profiler/src/cli/opcodes_flamegraph_cmd.rs index 603ce56a64a4..649331891a29 100644 --- a/noir/noir-repo/tooling/profiler/src/cli/opcodes_flamegraph_cmd.rs +++ b/noir/noir-repo/tooling/profiler/src/cli/opcodes_flamegraph_cmd.rs @@ -5,10 +5,10 @@ use acir::circuit::{Circuit, Opcode, OpcodeLocation}; use clap::Args; use color_eyre::eyre::{self, Context}; +use noir_artifact_cli::fs::artifact::read_program_from_file; use noirc_artifacts::debug::DebugArtifact; use crate::flamegraph::{CompilationSample, FlamegraphGenerator, InfernoFlamegraphGenerator}; -use crate::fs::read_program_from_file; use crate::opcode_formatter::{format_acir_opcode, format_brillig_opcode}; /// Generates a flamegraph mapping ACIR opcodes to their associated locations in the source code. @@ -16,11 +16,11 @@ use crate::opcode_formatter::{format_acir_opcode, format_brillig_opcode}; pub(crate) struct OpcodesFlamegraphCommand { /// The path to the artifact JSON file #[clap(long, short)] - artifact_path: String, + artifact_path: PathBuf, /// The output folder for the flamegraph svg files #[clap(long, short)] - output: String, + output: PathBuf, /// Whether to skip brillig functions #[clap(long, short, action)] @@ -29,9 +29,9 @@ pub(crate) struct OpcodesFlamegraphCommand { pub(crate) fn run(args: OpcodesFlamegraphCommand) -> eyre::Result<()> { run_with_generator( - &PathBuf::from(args.artifact_path), + &args.artifact_path, &InfernoFlamegraphGenerator { count_name: "opcodes".to_string() }, - &PathBuf::from(args.output), + &args.output, args.skip_brillig, ) } diff --git a/noir/noir-repo/tooling/profiler/src/fs.rs b/noir/noir-repo/tooling/profiler/src/fs.rs deleted file mode 100644 index a4f3d6b54a68..000000000000 --- a/noir/noir-repo/tooling/profiler/src/fs.rs +++ /dev/null @@ -1,42 +0,0 @@ -use std::{collections::BTreeMap, path::Path}; - -use color_eyre::eyre; -use noirc_abi::{ - Abi, InputMap, MAIN_RETURN_NAME, - input_parser::{Format, InputValue}, -}; -use noirc_artifacts::program::ProgramArtifact; - -pub(crate) fn read_program_from_file>( - circuit_path: P, -) -> eyre::Result { - let file_path = circuit_path.as_ref().with_extension("json"); - - let input_string = std::fs::read(file_path)?; - let program = serde_json::from_slice(&input_string)?; - - Ok(program) -} - -/// Returns the circuit's parameters and its return value, if one exists. -/// # Examples -/// -/// ```ignore -/// let (input_map, return_value): (InputMap, Option) = -/// read_inputs_from_file(path, "Verifier", Format::Toml, &abi)?; -/// ``` -pub(crate) fn read_inputs_from_file( - file_path: &Path, - format: Format, - abi: &Abi, -) -> eyre::Result<(InputMap, Option)> { - if abi.is_empty() { - return Ok((BTreeMap::new(), None)); - } - - let input_string = std::fs::read_to_string(file_path)?; - let mut input_map = format.parse(&input_string, abi)?; - let return_value = input_map.remove(MAIN_RETURN_NAME); - - Ok((input_map, return_value)) -} diff --git a/noir/noir-repo/tooling/profiler/src/main.rs b/noir/noir-repo/tooling/profiler/src/main.rs index 4c864b7a1006..e4a5bc153d2e 100644 --- a/noir/noir-repo/tooling/profiler/src/main.rs +++ b/noir/noir-repo/tooling/profiler/src/main.rs @@ -6,7 +6,6 @@ mod cli; mod errors; mod flamegraph; -mod fs; mod gates_provider; mod opcode_formatter;