diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index 2fe88c43c1c..c92a12e6a5c 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -1,6 +1,11 @@ name: Pull Request on: + # merge_group: + # The `merge_group` event is currently broken (Feb 9th) so we use `push` and scope the branches + push: + branches: + - gh-readonly-queue/master/* pull_request_target: types: - opened @@ -17,6 +22,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Check title + if: github.event_name == 'pull_request_target' uses: amannn/action-semantic-pull-request@v5 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 68d72bca72e..26b826ed4c8 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -17,6 +17,7 @@ jobs: id: release uses: google-github-actions/release-please-action@v3 with: + token: ${{ secrets.NOIR_RELEASES_TOKEN }} release-type: simple package-name: noir bump-minor-pre-major: true diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 590c9814a1e..fae0fbb6f6d 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -2,26 +2,24 @@ name: Rust on: [push, pull_request] +# This will cancel previous runs when a branch or PR is updated +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref || github.ref || github.run_id }} + cancel-in-progress: true + jobs: - check_n_test: - name: cargo check & test + test: + name: Cargo test uses: noir-lang/.github/.github/workflows/rust-test.yml@main clippy: - name: cargo clippy + name: Cargo clippy uses: noir-lang/.github/.github/workflows/rust-clippy.yml@main format: - name: cargo fmt + name: Cargo fmt uses: noir-lang/.github/.github/workflows/rust-format.yml@main spellcheck: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v3 - - uses: streetsidesoftware/cspell-action@v2 - with: - files: | - **/*.{md,rs} - incremental_files_only: true # Run this action on files which have changed in PR - strict: false # Do not fail, if a spelling mistake is found (This can be annoying for contributors) + name: Spellcheck + uses: noir-lang/.github/.github/workflows/spellcheck.yml@main diff --git a/Cargo.lock b/Cargo.lock index 666cce97cf8..55c25dff231 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8,7 +8,19 @@ version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "aad7977c11d19ae0dd983b50dc5fd9eb96c002072f75643e45daa6dc0c23fba5" dependencies = [ - "acir_field", + "acir_field 0.3.1", + "flate2", + "rmp-serde", + "serde", +] + +[[package]] +name = "acir" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d756bcab90b3a4a84dc53245890cf9bb8fcde31a1394931f5abca551b48eb20" +dependencies = [ + "acir_field 0.4.1", "flate2", "rmp-serde", "serde", @@ -31,15 +43,49 @@ dependencies = [ "serde", ] +[[package]] +name = "acir_field" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "deb7e1e30625a9125a0e700c6c6fd7442ffbcb1d235933100b791ba3786ef49e" +dependencies = [ + "ark-bn254", + "ark-ff", + "blake2", + "cfg-if 1.0.0", + "hex", + "num-bigint", + "num-traits", + "serde", +] + [[package]] name = "acvm" version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "99007127e84602134226eefc2245c59b7fe55853bfeba572714b04c5b3fefdea" dependencies = [ - "acir", - "acir_field", - "acvm_stdlib", + "acir 0.3.1", + "acir_field 0.3.1", + "acvm_stdlib 0.3.1", + "blake2", + "hex", + "indexmap", + "k256", + "num-bigint", + "num-traits", + "sha2", + "thiserror", +] + +[[package]] +name = "acvm" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6c4fae94e7f3fe0d21bec4796de00bbf0cd8f781271b5203dea54897aa5387b9" +dependencies = [ + "acir 0.4.1", + "acvm_stdlib 0.4.1", "blake2", "hex", "indexmap", @@ -56,8 +102,17 @@ version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b4f5ef183f4a10b4a257d25c3a37fd090b9e8fbb7dff0902329fb6606b524114" dependencies = [ - "acir", - "acir_field", + "acir 0.3.1", + "acir_field 0.3.1", +] + +[[package]] +name = "acvm_stdlib" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eaf6617b72c2cd4e965d425bc768bb77a803e485a7e37cbc09cccc5967becd7a" +dependencies = [ + "acir 0.4.1", ] [[package]] @@ -281,7 +336,7 @@ name = "arkworks_backend" version = "0.1.0" source = "git+https://github.com/noir-lang/arkworks_backend?rev=2f3f0db182004d5c01008c741bf519fe6798e24d#2f3f0db182004d5c01008c741bf519fe6798e24d" dependencies = [ - "acvm", + "acvm 0.3.1", "ark-bls12-381", "ark-bn254", "ark-ff", @@ -330,7 +385,7 @@ dependencies = [ [[package]] name = "barretenberg_static_lib" version = "0.1.0" -source = "git+https://github.com/noir-lang/aztec_backend?rev=9de36b642d125d1fb4facd1bf60db67946be70ae#9de36b642d125d1fb4facd1bf60db67946be70ae" +source = "git+https://github.com/noir-lang/aztec_backend?rev=d0e1257c22618f98f53781faba3c372ef91a0172#d0e1257c22618f98f53781faba3c372ef91a0172" dependencies = [ "barretenberg_wrapper", "blake2", @@ -350,7 +405,7 @@ dependencies = [ [[package]] name = "barretenberg_wasm" version = "0.1.0" -source = "git+https://github.com/noir-lang/aztec_backend?rev=9de36b642d125d1fb4facd1bf60db67946be70ae#9de36b642d125d1fb4facd1bf60db67946be70ae" +source = "git+https://github.com/noir-lang/aztec_backend?rev=d0e1257c22618f98f53781faba3c372ef91a0172#d0e1257c22618f98f53781faba3c372ef91a0172" dependencies = [ "blake2", "common", @@ -363,7 +418,7 @@ dependencies = [ [[package]] name = "barretenberg_wrapper" version = "0.1.0" -source = "git+https://github.com/noir-lang/aztec-connect?branch=kw/noir-dsl#85dcb7c404b824b420c92e1010a9e363fe91fbd2" +source = "git+https://github.com/noir-lang/aztec-connect?branch=kw/noir-dsl#f95b4d3551db4a62bbf7de56c6c3362523635e50" dependencies = [ "bindgen", "cmake", @@ -593,9 +648,9 @@ dependencies = [ [[package]] name = "common" version = "0.1.0" -source = "git+https://github.com/noir-lang/aztec_backend?rev=9de36b642d125d1fb4facd1bf60db67946be70ae#9de36b642d125d1fb4facd1bf60db67946be70ae" +source = "git+https://github.com/noir-lang/aztec_backend?rev=d0e1257c22618f98f53781faba3c372ef91a0172#d0e1257c22618f98f53781faba3c372ef91a0172" dependencies = [ - "acvm", + "acvm 0.4.1", "blake2", "dirs 3.0.2", "downloader", @@ -808,9 +863,9 @@ dependencies = [ [[package]] name = "darling" -version = "0.14.2" +version = "0.14.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b0dd3cd20dc6b5a876612a6e5accfe7f3dd883db6d07acfbf14c128f61550dfa" +checksum = "c0808e1bd8671fb44a113a14e13497557533369847788fa2ae912b6ebfce9fa8" dependencies = [ "darling_core", "darling_macro", @@ -818,9 +873,9 @@ dependencies = [ [[package]] name = "darling_core" -version = "0.14.2" +version = "0.14.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a784d2ccaf7c98501746bf0be29b2022ba41fd62a2e622af997a03e9f972859f" +checksum = "001d80444f28e193f30c2f293455da62dcf9a6b29918a4253152ae2b1de592cb" dependencies = [ "fnv", "ident_case", @@ -831,9 +886,9 @@ dependencies = [ [[package]] name = "darling_macro" -version = "0.14.2" +version = "0.14.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7618812407e9402654622dd402b0a89dff9ba93badd6540781526117b92aab7e" +checksum = "b36230598a2d5de7ec1c6f51f72d8a99a9208daff41de2084d06e3fd3ea56685" dependencies = [ "darling_core", "quote", @@ -1618,7 +1673,7 @@ name = "marlin_arkworks_backend" version = "0.1.0" source = "git+https://github.com/noir-lang/marlin_arkworks_backend?rev=144378edad821bfaa52bf2cacca8ecc87514a4fc#144378edad821bfaa52bf2cacca8ecc87514a4fc" dependencies = [ - "acvm", + "acvm 0.3.1", "arkworks_backend", ] @@ -1698,7 +1753,7 @@ checksum = "7843ec2de400bcbc6a6328c958dc38e5359da6e93e72e37bc5246bf1ae776389" name = "nargo" version = "0.1.1" dependencies = [ - "acvm", + "acvm 0.4.1", "barretenberg_static_lib", "barretenberg_wasm", "cfg-if 1.0.0", @@ -1726,7 +1781,7 @@ dependencies = [ name = "noir_wasm" version = "0.1.1" dependencies = [ - "acvm", + "acvm 0.4.1", "console_error_panic_hook", "getrandom", "gloo-utils", @@ -1740,7 +1795,7 @@ dependencies = [ name = "noirc_abi" version = "0.1.1" dependencies = [ - "acvm", + "acvm 0.4.1", "iter-extended", "serde", "serde_json", @@ -1752,7 +1807,7 @@ dependencies = [ name = "noirc_driver" version = "0.1.1" dependencies = [ - "acvm", + "acvm 0.4.1", "dirs 4.0.0", "fm", "noirc_abi", @@ -1776,7 +1831,7 @@ dependencies = [ name = "noirc_evaluator" version = "0.1.1" dependencies = [ - "acvm", + "acvm 0.4.1", "arena", "fm", "iter-extended", @@ -1785,6 +1840,7 @@ dependencies = [ "noirc_frontend", "num-bigint", "num-traits", + "rand 0.8.5", "thiserror", ] @@ -1792,7 +1848,7 @@ dependencies = [ name = "noirc_frontend" version = "0.1.1" dependencies = [ - "acvm", + "acvm 0.4.1", "arena", "chumsky", "fm", @@ -1953,9 +2009,9 @@ checksum = "478c572c3d73181ff3c2539045f6eb99e5491218eae919370993b890cdbdd98e" [[package]] name = "pest" -version = "2.5.4" +version = "2.5.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4ab62d2fa33726dbe6321cc97ef96d8cde531e3eeaf858a058de53a8a6d40d8f" +checksum = "028accff104c4e513bad663bbcd2ad7cfd5304144404c31ed0a77ac103d00660" dependencies = [ "thiserror", "ucd-trie", @@ -2020,9 +2076,9 @@ checksum = "dc375e1527247fe1a97d8b7156678dfe7c1af2fc075c9a4db3690ecd2a148068" [[package]] name = "proc-macro2" -version = "1.0.50" +version = "1.0.51" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6ef7d57beacfaf2d8aee5937dab7b7f28de3cb8b1828479bb5de2a7106f2bae2" +checksum = "5d727cae5b39d21da60fa540906919ad737832fe0b1c165da3a34d6548c849d6" dependencies = [ "unicode-ident", ] @@ -2471,9 +2527,9 @@ dependencies = [ [[package]] name = "serde_bytes" -version = "0.11.8" +version = "0.11.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "718dc5fff5b36f99093fc49b280cfc96ce6fc824317783bff5a1fed0c7a64819" +checksum = "416bda436f9aab92e02c8e10d49a15ddd339cea90b6e340fe51ed97abb548294" dependencies = [ "serde", ] @@ -2491,9 +2547,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.91" +version = "1.0.93" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "877c235533714907a8c2464236f5c4b2a17262ef1bd71f38f35ea592c8da6883" +checksum = "cad406b69c91885b5107daf2c29572f6c8cdb3c66826821e286c533490c0bc76" dependencies = [ "itoa", "ryu", @@ -2574,9 +2630,9 @@ checksum = "a507befe795404456341dfab10cef66ead4c041f62b8b11bbb92bffe5d0953e0" [[package]] name = "smol_str" -version = "0.1.23" +version = "0.1.24" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7475118a28b7e3a2e157ce0131ba8c5526ea96e90ee601d9f6bb2e286a35ab44" +checksum = "fad6c857cbab2627dcf01ec85a623ca4e7dcb5691cbaa3d7fb7653671f0d09c9" dependencies = [ "serde", ] @@ -2742,9 +2798,9 @@ dependencies = [ [[package]] name = "tinyvec_macros" -version = "0.1.0" +version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cda74da7e1a664f795bb1f8a87ec406fb89a02522cf6e50620d016add6dbbf5c" +checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" [[package]] name = "tokio" @@ -2776,9 +2832,9 @@ dependencies = [ [[package]] name = "tokio-util" -version = "0.7.4" +version = "0.7.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0bb2e075f03b3d66d8d8785356224ba688d2906a371015e225beeb65ca92c740" +checksum = "7e267c18a719545b481171952a79f8c25c80361463ba44bc7fa9eba7c742ef4f" dependencies = [ "bytes", "futures-core", @@ -3026,9 +3082,9 @@ dependencies = [ [[package]] name = "wasm-encoder" -version = "0.22.0" +version = "0.22.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ef126be0e14bdf355ac1a8b41afc89195289e5c7179f80118e3abddb472f0810" +checksum = "9a584273ccc2d9311f1dd19dc3fb26054661fa3e373d53ede5d1144ba07a9acd" dependencies = [ "leb128", ] @@ -3272,9 +3328,9 @@ checksum = "718ed7c55c2add6548cca3ddd6383d738cd73b892df400e96b9aa876f0141d7a" [[package]] name = "wast" -version = "52.0.2" +version = "52.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "707a9fd59b0144c530f0a31f21737036ffea6ece492918cae0843dd09b6f9bc9" +checksum = "15942180f265280eede7bc38b239e9770031d1821c02d905284216c645316430" dependencies = [ "leb128", "memchr", @@ -3284,9 +3340,9 @@ dependencies = [ [[package]] name = "wat" -version = "1.0.56" +version = "1.0.57" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "91d73cbaa81acc2f8a3303e2289205c971d99c89245c2f56ab8765c4daabc2be" +checksum = "37212100d4cbe6f0f6ff6e707f1e5a5b5b675f0451231ed9e4235e234e127ed3" dependencies = [ "wast", ] diff --git a/Cargo.toml b/Cargo.toml index 1b23ff98062..f245fd06b67 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,10 +20,10 @@ version = "0.1.1" # x-release-please-end authors = ["The Noir Team "] edition = "2021" -rust-version = "1.64" +rust-version = "1.66" [workspace.dependencies] -acvm = "0.3.1" +acvm = "0.4.1" arena = { path = "crates/arena" } fm = { path = "crates/fm" } iter-extended = { path = "crates/iter-extended" } diff --git a/README.md b/README.md index c03b9de897e..c845c00b73e 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ Noir is a Domain Specific Language for SNARK proving systems. It has been design Read the installation section [here](https://noir-lang.github.io/book/getting_started/nargo/installation.html). -Once you have read through the documentation, you can also run the examples located in the `examples` folder. +Once you have read through the documentation, you can visit [Awesome Noir](https://github.com/noir-lang/awesome-noir) to run some of the examples that others have created. ## Current Features @@ -63,14 +63,10 @@ Concretely the following items are on the road map: ## Minimum Rust version -This crate's minimum supported rustc version is 1.64.0. +This crate's minimum supported rustc version is 1.66.0. ## License Noir is free and open source. It is distributed under a dual license. (MIT/APACHE) Unless you explicitly state otherwise, any contribution intentionally submitted for inclusion in this crate by you, as defined in the Apache-2.0 license, shall be dual licensed as above, without any additional terms or conditions. - -## Barretenberg License - -Barretenberg is licensed under GPL V2.0. diff --git a/crates/arena/src/lib.rs b/crates/arena/src/lib.rs index 278178265a4..bbb04a26d84 100644 --- a/crates/arena/src/lib.rs +++ b/crates/arena/src/lib.rs @@ -1,2 +1,3 @@ +#![forbid(unsafe_code)] // For now we use a wrapper around generational-arena pub use generational_arena::{Arena, Index}; diff --git a/crates/fm/src/file_map.rs b/crates/fm/src/file_map.rs index 2a1cd662d4e..d893b6c14d8 100644 --- a/crates/fm/src/file_map.rs +++ b/crates/fm/src/file_map.rs @@ -51,7 +51,7 @@ impl FileId { pub struct File<'input>(&'input SimpleFile); impl<'input> File<'input> { - pub fn get_source(self) -> &'input str { + pub fn source(self) -> &'input str { self.0.source() } } diff --git a/crates/fm/src/lib.rs b/crates/fm/src/lib.rs index ebc3d7021a8..c365f578c4a 100644 --- a/crates/fm/src/lib.rs +++ b/crates/fm/src/lib.rs @@ -1,3 +1,4 @@ +#![forbid(unsafe_code)] mod file_map; mod file_reader; diff --git a/crates/iter-extended/src/lib.rs b/crates/iter-extended/src/lib.rs index b840dc1c9b3..b5729c9a895 100644 --- a/crates/iter-extended/src/lib.rs +++ b/crates/iter-extended/src/lib.rs @@ -1,3 +1,4 @@ +#![forbid(unsafe_code)] use std::collections::BTreeMap; /// Equivalent to .into_iter().map(f).collect::>() diff --git a/crates/nargo/Cargo.toml b/crates/nargo/Cargo.toml index 8daffdcf56b..1e5ecdc3c07 100644 --- a/crates/nargo/Cargo.toml +++ b/crates/nargo/Cargo.toml @@ -31,8 +31,8 @@ termcolor = "1.1.2" tempdir = "0.3.7" # Backends -aztec_backend = { optional = true, package = "barretenberg_static_lib", git = "https://github.com/noir-lang/aztec_backend", rev = "9de36b642d125d1fb4facd1bf60db67946be70ae" } -aztec_wasm_backend = { optional = true, package = "barretenberg_wasm", git = "https://github.com/noir-lang/aztec_backend", rev = "9de36b642d125d1fb4facd1bf60db67946be70ae" } +aztec_backend = { optional = true, package = "barretenberg_static_lib", git = "https://github.com/noir-lang/aztec_backend", rev = "d0e1257c22618f98f53781faba3c372ef91a0172" } +aztec_wasm_backend = { optional = true, package = "barretenberg_wasm", git = "https://github.com/noir-lang/aztec_backend", rev = "d0e1257c22618f98f53781faba3c372ef91a0172" } marlin_arkworks_backend = { optional = true, git = "https://github.com/noir-lang/marlin_arkworks_backend", rev = "144378edad821bfaa52bf2cacca8ecc87514a4fc" } [features] diff --git a/crates/nargo/build.rs b/crates/nargo/build.rs index 21889d81b36..9bd33aab0e7 100644 --- a/crates/nargo/build.rs +++ b/crates/nargo/build.rs @@ -18,8 +18,8 @@ fn rerun_if_stdlib_changes(directory: &Path) { fn check_rustc_version() { assert!( - version().unwrap() >= Version::parse("1.6.4").unwrap(), - "The minimal supported rustc version is 1.64.0." + version().unwrap() >= Version::parse("1.66.0").unwrap(), + "The minimal supported rustc version is 1.66.0." ); } diff --git a/crates/nargo/src/cli/check_cmd.rs b/crates/nargo/src/cli/check_cmd.rs index 27324926511..c01d0541a87 100644 --- a/crates/nargo/src/cli/check_cmd.rs +++ b/crates/nargo/src/cli/check_cmd.rs @@ -15,8 +15,10 @@ pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> { let args = args.subcommand_matches("check").unwrap(); let allow_warnings = args.is_present("allow-warnings"); - let package_dir = std::env::current_dir().unwrap(); - check_from_path(package_dir, allow_warnings)?; + let program_dir = + args.value_of("path").map_or_else(|| std::env::current_dir().unwrap(), PathBuf::from); + + check_from_path(program_dir, allow_warnings)?; println!("Constraint system successfully built!"); Ok(()) } diff --git a/crates/nargo/src/cli/compile_cmd.rs b/crates/nargo/src/cli/compile_cmd.rs index 5f335e4ac8a..8f853d5ebed 100644 --- a/crates/nargo/src/cli/compile_cmd.rs +++ b/crates/nargo/src/cli/compile_cmd.rs @@ -17,14 +17,15 @@ pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> { let circuit_name = args.value_of("circuit_name").unwrap(); let witness = args.is_present("witness"); let allow_warnings = args.is_present("allow-warnings"); + let program_dir = + args.value_of("path").map_or_else(|| std::env::current_dir().unwrap(), PathBuf::from); - let current_dir = std::env::current_dir().unwrap(); - let mut circuit_path = PathBuf::new(); + let mut circuit_path = program_dir.clone(); circuit_path.push(TARGET_DIR); generate_circuit_and_witness_to_disk( circuit_name, - current_dir, + program_dir, circuit_path, witness, allow_warnings, diff --git a/crates/nargo/src/cli/contract_cmd.rs b/crates/nargo/src/cli/contract_cmd.rs index 68191be7730..dcdd6745113 100644 --- a/crates/nargo/src/cli/contract_cmd.rs +++ b/crates/nargo/src/cli/contract_cmd.rs @@ -1,23 +1,25 @@ +use std::path::PathBuf; + use super::{create_named_dir, write_to_file}; use crate::{cli::compile_cmd::compile_circuit, constants::CONTRACT_DIR, errors::CliError}; use acvm::SmartContract; use clap::ArgMatches; pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> { - let cmd = args.subcommand_matches("contract").unwrap(); - - let package_dir = match cmd.value_of("path") { - Some(path) => std::path::PathBuf::from(path), - None => std::env::current_dir().unwrap(), - }; + let args = args.subcommand_matches("contract").unwrap(); let allow_warnings = args.is_present("allow-warnings"); - let compiled_program = compile_circuit(package_dir, false, allow_warnings)?; + let program_dir = + args.value_of("path").map_or_else(|| std::env::current_dir().unwrap(), PathBuf::from); + + let compiled_program = compile_circuit(program_dir.clone(), false, allow_warnings)?; let backend = crate::backends::ConcreteBackend; let smart_contract_string = backend.eth_contract_from_cs(compiled_program.circuit); - let mut contract_path = create_named_dir(CONTRACT_DIR.as_ref(), "contract"); + let mut contract_dir = program_dir; + contract_dir.push(CONTRACT_DIR); + let mut contract_path = create_named_dir(contract_dir.as_ref(), "contract"); contract_path.push("plonk_vk"); contract_path.set_extension("sol"); diff --git a/crates/nargo/src/cli/execute_cmd.rs b/crates/nargo/src/cli/execute_cmd.rs index ba444b57bb2..a89a0fcaa21 100644 --- a/crates/nargo/src/cli/execute_cmd.rs +++ b/crates/nargo/src/cli/execute_cmd.rs @@ -10,6 +10,7 @@ use noirc_driver::CompiledProgram; use super::{create_named_dir, read_inputs_from_file, write_to_file, InputMap, WitnessMap}; use crate::{ + cli::compile_cmd::compile_circuit, constants::{PROVER_INPUT_FILE, TARGET_DIR, WITNESS_EXT}, errors::CliError, }; @@ -19,14 +20,17 @@ pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> { let witness_name = args.value_of("witness_name"); let show_ssa = args.is_present("show-ssa"); let allow_warnings = args.is_present("allow-warnings"); - let (return_value, solved_witness) = execute(show_ssa, allow_warnings)?; + let program_dir = + args.value_of("path").map_or_else(|| std::env::current_dir().unwrap(), PathBuf::from); + + let (return_value, solved_witness) = execute_with_path(&program_dir, show_ssa, allow_warnings)?; println!("Circuit witness successfully solved"); if let Some(return_value) = return_value { println!("Circuit output: {return_value:?}"); } if let Some(witness_name) = witness_name { - let mut witness_dir = std::env::current_dir().unwrap(); + let mut witness_dir = program_dir; witness_dir.push(TARGET_DIR); let witness_path = save_witness_to_dir(solved_witness, witness_name, witness_dir)?; @@ -40,18 +44,16 @@ pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> { /// So when we add witness values, their index start from 1. const WITNESS_OFFSET: u32 = 1; -fn execute( +fn execute_with_path>( + program_dir: P, show_ssa: bool, allow_warnings: bool, ) -> Result<(Option, WitnessMap), CliError> { - let current_dir = std::env::current_dir().unwrap(); - - let compiled_program = - super::compile_cmd::compile_circuit(¤t_dir, show_ssa, allow_warnings)?; + let compiled_program = compile_circuit(&program_dir, show_ssa, allow_warnings)?; // Parse the initial witness values from Prover.toml let inputs_map = read_inputs_from_file( - current_dir, + &program_dir, PROVER_INPUT_FILE, Format::Toml, compiled_program.abi.as_ref().unwrap().clone(), diff --git a/crates/nargo/src/cli/gates_cmd.rs b/crates/nargo/src/cli/gates_cmd.rs index 04f0ecab759..264fa4ab709 100644 --- a/crates/nargo/src/cli/gates_cmd.rs +++ b/crates/nargo/src/cli/gates_cmd.rs @@ -1,6 +1,6 @@ use acvm::ProofSystemCompiler; use clap::ArgMatches; -use std::path::Path; +use std::path::{Path, PathBuf}; use crate::cli::compile_cmd::compile_circuit; use crate::errors::CliError; @@ -9,12 +9,10 @@ pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> { let args = args.subcommand_matches("gates").unwrap(); let show_ssa = args.is_present("show-ssa"); let allow_warnings = args.is_present("allow-warnings"); - count_gates(show_ssa, allow_warnings) -} + let program_dir = + args.value_of("path").map_or_else(|| std::env::current_dir().unwrap(), PathBuf::from); -pub fn count_gates(show_ssa: bool, allow_warnings: bool) -> Result<(), CliError> { - let current_dir = std::env::current_dir().unwrap(); - count_gates_with_path(current_dir, show_ssa, allow_warnings) + count_gates_with_path(program_dir, show_ssa, allow_warnings) } pub fn count_gates_with_path>( diff --git a/crates/nargo/src/cli/mod.rs b/crates/nargo/src/cli/mod.rs index 8753fa6241e..92cc68b41d9 100644 --- a/crates/nargo/src/cli/mod.rs +++ b/crates/nargo/src/cli/mod.rs @@ -120,7 +120,8 @@ pub fn start_cli() { .arg( Arg::with_name("witness_name") .long("witness_name") - .help("Write the execution witness to named file"), + .help("Write the execution witness to named file") + .takes_value(true), ) .arg(show_ssa) .arg(allow_warnings), @@ -184,7 +185,7 @@ pub fn read_inputs_from_file>( dir_path }; if !file_path.exists() { - return Err(CliError::MissingTomlFile(file_path)); + return Err(CliError::MissingTomlFile(file_name.to_owned(), file_path)); } let input_string = std::fs::read_to_string(file_path).unwrap(); diff --git a/crates/nargo/src/cli/new_cmd.rs b/crates/nargo/src/cli/new_cmd.rs index 967445af8f0..ab16eae69be 100644 --- a/crates/nargo/src/cli/new_cmd.rs +++ b/crates/nargo/src/cli/new_cmd.rs @@ -5,17 +5,16 @@ use crate::{ use super::{create_named_dir, write_to_file}; use clap::ArgMatches; -use std::path::Path; +use std::path::{Path, PathBuf}; pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> { let cmd = args.subcommand_matches("new").unwrap(); let package_name = cmd.value_of("package_name").unwrap(); - let mut package_dir = match cmd.value_of("path") { - Some(path) => std::path::PathBuf::from(path), - None => std::env::current_dir().unwrap(), - }; + let mut package_dir = + args.value_of("path").map_or_else(|| std::env::current_dir().unwrap(), PathBuf::from); + package_dir.push(Path::new(package_name)); if package_dir.exists() { return Err(CliError::DestinationAlreadyExists(package_dir)); diff --git a/crates/nargo/src/cli/prove_cmd.rs b/crates/nargo/src/cli/prove_cmd.rs index eb4fc5e5b22..d67cdceffb9 100644 --- a/crates/nargo/src/cli/prove_cmd.rs +++ b/crates/nargo/src/cli/prove_cmd.rs @@ -24,21 +24,13 @@ pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> { let show_ssa = args.is_present("show-ssa"); let allow_warnings = args.is_present("allow-warnings"); - prove(proof_name, check_proof, show_ssa, allow_warnings) -} - -fn prove( - proof_name: Option<&str>, - check_proof: bool, - show_ssa: bool, - allow_warnings: bool, -) -> Result<(), CliError> { - let current_dir = std::env::current_dir().unwrap(); + let program_dir = + args.value_of("path").map_or_else(|| std::env::current_dir().unwrap(), PathBuf::from); - let mut proof_dir = PathBuf::new(); + let mut proof_dir = program_dir.clone(); proof_dir.push(PROOFS_DIR); - prove_with_path(proof_name, current_dir, proof_dir, check_proof, show_ssa, allow_warnings)?; + prove_with_path(proof_name, program_dir, proof_dir, check_proof, show_ssa, allow_warnings)?; Ok(()) } diff --git a/crates/nargo/src/cli/test_cmd.rs b/crates/nargo/src/cli/test_cmd.rs index 0a46f35e585..141bc034f40 100644 --- a/crates/nargo/src/cli/test_cmd.rs +++ b/crates/nargo/src/cli/test_cmd.rs @@ -1,4 +1,8 @@ -use std::{collections::BTreeMap, io::Write}; +use std::{ + collections::BTreeMap, + io::Write, + path::{Path, PathBuf}, +}; use acvm::{PartialWitnessGenerator, ProofSystemCompiler}; use clap::ArgMatches; @@ -14,14 +18,16 @@ pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> { let args = args.subcommand_matches("test").unwrap(); let test_name = args.value_of("test_name").unwrap_or(""); let allow_warnings = args.is_present("allow-warnings"); - run_tests(test_name, allow_warnings) + let program_dir = + args.value_of("path").map_or_else(|| std::env::current_dir().unwrap(), PathBuf::from); + + run_tests(&program_dir, test_name, allow_warnings) } -fn run_tests(test_name: &str, allow_warnings: bool) -> Result<(), CliError> { +fn run_tests(program_dir: &Path, test_name: &str, allow_warnings: bool) -> Result<(), CliError> { let backend = crate::backends::ConcreteBackend; - let package_dir = std::env::current_dir().unwrap(); - let mut driver = Resolver::resolve_root_config(&package_dir, backend.np_language())?; + let mut driver = Resolver::resolve_root_config(program_dir, backend.np_language())?; add_std_lib(&mut driver); if driver.check_crate(allow_warnings).is_err() { diff --git a/crates/nargo/src/cli/verify_cmd.rs b/crates/nargo/src/cli/verify_cmd.rs index 962f9a09ea6..60efc882ec7 100644 --- a/crates/nargo/src/cli/verify_cmd.rs +++ b/crates/nargo/src/cli/verify_cmd.rs @@ -11,33 +11,29 @@ use clap::ArgMatches; use noirc_abi::errors::AbiError; use noirc_abi::input_parser::Format; use noirc_driver::CompiledProgram; -use std::{collections::BTreeMap, path::Path, path::PathBuf}; +use std::{ + collections::BTreeMap, + path::{Path, PathBuf}, +}; pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> { let args = args.subcommand_matches("verify").unwrap(); let proof_name = args.value_of("proof").unwrap(); - let mut proof_path = std::path::PathBuf::new(); - proof_path.push(Path::new(PROOFS_DIR)); + let program_dir = + args.value_of("path").map_or_else(|| std::env::current_dir().unwrap(), PathBuf::from); + let mut proof_path = program_dir.clone(); + proof_path.push(Path::new(PROOFS_DIR)); proof_path.push(Path::new(proof_name)); proof_path.set_extension(PROOF_EXT); let allow_warnings = args.is_present("allow-warnings"); - let result = verify(proof_name, allow_warnings)?; + let result = verify_with_path(program_dir, proof_path, false, allow_warnings)?; println!("Proof verified : {result}\n"); Ok(()) } -fn verify(proof_name: &str, allow_warnings: bool) -> Result { - let current_dir = std::env::current_dir().unwrap(); - let mut proof_path = PathBuf::new(); // or current_dir? - proof_path.push(PROOFS_DIR); - proof_path.push(Path::new(proof_name)); - proof_path.set_extension(PROOF_EXT); - verify_with_path(¤t_dir, &proof_path, false, allow_warnings) -} - pub fn verify_with_path>( program_dir: P, proof_path: P, diff --git a/crates/nargo/src/errors.rs b/crates/nargo/src/errors.rs index b569b979bdf..fc3db1c9656 100644 --- a/crates/nargo/src/errors.rs +++ b/crates/nargo/src/errors.rs @@ -15,8 +15,10 @@ pub enum CliError { PathNotValid(PathBuf), #[error("Error: could not parse proof data ({0})")] ProofNotValid(FromHexError), - #[error("cannot find input file located at {0:?}, run nargo build to generate the missing Prover and/or Verifier toml files")] - MissingTomlFile(PathBuf), + #[error( + " Error: cannot find {0}.toml file.\n Expected location: {1:?} \n Please generate this file at the expected location." + )] + MissingTomlFile(String, PathBuf), } impl From for CliError { diff --git a/crates/nargo/src/lib.rs b/crates/nargo/src/lib.rs index 526f7be373f..fd001dd3e98 100644 --- a/crates/nargo/src/lib.rs +++ b/crates/nargo/src/lib.rs @@ -1,3 +1,4 @@ +#![forbid(unsafe_code)] use noirc_frontend::graph::CrateType; use std::path::{Path, PathBuf}; diff --git a/crates/nargo/tests/test_data/array_len/src/main.nr b/crates/nargo/tests/test_data/array_len/src/main.nr index 45c474f9dca..2531bbc42fb 100644 --- a/crates/nargo/tests/test_data/array_len/src/main.nr +++ b/crates/nargo/tests/test_data/array_len/src/main.nr @@ -20,4 +20,12 @@ fn main(len3: [u8; 3], len4: [Field; 4]) { // std::array::len returns a comptime value constrain len4[std::array::len(len3)] == 4; + + // test for std::array::sort + let mut unsorted = len3; + unsorted[0] = len3[1]; + unsorted[1] = len3[0]; + constrain unsorted[0] > unsorted[1]; + let sorted = std::array::sort(unsorted); + constrain sorted[0] < sorted[1]; } diff --git a/crates/nargo/tests/test_data/generics/src/main.nr b/crates/nargo/tests/test_data/generics/src/main.nr index f9746690104..56078a304e0 100644 --- a/crates/nargo/tests/test_data/generics/src/main.nr +++ b/crates/nargo/tests/test_data/generics/src/main.nr @@ -8,10 +8,45 @@ fn foo(bar: Bar) { constrain bar.one == bar.two; } +struct BigInt { + limbs: [u32; N], +} + +impl BigInt { + // `N` is in scope of all methods in the impl + fn first(first: BigInt, second: BigInt) -> Self { + constrain first.limbs != second.limbs; + first + } + + fn second(first: BigInt, second: Self) -> Self { + constrain first.limbs != second.limbs; + second + } +} + +impl Bar { + fn get_other(self) -> Field { + self.other + } +} + fn main(x: Field, y: Field) { let bar1: Bar = Bar { one: x, two: y, other: 0 }; let bar2 = Bar { one: x, two: y, other: [0] }; foo(bar1); foo(bar2); + + // Test generic impls + let int1 = BigInt { limbs: [1] }; + let int2 = BigInt { limbs: [2] }; + let BigInt { limbs } = int1.second(int2).first(int1); + constrain limbs == int2.limbs; + + // Test impl exclusively for Bar + constrain bar1.get_other() == bar1.other; + + // Expected type error + // constrain bar2.get_other() == bar2.other; } diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index 821cb663912..7d8bc645102 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -1,3 +1,4 @@ +#![forbid(unsafe_code)] use std::{collections::BTreeMap, convert::TryInto, str}; use acvm::FieldElement; diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index d02271791a1..69d19517585 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -1,9 +1,10 @@ +#![forbid(unsafe_code)] use acvm::acir::circuit::Circuit; use acvm::Language; use fm::FileType; use noirc_abi::Abi; -use noirc_errors::{DiagnosableError, ReportedError, Reporter}; +use noirc_errors::{reporter, ReportedError}; use noirc_evaluator::create_circuit; use noirc_frontend::graph::{CrateId, CrateName, CrateType, LOCAL_CRATE}; use noirc_frontend::hir::def_map::CrateDefMap; @@ -47,14 +48,7 @@ impl Driver { pub fn file_compiles(&mut self) -> bool { let mut errs = vec![]; CrateDefMap::collect_defs(LOCAL_CRATE, &mut self.context, &mut errs); - for errors in &errs { - Reporter::with_diagnostics( - Some(errors.file_id), - &self.context.file_manager, - &errors.errors, - false, - ); - } + reporter::report_all(&self.context.file_manager, &errs, false); errs.is_empty() } @@ -135,17 +129,8 @@ impl Driver { pub fn check_crate(&mut self, allow_warnings: bool) -> Result<(), ReportedError> { let mut errs = vec![]; CrateDefMap::collect_defs(LOCAL_CRATE, &mut self.context, &mut errs); - let mut error_count = 0; - for errors in &errs { - error_count += Reporter::with_diagnostics( - Some(errors.file_id), - &self.context.file_manager, - &errors.errors, - allow_warnings, - ); - } - - Reporter::finish(error_count) + let error_count = reporter::report_all(&self.context.file_manager, &errs, allow_warnings); + reporter::finish_report(error_count) } pub fn compute_abi(&self) -> Option { @@ -201,18 +186,16 @@ impl Driver { let program = monomorphize(main_function, &self.context.def_interner); - let blackbox_supported = acvm::default_is_blackbox_supported(np_language.clone()); + let blackbox_supported = acvm::default_is_black_box_supported(np_language.clone()); match create_circuit(program, np_language, blackbox_supported, show_ssa) { Ok(circuit) => Ok(CompiledProgram { circuit, abi: Some(abi) }), Err(err) => { // The FileId here will be the file id of the file with the main file // Errors will be shown at the call site without a stacktrace - let file_id = err.location.map(|loc| loc.file); - let error = &[err.to_diagnostic()]; + let file = err.location.map(|loc| loc.file); let files = &self.context.file_manager; - - let error_count = Reporter::with_diagnostics(file_id, files, error, allow_warnings); - Reporter::finish(error_count)?; + let error = reporter::report(files, &err.into(), file, allow_warnings); + reporter::finish_report(error as u32)?; Err(ReportedError) } } @@ -223,8 +206,10 @@ impl Driver { /// will return all functions marked with #[test]. pub fn get_all_test_functions_in_crate_matching(&self, pattern: &str) -> Vec { let interner = &self.context.def_interner; - interner - .get_all_test_functions() + self.context + .def_map(LOCAL_CRATE) + .expect("The local crate should be analyzed already") + .get_all_test_functions(interner) .filter_map(|id| interner.function_name(&id).contains(pattern).then_some(id)) .collect() } diff --git a/crates/noirc_errors/src/lib.rs b/crates/noirc_errors/src/lib.rs index a4d412dd82b..99dd6554dbb 100644 --- a/crates/noirc_errors/src/lib.rs +++ b/crates/noirc_errors/src/lib.rs @@ -1,19 +1,15 @@ +#![forbid(unsafe_code)] mod position; -mod reporter; - +pub mod reporter; pub use position::{Location, Position, Span, Spanned}; -pub use reporter::*; - -pub trait DiagnosableError { - fn to_diagnostic(&self) -> CustomDiagnostic; -} +pub use reporter::{CustomDiagnostic, DiagnosticKind}; /// Returned when the Reporter finishes after reporting errors #[derive(Copy, Clone)] pub struct ReportedError; #[derive(Debug, PartialEq, Eq)] -pub struct CollectedErrors { +pub struct FileDiagnostic { pub file_id: fm::FileId, - pub errors: Vec, + pub diagnostic: CustomDiagnostic, } diff --git a/crates/noirc_errors/src/reporter.rs b/crates/noirc_errors/src/reporter.rs index 60a5e07fcaf..dec215295ad 100644 --- a/crates/noirc_errors/src/reporter.rs +++ b/crates/noirc_errors/src/reporter.rs @@ -1,10 +1,9 @@ -use crate::{ReportedError, Span}; +use crate::{FileDiagnostic, ReportedError, Span}; use codespan_reporting::diagnostic::{Diagnostic, Label}; use codespan_reporting::term; use codespan_reporting::term::termcolor::{ Color, ColorChoice, ColorSpec, StandardStream, WriteColor, }; -use fm::FileId; use std::io::Write; #[derive(Debug, PartialEq, Eq)] @@ -57,6 +56,10 @@ impl CustomDiagnostic { } } + pub fn in_file(self, file_id: fm::FileId) -> FileDiagnostic { + FileDiagnostic { file_id, diagnostic: self } + } + pub fn add_note(&mut self, message: String) { self.notes.push(message); } @@ -64,6 +67,10 @@ impl CustomDiagnostic { pub fn add_secondary(&mut self, message: String, span: Span) { self.secondaries.push(CustomLabel::new(message, span)); } + + fn is_error(&self) -> bool { + matches!(self.kind, DiagnosticKind::Error) + } } impl std::fmt::Display for CustomDiagnostic { @@ -94,71 +101,72 @@ impl CustomLabel { } } -pub struct Reporter; - -impl Reporter { - /// Writes the given diagnostics to stderr and returns the count - /// of diagnostics that were errors. - pub fn with_diagnostics( - file_id: Option, - files: &fm::FileManager, - diagnostics: &[CustomDiagnostic], - allow_warnings: bool, - ) -> usize { - let mut error_count = 0; - - // Convert each Custom Diagnostic into a diagnostic - let diagnostics = diagnostics.iter().map(|cd| { - let diagnostic = match (cd.kind, allow_warnings) { - (DiagnosticKind::Warning, true) => Diagnostic::warning(), - _ => { - error_count += 1; - Diagnostic::error() - } - }; - - let secondary_labels = if let Some(f_id) = file_id { - cd.secondaries - .iter() - .map(|sl| { - let start_span = sl.span.start() as usize; - let end_span = sl.span.end() as usize + 1; - - Label::secondary(f_id.as_usize(), start_span..end_span) - .with_message(&sl.message) - }) - .collect() - } else { - Vec::new() - }; - diagnostic - .with_message(&cd.message) - .with_labels(secondary_labels) - .with_notes(cd.notes.clone()) - }); +/// Writes the given diagnostics to stderr and returns the count +/// of diagnostics that were errors. +pub fn report_all( + files: &fm::FileManager, + diagnostics: &[FileDiagnostic], + allow_warnings: bool, +) -> u32 { + diagnostics + .iter() + .map(|error| report(files, &error.diagnostic, Some(error.file_id), allow_warnings) as u32) + .sum() +} - let writer = StandardStream::stderr(ColorChoice::Always); - let config = codespan_reporting::term::Config::default(); +/// Report the given diagnostic, and return true if it was an error +pub fn report( + files: &fm::FileManager, + custom_diagnostic: &CustomDiagnostic, + file: Option, + allow_warnings: bool, +) -> bool { + let writer = StandardStream::stderr(ColorChoice::Always); + let config = codespan_reporting::term::Config::default(); - for diagnostic in diagnostics { - term::emit(&mut writer.lock(), &config, files.as_simple_files(), &diagnostic).unwrap(); - } + let diagnostic = convert_diagnostic(custom_diagnostic, file, allow_warnings); + term::emit(&mut writer.lock(), &config, files.as_simple_files(), &diagnostic).unwrap(); - error_count - } + !allow_warnings || custom_diagnostic.is_error() +} - pub fn finish(error_count: usize) -> Result<(), ReportedError> { - if error_count != 0 { - let writer = StandardStream::stderr(ColorChoice::Always); - let mut writer = writer.lock(); +fn convert_diagnostic( + cd: &CustomDiagnostic, + file: Option, + allow_warnings: bool, +) -> Diagnostic { + let diagnostic = match (cd.kind, allow_warnings) { + (DiagnosticKind::Warning, true) => Diagnostic::warning(), + _ => Diagnostic::error(), + }; + + let secondary_labels = if let Some(file_id) = file { + cd.secondaries + .iter() + .map(|sl| { + let start_span = sl.span.start() as usize; + let end_span = sl.span.end() as usize + 1; + Label::secondary(file_id.as_usize(), start_span..end_span).with_message(&sl.message) + }) + .collect() + } else { + vec![] + }; + + diagnostic.with_message(&cd.message).with_labels(secondary_labels).with_notes(cd.notes.clone()) +} - writer.set_color(ColorSpec::new().set_fg(Some(Color::Red))).unwrap(); - writeln!(&mut writer, "error: aborting due to {error_count} previous errors").unwrap(); - writer.reset().ok(); // Ignore any IO errors, we're exiting at this point anyway +pub fn finish_report(error_count: u32) -> Result<(), ReportedError> { + if error_count != 0 { + let writer = StandardStream::stderr(ColorChoice::Always); + let mut writer = writer.lock(); - Err(ReportedError) - } else { - Ok(()) - } + writer.set_color(ColorSpec::new().set_fg(Some(Color::Red))).unwrap(); + writeln!(&mut writer, "error: aborting due to {error_count} previous errors").unwrap(); + writer.reset().ok(); + + Err(ReportedError) + } else { + Ok(()) } } diff --git a/crates/noirc_evaluator/Cargo.toml b/crates/noirc_evaluator/Cargo.toml index d2a137afaa9..c428a68609b 100644 --- a/crates/noirc_evaluator/Cargo.toml +++ b/crates/noirc_evaluator/Cargo.toml @@ -17,3 +17,6 @@ iter-extended.workspace = true thiserror.workspace = true num-bigint = "0.4" num-traits = "0.2.8" + +[dev-dependencies] +rand="0.8.5" \ No newline at end of file diff --git a/crates/noirc_evaluator/src/errors.rs b/crates/noirc_evaluator/src/errors.rs index 358bd3717e3..e90726a1211 100644 --- a/crates/noirc_evaluator/src/errors.rs +++ b/crates/noirc_evaluator/src/errors.rs @@ -1,4 +1,4 @@ -use noirc_errors::{CustomDiagnostic as Diagnostic, DiagnosableError, Location}; +use noirc_errors::{CustomDiagnostic as Diagnostic, Location}; use thiserror::Error; #[derive(Debug)] @@ -64,11 +64,11 @@ impl RuntimeErrorKind { } } -impl DiagnosableError for RuntimeError { - fn to_diagnostic(&self) -> Diagnostic { +impl From for Diagnostic { + fn from(error: RuntimeError) -> Diagnostic { let span = - if let Some(loc) = self.location { loc.span } else { noirc_errors::Span::new(0..0) }; - match &self.kind { + if let Some(loc) = error.location { loc.span } else { noirc_errors::Span::new(0..0) }; + match &error.kind { RuntimeErrorKind::ArrayOutOfBounds { index, bound } => Diagnostic::simple_error( "index out of bounds".to_string(), format!("out of bounds error, index is {index} but length is {bound}"), diff --git a/crates/noirc_evaluator/src/lib.rs b/crates/noirc_evaluator/src/lib.rs index a7c5f43fead..a4b81469f2d 100644 --- a/crates/noirc_evaluator/src/lib.rs +++ b/crates/noirc_evaluator/src/lib.rs @@ -1,3 +1,4 @@ +#![forbid(unsafe_code)] mod errors; mod ssa; @@ -11,7 +12,7 @@ use errors::{RuntimeError, RuntimeErrorKind}; use iter_extended::btree_map; use noirc_abi::{AbiType, AbiVisibility}; use noirc_frontend::monomorphization::ast::*; -use ssa::{code_gen::IRGenerator, node}; +use ssa::{node, ssa_gen::IrGenerator}; use std::collections::BTreeMap; pub struct Evaluator { @@ -110,11 +111,11 @@ impl Evaluator { program: Program, enable_logging: bool, ) -> Result<(), RuntimeError> { - let mut ir_gen = IRGenerator::new(program); + let mut ir_gen = IrGenerator::new(program); self.parse_abi_alt(&mut ir_gen); // Now call the main function - ir_gen.codegen_main()?; + ir_gen.ssa_gen_main()?; //Generates ACIR representation: ir_gen.context.ir_to_acir(self, enable_logging)?; @@ -140,7 +141,7 @@ impl Evaluator { def: Definition, param_type: &AbiType, visibility: &AbiVisibility, - ir_gen: &mut IRGenerator, + ir_gen: &mut IrGenerator, ) -> Result<(), RuntimeErrorKind> { match param_type { AbiType::Field => { @@ -274,7 +275,7 @@ impl Evaluator { /// Noted in the noirc_abi, it is possible to convert Toml -> NoirTypes /// However, this intermediate representation is useful as it allows us to have /// intermediate Types which the core type system does not know about like Strings. - fn parse_abi_alt(&mut self, ir_gen: &mut IRGenerator) { + fn parse_abi_alt(&mut self, ir_gen: &mut IrGenerator) { // XXX: Currently, the syntax only supports public witnesses // u8 and arrays are assumed to be private // This is not a short-coming of the ABI, but of the grammar diff --git a/crates/noirc_evaluator/src/ssa/acir_gen.rs b/crates/noirc_evaluator/src/ssa/acir_gen.rs index 06388c94f6c..52fe4de857f 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen.rs @@ -29,7 +29,7 @@ pub struct Acir { impl Acir { /// Generate ACIR opcodes based on the given instruction - pub fn evaluate_instruction( + pub fn acir_gen_instruction( &mut self, ins: &Instruction, evaluator: &mut Evaluator, diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/internal_var.rs b/crates/noirc_evaluator/src/ssa/acir_gen/internal_var.rs index f3c23feb2c3..c614d940d50 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/internal_var.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/internal_var.rs @@ -1,5 +1,8 @@ use crate::{ - ssa::acir_gen::{const_from_expression, expression_to_witness, optional_expression_to_witness}, + ssa::acir_gen::{ + const_from_expression, expression_from_witness, expression_to_witness, + optional_expression_to_witness, + }, ssa::node::NodeId, Evaluator, }; @@ -46,6 +49,14 @@ impl InternalVar { &self.cached_witness } + pub fn to_expression(&self) -> Expression { + if let Some(w) = self.cached_witness { + expression_from_witness(w) + } else { + self.expression().clone() + } + } + /// If the InternalVar holds a constant expression /// Return that constant.Otherwise, return None. pub(super) fn to_const(&self) -> Option { diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/operations.rs b/crates/noirc_evaluator/src/ssa/acir_gen/operations.rs index b92fdb8c934..1af21131ee9 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/operations.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/operations.rs @@ -8,5 +8,6 @@ pub mod intrinsics; pub mod load; pub mod not; pub mod r#return; +mod sort; pub mod store; pub mod truncate; diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/operations/binary.rs b/crates/noirc_evaluator/src/ssa/acir_gen/operations/binary.rs index e95a613e31a..57bc0782d16 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/operations/binary.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/operations/binary.rs @@ -37,6 +37,9 @@ pub(crate) fn evaluate( let r_size = ctx[binary.rhs].size_in_bits(); let l_size = ctx[binary.lhs].size_in_bits(); let max_size = u32::max(r_size, l_size); + if binary.predicate == Some(ctx.zero()) { + return None; + } let binary_output = match &binary.operator { BinaryOp::Add | BinaryOp::SafeAdd => { diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/operations/intrinsics.rs b/crates/noirc_evaluator/src/ssa/acir_gen/operations/intrinsics.rs index 8beed8119cf..7feac203bb3 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/operations/intrinsics.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/operations/intrinsics.rs @@ -1,15 +1,23 @@ use crate::{ ssa::{ - acir_gen::{constraints::to_radix_base, InternalVar, InternalVarCache, MemoryMap}, + acir_gen::{ + constraints::{bound_constraint_with_offset, to_radix_base}, + expression_from_witness, + operations::sort::evaluate_permutation, + InternalVar, InternalVarCache, MemoryMap, + }, builtin, context::SsaContext, - mem::ArrayId, + mem::{ArrayId, Memory}, node::{self, Instruction, Node, NodeId, ObjectType}, }, Evaluator, }; use acvm::acir::{ - circuit::opcodes::{BlackBoxFuncCall, FunctionInput, Opcode as AcirOpcode}, + circuit::{ + directives::Directive, + opcodes::{BlackBoxFuncCall, FunctionInput, Opcode as AcirOpcode}, + }, native_types::{Expression, Witness}, }; use iter_extended::vecmap; @@ -66,6 +74,42 @@ pub(crate) fn evaluate( }; evaluator.opcodes.push(AcirOpcode::BlackBoxFuncCall(func_call)); } + Opcode::Sort => { + let mut in_expr = Vec::new(); + let array_id = Memory::deref(ctx, args[0]).unwrap(); + let array = &ctx.mem[array_id]; + let num_bits = array.element_type.bits(); + for i in 0..array.len { + in_expr.push( + memory_map.load_array_element_constant_index(array, i).unwrap().to_expression(), + ); + } + outputs = prepare_outputs(memory_map, instruction_id, array.len, ctx, evaluator); + let out_expr: Vec = + outputs.iter().map(|w| expression_from_witness(*w)).collect(); + for i in 0..(out_expr.len() - 1) { + bound_constraint_with_offset( + &out_expr[i], + &out_expr[i + 1], + &Expression::zero(), + num_bits, + evaluator, + ); + } + let bits = evaluate_permutation(&in_expr, &out_expr, evaluator); + let inputs = in_expr.iter().map(|a| vec![a.clone()]).collect(); + evaluator.opcodes.push(AcirOpcode::Directive(Directive::PermutationSort { + inputs, + tuple: 1, + bits, + sort_by: vec![0], + })); + if let node::ObjectType::Pointer(a) = res_type { + memory_map.map_array(a, &outputs, ctx); + } else { + unreachable!(); + } + } } // If more than witness is returned, diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/operations/sort.rs b/crates/noirc_evaluator/src/ssa/acir_gen/operations/sort.rs new file mode 100644 index 00000000000..430d1180b6e --- /dev/null +++ b/crates/noirc_evaluator/src/ssa/acir_gen/operations/sort.rs @@ -0,0 +1,172 @@ +use acvm::{ + acir::native_types::Expression, + acir::{circuit::opcodes::Opcode as AcirOpcode, native_types::Witness}, + FieldElement, +}; + +use crate::{ + ssa::acir_gen::{ + constraints::{add, mul_with_witness, subtract}, + expression_from_witness, + }, + Evaluator, +}; + +// Generate gates which ensure that out_expr is a permutation of in_expr +// Returns the control bits of the sorting network used to generate the constrains +pub fn evaluate_permutation( + in_expr: &Vec, + out_expr: &Vec, + evaluator: &mut Evaluator, +) -> Vec { + let (w, b) = permutation_layer(in_expr, evaluator); + // we contrain the network output to out_expr + for (b, o) in b.iter().zip(out_expr) { + evaluator.opcodes.push(AcirOpcode::Arithmetic(subtract(b, FieldElement::one(), o))); + } + w +} + +// Generates gates for a sorting network +// returns witness corresponding to the network configuration and the expressions corresponding to the network output +// in_expr: inputs of the sorting network +pub fn permutation_layer( + in_expr: &Vec, + evaluator: &mut Evaluator, +) -> (Vec, Vec) { + let n = in_expr.len(); + if n == 1 { + return (Vec::new(), in_expr.clone()); + } + let n1 = n / 2; + let mut conf = Vec::new(); + // witness for the input switches + for _ in 0..n1 { + conf.push(evaluator.add_witness_to_cs()); + } + // compute expressions after the input switches + // If inputs are a1,a2, and the switch value is c, then we compute expresions b1,b2 where + // b1 = a1+q, b2 = a2-q, q = c(a2-a1) + let mut in_sub1 = Vec::new(); + let mut in_sub2 = Vec::new(); + for i in 0..n1 { + //q = c*(a2-a1); + let intermediate = mul_with_witness( + evaluator, + &expression_from_witness(conf[i]), + &subtract(&in_expr[2 * i + 1], FieldElement::one(), &in_expr[2 * i]), + ); + //b1=a1+q + in_sub1.push(add(&intermediate, FieldElement::one(), &in_expr[2 * i])); + //b2=a2-q + in_sub2.push(subtract(&in_expr[2 * i + 1], FieldElement::one(), &intermediate)); + } + if n % 2 == 1 { + in_sub2.push(in_expr.last().unwrap().clone()); + } + let mut out_expr = Vec::new(); + // compute results for the sub networks + let (w1, b1) = permutation_layer(&in_sub1, evaluator); + let (w2, b2) = permutation_layer(&in_sub2, evaluator); + // apply the output swithces + for i in 0..(n - 1) / 2 { + let c = evaluator.add_witness_to_cs(); + conf.push(c); + let intermediate = mul_with_witness( + evaluator, + &expression_from_witness(c), + &subtract(&b2[i], FieldElement::one(), &b1[i]), + ); + out_expr.push(add(&intermediate, FieldElement::one(), &b1[i])); + out_expr.push(subtract(&b2[i], FieldElement::one(), &intermediate)); + } + if n % 2 == 0 { + out_expr.push(b1.last().unwrap().clone()); + } + out_expr.push(b2.last().unwrap().clone()); + conf.extend(w1); + conf.extend(w2); + (conf, out_expr) +} + +#[cfg(test)] +mod test { + use std::collections::BTreeMap; + + use acvm::{ + acir::{circuit::opcodes::BlackBoxFuncCall, native_types::Witness}, + FieldElement, OpcodeResolutionError, PartialWitnessGenerator, + }; + + use crate::{ + ssa::acir_gen::{expression_from_witness, operations::sort::evaluate_permutation}, + Evaluator, + }; + use rand::prelude::*; + + struct MockBackend {} + impl PartialWitnessGenerator for MockBackend { + fn solve_black_box_function_call( + _initial_witness: &mut BTreeMap, + _func_call: &BlackBoxFuncCall, + ) -> Result<(), OpcodeResolutionError> { + unreachable!(); + } + } + + // Check that a random network constrains its output to be a permutation of any random input + #[test] + fn test_permutation() { + let mut rng = rand::thread_rng(); + for n in 2..50 { + let mut eval = Evaluator { + current_witness_index: 0, + num_witnesses_abi_len: 0, + public_inputs: Vec::new(), + opcodes: Vec::new(), + }; + + //we generate random inputs + let mut input = Vec::new(); + let mut a_val = Vec::new(); + let mut b_wit = Vec::new(); + let mut solved_witness: BTreeMap = BTreeMap::new(); + for i in 0..n { + let w = eval.add_witness_to_cs(); + input.push(expression_from_witness(w)); + a_val.push(FieldElement::from(rng.next_u32() as i128)); + solved_witness.insert(w, a_val[i]); + } + + let mut output = Vec::new(); + for _i in 0..n { + let w = eval.add_witness_to_cs(); + b_wit.push(w); + output.push(expression_from_witness(w)); + } + //generate constraints for the inputs + let w = evaluate_permutation(&input, &output, &mut eval); + + //we generate random network + let mut c = Vec::new(); + for _i in 0..w.len() { + c.push(rng.next_u32() % 2 != 0); + } + // intialise bits + for i in 0..w.len() { + solved_witness.insert(w[i], FieldElement::from(c[i] as i128)); + } + // compute the network output by solving the constraints + let backend = MockBackend {}; + backend + .solve(&mut solved_witness, eval.opcodes.clone()) + .expect("Could not solve permutation constraints"); + let mut b_val = Vec::new(); + for i in 0..output.len() { + b_val.push(solved_witness[&b_wit[i]]); + } + // ensure the outputs are a permutation of the inputs + assert_eq!(a_val.sort(), b_val.sort()); + } + } +} diff --git a/crates/noirc_evaluator/src/ssa/anchor.rs b/crates/noirc_evaluator/src/ssa/anchor.rs index 2658e36b0b5..3de6348fd1d 100644 --- a/crates/noirc_evaluator/src/ssa/anchor.rs +++ b/crates/noirc_evaluator/src/ssa/anchor.rs @@ -110,7 +110,7 @@ impl Anchor { ctx: &SsaContext, id: NodeId, ) -> Result<(), RuntimeError> { - let ins = ctx.get_instruction(id); + let ins = ctx.instruction(id); let (array_id, index, is_load) = Anchor::get_mem_op(&ins.operation); self.use_array(array_id, ctx.mem[array_id].len as usize); let prev_list = self.mem_list.get_mut(&array_id).unwrap(); diff --git a/crates/noirc_evaluator/src/ssa/block.rs b/crates/noirc_evaluator/src/ssa/block.rs index 2aa472802d9..1906d82b812 100644 --- a/crates/noirc_evaluator/src/ssa/block.rs +++ b/crates/noirc_evaluator/src/ssa/block.rs @@ -430,7 +430,7 @@ pub fn short_circuit_instructions( Some(target), )); let nop = instructions[0]; - debug_assert_eq!(ctx.get_instruction(nop).operation, node::Operation::Nop); + debug_assert_eq!(ctx.instruction(nop).operation, node::Operation::Nop); let mut stack = vec![nop, unreachable_ins]; //return: for &i in instructions.iter() { @@ -463,13 +463,13 @@ pub fn zero_instructions(ctx: &mut SsaContext, instructions: &[NodeId], avoid: O let mut zeros = HashMap::new(); let mut zero_keys = Vec::new(); for i in instructions { - let ins = ctx.get_instruction(*i); + let ins = ctx.instruction(*i); if ins.res_type != node::ObjectType::NotAnObject { zeros.insert(ins.res_type, ctx.zero_with_type(ins.res_type)); } else if let node::Operation::Return(ret) = &ins.operation { for i in ret { if *i != NodeId::dummy() { - let typ = ctx.get_object_type(*i); + let typ = ctx.object_type(*i); assert_ne!(typ, node::ObjectType::NotAnObject); zero_keys.push(typ); } else { @@ -488,7 +488,7 @@ pub fn zero_instructions(ctx: &mut SsaContext, instructions: &[NodeId], avoid: O } for i in instructions.iter().filter(|x| Some(*x) != avoid) { - let ins = ctx.get_mut_instruction(*i); + let ins = ctx.instruction_mut(*i); if ins.res_type != node::ObjectType::NotAnObject { ins.mark = Mark::ReplaceWith(zeros[&ins.res_type]); } else if ins.operation.opcode() != Opcode::Nop { diff --git a/crates/noirc_evaluator/src/ssa/builtin.rs b/crates/noirc_evaluator/src/ssa/builtin.rs index ff6f14b6272..8c7ee5a9e24 100644 --- a/crates/noirc_evaluator/src/ssa/builtin.rs +++ b/crates/noirc_evaluator/src/ssa/builtin.rs @@ -1,13 +1,21 @@ -use crate::ssa::node::ObjectType; +use crate::ssa::{ + context::SsaContext, + node::{NodeId, ObjectType}, +}; use acvm::{acir::BlackBoxFunc, FieldElement}; use num_bigint::BigUint; use num_traits::{One, Zero}; +/// Opcode here refers to either a black box function +/// defined in ACIR, or a function which has its +/// function signature defined in Noir, but its +/// function definition is implemented in the compiler. #[derive(Clone, Debug, Hash, Copy, PartialEq, Eq)] pub enum Opcode { LowLevel(BlackBoxFunc), ToBits, ToRadix, + Sort, } impl std::fmt::Display for Opcode { @@ -17,19 +25,26 @@ impl std::fmt::Display for Opcode { } impl Opcode { + /// Searches for an `Opcode` using its + /// string equivalent name. + /// + /// Returns `None` if there is no string that + /// corresponds to any of the opcodes. pub fn lookup(op_name: &str) -> Option { match op_name { "to_le_bits" => Some(Opcode::ToBits), "to_radix" => Some(Opcode::ToRadix), + "arraysort" => Some(Opcode::Sort), _ => BlackBoxFunc::lookup(op_name).map(Opcode::LowLevel), } } - pub fn name(&self) -> &str { + fn name(&self) -> &str { match self { Opcode::LowLevel(op) => op.name(), Opcode::ToBits => "to_le_bits", Opcode::ToRadix => "to_radix", + Opcode::Sort => "arraysort", } } @@ -37,42 +52,54 @@ impl Opcode { match self { Opcode::LowLevel(op) => { match op { + // Pointers do not overflow BlackBoxFunc::SHA256 | BlackBoxFunc::Blake2s | BlackBoxFunc::Pedersen - | BlackBoxFunc::FixedBaseScalarMul => BigUint::zero(), //pointers do not overflow + | BlackBoxFunc::FixedBaseScalarMul => BigUint::zero(), + // Verify returns zero or one BlackBoxFunc::SchnorrVerify | BlackBoxFunc::EcdsaSecp256k1 - | BlackBoxFunc::MerkleMembership => BigUint::one(), //verify returns 0 or 1 + | BlackBoxFunc::MerkleMembership => BigUint::one(), BlackBoxFunc::HashToField128Security => ObjectType::NativeField.max_size(), - _ => todo!("max value must be implemented for opcode {} ", op), + BlackBoxFunc::AES => { + todo!("ICE: AES is unimplemented") + } + BlackBoxFunc::RANGE | BlackBoxFunc::AND | BlackBoxFunc::XOR => { + unimplemented!("ICE: these opcodes do not have Noir builtin functions") + } } } - Opcode::ToBits | Opcode::ToRadix => BigUint::zero(), //pointers do not overflow + Opcode::ToBits | Opcode::ToRadix | Opcode::Sort => BigUint::zero(), //pointers do not overflow } } - //Returns the number of elements and their type, of the output result corresponding to the OPCODE function. - pub fn get_result_type(&self) -> (u32, ObjectType) { + /// Returns the number of elements that the `Opcode` should return + /// and the type. + pub fn get_result_type(&self, args: &[NodeId], ctx: &SsaContext) -> (u32, ObjectType) { match self { Opcode::LowLevel(op) => { match op { - BlackBoxFunc::AES => (0, ObjectType::NotAnObject), //Not implemented - BlackBoxFunc::SHA256 => (32, ObjectType::Unsigned(8)), - BlackBoxFunc::Blake2s => (32, ObjectType::Unsigned(8)), + BlackBoxFunc::AES => todo!("ICE: AES is unimplemented"), + BlackBoxFunc::SHA256 | BlackBoxFunc::Blake2s => (32, ObjectType::Unsigned(8)), BlackBoxFunc::HashToField128Security => (1, ObjectType::NativeField), - BlackBoxFunc::MerkleMembership => (1, ObjectType::NativeField), //or bool? - BlackBoxFunc::SchnorrVerify => (1, ObjectType::NativeField), //or bool? + // See issue #775 on changing this to return a boolean + BlackBoxFunc::MerkleMembership + | BlackBoxFunc::SchnorrVerify + | BlackBoxFunc::EcdsaSecp256k1 => (1, ObjectType::NativeField), BlackBoxFunc::Pedersen => (2, ObjectType::NativeField), - BlackBoxFunc::EcdsaSecp256k1 => (1, ObjectType::NativeField), //field? BlackBoxFunc::FixedBaseScalarMul => (2, ObjectType::NativeField), - BlackBoxFunc::AND => (1, ObjectType::NativeField), - BlackBoxFunc::XOR => (1, ObjectType::NativeField), - BlackBoxFunc::RANGE => (0, ObjectType::NotAnObject), + BlackBoxFunc::RANGE | BlackBoxFunc::AND | BlackBoxFunc::XOR => { + unreachable!("ICE: these opcodes do not have Noir builtin functions") + } } } Opcode::ToBits => (FieldElement::max_num_bits(), ObjectType::Boolean), Opcode::ToRadix => (FieldElement::max_num_bits(), ObjectType::NativeField), + Opcode::Sort => { + let a = super::mem::Memory::deref(ctx, args[0]).unwrap(); + (ctx.mem[a].len, ctx.mem[a].element_type) + } } } } diff --git a/crates/noirc_evaluator/src/ssa/conditional.rs b/crates/noirc_evaluator/src/ssa/conditional.rs index 1479a2487ff..8dc535249f5 100644 --- a/crates/noirc_evaluator/src/ssa/conditional.rs +++ b/crates/noirc_evaluator/src/ssa/conditional.rs @@ -507,7 +507,7 @@ impl DecisionTree { ass_value = self[predicate].value.unwrap_or_else(NodeId::dummy); } assert!(!ctx.is_zero(ass_value), "code should have been already simplified"); - let ins1 = ctx.get_instruction(ins_id); + let ins1 = ctx.instruction(ins_id); match &ins1.operation { Operation::Call { returned_arrays, .. } => { for a in returned_arrays { @@ -529,7 +529,7 @@ impl DecisionTree { let ins = ins1.clone(); if short_circuit { stack.set_zero(ctx, ins.res_type); - let ins2 = ctx.get_mut_instruction(ins_id); + let ins2 = ctx.instruction_mut(ins_id); if ins2.res_type == ObjectType::NotAnObject { ins2.mark = Mark::Deleted; } else { @@ -540,7 +540,7 @@ impl DecisionTree { Operation::Phi { block_args, .. } => { if ctx[stack.block].kind == BlockType::IfJoin { assert_eq!(block_args.len(), 2); - let ins2 = ctx.get_mut_instruction(ins_id); + let ins2 = ctx.instruction_mut(ins_id); ins2.operation = Operation::Cond { condition: ass_cond, val_true: block_args[0].0, @@ -604,7 +604,7 @@ impl DecisionTree { return Ok(false); } if ctx.under_assumption(cond) { - let ins2 = ctx.get_mut_instruction(ins_id); + let ins2 = ctx.instruction_mut(ins_id); ins2.operation = Operation::Binary(crate::node::Binary { lhs: binary_op.lhs, rhs: binary_op.rhs, @@ -651,7 +651,7 @@ impl DecisionTree { stack.push(dummy); stack.push(cond); //store the conditional value - let ins2 = ctx.get_mut_instruction(ins_id); + let ins2 = ctx.instruction_mut(ins_id); ins2.operation = Operation::Store { array_id: *array_id, index: *index, @@ -670,7 +670,7 @@ impl DecisionTree { let name = array.name.to_string() + DUPLICATED; ctx.new_array(&name, array.element_type, array.len, None); let array_dup = ctx.mem.last_id(); - let ins2 = ctx.get_mut_instruction(ins_id); + let ins2 = ctx.instruction_mut(ins_id); ins2.res_type = ObjectType::Pointer(array_dup); let mut memcpy_stack = StackFrame::new(stack.block); @@ -699,7 +699,7 @@ impl DecisionTree { } => { if ctx.under_assumption(ass_value) { assert!(*ins_pred == AssumptionId::dummy()); - let mut ins2 = ctx.get_mut_instruction(ins_id); + let mut ins2 = ctx.instruction_mut(ins_id); ins2.operation = Operation::Call { func: *func_id, arguments: arguments.clone(), @@ -726,7 +726,7 @@ impl DecisionTree { Some(stack.block), )); stack.push(cond); - let ins2 = ctx.get_mut_instruction(ins_id); + let ins2 = ctx.instruction_mut(ins_id); ins2.operation = Operation::Constrain(cond, *loc); if ctx.is_zero(*expr) { stack.push(ins_id); @@ -760,15 +760,15 @@ impl DecisionTree { // 1. find potential matches between the two blocks let mut candidates = Vec::new(); let keep_call_and_store = |node_id: NodeId| -> bool { - let ins = ctx.get_instruction(node_id); + let ins = ctx.instruction(node_id); matches!(ins.operation.opcode(), Opcode::Call(_) | Opcode::Store(_)) }; let l_iter = left.iter().enumerate().filter(|&i| keep_call_and_store(*i.1)); let mut r_iter = right.iter().enumerate().filter(|&i| keep_call_and_store(*i.1)); for left_node in l_iter { - let left_ins = ctx.get_instruction(*left_node.1); + let left_ins = ctx.instruction(*left_node.1); for right_node in r_iter.by_ref() { - let right_ins = ctx.get_instruction(*right_node.1); + let right_ins = ctx.instruction(*right_node.1); match (&left_ins.operation, &right_ins.operation) { ( Operation::Call { func: left_func, returned_arrays: left_arrays, .. }, @@ -809,8 +809,8 @@ impl DecisionTree { result.extend_from_slice(&right[right_pos..i.right.0]); right_pos = i.right.0; //merge i: - let left_ins = ctx.get_instruction(left[left_pos]); - let right_ins = ctx.get_instruction(right[right_pos]); + let left_ins = ctx.instruction(left[left_pos]); + let right_ins = ctx.instruction(right[right_pos]); let assumption = &self[ctx[block_id].assumption]; let mut to_merge = Vec::new(); @@ -832,7 +832,7 @@ impl DecisionTree { val_true: *a.1, val_false: right_arg[a.0], }; - let typ = ctx.get_object_type(*a.1); + let typ = ctx.object_type(*a.1); to_merge.push(Instruction::new(op, typ, Some(block_id))); } Operation::Call { @@ -876,10 +876,10 @@ impl DecisionTree { if let Operation::Call { arguments, .. } = &mut merged_op { *arguments = merge_ids; } - let left_ins = ctx.get_mut_instruction(left[left_pos]); + let left_ins = ctx.instruction_mut(left[left_pos]); left_ins.mark = node::Mark::ReplaceWith(right[right_pos]); } - let ins1 = ctx.get_mut_instruction(right[right_pos]); + let ins1 = ctx.instruction_mut(right[right_pos]); ins1.operation = merged_op; result.push(ins1.id); left_pos += 1; diff --git a/crates/noirc_evaluator/src/ssa/context.rs b/crates/noirc_evaluator/src/ssa/context.rs index 0e70ba69ad5..58f9d9d94cf 100644 --- a/crates/noirc_evaluator/src/ssa/context.rs +++ b/crates/noirc_evaluator/src/ssa/context.rs @@ -3,7 +3,7 @@ use crate::ssa::{ acir_gen::Acir, block::{BasicBlock, BlockId}, conditional::{DecisionTree, TreeBuilder}, - function::{FuncIndex, SSAFunction}, + function::{FuncIndex, SsaFunction}, inline::StackFrame, mem::{ArrayId, Memory}, node::{ @@ -32,7 +32,7 @@ pub struct SsaContext { pub sealed_blocks: HashSet, pub mem: Memory, - pub functions: HashMap, + pub functions: HashMap, pub opcode_ids: HashMap, //Adjacency Matrix of the call graph; list of rows where each row indicates the functions called by the function whose FuncIndex is the row number @@ -83,7 +83,7 @@ impl SsaContext { if id == NodeId::dummy() { return false; } - let typ = self.get_object_type(id); + let typ = self.object_type(id); if let Some(one) = self.find_const_with_type(&BigUint::one(), typ) { id == one } else { @@ -95,7 +95,7 @@ impl SsaContext { if id == NodeId::dummy() { return false; } - let typ = self.get_object_type(id); + let typ = self.object_type(id); if let Some(zero) = self.find_const_with_type(&BigUint::zero(), typ) { id == zero } else { @@ -374,7 +374,7 @@ impl SsaContext { id } - pub fn get_ssa_func(&self, func_id: FuncId) -> Option<&SSAFunction> { + pub fn ssa_func(&self, func_id: FuncId) -> Option<&SsaFunction> { self.functions.get(&func_id) } @@ -385,8 +385,8 @@ impl SsaContext { } } - pub fn try_get_ssa_func(&self, id: NodeId) -> Option<&SSAFunction> { - self.try_get_func_id(id).and_then(|id| self.get_ssa_func(id)) + pub fn try_get_ssa_func(&self, id: NodeId) -> Option<&SsaFunction> { + self.try_get_func_id(id).and_then(|id| self.ssa_func(id)) } pub fn dummy_id() -> arena::Index { @@ -401,7 +401,7 @@ impl SsaContext { self.nodes.get_mut(id.0) } - pub fn get_object_type(&self, id: NodeId) -> node::ObjectType { + pub fn object_type(&self, id: NodeId) -> node::ObjectType { self[id].get_type() } @@ -413,11 +413,11 @@ impl SsaContext { None } - pub fn get_instruction(&self, id: NodeId) -> &node::Instruction { + pub fn instruction(&self, id: NodeId) -> &node::Instruction { self.try_get_instruction(id).expect("Index not found or not an instruction") } - pub fn get_mut_instruction(&mut self, id: NodeId) -> &mut node::Instruction { + pub fn instruction_mut(&mut self, id: NodeId) -> &mut node::Instruction { self.try_get_mut_instruction(id).expect("Index not found or not an instruction") } @@ -479,8 +479,8 @@ impl SsaContext { None } - pub fn get_root_value(&self, id: NodeId) -> NodeId { - self.get_variable(id).map(|v| v.get_root()).unwrap_or(id) + pub fn root_value(&self, id: NodeId) -> NodeId { + self.get_variable(id).map(|v| v.root()).unwrap_or(id) } pub fn add_variable(&mut self, obj: node::Variable, root: Option) -> NodeId { @@ -502,7 +502,7 @@ impl SsaContext { new_value: NodeId, block_id: BlockId, ) { - let root_id = self.get_root_value(var_id); + let root_id = self.root_value(var_id); let root = self.get_variable(root_id).unwrap(); let root_name = root.name.clone(); let cb = &mut self[block_id]; @@ -604,7 +604,7 @@ impl SsaContext { }) } - //Return the type of the operation result, based on the left hand type + // Return the type of the operation result, based on the left hand type pub fn get_result_type(&self, op: &Operation, lhs_type: node::ObjectType) -> node::ObjectType { use {BinaryOp::*, Operation::*}; match op { @@ -656,7 +656,7 @@ impl SsaContext { (self.add_variable(new_var, None), array_index) } - //returns the value of the element array[index], if it exists in the memory_map + // Returns the value of the element array[index], if it exists in the memory_map pub fn get_indexed_value(&self, array_id: ArrayId, index: NodeId) -> Option<&NodeId> { if let Some(idx) = Memory::to_u32(self, index) { self.mem.get_value_from_map(array_id, idx) @@ -664,7 +664,7 @@ impl SsaContext { None } } - //blocks///////////////////////// + pub fn try_get_block_mut(&mut self, id: BlockId) -> Option<&mut block::BasicBlock> { self.blocks.get_mut(id.0) } @@ -753,8 +753,8 @@ impl SsaContext { let mut fb = Some(&self[self.first_block]); while let Some(block) = fb { for iter in &block.instructions { - let ins = self.get_instruction(*iter); - acir.evaluate_instruction(ins, evaluator, self).map_err(RuntimeError::from)?; + let ins = self.instruction(*iter); + acir.acir_gen_instruction(ins, evaluator, self).map_err(RuntimeError::from)?; } //TODO we should rather follow the jumps fb = block.left.map(|block_id| &self[block_id]); @@ -775,7 +775,7 @@ impl SsaContext { } } - let v_type = self.get_object_type(phi_root); + let v_type = self.object_type(phi_root); let operation = Operation::Phi { root: phi_root, block_args: vec![] }; let new_phi = Instruction::new(operation, v_type, Some(target_block)); let phi_id = self.add_instruction(new_phi); @@ -823,8 +823,8 @@ impl SsaContext { index: Option, rhs: NodeId, ) -> Result { - let lhs_type = self.get_object_type(lhs); - let rhs_type = self.get_object_type(rhs); + let lhs_type = self.object_type(lhs); + let rhs_type = self.object_type(rhs); let mut ret_array = None; if let Some(Instruction { @@ -849,7 +849,7 @@ impl SsaContext { //Issue #579: we initialize the array, unless it is also in arguments in which case it is already initialized. let mut init = false; for i in arguments.clone() { - if let ObjectType::Pointer(b) = self.get_object_type(i) { + if let ObjectType::Pointer(b) = self.object_type(i) { if a == b { init = true; } @@ -907,7 +907,7 @@ impl SsaContext { witness: None, parent_block: self.current_block, }; - let ls_root = lhs_obj.get_root(); + let ls_root = lhs_obj.root(); //ssa: we create a new variable a1 linked to a let new_var_id = self.add_variable(new_var, Some(ls_root)); let op = Operation::Binary(node::Binary { @@ -979,8 +979,8 @@ impl SsaContext { stack_frame: &mut inline::StackFrame, block_id: BlockId, ) -> NodeId { - let lhs_type = self.get_object_type(lhs); - let rhs_type = self.get_object_type(rhs); + let lhs_type = self.object_type(lhs); + let rhs_type = self.object_type(rhs); if let ObjectType::Pointer(a) = lhs_type { //Array let b = stack_frame.get_or_default(a); @@ -998,7 +998,7 @@ impl SsaContext { witness: None, parent_block: self.current_block, }; - let ls_root = lhs_obj.get_root(); + let ls_root = lhs_obj.root(); //ssa: we create a new variable a1 linked to a let new_var_id = self.add_variable(new_var, Some(ls_root)); //ass @@ -1040,7 +1040,7 @@ impl SsaContext { let block1 = self[exit_block].predecessor[0]; let block2 = self[exit_block].predecessor[1]; - let a_type = self.get_object_type(a); + let a_type = self.object_type(a); let name = format!("if_{}_ret{c}", exit_block.0.into_raw_parts().0); *c += 1; @@ -1093,7 +1093,7 @@ impl SsaContext { } pub fn function_already_compiled(&self, func_id: FuncId) -> bool { - self.get_ssa_func(func_id).is_some() + self.ssa_func(func_id).is_some() } pub fn get_or_create_opcode_node_id(&mut self, opcode: builtin::Opcode) -> NodeId { diff --git a/crates/noirc_evaluator/src/ssa/function.rs b/crates/noirc_evaluator/src/ssa/function.rs index 5012665ce37..34f98c7a886 100644 --- a/crates/noirc_evaluator/src/ssa/function.rs +++ b/crates/noirc_evaluator/src/ssa/function.rs @@ -1,11 +1,11 @@ use crate::errors::RuntimeError; use crate::ssa::{ block::BlockId, - code_gen::IRGenerator, conditional::{AssumptionId, DecisionTree, TreeBuilder}, context::SsaContext, mem::ArrayId, node::{Node, NodeId, ObjectType, Opcode, Operation}, + ssa_gen::IrGenerator, {block, builtin, node, ssa_form}, }; use iter_extended::try_vecmap; @@ -22,7 +22,7 @@ impl FuncIndex { } #[derive(Clone, Debug)] -pub struct SSAFunction { +pub struct SsaFunction { pub entry_block: BlockId, pub id: FuncId, pub idx: FuncIndex, @@ -35,15 +35,15 @@ pub struct SSAFunction { pub decision: DecisionTree, } -impl SSAFunction { +impl SsaFunction { pub fn new( id: FuncId, name: &str, block_id: BlockId, idx: FuncIndex, ctx: &mut SsaContext, - ) -> SSAFunction { - SSAFunction { + ) -> SsaFunction { + SsaFunction { entry_block: block_id, id, node_id: ctx.push_function_id(id, name), @@ -55,7 +55,7 @@ impl SSAFunction { } } - pub fn compile(&self, ir_gen: &mut IRGenerator) -> Result { + pub fn compile(&self, ir_gen: &mut IrGenerator) -> Result { let function_cfg = block::bfs(self.entry_block, None, &ir_gen.context); block::compute_sub_dom(&mut ir_gen.context, &function_cfg); //Optimization @@ -68,7 +68,7 @@ impl SSAFunction { let mut decision = DecisionTree::new(&ir_gen.context); let mut builder = TreeBuilder::new(self.entry_block); for (arg, _) in &self.arguments { - if let ObjectType::Pointer(a) = ir_gen.context.get_object_type(*arg) { + if let ObjectType::Pointer(a) = ir_gen.context.object_type(*arg) { builder.stack.created_arrays.insert(a, self.entry_block); } } @@ -88,7 +88,7 @@ impl SSAFunction { ); if self.entry_block != exit { for i in &stack { - ir_gen.context.get_mut_instruction(*i).parent_block = self.entry_block; + ir_gen.context.instruction_mut(*i).parent_block = self.entry_block; } } @@ -137,7 +137,7 @@ impl SSAFunction { } } -impl IRGenerator { +impl IrGenerator { /// Creates an ssa function and returns its type upon success pub fn create_function( &mut self, @@ -150,7 +150,7 @@ impl IRGenerator { let function = &mut self.program[func_id]; let mut func = - SSAFunction::new(func_id, &function.name, func_block, index, &mut self.context); + SsaFunction::new(func_id, &function.name, func_block, index, &mut self.context); //arguments: for (param_id, mutable, name, typ) in std::mem::take(&mut function.parameters) { @@ -173,7 +173,7 @@ impl IRGenerator { self.context.functions.insert(func_id, func.clone()); let function_body = self.program.take_function_body(func_id); - let last_value = self.codegen_expression(&function_body)?; + let last_value = self.ssa_gen_expression(&function_body)?; let return_values = last_value.to_node_ids(); func.result_types.clear(); @@ -227,8 +227,8 @@ impl IRGenerator { //generates an instruction for calling the function pub fn call(&mut self, call: &Call) -> Result, RuntimeError> { - let func = self.codegen_expression(&call.func)?.unwrap_id(); - let arguments = self.codegen_expression_list(&call.arguments); + let func = self.ssa_gen_expression(&call.func)?.unwrap_id(); + let arguments = self.ssa_gen_expression_list(&call.arguments); if let Some(opcode) = self.context.get_builtin_opcode(func) { return self.call_low_level(opcode, arguments); @@ -249,7 +249,7 @@ impl IRGenerator { self.context.new_instruction(call_op.clone(), ObjectType::NotAnObject)?; if let Some(id) = self.context.try_get_func_id(func) { - let callee = self.context.get_ssa_func(id).unwrap().idx; + let callee = self.context.ssa_func(id).unwrap().idx; if let Some(caller) = self.function_context { update_call_graph(&mut self.context.call_graph, caller, callee); } @@ -269,7 +269,7 @@ impl IRGenerator { *i.1, )?); } - let ssa_func = self.context.get_ssa_func(func_id).unwrap(); + let ssa_func = self.context.ssa_func(func_id).unwrap(); let func_arguments = ssa_func.arguments.clone(); for (caller_arg, func_arg) in arguments.iter().zip(func_arguments) { let mut is_array_result = false; @@ -299,7 +299,7 @@ impl IRGenerator { // Fixup the returned_arrays, they will be incorrectly tracked for higher order functions // otherwise. - self.context.get_mut_instruction(call_instruction).operation = call_op; + self.context.instruction_mut(call_instruction).operation = call_op; result_ids } @@ -333,7 +333,7 @@ impl IRGenerator { op: builtin::Opcode, args: Vec, ) -> Result, RuntimeError> { - let (len, elem_type) = op.get_result_type(); + let (len, elem_type) = op.get_result_type(&args, &self.context); let result_type = if len > 1 { //We create an array that will contain the result and set the res_type to point to that array @@ -342,7 +342,6 @@ impl IRGenerator { } else { elem_type }; - //when the function returns an array, we use ins.res_type(array) //else we map ins.id to the returned witness let id = self.context.new_instruction(node::Operation::Intrinsic(op, args), result_type)?; diff --git a/crates/noirc_evaluator/src/ssa/inline.rs b/crates/noirc_evaluator/src/ssa/inline.rs index f25786d47e2..4b009d99b5a 100644 --- a/crates/noirc_evaluator/src/ssa/inline.rs +++ b/crates/noirc_evaluator/src/ssa/inline.rs @@ -39,7 +39,7 @@ pub fn inline_cfg( to_inline: Option, ) -> Result { let mut result = true; - let func = ctx.get_ssa_func(func_id).unwrap(); + let func = ctx.ssa_func(func_id).unwrap(); let func_cfg = block::bfs(func.entry_block, None, ctx); let decision = func.decision.clone(); for block_id in func_cfg { @@ -78,7 +78,7 @@ fn inline_block( let mut result = true; for (ins_id, f, args, arrays, parent_block) in call_ins { if let Some(func_id) = ctx.try_get_func_id(f) { - let f_copy = ctx.get_ssa_func(func_id).unwrap().clone(); + let f_copy = ctx.ssa_func(func_id).unwrap().clone(); if !inline(ctx, &f_copy, &args, &arrays, parent_block, ins_id, decision)? { result = false; } @@ -185,7 +185,7 @@ impl StackFrame { //Return false if the inlined function performs a function call pub fn inline( ctx: &mut SsaContext, - ssa_func: &function::SSAFunction, + ssa_func: &function::SsaFunction, args: &[NodeId], arrays: &[(ArrayId, u32)], block: BlockId, @@ -209,8 +209,8 @@ pub fn inline( //2. by copy parameters: for (&arg_caller, &arg_function) in args.iter().zip(&func_arg) { //pass by-ref const array arguments - if let node::ObjectType::Pointer(x) = ctx.get_object_type(arg_function.0) { - if let node::ObjectType::Pointer(y) = ctx.get_object_type(arg_caller) { + if let node::ObjectType::Pointer(x) = ctx.object_type(arg_function.0) { + if let node::ObjectType::Pointer(y) = ctx.object_type(arg_caller) { if !arg_function.1 && !stack_frame.array_map.contains_key(&x) { stack_frame.array_map.insert(x, y); continue; @@ -254,12 +254,11 @@ pub fn inline_in_block( let block_func = &ctx[block_id]; let next_block = block_func.left; let block_func_instructions = &block_func.instructions.clone(); - let predicate = - if let Operation::Call { predicate, .. } = &ctx.get_instruction(call_id).operation { - *predicate - } else { - unreachable!("invalid call id"); - }; + let predicate = if let Operation::Call { predicate, .. } = &ctx.instruction(call_id).operation { + *predicate + } else { + unreachable!("invalid call id"); + }; let mut short_circuit = false; *nested_call = false; @@ -295,7 +294,7 @@ pub fn inline_in_block( result.mark = Mark::ReplaceWith(*value); } } - let call_ins = ctx.get_mut_instruction(call_id); + let call_ins = ctx.instruction_mut(call_id); call_ins.mark = Mark::Deleted; } Operation::Call { .. } => { @@ -438,7 +437,7 @@ impl node::Operation { return id; } } - function::SSAFunction::get_mapped_value(Some(&id), ctx, inline_map, block_id) + function::SsaFunction::get_mapped_value(Some(&id), ctx, inline_map, block_id) }); } //However we deliberately not use the default case to force review of the behavior if a new type of operation is added. @@ -451,7 +450,7 @@ impl node::Operation { | Operation::Return(_) | Operation::Load {.. } | Operation::Store { .. } | Operation::Call { .. } => { self.map_id_mut(|id| { - function::SSAFunction::get_mapped_value(Some(&id), ctx, inline_map, block_id) + function::SsaFunction::get_mapped_value(Some(&id), ctx, inline_map, block_id) }); } } diff --git a/crates/noirc_evaluator/src/ssa/integer.rs b/crates/noirc_evaluator/src/ssa/integer.rs index 95cb4e10fab..20e3b5a26fa 100644 --- a/crates/noirc_evaluator/src/ssa/integer.rs +++ b/crates/noirc_evaluator/src/ssa/integer.rs @@ -442,7 +442,6 @@ fn get_load_max( max_map: &mut HashMap, value_map: &HashMap, array: ArrayId, - // obj_type: ObjectType, ) -> BigUint { if let Some(&value) = ctx.get_indexed_value(array, address) { return get_obj_max_value(ctx, value, max_map, value_map); diff --git a/crates/noirc_evaluator/src/ssa/mem.rs b/crates/noirc_evaluator/src/ssa/mem.rs index 6510a962108..897ce25e8ab 100644 --- a/crates/noirc_evaluator/src/ssa/mem.rs +++ b/crates/noirc_evaluator/src/ssa/mem.rs @@ -93,7 +93,10 @@ impl Memory { self.last_adr += len; id } - + /// Coerces a FieldElement into a u32 + /// By taking its value modulo 2^32 + /// + /// See issue #785 on whether this is safe pub fn as_u32(value: FieldElement) -> u32 { let big_v = BigUint::from_bytes_be(&value.to_be_bytes()); let mut modulus = BigUint::from(2_u32); diff --git a/crates/noirc_evaluator/src/ssa/mod.rs b/crates/noirc_evaluator/src/ssa/mod.rs index 2512890fc1a..85227d6c174 100644 --- a/crates/noirc_evaluator/src/ssa/mod.rs +++ b/crates/noirc_evaluator/src/ssa/mod.rs @@ -1,15 +1,16 @@ pub mod acir_gen; -pub mod anchor; -pub mod block; -pub mod builtin; -pub mod code_gen; -pub mod conditional; -pub mod context; -pub mod flatten; -pub mod function; -pub mod inline; -pub mod integer; -pub mod mem; +mod anchor; +mod block; +mod builtin; +mod conditional; +mod context; +mod flatten; +mod function; +mod inline; +mod integer; +mod mem; pub mod node; -pub mod optimizations; -pub mod ssa_form; +mod optimizations; +mod ssa_form; +pub mod ssa_gen; +mod value; diff --git a/crates/noirc_evaluator/src/ssa/node.rs b/crates/noirc_evaluator/src/ssa/node.rs index 2efea1a07a5..5504325ed64 100644 --- a/crates/noirc_evaluator/src/ssa/node.rs +++ b/crates/noirc_evaluator/src/ssa/node.rs @@ -14,7 +14,7 @@ use std::ops::{Add, BitAnd, BitOr, BitXor, Mul, Shl, Shr, Sub}; pub trait Node: std::fmt::Display { fn get_type(&self) -> ObjectType; - fn get_id(&self) -> NodeId; + fn id(&self) -> NodeId; fn size_in_bits(&self) -> u32; } @@ -52,7 +52,7 @@ impl Node for Variable { self.get_type().bits() } - fn get_id(&self) -> NodeId { + fn id(&self) -> NodeId { self.id } } @@ -76,11 +76,11 @@ impl Node for NodeObject { } } - fn get_id(&self) -> NodeId { + fn id(&self) -> NodeId { match self { - NodeObject::Obj(o) => o.get_id(), + NodeObject::Obj(o) => o.id(), NodeObject::Instr(i) => i.id, - NodeObject::Const(c) => c.get_id(), + NodeObject::Const(c) => c.id(), NodeObject::Function(_, id, _) => *id, } } @@ -95,7 +95,7 @@ impl Node for Constant { self.value.bits().try_into().unwrap() } - fn get_id(&self) -> NodeId { + fn id(&self) -> NodeId { self.id } } @@ -149,7 +149,7 @@ pub struct Variable { } impl Variable { - pub fn get_root(&self) -> NodeId { + pub fn root(&self) -> NodeId { self.root.unwrap_or(self.id) } @@ -173,17 +173,12 @@ impl Variable { #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] pub enum ObjectType { - //Numeric(NumericType), NativeField, - // custom_field(BigUint), //TODO requires copy trait for BigUint Boolean, Unsigned(u32), //bit size Signed(u32), //bit size Pointer(ArrayId), - Function, - //TODO big_int - //TODO floats NotAnObject, //not an object } @@ -497,7 +492,6 @@ pub enum Operation { root: NodeId, block_args: Vec<(NodeId, BlockId)>, }, - //Call(function::FunctionCall), Call { func: NodeId, arguments: Vec, @@ -704,7 +698,11 @@ impl Binary { } fn zero_div_error(&self) -> Result<(), RuntimeError> { - Err(RuntimeErrorKind::Spanless("Panic - division by zero".to_string()).into()) + if self.predicate.is_none() { + Err(RuntimeErrorKind::Spanless("Panic - division by zero".to_string()).into()) + } else { + Ok(()) + } } fn evaluate( @@ -719,8 +717,8 @@ impl Binary { { let l_eval = eval_fn(ctx, self.lhs)?; let r_eval = eval_fn(ctx, self.rhs)?; - let l_type = ctx.get_object_type(self.lhs); - let r_type = ctx.get_object_type(self.rhs); + let l_type = ctx.object_type(self.lhs); + let r_type = ctx.object_type(self.rhs); let lhs = l_eval.into_const_value(); let rhs = r_eval.into_const_value(); diff --git a/crates/noirc_evaluator/src/ssa/optimizations.rs b/crates/noirc_evaluator/src/ssa/optimizations.rs index f002fd9337e..f66ce3e817f 100644 --- a/crates/noirc_evaluator/src/ssa/optimizations.rs +++ b/crates/noirc_evaluator/src/ssa/optimizations.rs @@ -9,7 +9,7 @@ use crate::ssa::{ use acvm::FieldElement; pub fn simplify_id(ctx: &mut SsaContext, ins_id: NodeId) -> Result<(), RuntimeError> { - let mut ins = ctx.get_instruction(ins_id).clone(); + let mut ins = ctx.instruction(ins_id).clone(); simplify(ctx, &mut ins)?; ctx[ins_id] = super::node::NodeObject::Instr(ins); Ok(()) @@ -21,12 +21,6 @@ pub fn simplify(ctx: &mut SsaContext, ins: &mut Instruction) -> Result<(), Runti if ins.is_deleted() { return Ok(()); } - if let Operation::Binary(bin) = &ins.operation { - if bin.predicate == Some(ctx.zero_with_type(ObjectType::Boolean)) { - ins.mark = Mark::Deleted; - return Ok(()); - } - } //1. constant folding let new_id = ins.evaluate(ctx)?.to_index(ctx); @@ -107,7 +101,10 @@ fn evaluate_intrinsic( _ => todo!(), } } -////////////////////CSE//////////////////////////////////////// + +// +// The following code will be concerned with Common Subexpression Elimination (CSE) +// pub fn propagate(ctx: &SsaContext, id: NodeId, modified: &mut bool) -> NodeId { if let Some(obj) = ctx.try_get_instruction(id) { @@ -220,13 +217,13 @@ fn cse_block_with_anchor( match &operator { Operation::Binary(binary) => { - if let ObjectType::Pointer(a) = ctx.get_object_type(binary.lhs) { + if let ObjectType::Pointer(a) = ctx.object_type(binary.lhs) { //No CSE for arrays because they are not in SSA form //We could improve this in future by checking if the arrays are immutable or not modified in-between let id = ctx.get_dummy_load(a); anchor.push_mem_instruction(ctx, id)?; - if let ObjectType::Pointer(a) = ctx.get_object_type(binary.rhs) { + if let ObjectType::Pointer(a) = ctx.object_type(binary.rhs) { let id = ctx.get_dummy_load(a); anchor.push_mem_instruction(ctx, id)?; } @@ -366,7 +363,7 @@ fn cse_block_with_anchor( } } - let update = ctx.get_mut_instruction(*ins_id); + let update = ctx.instruction_mut(*ins_id); update.operation = operator; update.mark = new_mark; @@ -399,7 +396,7 @@ fn cse_block_with_anchor( )); } } - let update3 = ctx.get_mut_instruction(*ins_id); + let update3 = ctx.instruction_mut(*ins_id); *update3 = update2; } } @@ -409,7 +406,7 @@ fn cse_block_with_anchor( Ok(last) } -pub fn is_some(ctx: &SsaContext, id: NodeId) -> bool { +fn is_some(ctx: &SsaContext, id: NodeId) -> bool { if id == NodeId::dummy() { return false; } diff --git a/crates/noirc_evaluator/src/ssa/ssa_form.rs b/crates/noirc_evaluator/src/ssa/ssa_form.rs index d8c924f5702..530158c6557 100644 --- a/crates/noirc_evaluator/src/ssa/ssa_form.rs +++ b/crates/noirc_evaluator/src/ssa/ssa_form.rs @@ -7,7 +7,7 @@ use crate::ssa::{ use std::collections::HashSet; // create phi arguments from the predecessors of the block (containing phi) -pub fn write_phi(ctx: &mut SsaContext, predecessors: &[BlockId], var: NodeId, phi: NodeId) { +fn write_phi(ctx: &mut SsaContext, predecessors: &[BlockId], var: NodeId, phi: NodeId) { let mut result = Vec::new(); for b in predecessors { let v = get_current_value_in_block(ctx, var, *b); @@ -54,7 +54,7 @@ pub fn seal_block(ctx: &mut SsaContext, block_id: BlockId, entry_block: BlockId) } // write dummy store for join block -pub fn add_dummy_store(ctx: &mut SsaContext, entry: BlockId, join: BlockId) { +fn add_dummy_store(ctx: &mut SsaContext, entry: BlockId, join: BlockId) { //retrieve modified arrays let mut modified = HashSet::new(); if entry == join { @@ -74,7 +74,7 @@ pub fn add_dummy_store(ctx: &mut SsaContext, entry: BlockId, join: BlockId) { } //look-up recursively into predecessors -pub fn get_block_value(ctx: &mut SsaContext, root: NodeId, block_id: BlockId) -> NodeId { +fn get_block_value(ctx: &mut SsaContext, root: NodeId, block_id: BlockId) -> NodeId { let result = if !ctx.sealed_blocks.contains(&block_id) { //incomplete CFG ctx.generate_empty_phi(block_id, root) @@ -107,7 +107,7 @@ pub fn get_current_value_in_block( var_id: NodeId, block_id: BlockId, ) -> NodeId { - let root = ctx.get_root_value(var_id); + let root = ctx.root_value(var_id); ctx[block_id] .get_current_value(root) //Local value numbering diff --git a/crates/noirc_evaluator/src/ssa/code_gen.rs b/crates/noirc_evaluator/src/ssa/ssa_gen.rs similarity index 77% rename from crates/noirc_evaluator/src/ssa/code_gen.rs rename to crates/noirc_evaluator/src/ssa/ssa_gen.rs index cb3186a0e91..5408359842a 100644 --- a/crates/noirc_evaluator/src/ssa/code_gen.rs +++ b/crates/noirc_evaluator/src/ssa/ssa_gen.rs @@ -4,6 +4,7 @@ use crate::ssa::{ function::FuncIndex, mem::ArrayId, node::{Binary, BinaryOp, NodeId, ObjectType, Operation, Variable}, + value::Value, {block, builtin, node, ssa_form}, }; use crate::{errors, errors::RuntimeError}; @@ -20,7 +21,7 @@ use num_bigint::BigUint; use num_traits::Zero; use std::collections::{BTreeMap, HashMap}; -pub struct IRGenerator { +pub struct IrGenerator { pub context: SsaContext, pub function_context: Option, @@ -31,95 +32,9 @@ pub struct IRGenerator { pub program: Program, } -#[derive(Debug, Clone)] -pub enum Value { - Single(NodeId), - Tuple(Vec), -} - -impl Value { - pub fn unwrap_id(&self) -> NodeId { - match self { - Value::Single(id) => *id, - Value::Tuple(_) => panic!("Tried to unwrap a struct into a single value"), - } - } - - pub fn dummy() -> Value { - Value::Single(NodeId::dummy()) - } - - pub fn is_dummy(&self) -> bool { - match self { - Value::Single(id) => *id == NodeId::dummy(), - _ => false, - } - } - - pub fn to_node_ids(&self) -> Vec { - match self { - Value::Single(id) => vec![*id], - Value::Tuple(v) => v.iter().flat_map(|i| i.to_node_ids()).collect(), - } - } - - pub fn zip(&self, v2: &Value, f: &mut F) -> Value - where - F: FnMut(NodeId, NodeId) -> NodeId, - { - if self.is_dummy() || v2.is_dummy() { - return Value::dummy(); - } - - match (self, v2) { - (Value::Single(id1), Value::Single(id2)) => Value::Single(f(*id1, *id2)), - (Value::Tuple(tup1), Value::Tuple(tup2)) => { - Value::Tuple(vecmap(tup1.iter().zip(tup2), |(v1, v2)| v1.zip(v2, f))) - } - _ => unreachable!(), - } - } - - pub fn into_field_member(self, field_index: usize) -> Value { - match self { - Value::Single(_) => { - unreachable!("Runtime type error, expected struct but found a single value") - } - Value::Tuple(mut fields) => fields.remove(field_index), - } - } - - pub fn get_field_member(&self, field_index: usize) -> &Value { - match self { - Value::Single(_) => { - unreachable!("Runtime type error, expected struct but found a single value") - } - Value::Tuple(fields) => &fields[field_index], - } - } - - //Reconstruct a value whose type is provided in argument, from a bunch of NodeIds - fn reshape(value_type: &Type, iter: &mut core::slice::Iter) -> Value { - match value_type { - Type::Tuple(tup) => { - let values = vecmap(tup, |v| Self::reshape(v, iter)); - Value::Tuple(values) - } - _ => Value::Single(*iter.next().unwrap()), - } - } - - fn from_slice(value_type: &Type, slice: &[NodeId]) -> Value { - let mut iter = slice.iter(); - let result = Value::reshape(value_type, &mut iter); - assert!(iter.next().is_none()); - result - } -} - -impl IRGenerator { - pub fn new(program: Program) -> IRGenerator { - IRGenerator { +impl IrGenerator { + pub fn new(program: Program) -> IrGenerator { + IrGenerator { context: SsaContext::new(), variable_values: HashMap::new(), function_context: None, @@ -127,9 +42,9 @@ impl IRGenerator { } } - pub fn codegen_main(&mut self) -> Result<(), RuntimeError> { + pub fn ssa_gen_main(&mut self) -> Result<(), RuntimeError> { let main_body = self.program.take_main_body(); - let value = self.codegen_expression(&main_body)?; + let value = self.ssa_gen_expression(&main_body)?; let node_ids = value.to_node_ids(); if self.program.main().return_type != Type::Unit { @@ -144,7 +59,7 @@ impl IRGenerator { pub fn get_current_value(&mut self, value: &Value) -> Value { match value { - Value::Single(id) => Value::Single(ssa_form::get_current_value(&mut self.context, *id)), + Value::Node(id) => Value::Node(ssa_form::get_current_value(&mut self.context, *id)), Value::Tuple(fields) => { Value::Tuple(vecmap(fields, |value| self.get_current_value(value))) } @@ -199,7 +114,7 @@ impl IRGenerator { noirc_abi::AbiType::Array { length, typ } => { let v_id = self.abi_array(&new_name, None, typ, *length, witnesses[&new_name].clone()); - Value::Single(v_id) + Value::Node(v_id) } noirc_abi::AbiType::Struct { fields, .. } => { let new_name = format!("{struct_name}.{name}"); @@ -215,7 +130,7 @@ impl IRGenerator { *length, witnesses[&new_name].clone(), ); - Value::Single(v_id) + Value::Node(v_id) } _ => { let obj_type = self.get_object_type_from_abi(field_typ); @@ -225,14 +140,14 @@ impl IRGenerator { obj_type, Some(witnesses[&new_name][0]), ); - Value::Single(v_id) + Value::Node(v_id) } } }); self.insert_new_struct(ident_def, values) } - fn codegen_identifier(&mut self, ident: &Ident) -> Result { + fn ssa_gen_identifier(&mut self, ident: &Ident) -> Result { // Check if we have already code-gen'd the definition of this variable if let Some(value) = self.variable_values.get(&ident.definition) { Ok(self.get_current_value(&value.clone())) @@ -250,27 +165,27 @@ impl IRGenerator { self.create_function(id, index)?; } - let expect_msg = "Expected called function to already be codegen'd"; + let expect_msg = "Expected called function to already be ssa_gen'd"; let function_node_id = self.context.get_function_node_id(id).expect(expect_msg); - Ok(Value::Single(function_node_id)) + Ok(Value::Node(function_node_id)) } Definition::Builtin(opcode) | Definition::LowLevel(opcode) => { let opcode = builtin::Opcode::lookup(opcode).unwrap_or_else(|| { unreachable!("Unknown builtin/low level opcode '{}'", opcode) }); let function_node_id = self.context.get_or_create_opcode_node_id(opcode); - Ok(Value::Single(function_node_id)) + Ok(Value::Node(function_node_id)) } } } } - fn codegen_prefix_expression( + fn ssa_gen_prefix_expression( &mut self, rhs: NodeId, op: UnaryOp, ) -> Result { - let rhs_type = self.context.get_object_type(rhs); + let rhs_type = self.context.object_type(rhs); match op { UnaryOp::Minus => { let lhs = self.context.zero_with_type(rhs_type); @@ -282,27 +197,27 @@ impl IRGenerator { } } - fn codegen_infix_expression( + fn ssa_gen_infix_expression( &mut self, lhs: NodeId, rhs: NodeId, op: BinaryOpKind, ) -> Result { - let lhs_type = self.context.get_object_type(lhs); + let lhs_type = self.context.object_type(lhs); // Get the opcode from the infix operator let opcode = Operation::Binary(Binary::from_ast(op, lhs_type, lhs, rhs)); let op_type = self.context.get_result_type(&opcode, lhs_type); self.context.new_instruction(opcode, op_type) } - fn codegen_indexed_value( + fn ssa_gen_indexed_value( &mut self, array: &LValue, index: &Expression, ) -> Result<(NodeId, NodeId), RuntimeError> { let value = self.lvalue_to_value(array); let lhs = value.unwrap_id(); - let index = self.codegen_expression(index)?.unwrap_id(); + let index = self.ssa_gen_expression(index)?.unwrap_id(); Ok((lhs, index)) } @@ -345,7 +260,7 @@ impl IRGenerator { parent_block: self.context.current_block, }; let v_id = self.context.add_variable(new_var, None); - let v_value = Value::Single(v_id); + let v_value = Value::Node(v_id); if let Some(def) = def { self.variable_values.insert(def, v_value); } @@ -380,19 +295,19 @@ impl IRGenerator { let obj_type = self.context.convert_type(elem); let len = *len; let (v_id, _) = self.new_array(base_name, obj_type, len.try_into().unwrap(), def); - Value::Single(v_id) + Value::Node(v_id) } Type::String(len) => { let obj_type = ObjectType::Unsigned(8); let len = *len; let (v_id, _) = self.new_array(base_name, obj_type, len.try_into().unwrap(), def); - Value::Single(v_id) + Value::Node(v_id) } _ => { let obj_type = self.context.convert_type(typ); let v_id = self.create_new_variable(base_name.to_string(), def, obj_type, None); self.context.get_current_block_mut().update_variable(v_id, v_id); - Value::Single(v_id) + Value::Node(v_id) } } } @@ -406,18 +321,18 @@ impl IRGenerator { ) -> (NodeId, ArrayId) { let (id, array_id) = self.context.new_array(name, element_type, len, def.clone()); if let Some(def) = def { - self.variable_values.insert(def, super::code_gen::Value::Single(id)); + self.variable_values.insert(def, super::ssa_gen::Value::Node(id)); } (id, array_id) } // Add a constraint to constrain two expression together - fn codegen_constrain( + fn ssa_gen_constrain( &mut self, expr: &Expression, location: noirc_errors::Location, ) -> Result { - let cond = self.codegen_expression(expr)?.unwrap_id(); + let cond = self.ssa_gen_expression(expr)?.unwrap_id(); let operation = Operation::Constrain(cond, Some(location)); self.context.new_instruction(operation, ObjectType::NotAnObject)?; Ok(Value::dummy()) @@ -428,8 +343,8 @@ impl IRGenerator { fn bind_id(&mut self, id: LocalId, value: Value, name: &str) -> Result<(), RuntimeError> { let definition = Definition::Local(id); match value { - Value::Single(node_id) => { - let object_type = self.context.get_object_type(node_id); + Value::Node(node_id) => { + let object_type = self.context.object_type(node_id); let value = self.bind_variable( name.to_owned(), Some(definition.clone()), @@ -452,8 +367,8 @@ impl IRGenerator { /// This function could use a clearer name fn bind_fresh_pattern(&mut self, basename: &str, value: Value) -> Result { match value { - Value::Single(node_id) => { - let object_type = self.context.get_object_type(node_id); + Value::Node(node_id) => { + let object_type = self.context.object_type(node_id); self.bind_variable(basename.to_owned(), None, object_type, node_id) } Value::Tuple(field_values) => { @@ -488,7 +403,7 @@ impl IRGenerator { self.context.add_variable(new_var, None) }; //Assign rhs to lhs - Ok(Value::Single(self.context.handle_assign(id, None, value_id)?)) + Ok(Value::Node(self.context.handle_assign(id, None, value_id)?)) } //same as update_variable but using the var index instead of var @@ -496,13 +411,13 @@ impl IRGenerator { self.context.update_variable_id(var_id, new_var, new_value); } - fn codegen_assign( + fn ssa_gen_assign( &mut self, lvalue: &LValue, expression: &Expression, ) -> Result { let ident_def = Self::lvalue_ident_def(lvalue); - let rhs = self.codegen_expression(expression)?; + let rhs = self.ssa_gen_expression(expression)?; match lvalue { LValue::Ident(_) => { @@ -514,7 +429,7 @@ impl IRGenerator { self.variable_values.insert(ident_def.clone(), result); } LValue::Index { array, index, .. } => { - let (lhs_id, array_idx) = self.codegen_indexed_value(array.as_ref(), index)?; + let (lhs_id, array_idx) = self.ssa_gen_indexed_value(array.as_ref(), index)?; let rhs_id = rhs.unwrap_id(); self.context.handle_assign(lhs_id, Some(array_idx), rhs_id)?; } @@ -532,8 +447,8 @@ impl IRGenerator { /// each value rather than defining new variables. fn assign_pattern(&mut self, lhs: &Value, rhs: Value) -> Result { match (lhs, rhs) { - (Value::Single(lhs_id), Value::Single(rhs_id)) => { - Ok(Value::Single(self.context.handle_assign(*lhs_id, None, rhs_id)?)) + (Value::Node(lhs_id), Value::Node(rhs_id)) => { + Ok(Value::Node(self.context.handle_assign(*lhs_id, None, rhs_id)?)) } (Value::Tuple(lhs_fields), Value::Tuple(rhs_fields)) => { assert_eq!(lhs_fields.len(), rhs_fields.len()); @@ -543,23 +458,23 @@ impl IRGenerator { Ok(Value::Tuple(fields)) } - (Value::Single(_), Value::Tuple(_)) => unreachable!("variables with tuple/struct types should already be decomposed into multiple variables"), - (Value::Tuple(_), Value::Single(_)) => unreachable!("Uncaught type error, tried to assign a single value to a tuple/struct type"), + (Value::Node(_), Value::Tuple(_)) => unreachable!("variables with tuple/struct types should already be decomposed into multiple variables"), + (Value::Tuple(_), Value::Node(_)) => unreachable!("Uncaught type error, tried to assign a single value to a tuple/struct type"), } } // Let statements are used to declare higher level objects - fn codegen_let(&mut self, let_expr: &Let) -> Result { - let rhs = self.codegen_expression(&let_expr.expression)?; + fn ssa_gen_let(&mut self, let_expr: &Let) -> Result { + let rhs = self.ssa_gen_expression(&let_expr.expression)?; self.bind_id(let_expr.id, rhs, &let_expr.name)?; Ok(Value::dummy()) } - pub(crate) fn codegen_expression(&mut self, expr: &Expression) -> Result { + pub(crate) fn ssa_gen_expression(&mut self, expr: &Expression) -> Result { match expr { Expression::Literal(Literal::Integer(x, typ)) => { let typ = self.context.convert_type(typ); - Ok(Value::Single(self.context.get_or_create_const(*x, typ))) + Ok(Value::Node(self.context.get_or_create_const(*x, typ))) } Expression::Literal(Literal::Array(arr_lit)) => { let element_type = self.context.convert_type(&arr_lit.element_type); @@ -567,7 +482,7 @@ impl IRGenerator { let (new_var, array_id) = self.context.new_array("", element_type, arr_lit.contents.len() as u32, None); - let elements = self.codegen_expression_list(&arr_lit.contents); + let elements = self.ssa_gen_expression_list(&arr_lit.contents); for (pos, object) in elements.into_iter().enumerate() { let lhs_adr = self.context.get_or_create_const( FieldElement::from((pos as u32) as u128), @@ -576,7 +491,7 @@ impl IRGenerator { let store = Operation::Store { array_id, index: lhs_adr, value: object }; self.context.new_instruction(store, element_type)?; } - Ok(Value::Single(new_var)) + Ok(Value::Node(new_var)) } Expression::Literal(Literal::Str(string)) => { let string_as_integers = vecmap(string.bytes(), |byte| { @@ -593,16 +508,16 @@ impl IRGenerator { }; let new_value = self - .codegen_expression(&Expression::Literal(Literal::Array(string_arr_literal)))?; + .ssa_gen_expression(&Expression::Literal(Literal::Array(string_arr_literal)))?; Ok(new_value) } - Expression::Ident(ident) => self.codegen_identifier(ident), + Expression::Ident(ident) => self.ssa_gen_identifier(ident), Expression::Binary(binary) => { // Note: we disallows structs/tuples in infix expressions. // The type checker currently disallows this as well but not if they come from generic type // We could allow some in the future, e.g. struct == struct - let lhs = self.codegen_expression(&binary.lhs)?.to_node_ids(); - let rhs = self.codegen_expression(&binary.rhs)?.to_node_ids(); + let lhs = self.ssa_gen_expression(&binary.lhs)?.to_node_ids(); + let rhs = self.ssa_gen_expression(&binary.rhs)?.to_node_ids(); if lhs.len() != 1 || rhs.len() != 1 { return Err(errors::RuntimeErrorKind::UnsupportedOp { op: binary.operator.to_string(), @@ -611,60 +526,60 @@ impl IRGenerator { } .into()); } - Ok(Value::Single(self.codegen_infix_expression(lhs[0], rhs[0], binary.operator)?)) + Ok(Value::Node(self.ssa_gen_infix_expression(lhs[0], rhs[0], binary.operator)?)) } Expression::Cast(cast_expr) => { - let lhs = self.codegen_expression(&cast_expr.lhs)?.unwrap_id(); + let lhs = self.ssa_gen_expression(&cast_expr.lhs)?.unwrap_id(); let object_type = self.context.convert_type(&cast_expr.r#type); - Ok(Value::Single(self.context.new_instruction(Operation::Cast(lhs), object_type)?)) + Ok(Value::Node(self.context.new_instruction(Operation::Cast(lhs), object_type)?)) } Expression::Index(indexed_expr) => { // Evaluate the 'array' expression - let expr_node = self.codegen_expression(&indexed_expr.collection)?.unwrap_id(); - let array = match self.context.get_object_type(expr_node) { + let expr_node = self.ssa_gen_expression(&indexed_expr.collection)?.unwrap_id(); + let array = match self.context.object_type(expr_node) { ObjectType::Pointer(array_id) => &self.context.mem[array_id], other => unreachable!("Expected Pointer type, found {:?}", other), }; let array_id = array.id; let e_type = array.element_type; // Evaluate the index expression - let index_as_obj = self.codegen_expression(&indexed_expr.index)?.unwrap_id(); + let index_as_obj = self.ssa_gen_expression(&indexed_expr.index)?.unwrap_id(); let load = Operation::Load { array_id, index: index_as_obj }; - Ok(Value::Single(self.context.new_instruction(load, e_type)?)) + Ok(Value::Node(self.context.new_instruction(load, e_type)?)) } Expression::Call(call_expr) => { let results = self.call(call_expr)?; Ok(Value::from_slice(&call_expr.return_type, &results)) } - Expression::For(for_expr) => self.codegen_for(for_expr), - Expression::Tuple(fields) => self.codegen_tuple(fields), + Expression::For(for_expr) => self.ssa_gen_for(for_expr), + Expression::Tuple(fields) => self.ssa_gen_tuple(fields), Expression::If(if_expr) => self.handle_if_expr(if_expr), Expression::Unary(prefix) => { - let rhs = self.codegen_expression(&prefix.rhs)?.unwrap_id(); - self.codegen_prefix_expression(rhs, prefix.operator).map(Value::Single) + let rhs = self.ssa_gen_expression(&prefix.rhs)?.unwrap_id(); + self.ssa_gen_prefix_expression(rhs, prefix.operator).map(Value::Node) } - Expression::Literal(l) => Ok(Value::Single(self.codegen_literal(l))), - Expression::Block(block) => self.codegen_block(block), + Expression::Literal(l) => Ok(Value::Node(self.ssa_gen_literal(l))), + Expression::Block(block) => self.ssa_gen_block(block), Expression::ExtractTupleField(expr, field) => { - let tuple = self.codegen_expression(expr.as_ref())?; + let tuple = self.ssa_gen_expression(expr.as_ref())?; Ok(tuple.into_field_member(*field)) } - Expression::Let(let_expr) => self.codegen_let(let_expr), + Expression::Let(let_expr) => self.ssa_gen_let(let_expr), Expression::Constrain(expr, location) => { - self.codegen_constrain(expr.as_ref(), *location) + self.ssa_gen_constrain(expr.as_ref(), *location) } Expression::Assign(assign) => { - self.codegen_assign(&assign.lvalue, assign.expression.as_ref()) + self.ssa_gen_assign(&assign.lvalue, assign.expression.as_ref()) } Expression::Semi(expr) => { - self.codegen_expression(expr.as_ref())?; + self.ssa_gen_expression(expr.as_ref())?; Ok(Value::dummy()) } } } - fn codegen_literal(&mut self, l: &Literal) -> NodeId { + fn ssa_gen_literal(&mut self, l: &Literal) -> NodeId { match l { Literal::Bool(b) => { if *b { @@ -682,28 +597,28 @@ impl IRGenerator { } /// A tuple is much the same as a constructor, we just give it fields with numbered names - fn codegen_tuple(&mut self, fields: &[Expression]) -> Result { + fn ssa_gen_tuple(&mut self, fields: &[Expression]) -> Result { let fields = fields .iter() - .map(|field| self.codegen_expression(field)) + .map(|field| self.ssa_gen_expression(field)) .collect::, _>>()?; Ok(Value::Tuple(fields)) } - pub fn codegen_expression_list(&mut self, exprs: &[Expression]) -> Vec { + pub fn ssa_gen_expression_list(&mut self, exprs: &[Expression]) -> Vec { let mut result = Vec::with_capacity(exprs.len()); for expr in exprs { - let value = self.codegen_expression(expr); + let value = self.ssa_gen_expression(expr); result.extend(value.unwrap().to_node_ids()); } result } - fn codegen_for(&mut self, for_expr: &For) -> Result { + fn ssa_gen_for(&mut self, for_expr: &For) -> Result { //we add the 'i = start' instruction (in the block before the join) - let start_idx = self.codegen_expression(&for_expr.start_range).unwrap().unwrap_id(); - let end_idx = self.codegen_expression(&for_expr.end_range).unwrap().unwrap_id(); + let start_idx = self.ssa_gen_expression(&for_expr.start_range).unwrap().unwrap_id(); + let end_idx = self.ssa_gen_expression(&for_expr.end_range).unwrap().unwrap_id(); //We support only const range for now let iter_def = Definition::Local(for_expr.index_variable); @@ -745,7 +660,7 @@ impl IRGenerator { let body_block1 = &mut self.context[body_id]; body_block1.update_variable(iter_id, phi); //TODO try with just a get_current_value(iter) - self.codegen_expression(for_expr.block.as_ref())?; + self.ssa_gen_expression(for_expr.block.as_ref())?; //increment iter let one = self.context.get_or_create_const(FieldElement::one(), iter_type); @@ -773,14 +688,14 @@ impl IRGenerator { //seal join ssa_form::seal_block(&mut self.context, join_idx, join_idx); - Ok(Value::Single(exit_first)) + Ok(Value::Node(exit_first)) } //Parse a block of AST statements into ssa form - pub fn codegen_block(&mut self, block: &[Expression]) -> Result { + pub fn ssa_gen_block(&mut self, block: &[Expression]) -> Result { let mut last_value = Value::dummy(); for expr in block { - last_value = self.codegen_expression(expr)?; + last_value = self.ssa_gen_expression(expr)?; } Ok(last_value) } @@ -793,17 +708,17 @@ impl IRGenerator { block::new_sealed_block(&mut self.context, block::BlockType::Normal, true); } - let condition = self.codegen_expression(if_expr.condition.as_ref())?.unwrap_id(); + let condition = self.ssa_gen_expression(if_expr.condition.as_ref())?.unwrap_id(); if let Some(cond) = node::NodeEval::from_id(&self.context, condition).into_const_value() { if cond.is_zero() { if let Some(alt) = &if_expr.alternative { - return self.codegen_expression(alt); + return self.ssa_gen_expression(alt); } else { return Ok(Value::dummy()); } } else { - return self.codegen_expression(if_expr.consequence.as_ref()); + return self.ssa_gen_expression(if_expr.consequence.as_ref()); } } @@ -813,7 +728,7 @@ impl IRGenerator { //Then block block::new_sealed_block(&mut self.context, block::BlockType::Normal, true); - let v1 = self.codegen_expression(if_expr.consequence.as_ref())?; + let v1 = self.ssa_gen_expression(if_expr.consequence.as_ref())?; //Exit block let exit_block = @@ -827,14 +742,14 @@ impl IRGenerator { //Fixup the jump if let node::Instruction { operation: Operation::Jeq(_, target), .. } = - self.context.get_mut_instruction(jump_ins) + self.context.instruction_mut(jump_ins) { *target = block2; } let mut v2 = Value::dummy(); if let Some(alt) = if_expr.alternative.as_ref() { - v2 = self.codegen_expression(alt)?; + v2 = self.ssa_gen_expression(alt)?; } //Connect with the exit block diff --git a/crates/noirc_evaluator/src/ssa/value.rs b/crates/noirc_evaluator/src/ssa/value.rs new file mode 100644 index 00000000000..a023bca4b96 --- /dev/null +++ b/crates/noirc_evaluator/src/ssa/value.rs @@ -0,0 +1,113 @@ +use crate::ssa::node::NodeId; +use iter_extended::vecmap; +use noirc_frontend::monomorphization::ast::Type; + +/// `Value` is used only to construct the SSA IR. +#[derive(Debug, Clone)] +pub enum Value { + Node(NodeId), + Tuple(Vec), +} + +impl Value { + /// Returns a single NodeId. + /// Panics: If `Value` holds multiple Values + pub fn unwrap_id(&self) -> NodeId { + match self { + Value::Node(id) => *id, + Value::Tuple(_) => panic!("Tried to unwrap a struct/tuple into a NodeId"), + } + } + + /// Returns a placeholder NodeId that can + /// be used to represent the absence of a value. + pub fn dummy() -> Value { + Value::Node(NodeId::dummy()) + } + + /// Checks if the `Value` corresponds to + /// `Option::None` or no value. + pub fn is_dummy(&self) -> bool { + match self { + Value::Node(id) => *id == NodeId::dummy(), + _ => false, + } + } + + /// Converts `Value` into a vector of NodeId's + pub fn to_node_ids(&self) -> Vec { + match self { + Value::Node(id) => vec![*id], + Value::Tuple(v) => v.iter().flat_map(|i| i.to_node_ids()).collect(), + } + } + + /// Calls the function `f` on `self` and the given `Value` + /// Panics: If `self` and the given value are not the same + /// enum variant + pub fn zip(&self, rhs_value: &Value, f: &mut F) -> Value + where + F: FnMut(NodeId, NodeId) -> NodeId, + { + if self.is_dummy() || rhs_value.is_dummy() { + return Value::dummy(); + } + + match (self, rhs_value) { + (Value::Node(lhs), Value::Node(rhs)) => Value::Node(f(*lhs, *rhs)), + (Value::Tuple(lhs), Value::Tuple(rhs)) => { + Value::Tuple(vecmap(lhs.iter().zip(rhs), |(lhs_value, rhs_value)| { + lhs_value.zip(rhs_value, f) + })) + } + _ => { + unreachable!("ICE: expected both `Value` instances to be of the same enum variant") + } + } + } + + /// Returns the `Value` at position `field_index` in the + /// Tuple Variant. + /// Panics: If the `self` is not the `Tuple` Variant. + pub fn into_field_member(self, field_index: usize) -> Value { + match self { + Value::Node(_) => { + unreachable!("Runtime type error, expected struct/tuple but found a NodeId") + } + Value::Tuple(mut fields) => fields.remove(field_index), + } + } + pub fn get_field_member(&self, field_index: usize) -> &Value { + match self { + Value::Node(_) => { + unreachable!("Runtime type error, expected struct but found a NodeId") + } + Value::Tuple(fields) => &fields[field_index], + } + } + + /// Reconstruct a `Value` instance whose type is `value_type` + fn reshape(value_type: &Type, iter: &mut core::slice::Iter) -> Value { + match value_type { + Type::Tuple(tup) => { + let values = vecmap(tup, |v| Self::reshape(v, iter)); + Value::Tuple(values) + } + Type::Unit + | Type::Function(..) + | Type::Array(..) + | Type::String(..) + | Type::Integer(..) + | Type::Bool + | Type::Field => Value::Node(*iter.next().unwrap()), + } + } + + /// Reconstruct a `Value` instance from a slice of NodeId's + pub(crate) fn from_slice(value_type: &Type, slice: &[NodeId]) -> Value { + let mut iter = slice.iter(); + let result = Value::reshape(value_type, &mut iter); + assert!(iter.next().is_none()); + result + } +} diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index db08c7dae01..15ae1d8aaa8 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -26,6 +26,10 @@ pub enum ExpressionKind { Error, } +/// A Vec of unresolved names for type variables. +/// For `fn foo(...)` this corresponds to vec!["A", "B"]. +pub type UnresolvedGenerics = Vec; + impl ExpressionKind { pub fn into_path(self) -> Option { match self { @@ -306,7 +310,7 @@ pub struct Lambda { pub struct FunctionDefinition { pub name: Ident, pub attribute: Option, // XXX: Currently we only have one attribute defined. If more attributes are needed per function, we can make this a vector and make attribute definition more expressive - pub generics: Vec, + pub generics: UnresolvedGenerics, pub parameters: Vec<(Pattern, UnresolvedType, noirc_abi::AbiVisibility)>, pub body: BlockExpression, pub span: Span, diff --git a/crates/noirc_frontend/src/ast/mod.rs b/crates/noirc_frontend/src/ast/mod.rs index 725455a8598..057fa42345d 100644 --- a/crates/noirc_frontend/src/ast/mod.rs +++ b/crates/noirc_frontend/src/ast/mod.rs @@ -18,7 +18,7 @@ use iter_extended::vecmap; /// The parser parses types as 'UnresolvedType's which /// require name resolution to resolve any type names used /// for structs within, but are otherwise identical to Types. -#[derive(Debug, PartialEq, Eq, Clone)] +#[derive(Debug, PartialEq, Eq, Clone, Hash)] pub enum UnresolvedType { FieldElement(CompTime), Array(Option, Box), // [4]Witness = Array(4, Witness) @@ -43,7 +43,7 @@ pub enum UnresolvedType { /// The precursor to TypeExpression, this is the type that the parser allows /// to be used in the length position of an array type. Only constants, variables, /// and numeric binary operators are allowed here. -#[derive(Debug, PartialEq, Eq, Clone)] +#[derive(Debug, PartialEq, Eq, Clone, Hash)] pub enum UnresolvedTypeExpression { Variable(Path), Constant(u64, Span), diff --git a/crates/noirc_frontend/src/ast/structure.rs b/crates/noirc_frontend/src/ast/structure.rs index dc01a7d5c90..47dd432e739 100644 --- a/crates/noirc_frontend/src/ast/structure.rs +++ b/crates/noirc_frontend/src/ast/structure.rs @@ -1,12 +1,14 @@ use std::fmt::Display; -use crate::{Ident, NoirFunction, Path, UnresolvedType}; +use crate::{Ident, NoirFunction, UnresolvedGenerics, UnresolvedType}; +use iter_extended::vecmap; use noirc_errors::Span; +/// Ast node for a struct #[derive(Clone, Debug, PartialEq, Eq)] pub struct NoirStruct { pub name: Ident, - pub generics: Vec, + pub generics: UnresolvedGenerics, pub fields: Vec<(Ident, UnresolvedType)>, pub span: Span, } @@ -22,15 +24,21 @@ impl NoirStruct { } } +/// Ast node for an impl #[derive(Clone, Debug)] pub struct NoirImpl { - pub type_path: Path, + pub object_type: UnresolvedType, + pub type_span: Span, + pub generics: UnresolvedGenerics, pub methods: Vec, } impl Display for NoirStruct { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - writeln!(f, "struct {} {{", self.name)?; + let generics = vecmap(&self.generics, |generic| generic.to_string()); + let generics = if generics.is_empty() { "".into() } else { generics.join(", ") }; + + writeln!(f, "struct {}{} {{", self.name, generics)?; for (name, typ) in self.fields.iter() { writeln!(f, " {name}: {typ},")?; @@ -42,7 +50,10 @@ impl Display for NoirStruct { impl Display for NoirImpl { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - writeln!(f, "impl {} {{", self.type_path)?; + let generics = vecmap(&self.generics, |generic| generic.to_string()); + let generics = if generics.is_empty() { "".into() } else { generics.join(", ") }; + + writeln!(f, "impl{} {} {{", generics, self.object_type)?; for method in self.methods.iter() { let method = method.to_string(); diff --git a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs index 1b4bd7991d1..1c5dc4f178f 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -12,13 +12,16 @@ use crate::hir::type_check::type_check; use crate::hir::type_check::type_check_func; use crate::hir::Context; use crate::node_interner::{FuncId, NodeInterner, StmtId, StructId}; -use crate::{Generics, Ident, LetStatement, NoirFunction, NoirStruct, ParsedModule, Path, Type}; +use crate::{ + Generics, Ident, LetStatement, NoirFunction, NoirStruct, ParsedModule, Shared, Type, + TypeBinding, UnresolvedGenerics, UnresolvedType, +}; use fm::FileId; -use noirc_errors::CollectedErrors; -use noirc_errors::DiagnosableError; -use std::collections::{BTreeMap, HashMap}; - use iter_extended::vecmap; +use noirc_errors::Span; +use noirc_errors::{CustomDiagnostic, FileDiagnostic}; +use std::collections::{BTreeMap, HashMap}; +use std::rc::Rc; /// Stores all of the unresolved functions in a particular file/mod pub struct UnresolvedFunctions { @@ -53,11 +56,15 @@ pub struct DefCollector { pub(crate) collected_functions: Vec, pub(crate) collected_types: HashMap, pub(crate) collected_globals: Vec, - /// collected impls maps the type name and the module id in which - /// the impl is defined to the functions contained in that impl - pub(crate) collected_impls: HashMap<(Path, LocalModuleId), Vec>, + pub(crate) collected_impls: ImplMap, } +/// Maps the type and the module id in which the impl is defined to the functions contained in that +/// impl along with the generics declared on the impl itself. This also contains the Span +/// of the object_type of the impl, used to issue an error if the object type fails to resolve. +type ImplMap = + HashMap<(UnresolvedType, LocalModuleId), Vec<(UnresolvedGenerics, Span, UnresolvedFunctions)>>; + impl DefCollector { fn new(def_map: CrateDefMap) -> DefCollector { DefCollector { @@ -78,7 +85,7 @@ impl DefCollector { context: &mut Context, ast: ParsedModule, root_file_id: FileId, - errors: &mut Vec, + errors: &mut Vec, ) { let crate_id = def_map.krate; @@ -122,10 +129,8 @@ impl DefCollector { for unresolved_import in unresolved.into_iter() { // File if that the import was declared let file_id = current_def_map.modules[unresolved_import.module_id.0].origin.file_id(); - let diagnostic = DefCollectorErrorKind::UnresolvedImport { import: unresolved_import } - .to_diagnostic(); - let err = CollectedErrors { file_id, errors: vec![diagnostic] }; - errors.push(err); + let error = DefCollectorErrorKind::UnresolvedImport { import: unresolved_import }; + errors.push(error.into_file_diagnostic(file_id)); } // Populate module namespaces according to the imports used @@ -139,11 +144,7 @@ impl DefCollector { if let Err((first_def, second_def)) = result { let err = DefCollectorErrorKind::DuplicateImport { first_def, second_def }; - - errors.push(CollectedErrors { - file_id: root_file_id, - errors: vec![err.to_diagnostic()], - }); + errors.push(err.into_file_diagnostic(root_file_id)); } } } @@ -161,7 +162,7 @@ impl DefCollector { collect_impls(context, crate_id, &def_collector.collected_impls, errors); // Lower each function in the crate. This is now possible since imports have been resolved - let file_func_ids = resolve_functions( + let file_func_ids = resolve_free_functions( &mut context.def_interner, crate_id, &context.def_maps, @@ -190,33 +191,29 @@ impl DefCollector { fn collect_impls( context: &mut Context, crate_id: CrateId, - collected_impls: &HashMap<(Path, LocalModuleId), Vec>, - errors: &mut Vec, + collected_impls: &ImplMap, + errors: &mut Vec, ) { let interner = &mut context.def_interner; let def_maps = &mut context.def_maps; - for ((path, module_id), methods) in collected_impls { + for ((unresolved_type, module_id), methods) in collected_impls { let path_resolver = StandardPathResolver::new(ModuleId { local_id: *module_id, krate: crate_id }); let file = def_maps[&crate_id].module_file_id(*module_id); - for unresolved in methods { - let resolver = Resolver::new(interner, &path_resolver, def_maps, file); - let (typ, more_errors) = resolver.lookup_type_for_impl(path.clone()); - if !more_errors.is_empty() { - errors.push(CollectedErrors { - file_id: unresolved.file_id, - errors: vecmap(more_errors, |err| err.into_diagnostic()), - }) - } + for (generics, span, unresolved) in methods { + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); + resolver.add_generics(generics); + let typ = resolver.resolve_type(unresolved_type.clone()); + + extend_errors(errors, unresolved.file_id, resolver.take_errors()); - if typ != StructId::dummy_id() { + if let Some(type_module) = get_local_id_from_type(&typ) { // Grab the scope defined by the struct type. Note that impls are a case // where the scope the methods are added to is not the same as the scope // they are resolved in. - let type_module = typ.0.local_id; let scope = &mut def_maps.get_mut(&crate_id).unwrap().modules[type_module.0].scope; // .define_func_def(name, func_id); @@ -226,17 +223,33 @@ fn collect_impls( if let Err((first_def, second_def)) = result { let err = DefCollectorErrorKind::DuplicateFunction { first_def, second_def }; - errors.push(CollectedErrors { - file_id: unresolved.file_id, - errors: vec![err.to_diagnostic()], - }); + errors.push(err.into_file_diagnostic(unresolved.file_id)); } } + } else if typ != Type::Error { + let span = *span; + let error = DefCollectorErrorKind::NonStructTypeInImpl { span }; + errors.push(error.into_file_diagnostic(unresolved.file_id)) } } } } +fn get_local_id_from_type(typ: &Type) -> Option { + match typ { + Type::Struct(definition, _) => Some(definition.borrow().id.0.local_id), + _ => None, + } +} + +fn extend_errors(errors: &mut Vec, file: fm::FileId, new_errors: Errs) +where + Errs: IntoIterator, + Err: Into, +{ + errors.extend(new_errors.into_iter().map(|err| err.into().in_file(file))) +} + fn resolve_globals( context: &mut Context, globals: Vec, @@ -271,16 +284,12 @@ fn resolve_globals( fn type_check_globals( interner: &mut NodeInterner, global_ids: Vec<(FileId, StmtId)>, - all_errors: &mut Vec, + all_errors: &mut Vec, ) { for (file_id, stmt_id) in global_ids { - let mut type_check_errs = vec![]; - type_check(interner, &stmt_id, &mut type_check_errs); - let errors = vecmap(type_check_errs, |error| error.into_diagnostic()); - - if !errors.is_empty() { - all_errors.push(CollectedErrors { file_id, errors }); - } + let mut errors = vec![]; + type_check(interner, &stmt_id, &mut errors); + extend_errors(all_errors, file_id, errors); } } @@ -290,7 +299,7 @@ fn resolve_structs( context: &mut Context, structs: HashMap, crate_id: CrateId, - errors: &mut Vec, + errors: &mut Vec, ) { // We must first go through the struct list once to ensure all IDs are pushed to // the def_interner map. This lets structs refer to each other regardless of declaration order @@ -313,24 +322,18 @@ fn resolve_struct_fields( context: &mut Context, krate: CrateId, unresolved: UnresolvedStruct, - errors: &mut Vec, + all_errors: &mut Vec, ) -> (Generics, BTreeMap) { let path_resolver = StandardPathResolver::new(ModuleId { local_id: unresolved.module_id, krate }); let file = unresolved.file_id; - let (generics, fields, errs) = + let (generics, fields, errors) = Resolver::new(&mut context.def_interner, &path_resolver, &context.def_maps, file) .resolve_struct_fields(unresolved.struct_def); - if !errs.is_empty() { - errors.push(CollectedErrors { - file_id: unresolved.file_id, - errors: vecmap(errs, |err| err.into_diagnostic()), - }) - } - + extend_errors(all_errors, unresolved.file_id, errors); (generics, fields) } @@ -338,100 +341,118 @@ fn resolve_impls( interner: &mut NodeInterner, crate_id: CrateId, def_maps: &HashMap, - collected_impls: HashMap<(Path, LocalModuleId), Vec>, - errors: &mut Vec, + collected_impls: ImplMap, + errors: &mut Vec, ) -> Vec<(FileId, FuncId)> { let mut file_method_ids = Vec::new(); - for ((path, module_id), methods) in collected_impls { + for ((unresolved_type, module_id), methods) in collected_impls { let path_resolver = StandardPathResolver::new(ModuleId { local_id: module_id, krate: crate_id }); let file = def_maps[&crate_id].module_file_id(module_id); - let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); - let self_type = resolver.lookup_struct(path); - let self_type_id = self_type.as_ref().map(|typ| typ.borrow().id); - - let mut ids = - resolve_functions(interner, crate_id, def_maps, methods, self_type_id, errors); - - if let Some(typ) = self_type { - for (file_id, method_id) in &ids { - let method_name = interner.function_name(method_id).to_owned(); - let mut typ = typ.borrow_mut(); - - if let Some(first_fn) = typ.methods.insert(method_name.clone(), *method_id) { - let error = ResolverError::DuplicateDefinition { - name: method_name, - first_span: interner.function_ident(&first_fn).span(), - second_span: interner.function_ident(method_id).span(), - }; - - errors.push(CollectedErrors { - file_id: *file_id, - errors: vec![error.into_diagnostic()], - }); + for (generics, _, functions) in methods { + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); + resolver.add_generics(&generics); + let generics = resolver.get_generics().to_vec(); + let self_type = resolver.resolve_type(unresolved_type.clone()); + + let mut file_func_ids = resolve_function_set( + interner, + crate_id, + def_maps, + functions, + Some(self_type.clone()), + generics, + errors, + ); + + if self_type != Type::Error { + for (file_id, method_id) in &file_func_ids { + let method_name = interner.function_name(method_id).to_owned(); + + if let Some(first_fn) = + interner.add_method(&self_type, method_name.clone(), *method_id) + { + let error = ResolverError::DuplicateDefinition { + name: method_name, + first_span: interner.function_ident(&first_fn).span(), + second_span: interner.function_ident(method_id).span(), + }; + + errors.push(error.into_file_diagnostic(*file_id)); + } } } + file_method_ids.append(&mut file_func_ids); } - - file_method_ids.append(&mut ids); } file_method_ids } -fn resolve_functions( +fn resolve_free_functions( interner: &mut NodeInterner, crate_id: CrateId, def_maps: &HashMap, collected_functions: Vec, - self_type: Option, - errors: &mut Vec, + self_type: Option, + errors: &mut Vec, ) -> Vec<(FileId, FuncId)> { - let mut file_func_ids = Vec::new(); - // Lower each function in the crate. This is now possible since imports have been resolved - for unresolved_functions in collected_functions { - let file_id = unresolved_functions.file_id; - let mut collected_errors = CollectedErrors { file_id, errors: Vec::new() }; - - for (mod_id, func_id, func) in unresolved_functions.functions { - file_func_ids.push((file_id, func_id)); - - let path_resolver = - StandardPathResolver::new(ModuleId { local_id: mod_id, krate: crate_id }); - - let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file_id); - resolver.set_self_type(self_type); + collected_functions + .into_iter() + .flat_map(|unresolved_functions| { + resolve_function_set( + interner, + crate_id, + def_maps, + unresolved_functions, + self_type.clone(), + vec![], // no impl generics + errors, + ) + }) + .collect() +} - let (hir_func, func_meta, errs) = resolver.resolve_function(func, func_id); - interner.push_fn_meta(func_meta, func_id); - interner.update_fn(func_id, hir_func); - collected_errors.errors.extend(errs.into_iter().map(|err| err.into_diagnostic())); - } - if !collected_errors.errors.is_empty() { - errors.push(collected_errors); - } - } +fn resolve_function_set( + interner: &mut NodeInterner, + crate_id: CrateId, + def_maps: &HashMap, + unresolved_functions: UnresolvedFunctions, + self_type: Option, + impl_generics: Vec<(Rc, Shared, Span)>, + errors: &mut Vec, +) -> Vec<(FileId, FuncId)> { + let file_id = unresolved_functions.file_id; - file_func_ids + vecmap(unresolved_functions.functions, |(mod_id, func_id, func)| { + let path_resolver = + StandardPathResolver::new(ModuleId { local_id: mod_id, krate: crate_id }); + + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file_id); + // Must use set_generics here to ensure we re-use the same generics from when + // the impl was originally collected. Otherwise the function will be using different + // TypeVariables for the same generic, causing it to instantiate incorrectly. + resolver.set_generics(impl_generics.clone()); + resolver.set_self_type(self_type.clone()); + + let (hir_func, func_meta, errs) = resolver.resolve_function(func, func_id); + interner.push_fn_meta(func_meta, func_id); + interner.update_fn(func_id, hir_func); + extend_errors(errors, file_id, errs); + (file_id, func_id) + }) } fn type_check_functions( interner: &mut NodeInterner, file_func_ids: Vec<(FileId, FuncId)>, - errors: &mut Vec, + errors: &mut Vec, ) { - file_func_ids - .into_iter() - .map(|(file_id, func_id)| { - let errors = - vecmap(type_check_func(interner, func_id), |error| error.into_diagnostic()); - - CollectedErrors { file_id, errors } - }) - .filter(|collected| !collected.errors.is_empty()) - .for_each(|error| errors.push(error)); + for (file, func) in file_func_ids { + extend_errors(errors, file, type_check_func(interner, func)) + } } diff --git a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs index 256efa148b1..fb0c797d0cb 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -1,5 +1,5 @@ use fm::FileId; -use noirc_errors::{CollectedErrors, CustomDiagnostic, DiagnosableError}; +use noirc_errors::FileDiagnostic; use crate::{ graph::CrateId, hir::def_collector::dc_crate::UnresolvedStruct, node_interner::StructId, @@ -29,7 +29,7 @@ pub fn collect_defs( module_id: LocalModuleId, crate_id: CrateId, context: &mut Context, - errors: &mut Vec, + errors: &mut Vec, ) { let mut collector = ModCollector { def_collector, file_id, module_id }; @@ -53,13 +53,9 @@ pub fn collect_defs( collector.collect_structs(ast.types, crate_id, errors); - let errors_in_same_file = collector.collect_functions(context, ast.functions); + collector.collect_functions(context, ast.functions, errors); collector.collect_impls(context, ast.impls); - - if !errors_in_same_file.is_empty() { - errors.push(CollectedErrors { file_id: collector.file_id, errors: errors_in_same_file }); - } } impl<'a> ModCollector<'a> { @@ -67,7 +63,7 @@ impl<'a> ModCollector<'a> { &mut self, context: &mut Context, globals: Vec, - errors: &mut Vec, + errors: &mut Vec, ) { for global in globals { let name = global.pattern.name_ident().clone(); @@ -83,11 +79,7 @@ impl<'a> ModCollector<'a> { if let Err((first_def, second_def)) = result { let err = DefCollectorErrorKind::DuplicateGlobal { first_def, second_def }; - - errors.push(CollectedErrors { - file_id: self.file_id, - errors: vec![err.to_diagnostic()], - }); + errors.push(err.into_file_diagnostic(self.file_id)); } self.def_collector.collected_globals.push(UnresolvedGlobal { @@ -104,15 +96,15 @@ impl<'a> ModCollector<'a> { let mut unresolved_functions = UnresolvedFunctions { file_id: self.file_id, functions: Vec::new() }; - for method in r#impl.methods.iter() { + for method in r#impl.methods { let func_id = context.def_interner.push_empty_fn(); - unresolved_functions.push_fn(self.module_id, func_id, method.clone()); context.def_interner.push_function_definition(method.name().to_owned(), func_id); + unresolved_functions.push_fn(self.module_id, func_id, method); } - let key = (r#impl.type_path.clone(), self.module_id); + let key = (r#impl.object_type, self.module_id); let methods = self.def_collector.collected_impls.entry(key).or_default(); - methods.push(unresolved_functions); + methods.push((r#impl.generics, r#impl.type_span, unresolved_functions)); } } @@ -120,9 +112,8 @@ impl<'a> ModCollector<'a> { &mut self, context: &mut Context, functions: Vec, - ) -> Vec { - let mut errors = vec![]; - + errors: &mut Vec, + ) { let mut unresolved_functions = UnresolvedFunctions { file_id: self.file_id, functions: Vec::new() }; @@ -148,16 +139,12 @@ impl<'a> ModCollector<'a> { .define_func_def(name, func_id); if let Err((first_def, second_def)) = result { - errors.push( - DefCollectorErrorKind::DuplicateFunction { first_def, second_def } - .to_diagnostic(), - ); + let error = DefCollectorErrorKind::DuplicateFunction { first_def, second_def }; + errors.push(error.into_file_diagnostic(self.file_id)); } } self.def_collector.collected_functions.push(unresolved_functions); - - errors } /// Collect any struct definitions declared within the ast. @@ -166,18 +153,15 @@ impl<'a> ModCollector<'a> { &mut self, types: Vec, krate: CrateId, - errors: &mut Vec, + errors: &mut Vec, ) { for struct_definition in types { let name = struct_definition.name.clone(); // Create the corresponding module for the struct namespace - let id = match self.push_child_module(&name, self.file_id, false) { - Ok(local_id) => StructId(ModuleId { krate, local_id }), - Err(mut more_errors) => { - errors.append(&mut more_errors); - continue; - } + let id = match self.push_child_module(&name, self.file_id, false, errors) { + Some(local_id) => StructId(ModuleId { krate, local_id }), + None => continue, }; // Add the struct to scope so its path can be looked up later @@ -187,11 +171,7 @@ impl<'a> ModCollector<'a> { if let Err((first_def, second_def)) = result { let err = DefCollectorErrorKind::DuplicateFunction { first_def, second_def }; - - errors.push(CollectedErrors { - file_id: self.file_id, - errors: vec![err.to_diagnostic()], - }); + errors.push(err.into_file_diagnostic(self.file_id)); } // And store the TypeId -> StructType mapping somewhere it is reachable @@ -210,20 +190,19 @@ impl<'a> ModCollector<'a> { crate_id: CrateId, submodules: Vec, file_id: FileId, - errors: &mut Vec, + errors: &mut Vec, ) { for submodule in submodules { - match self.push_child_module(&submodule.name, file_id, true) { - Err(mut more_errors) => errors.append(&mut more_errors), - Ok(child_mod_id) => collect_defs( + if let Some(child) = self.push_child_module(&submodule.name, file_id, true, errors) { + collect_defs( self.def_collector, submodule.contents, file_id, - child_mod_id, + child, crate_id, context, errors, - ), + ); } } } @@ -236,7 +215,7 @@ impl<'a> ModCollector<'a> { context: &mut Context, mod_name: &Ident, crate_id: CrateId, - errors: &mut Vec, + errors: &mut Vec, ) { let child_file_id = match context.file_manager.resolve_path(self.file_id, &mod_name.0.contents) { @@ -244,11 +223,7 @@ impl<'a> ModCollector<'a> { Err(_) => { let err = DefCollectorErrorKind::UnresolvedModuleDecl { mod_name: mod_name.clone() }; - - errors.push(CollectedErrors { - file_id: self.file_id, - errors: vec![err.to_diagnostic()], - }); + errors.push(err.into_file_diagnostic(self.file_id)); return; } }; @@ -257,9 +232,8 @@ impl<'a> ModCollector<'a> { let ast = parse_file(&mut context.file_manager, child_file_id, errors); // Add module into def collector and get a ModuleId - match self.push_child_module(mod_name, child_file_id, true) { - Err(mut more_errors) => errors.append(&mut more_errors), - Ok(child_mod_id) => collect_defs( + if let Some(child_mod_id) = self.push_child_module(mod_name, child_file_id, true, errors) { + collect_defs( self.def_collector, ast, child_file_id, @@ -267,16 +241,19 @@ impl<'a> ModCollector<'a> { crate_id, context, errors, - ), + ); } } - pub fn push_child_module( + /// Add a child module to the current def_map. + /// On error this returns None and pushes to `errors` + fn push_child_module( &mut self, mod_name: &Ident, file_id: FileId, add_to_parent_scope: bool, - ) -> Result> { + errors: &mut Vec, + ) -> Option { // Create a new default module let module_id = self.def_collector.def_map.modules.insert(ModuleData::default()); @@ -305,19 +282,16 @@ impl<'a> ModCollector<'a> { krate: self.def_collector.def_map.krate, local_id: LocalModuleId(module_id), }; - modules[self.module_id.0] - .scope - .define_module_def(mod_name.to_owned(), mod_id) - .map_err(|(first_def, second_def)| { - let err = DefCollectorErrorKind::DuplicateModuleDecl { first_def, second_def }; - - vec![CollectedErrors { - file_id: self.file_id, - errors: vec![err.to_diagnostic()], - }] - })?; + + if let Err((first_def, second_def)) = + modules[self.module_id.0].scope.define_module_def(mod_name.to_owned(), mod_id) + { + let err = DefCollectorErrorKind::DuplicateModuleDecl { first_def, second_def }; + errors.push(err.into_file_diagnostic(self.file_id)); + return None; + } } - Ok(LocalModuleId(module_id)) + Some(LocalModuleId(module_id)) } } diff --git a/crates/noirc_frontend/src/hir/def_collector/errors.rs b/crates/noirc_frontend/src/hir/def_collector/errors.rs index 93d466077c3..6ee97061005 100644 --- a/crates/noirc_frontend/src/hir/def_collector/errors.rs +++ b/crates/noirc_frontend/src/hir/def_collector/errors.rs @@ -1,7 +1,8 @@ use crate::{hir::resolution::import::ImportDirective, Ident}; use noirc_errors::CustomDiagnostic as Diagnostic; -use noirc_errors::DiagnosableError; +use noirc_errors::FileDiagnostic; +use noirc_errors::Span; use thiserror::Error; #[derive(Error, Debug)] @@ -18,11 +19,19 @@ pub enum DefCollectorErrorKind { UnresolvedModuleDecl { mod_name: Ident }, #[error("unresolved import")] UnresolvedImport { import: ImportDirective }, + #[error("Non-struct type used in impl")] + NonStructTypeInImpl { span: Span }, } -impl DiagnosableError for DefCollectorErrorKind { - fn to_diagnostic(&self) -> Diagnostic { - match self { +impl DefCollectorErrorKind { + pub fn into_file_diagnostic(self, file: fm::FileId) -> FileDiagnostic { + Diagnostic::from(self).in_file(file) + } +} + +impl From for Diagnostic { + fn from(error: DefCollectorErrorKind) -> Diagnostic { + match error { DefCollectorErrorKind::DuplicateFunction { first_def, second_def } => { let first_span = first_def.0.span(); let second_span = second_def.0.span(); @@ -97,6 +106,11 @@ impl DiagnosableError for DefCollectorErrorKind { span, ) } + DefCollectorErrorKind::NonStructTypeInImpl { span } => Diagnostic::simple_error( + "Non-struct type used in impl".into(), + "Only struct types may have implementation methods".into(), + span, + ), } } } diff --git a/crates/noirc_frontend/src/hir/def_map/mod.rs b/crates/noirc_frontend/src/hir/def_map/mod.rs index a1873d58530..2fb161e20b3 100644 --- a/crates/noirc_frontend/src/hir/def_map/mod.rs +++ b/crates/noirc_frontend/src/hir/def_map/mod.rs @@ -1,11 +1,12 @@ use crate::graph::CrateId; use crate::hir::def_collector::dc_crate::DefCollector; use crate::hir::Context; -use crate::node_interner::FuncId; +use crate::node_interner::{FuncId, NodeInterner}; use crate::parser::{parse_program, ParsedModule}; +use crate::token::Attribute; use arena::{Arena, Index}; use fm::{FileId, FileManager}; -use noirc_errors::CollectedErrors; +use noirc_errors::FileDiagnostic; use std::collections::HashMap; mod module_def; @@ -50,7 +51,7 @@ impl CrateDefMap { pub fn collect_defs( crate_id: CrateId, context: &mut Context, - errors: &mut Vec, + errors: &mut Vec, ) { // Check if this Crate has already been compiled // XXX: There is probably a better alternative for this. @@ -112,19 +113,29 @@ impl CrateDefMap { pub fn module_file_id(&self, module_id: LocalModuleId) -> FileId { self.modules[module_id.0].origin.file_id() } + + /// Go through all modules in this crate, and find all functions in + /// each module with the #[test] attribute + pub fn get_all_test_functions<'a>( + &'a self, + interner: &'a NodeInterner, + ) -> impl Iterator + 'a { + self.modules.iter().flat_map(|(_, module)| { + let functions = module.scope.values().values().filter_map(|(id, _)| id.as_function()); + functions.filter(|id| interner.function_meta(id).attributes == Some(Attribute::Test)) + }) + } } /// Given a FileId, fetch the File, from the FileManager and parse it's content pub fn parse_file( fm: &mut FileManager, file_id: FileId, - all_errors: &mut Vec, + all_errors: &mut Vec, ) -> ParsedModule { let file = fm.fetch_file(file_id); - let (program, errors) = parse_program(file.get_source()); - if !errors.is_empty() { - all_errors.push(CollectedErrors { file_id, errors }); - }; + let (program, errors) = parse_program(file.source()); + all_errors.extend(errors.into_iter().map(|error| error.in_file(file_id))); program } diff --git a/crates/noirc_frontend/src/hir/resolution/errors.rs b/crates/noirc_frontend/src/hir/resolution/errors.rs index 01fe6f755a9..155288203d7 100644 --- a/crates/noirc_frontend/src/hir/resolution/errors.rs +++ b/crates/noirc_frontend/src/hir/resolution/errors.rs @@ -1,8 +1,8 @@ -use noirc_errors::CustomDiagnostic as Diagnostic; pub use noirc_errors::Span; +use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic}; use thiserror::Error; -use crate::Ident; +use crate::{Ident, Type}; #[derive(Error, Debug, Clone, PartialEq, Eq)] pub enum ResolverError { @@ -44,14 +44,26 @@ pub enum ResolverError { CapturedMutableVariable { span: Span }, #[error("Test functions are not allowed to have any parameters")] TestFunctionHasParameters { span: Span }, + #[error("Only struct types can be used in constructor expressions")] + NonStructUsedInConstructor { typ: Type, span: Span }, + #[error("Only struct types can have generics")] + NonStructWithGenerics { span: Span }, + #[error("Cannot apply generics on Self type")] + GenericsOnSelfType { span: Span }, } impl ResolverError { + pub fn into_file_diagnostic(self, file: fm::FileId) -> FileDiagnostic { + Diagnostic::from(self).in_file(file) + } +} + +impl From for Diagnostic { /// Only user errors can be transformed into a Diagnostic /// ICEs will make the compiler panic, as they could affect the /// soundness of the generated program - pub fn into_diagnostic(self) -> Diagnostic { - match self { + fn from(error: ResolverError) -> Diagnostic { + match error { ResolverError::DuplicateDefinition { name, first_span, second_span } => { let mut diag = Diagnostic::simple_error( format!("duplicate definitions of {name} found"), @@ -208,6 +220,21 @@ impl ResolverError { "Try removing the parameters or moving the test into a wrapper function".into(), span, ), + ResolverError::NonStructUsedInConstructor { typ, span } => Diagnostic::simple_error( + "Only struct types can be used in constructor expressions".into(), + format!("{typ} has no fields to construct it with"), + span, + ), + ResolverError::NonStructWithGenerics { span } => Diagnostic::simple_error( + "Only struct types can have generic arguments".into(), + "Try removing the generic arguments".into(), + span, + ), + ResolverError::GenericsOnSelfType { span } => Diagnostic::simple_error( + "Cannot apply generics to Self type".into(), + "Use an explicit type name or apply the generics at the start of the impl instead".into(), + span, + ), } } } diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index 5db43e09edf..0eb03248532 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -40,7 +40,8 @@ use crate::{ }; use crate::{ ArrayLiteral, Generics, LValue, NoirStruct, Path, Pattern, Shared, StructType, Type, - TypeBinding, TypeVariable, UnresolvedType, UnresolvedTypeExpression, ERROR_IDENT, + TypeBinding, TypeVariable, UnresolvedGenerics, UnresolvedType, UnresolvedTypeExpression, + ERROR_IDENT, }; use fm::FileId; use iter_extended::vecmap; @@ -56,6 +57,8 @@ use crate::hir_def::{ use super::errors::ResolverError; +const SELF_TYPE_NAME: &str = "Self"; + type Scope = GenericScope; type ScopeTree = GenericScopeTree; type ScopeForest = GenericScopeForest; @@ -69,11 +72,13 @@ pub struct Resolver<'a> { file: FileId, /// Set to the current type if we're resolving an impl - self_type: Option, + self_type: Option, - /// Contains a mapping of the current struct's generics to + /// Contains a mapping of the current struct or functions's generics to /// unique type variables if we're resolving a struct. Empty otherwise. - generics: HashMap, (TypeVariable, Span)>, + /// This is a Vec rather than a map to preserve the order a functions generics + /// were declared in. + generics: Vec<(Rc, TypeVariable, Span)>, /// Lambdas share the function scope of the function they're defined in, /// so to identify whether they use any variables from the parent function @@ -96,14 +101,14 @@ impl<'a> Resolver<'a> { scopes: ScopeForest::new(), interner, self_type: None, - generics: HashMap::new(), + generics: Vec::new(), errors: Vec::new(), lambda_index: 0, file, } } - pub fn set_self_type(&mut self, self_type: Option) { + pub fn set_self_type(&mut self, self_type: Option) { self.self_type = self_type; } @@ -130,7 +135,7 @@ impl<'a> Resolver<'a> { // Check whether the function has globals in the local module and add them to the scope self.resolve_local_globals(); - self.add_generics(func.def.generics.clone()); + self.add_generics(&func.def.generics); let (hir_func, func_meta) = self.intern_function(func, func_id); let func_scope_tree = self.scopes.end_function(); @@ -181,6 +186,7 @@ impl<'a> Resolver<'a> { &mut self, name: Ident, mutable: bool, + allow_shadowing: bool, definition: DefinitionKind, ) -> HirIdent { if definition.is_global() { @@ -194,13 +200,17 @@ impl<'a> Resolver<'a> { let scope = self.scopes.get_mut_scope(); let old_value = scope.add_key_value(name.0.contents.clone(), resolver_meta); - if let Some(old_value) = old_value { - self.push_err(ResolverError::DuplicateDefinition { - name: name.0.contents, - first_span: old_value.ident.location.span, - second_span: location.span, - }); + + if !allow_shadowing { + if let Some(old_value) = old_value { + self.push_err(ResolverError::DuplicateDefinition { + name: name.0.contents, + first_span: old_value.ident.location.span, + second_span: location.span, + }); + } } + ident } @@ -286,7 +296,7 @@ impl<'a> Resolver<'a> { FunctionKind::Normal => { let expr_id = self.intern_block(func.def.body); self.interner.push_expr_location(expr_id, func.def.span, self.file); - HirFunction::unsafe_from_expr(expr_id) + HirFunction::unchecked_from_expr(expr_id) } }; @@ -313,24 +323,7 @@ impl<'a> Resolver<'a> { UnresolvedType::Unit => Type::Unit, UnresolvedType::Unspecified => Type::Error, UnresolvedType::Error => Type::Error, - UnresolvedType::Named(path, args) => { - // Check if the path is a type variable first. We currently disallow generics on type - // variables since we do not support higher-kinded types. - if args.is_empty() && path.segments.len() == 1 { - let name = &path.last_segment().0.contents; - if let Some((name, (var, _))) = self.generics.get_key_value(name) { - return Type::NamedGeneric(var.clone(), name.clone()); - } - } - - match self.lookup_struct(path) { - Some(definition) => { - let args = vecmap(args, |arg| self.resolve_type_inner(arg, new_variables)); - Type::Struct(definition, args) - } - None => Type::Error, - } - } + UnresolvedType::Named(path, args) => self.resolve_named_type(path, args, new_variables), UnresolvedType::Tuple(fields) => { Type::Tuple(vecmap(fields, |field| self.resolve_type_inner(field, new_variables))) } @@ -342,6 +335,46 @@ impl<'a> Resolver<'a> { } } + fn find_generic(&self, target_name: &str) -> Option<&(Rc, TypeVariable, Span)> { + self.generics.iter().find(|(name, _, _)| name.as_ref() == target_name) + } + + fn resolve_named_type( + &mut self, + path: Path, + args: Vec, + new_variables: &mut Generics, + ) -> Type { + // Check if the path is a type variable first. We currently disallow generics on type + // variables since we do not support higher-kinded types. + if path.segments.len() == 1 { + let name = &path.last_segment().0.contents; + + if args.is_empty() { + if let Some((name, var, _)) = self.find_generic(name) { + return Type::NamedGeneric(var.clone(), name.clone()); + } + } + + if name == SELF_TYPE_NAME { + if let Some(self_type) = self.self_type.clone() { + if !args.is_empty() { + self.push_err(ResolverError::GenericsOnSelfType { span: path.span() }); + } + return self_type; + } + } + } + + match self.lookup_struct_or_error(path) { + Some(struct_type) => { + let args = vecmap(args, |arg| self.resolve_type_inner(arg, new_variables)); + Type::Struct(struct_type, args) + } + None => Type::Error, + } + } + fn resolve_array_size( &mut self, length: Option, @@ -367,7 +400,7 @@ impl<'a> Resolver<'a> { UnresolvedTypeExpression::Variable(path) => { if path.segments.len() == 1 { let name = &path.last_segment().0.contents; - if let Some((name, (var, _))) = self.generics.get_key_value(name) { + if let Some((name, var, _)) = self.find_generic(name) { return Type::NamedGeneric(var.clone(), name.clone()); } } @@ -424,10 +457,28 @@ impl<'a> Resolver<'a> { } /// Translates an UnresolvedType to a Type - fn resolve_type(&mut self, typ: UnresolvedType) -> Type { + pub fn resolve_type(&mut self, typ: UnresolvedType) -> Type { self.resolve_type_inner(typ, &mut vec![]) } + pub fn take_errors(self) -> Vec { + self.errors + } + + /// Return the current generics. + /// Needed to keep referring to the same type variables across many + /// methods in a single impl. + pub fn get_generics(&self) -> &[(Rc, TypeVariable, Span)] { + &self.generics + } + + /// Set the current generics that are in scope. + /// Unlike add_generics, this function will not create any new type variables, + /// opting to reuse the existing ones it is directly given. + pub fn set_generics(&mut self, generics: Vec<(Rc, TypeVariable, Span)>) { + self.generics = generics; + } + /// Translates a (possibly Unspecified) UnresolvedType to a Type. /// Any UnresolvedType::Unspecified encountered are replaced with fresh type variables. fn resolve_inferred_type(&mut self, typ: UnresolvedType) -> Type { @@ -437,7 +488,9 @@ impl<'a> Resolver<'a> { } } - fn add_generics(&mut self, generics: Vec) -> Generics { + /// Add the given generics to scope. + /// Each generic will have a fresh Shared associated with it. + pub fn add_generics(&mut self, generics: &UnresolvedGenerics) -> Generics { vecmap(generics, |generic| { // Map the generic to a fresh type variable let id = self.interner.next_type_variable_id(); @@ -446,14 +499,18 @@ impl<'a> Resolver<'a> { // Check for name collisions of this generic let name = Rc::new(generic.0.contents.clone()); - if let Some(old) = self.generics.insert(name, (typevar.clone(), span)) { + + if let Some((_, _, first_span)) = self.find_generic(&name) { let span = generic.0.span(); self.errors.push(ResolverError::DuplicateDefinition { - name: generic.0.contents, - first_span: old.1, + name: generic.0.contents.clone(), + first_span: *first_span, second_span: span, }) + } else { + self.generics.push((name, typevar.clone(), span)); } + (id, typevar) }) } @@ -462,7 +519,7 @@ impl<'a> Resolver<'a> { mut self, unresolved: NoirStruct, ) -> (Generics, BTreeMap, Vec) { - let generics = self.add_generics(unresolved.generics); + let generics = self.add_generics(&unresolved.generics); // Check whether the struct definition has globals in the local module and add them to the scope self.resolve_local_globals(); @@ -488,6 +545,8 @@ impl<'a> Resolver<'a> { /// Extract metadata from a NoirFunction /// to be used in analysis and intern the function parameters + /// Prerequisite: self.add_generics() has already been called with the given + /// function's generics, including any generics from the impl, if any. fn extract_meta(&mut self, func: &NoirFunction, func_id: FuncId) -> FuncMeta { let location = Location::new(func.name_ident().span(), self.file); let id = self.interner.function_definition_id(func_id); @@ -495,16 +554,10 @@ impl<'a> Resolver<'a> { let attributes = func.attribute().cloned(); - assert_eq!(self.generics.len(), func.def.generics.len()); - let mut generics = vecmap(&func.def.generics, |generic| { - // Always expect self.generics to contain all the generics of this function - let typevar = self.generics.get(&generic.0.contents).unwrap(); - match &*typevar.0.borrow() { - TypeBinding::Unbound(id) => (*id, typevar.0.clone()), - TypeBinding::Bound(binding) => unreachable!( - "Expected {} to be unbound, but it is bound to {}", - generic, binding - ), + let mut generics = vecmap(&self.generics, |(name, typevar, _)| match &*typevar.borrow() { + TypeBinding::Unbound(id) => (*id, typevar.clone()), + TypeBinding::Bound(binding) => { + unreachable!("Expected {} to be unbound, but it is bound to {}", name, binding) } }); @@ -677,8 +730,12 @@ impl<'a> Resolver<'a> { // TODO: For loop variables are currently mutable by default since we haven't // yet implemented syntax for them to be optionally mutable. let (identifier, block_id) = self.in_new_scope(|this| { - let decl = - this.add_variable_decl(identifier, false, DefinitionKind::Local(None)); + let decl = this.add_variable_decl( + identifier, + false, + false, + DefinitionKind::Local(None), + ); (decl, this.resolve_expression(block)) }); @@ -710,21 +767,24 @@ impl<'a> Resolver<'a> { ExpressionKind::Constructor(constructor) => { let span = constructor.type_name.span(); - if let Some(typ) = self.lookup_struct(constructor.type_name) { - let type_id = typ.borrow().id; - - HirExpression::Constructor(HirConstructorExpression { - type_id, - fields: self.resolve_constructor_fields( - type_id, - constructor.fields, - span, - Resolver::resolve_expression, - ), - r#type: typ, - }) - } else { - HirExpression::Error + match self.lookup_type_or_error(constructor.type_name) { + Some(Type::Struct(r#type, struct_generics)) => { + let typ = r#type.clone(); + let fields = constructor.fields; + let resolve_expr = Resolver::resolve_expression; + let fields = + self.resolve_constructor_fields(typ, fields, span, resolve_expr); + HirExpression::Constructor(HirConstructorExpression { + fields, + r#type, + struct_generics, + }) + } + Some(typ) => { + self.push_err(ResolverError::NonStructUsedInConstructor { typ, span }); + HirExpression::Error + } + None => HirExpression::Error, } } ExpressionKind::MemberAccess(access) => { @@ -782,7 +842,7 @@ impl<'a> Resolver<'a> { (Some(_), DefinitionKind::Local(_)) => DefinitionKind::Local(None), (_, other) => other, }; - let id = self.add_variable_decl(name, mutable.is_some(), definition); + let id = self.add_variable_decl(name, mutable.is_some(), false, definition); HirPattern::Identifier(id) } Pattern::Mutable(pattern, span) => { @@ -800,14 +860,33 @@ impl<'a> Resolver<'a> { HirPattern::Tuple(fields, span) } Pattern::Struct(name, fields, span) => { - let struct_id = self.lookup_type(name); - let struct_type = self.get_struct(struct_id); + let error_identifier = |this: &mut Self| { + // Must create a name here to return a HirPattern::Identifier. Allowing + // shadowing here lets us avoid further errors if we define ERROR_IDENT + // multiple times. + let name = ERROR_IDENT.into(); + let identifier = this.add_variable_decl(name, false, true, definition); + HirPattern::Identifier(identifier) + }; + + let (struct_type, generics) = match self.lookup_type_or_error(name) { + Some(Type::Struct(struct_type, generics)) => (struct_type, generics), + None => return error_identifier(self), + Some(typ) => { + self.push_err(ResolverError::NonStructUsedInConstructor { typ, span }); + return error_identifier(self); + } + }; + let resolve_field = |this: &mut Self, pattern| { this.resolve_pattern_mutable(pattern, mutable, definition) }; - let fields = - self.resolve_constructor_fields(struct_id, fields, span, resolve_field); - HirPattern::Struct(struct_type, fields, span) + + let typ = struct_type.clone(); + let fields = self.resolve_constructor_fields(typ, fields, span, resolve_field); + + let typ = Type::Struct(struct_type, generics); + HirPattern::Struct(typ, fields, span) } } } @@ -820,14 +899,14 @@ impl<'a> Resolver<'a> { /// and constructor patterns. fn resolve_constructor_fields( &mut self, - type_id: StructId, + struct_type: Shared, fields: Vec<(Ident, T)>, span: Span, mut resolve_function: impl FnMut(&mut Self, T) -> U, ) -> Vec<(Ident, U)> { let mut ret = Vec::with_capacity(fields.len()); let mut seen_fields = HashSet::new(); - let mut unseen_fields = self.get_field_names_of_type(type_id); + let mut unseen_fields = self.get_field_names_of_type(&struct_type); for (field, expr) in fields { let resolved = resolve_function(self, expr); @@ -842,7 +921,7 @@ impl<'a> Resolver<'a> { // field not required by struct self.push_err(ResolverError::NoSuchField { field: field.clone(), - struct_definition: self.get_struct(type_id).borrow().name.clone(), + struct_definition: struct_type.borrow().name.clone(), }); } @@ -853,7 +932,7 @@ impl<'a> Resolver<'a> { self.push_err(ResolverError::MissingFields { span, missing_fields: unseen_fields.into_iter().map(|field| field.to_string()).collect(), - struct_definition: self.get_struct(type_id).borrow().name.clone(), + struct_definition: struct_type.borrow().name.clone(), }); } @@ -864,10 +943,8 @@ impl<'a> Resolver<'a> { self.interner.get_struct(type_id) } - fn get_field_names_of_type(&self, type_id: StructId) -> BTreeSet { - let typ = self.get_struct(type_id); - let typ = typ.borrow(); - typ.field_names() + fn get_field_names_of_type(&self, typ: &Shared) -> BTreeSet { + typ.borrow().field_names() } fn lookup(&mut self, path: Path) -> Result { @@ -914,32 +991,40 @@ impl<'a> Resolver<'a> { Err(ResolverError::Expected { span, expected, got }) } - fn lookup_type(&mut self, path: Path) -> StructId { + /// Lookup a given struct type by name. + fn lookup_struct_or_error(&mut self, path: Path) -> Option> { + match self.lookup(path) { + Ok(struct_id) => Some(self.get_struct(struct_id)), + Err(error) => { + self.push_err(error); + None + } + } + } + + /// Looks up a given type by name. + /// This will also instantiate any struct types found. + fn lookup_type_or_error(&mut self, path: Path) -> Option { let ident = path.as_ident(); - if ident.map_or(false, |i| i == "Self") { - if let Some(id) = &self.self_type { - return *id; + if ident.map_or(false, |i| i == SELF_TYPE_NAME) { + if let Some(typ) = &self.self_type { + return Some(typ.clone()); } } match self.lookup(path) { - Ok(id) => id, + Ok(struct_id) => { + let struct_type = self.get_struct(struct_id); + let generics = struct_type.borrow().instantiate(self.interner); + Some(Type::Struct(struct_type, generics)) + } Err(error) => { self.push_err(error); - StructId::dummy_id() + None } } } - pub fn lookup_struct(&mut self, path: Path) -> Option> { - let id = self.lookup_type(path); - (id != StructId::dummy_id()).then(|| self.get_struct(id)) - } - - pub fn lookup_type_for_impl(mut self, path: Path) -> (StructId, Vec) { - (self.lookup_type(path), self.errors) - } - fn resolve_path(&mut self, path: Path) -> Result { let span = path.span(); let name = path.as_string(); diff --git a/crates/noirc_frontend/src/hir/type_check/errors.rs b/crates/noirc_frontend/src/hir/type_check/errors.rs index 82783940ad0..33e98e3d6af 100644 --- a/crates/noirc_frontend/src/hir/type_check/errors.rs +++ b/crates/noirc_frontend/src/hir/type_check/errors.rs @@ -35,15 +35,21 @@ pub enum TypeCheckError { } impl TypeCheckError { - pub fn into_diagnostic(self) -> Diagnostic { - match self { + pub fn add_context(self, ctx: &'static str) -> Self { + TypeCheckError::Context { err: Box::new(self), ctx } + } +} + +impl From for Diagnostic { + fn from(error: TypeCheckError) -> Diagnostic { + match error { TypeCheckError::TypeCannotBeUsed { typ, place, span } => Diagnostic::simple_error( format!("The type {} cannot be used in a {}", &typ, place), String::new(), span, ), TypeCheckError::Context { err, ctx } => { - let mut diag = err.into_diagnostic(); + let mut diag = Diagnostic::from(*err); diag.add_note(ctx.to_owned()); diag } @@ -92,8 +98,4 @@ impl TypeCheckError { ), } } - - pub fn add_context(self, ctx: &'static str) -> Self { - TypeCheckError::Context { err: Box::new(self), ctx } - } } diff --git a/crates/noirc_frontend/src/hir/type_check/expr.rs b/crates/noirc_frontend/src/hir/type_check/expr.rs index ad0b4a3710a..4d43a8970d9 100644 --- a/crates/noirc_frontend/src/hir/type_check/expr.rs +++ b/crates/noirc_frontend/src/hir/type_check/expr.rs @@ -216,7 +216,7 @@ pub(crate) fn type_check_expression( } HirExpression::If(if_expr) => check_if_expr(&if_expr, expr_id, interner, errors), HirExpression::Constructor(constructor) => { - check_constructor(&constructor, expr_id, interner, errors) + check_constructor(constructor, expr_id, interner, errors) } HirExpression::MemberAccess(access) => { check_member_access(access, interner, *expr_id, errors) @@ -353,21 +353,16 @@ fn lookup_method( errors: &mut Vec, ) -> Option { match &object_type { - Type::Struct(typ, _args) => { - let typ = typ.borrow(); - match typ.methods.get(method_name) { - Some(method_id) => Some(*method_id), - None => { - errors.push(TypeCheckError::Unstructured { - span: interner.expr_span(expr_id), - msg: format!( - "No method named '{method_name}' found for type '{object_type}'", - ), - }); - None - } + Type::Struct(typ, _args) => match interner.lookup_method(typ.borrow().id, method_name) { + Some(method_id) => Some(method_id), + None => { + errors.push(TypeCheckError::Unstructured { + span: interner.expr_span(expr_id), + msg: format!("No method named '{method_name}' found for type '{object_type}'",), + }); + None } - } + }, // If we fail to resolve the object to a struct type, we have no way of type // checking its arguments as we can't even resolve the name of the function Type::Error => None, @@ -412,8 +407,10 @@ fn type_check_method_call( } let (function_type, instantiation_bindings) = func_meta.typ.instantiate(interner); + interner.store_instantiation_bindings(*function_ident_id, instantiation_bindings); interner.push_expr_type(function_ident_id, function_type.clone()); + bind_function_type(function_type, arguments, span, interner, errors) } } @@ -513,19 +510,8 @@ pub fn infix_operand_type_rules( use Type::*; match (lhs_type, rhs_type) { - (Integer(comptime_x, sign_x, bit_width_x), Integer(comptime_y, sign_y, bit_width_y)) => { - if sign_x != sign_y { - return Err(make_error(format!("Integers must have the same signedness LHS is {sign_x:?}, RHS is {sign_y:?} "))) - } - if bit_width_x != bit_width_y { - return Err(make_error(format!("Integers must have the same bit width LHS is {bit_width_x}, RHS is {bit_width_y} "))) - } - let comptime = comptime_x.and(comptime_y, op.location.span); - Ok(Integer(comptime, *sign_x, *bit_width_x)) - } - (Integer(..), FieldElement(..)) | (FieldElement(..), Integer(..)) => { - Err(make_error("Cannot use an integer and a Field in a binary operation, try converting the Field into an integer".to_string())) - } + // Matches on PolymorphicInteger and TypeVariable must be first so that we follow any type + // bindings. (PolymorphicInteger(comptime, int), other) | (other, PolymorphicInteger(comptime, int)) => { if let TypeBinding::Bound(binding) = &*int.borrow() { @@ -555,6 +541,32 @@ pub fn infix_operand_type_rules( Err(make_error(format!("Types in a binary operation should match, but found {lhs_type} and {rhs_type}"))) } } + (TypeVariable(var), other) + | (other, TypeVariable(var)) => { + if let TypeBinding::Bound(binding) = &*var.borrow() { + return infix_operand_type_rules(binding, op, other, span, interner, errors); + } + + let comptime = CompTime::No(None); + if other.try_bind_to_polymorphic_int(var, &comptime, true, op.location.span).is_ok() || other == &Type::Error { + Ok(other.clone()) + } else { + Err(make_error(format!("Types in a binary operation should match, but found {lhs_type} and {rhs_type}"))) + } + } + (Integer(comptime_x, sign_x, bit_width_x), Integer(comptime_y, sign_y, bit_width_y)) => { + if sign_x != sign_y { + return Err(make_error(format!("Integers must have the same signedness LHS is {sign_x:?}, RHS is {sign_y:?} "))) + } + if bit_width_x != bit_width_y { + return Err(make_error(format!("Integers must have the same bit width LHS is {bit_width_x}, RHS is {bit_width_y} "))) + } + let comptime = comptime_x.and(comptime_y, op.location.span); + Ok(Integer(comptime, *sign_x, *bit_width_x)) + } + (Integer(..), FieldElement(..)) | (FieldElement(..), Integer(..)) => { + Err(make_error("Cannot use an integer and a Field in a binary operation, try converting the Field into an integer".to_string())) + } (Integer(..), typ) | (typ,Integer(..)) => { Err(make_error(format!("Integer cannot be used with type {typ}"))) } @@ -578,20 +590,6 @@ pub fn infix_operand_type_rules( (Bool(comptime_x), Bool(comptime_y)) => Ok(Bool(comptime_x.and(comptime_y, op.location.span))), - (TypeVariable(var), other) - | (other, TypeVariable(var)) => { - if let TypeBinding::Bound(binding) = &*var.borrow() { - return infix_operand_type_rules(binding, op, other, span, interner, errors); - } - - let comptime = CompTime::No(None); - if other.try_bind_to_polymorphic_int(var, &comptime, true, op.location.span).is_ok() || other == &Type::Error { - Ok(other.clone()) - } else { - Err(make_error(format!("Types in a binary operation should match, but found {lhs_type} and {rhs_type}"))) - } - } - (lhs, rhs) => Err(make_error(format!("Unsupported types for binary operation: {lhs} and {rhs}"))), } } @@ -644,12 +642,13 @@ fn check_if_expr( } fn check_constructor( - constructor: &expr::HirConstructorExpression, + constructor: expr::HirConstructorExpression, expr_id: &ExprId, interner: &mut NodeInterner, errors: &mut Vec, ) -> Type { - let typ = &constructor.r#type; + let typ = constructor.r#type; + let generics = constructor.struct_generics; // Sanity check, this should be caught during name resolution anyway assert_eq!(constructor.fields.len(), typ.borrow().num_fields()); @@ -657,15 +656,14 @@ fn check_constructor( // Sort argument types by name so we can zip with the struct type in the same ordering. // Note that we use a Vec to store the original arguments (rather than a BTreeMap) to // preserve the evaluation order of the source code. - let mut args = constructor.fields.clone(); + let mut args = constructor.fields; args.sort_by_key(|arg| arg.0.clone()); - let typ_ref = typ.borrow(); - let (generics, fields) = typ_ref.instantiate(interner); + let fields = typ.borrow().get_fields(&generics); for ((param_name, param_type), (arg_ident, arg)) in fields.into_iter().zip(args) { // Sanity check to ensure we're matching against the same field - assert_eq!(param_name, &arg_ident.0.contents); + assert_eq!(param_name, arg_ident.0.contents); let arg_type = type_check_expression(interner, &arg, errors); @@ -677,7 +675,7 @@ fn check_constructor( }); } - Type::Struct(typ.clone(), generics) + Type::Struct(typ, generics) } pub fn check_member_access( @@ -722,6 +720,32 @@ pub fn comparator_operand_type_rules( use crate::BinaryOpKind::{Equal, NotEqual}; use Type::*; match (lhs_type, rhs_type) { + // Matches on PolymorphicInteger and TypeVariable must be first to follow any type + // bindings. + (PolymorphicInteger(comptime, int), other) + | (other, PolymorphicInteger(comptime, int)) => { + if let TypeBinding::Bound(binding) = &*int.borrow() { + return comparator_operand_type_rules(other, binding, op, errors); + } + if other.try_bind_to_polymorphic_int(int, comptime, true, op.location.span).is_ok() || other == &Type::Error { + Ok(Bool(comptime.clone())) + } else { + Err(format!("Types in a binary operation should match, but found {lhs_type} and {rhs_type}")) + } + } + (TypeVariable(var), other) + | (other, TypeVariable(var)) => { + if let TypeBinding::Bound(binding) = &*var.borrow() { + return comparator_operand_type_rules(binding, other, op, errors); + } + + let comptime = CompTime::No(None); + if other.try_bind_to_polymorphic_int(var, &comptime, true, op.location.span).is_ok() || other == &Type::Error { + Ok(other.clone()) + } else { + Err(format!("Types in a binary operation should match, but found {lhs_type} and {rhs_type}")) + } + } (Integer(comptime_x, sign_x, bit_width_x), Integer(comptime_y, sign_y, bit_width_y)) => { if sign_x != sign_y { return Err(format!("Integers must have the same signedness LHS is {sign_x:?}, RHS is {sign_y:?} ")) @@ -735,17 +759,6 @@ pub fn comparator_operand_type_rules( (Integer(..), FieldElement(..)) | ( FieldElement(..), Integer(..) ) => { Err("Cannot use an integer and a Field in a binary operation, try converting the Field into an integer first".to_string()) } - (PolymorphicInteger(comptime, int), other) - | (other, PolymorphicInteger(comptime, int)) => { - if let TypeBinding::Bound(binding) = &*int.borrow() { - return comparator_operand_type_rules(other, binding, op, errors); - } - if other.try_bind_to_polymorphic_int(int, comptime, true, op.location.span).is_ok() || other == &Type::Error { - Ok(Bool(comptime.clone())) - } else { - Err(format!("Types in a binary operation should match, but found {lhs_type} and {rhs_type}")) - } - } (Integer(..), typ) | (typ,Integer(..)) => { Err(format!("Integer cannot be used with type {typ}")) } @@ -795,19 +808,6 @@ pub fn comparator_operand_type_rules( } Err(format!("Unsupported types for comparison: {name_a} and {name_b}")) } - (TypeVariable(var), other) - | (other, TypeVariable(var)) => { - if let TypeBinding::Bound(binding) = &*var.borrow() { - return comparator_operand_type_rules(binding, other, op, errors); - } - - let comptime = CompTime::No(None); - if other.try_bind_to_polymorphic_int(var, &comptime, true, op.location.span).is_ok() || other == &Type::Error { - Ok(other.clone()) - } else { - Err(format!("Types in a binary operation should match, but found {lhs_type} and {rhs_type}")) - } - } (String(x_size), String(y_size)) => { x_size.unify(y_size, op.location.span, errors, || { TypeCheckError::Unstructured { diff --git a/crates/noirc_frontend/src/hir/type_check/mod.rs b/crates/noirc_frontend/src/hir/type_check/mod.rs index 8453d5e5f77..29b1d7e0061 100644 --- a/crates/noirc_frontend/src/hir/type_check/mod.rs +++ b/crates/noirc_frontend/src/hir/type_check/mod.rs @@ -134,7 +134,7 @@ mod test { interner.push_expr_location(expr_id, Span::single_char(0), file); // Create function to enclose the let statement - let func = HirFunction::unsafe_from_expr(expr_id); + let func = HirFunction::unchecked_from_expr(expr_id); let func_id = interner.push_fn(func); let name = HirIdent { diff --git a/crates/noirc_frontend/src/hir/type_check/stmt.rs b/crates/noirc_frontend/src/hir/type_check/stmt.rs index ac4c18139dd..f79df00dbec 100644 --- a/crates/noirc_frontend/src/hir/type_check/stmt.rs +++ b/crates/noirc_frontend/src/hir/type_check/stmt.rs @@ -77,27 +77,25 @@ pub fn bind_pattern( }); } }, - HirPattern::Struct(struct_type, fields, span) => match typ { - Type::Struct(inner, args) if &inner == struct_type => { - let mut pattern_fields = fields.clone(); + HirPattern::Struct(struct_type, fields, span) => { + struct_type.unify(&typ, *span, errors, || TypeCheckError::TypeMismatch { + expected_typ: typ.to_string(), + expr_typ: struct_type.to_string(), + expr_span: *span, + }); + if let Type::Struct(struct_type, generics) = struct_type { + let mut pattern_fields = fields.clone(); pattern_fields.sort_by_key(|(ident, _)| ident.clone()); + let struct_type = struct_type.borrow(); - for pattern_field in pattern_fields { + for (field_name, field_pattern) in pattern_fields { let type_field = - inner.borrow().get_field(&pattern_field.0 .0.contents, &args).unwrap().0; - bind_pattern(interner, &pattern_field.1, type_field, errors); + struct_type.get_field(&field_name.0.contents, generics).unwrap().0; + bind_pattern(interner, &field_pattern, type_field, errors); } } - Type::Error => (), - other => { - errors.push(TypeCheckError::TypeMismatch { - expected_typ: other.to_string(), - expr_typ: other.to_string(), - expr_span: *span, - }); - } - }, + } } } diff --git a/crates/noirc_frontend/src/hir_def/expr.rs b/crates/noirc_frontend/src/hir_def/expr.rs index 3cc5db4565d..5ad5a3a6fea 100644 --- a/crates/noirc_frontend/src/hir_def/expr.rs +++ b/crates/noirc_frontend/src/hir_def/expr.rs @@ -2,7 +2,7 @@ use acvm::FieldElement; use fm::FileId; use noirc_errors::Location; -use crate::node_interner::{DefinitionId, ExprId, FuncId, NodeInterner, StmtId, StructId}; +use crate::node_interner::{DefinitionId, ExprId, FuncId, NodeInterner, StmtId}; use crate::{BinaryOp, BinaryOpKind, Ident, Shared, UnaryOp}; use super::stmt::HirPattern; @@ -149,8 +149,8 @@ impl HirMethodCallExpression { #[derive(Debug, Clone)] pub struct HirConstructorExpression { - pub type_id: StructId, pub r#type: Shared, + pub struct_generics: Vec, // NOTE: It is tempting to make this a BTreeSet to force ordering of field // names (and thus remove the need to normalize them during type checking) diff --git a/crates/noirc_frontend/src/hir_def/function.rs b/crates/noirc_frontend/src/hir_def/function.rs index d38d4638fbe..2fa9d5edf1b 100644 --- a/crates/noirc_frontend/src/hir_def/function.rs +++ b/crates/noirc_frontend/src/hir_def/function.rs @@ -18,9 +18,7 @@ impl HirFunction { HirFunction(ExprId::empty_block_id()) } - // This function is marked as unsafe because - // the expression kind is not being checked - pub const fn unsafe_from_expr(expr_id: ExprId) -> HirFunction { + pub const fn unchecked_from_expr(expr_id: ExprId) -> HirFunction { HirFunction(expr_id) } diff --git a/crates/noirc_frontend/src/hir_def/stmt.rs b/crates/noirc_frontend/src/hir_def/stmt.rs index 986d417027f..278126d224d 100644 --- a/crates/noirc_frontend/src/hir_def/stmt.rs +++ b/crates/noirc_frontend/src/hir_def/stmt.rs @@ -1,6 +1,6 @@ use super::expr::HirIdent; use crate::node_interner::ExprId; -use crate::{Ident, Shared, StructType, Type}; +use crate::{Ident, Type}; use fm::FileId; use noirc_errors::Span; @@ -51,7 +51,7 @@ pub enum HirPattern { Identifier(HirIdent), Mutable(Box, Span), Tuple(Vec, Span), - Struct(Shared, Vec<(Ident, HirPattern)>, Span), + Struct(Type, Vec<(Ident, HirPattern)>, Span), } impl HirPattern { diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index b99cb917df4..10b70ad2db1 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -9,10 +9,7 @@ use iter_extended::{btree_map, vecmap}; use noirc_abi::AbiType; use noirc_errors::Span; -use crate::{ - node_interner::{FuncId, StructId}, - Ident, Signedness, -}; +use crate::{node_interner::StructId, Ident, Signedness}; /// A shared, mutable reference to some T. /// Wrapper is required for Hash impl of RefCell. @@ -75,7 +72,6 @@ pub struct StructType { fields: BTreeMap, pub generics: Generics, - pub methods: HashMap, pub span: Span, } @@ -101,7 +97,7 @@ impl StructType { fields: BTreeMap, generics: Generics, ) -> StructType { - StructType { id, fields, name, span, generics, methods: HashMap::new() } + StructType { id, fields, name, span, generics } } pub fn set_fields(&mut self, fields: BTreeMap) { @@ -155,30 +151,9 @@ impl StructType { } /// Instantiate this struct type, returning a Vec of the new generic args (in - /// the same order as self.generics) and a map of each instantiated field - pub fn instantiate<'a>( - &'a self, - interner: &mut NodeInterner, - ) -> (Vec, BTreeMap<&'a str, Type>) { - let (generics, substitutions) = self - .generics - .iter() - .map(|(old_id, old_var)| { - let new = interner.next_type_variable(); - (new.clone(), (*old_id, (old_var.clone(), new))) - }) - .unzip(); - - let fields = self - .fields - .iter() - .map(|(name, typ)| { - let typ = typ.substitute(&substitutions); - (name.0.contents.as_str(), typ) - }) - .collect(); - - (generics, fields) + /// the same order as self.generics) + pub fn instantiate(&self, interner: &mut NodeInterner) -> Vec { + vecmap(&self.generics, |_| interner.next_type_variable()) } } diff --git a/crates/noirc_frontend/src/lexer/errors.rs b/crates/noirc_frontend/src/lexer/errors.rs index 86398de3f03..189bfdb4bb2 100644 --- a/crates/noirc_frontend/src/lexer/errors.rs +++ b/crates/noirc_frontend/src/lexer/errors.rs @@ -2,7 +2,7 @@ use crate::token::SpannedToken; use super::token::Token; use noirc_errors::CustomDiagnostic as Diagnostic; -use noirc_errors::{DiagnosableError, Span}; +use noirc_errors::Span; use thiserror::Error; #[derive(Error, Clone, Debug, PartialEq, Eq)] @@ -77,9 +77,9 @@ impl LexerErrorKind { } } -impl DiagnosableError for LexerErrorKind { - fn to_diagnostic(&self) -> Diagnostic { - let (primary, secondary, span) = self.parts(); +impl From for Diagnostic { + fn from(error: LexerErrorKind) -> Diagnostic { + let (primary, secondary, span) = error.parts(); Diagnostic::simple_error(primary, secondary, span) } } diff --git a/crates/noirc_frontend/src/lexer/lexer.rs b/crates/noirc_frontend/src/lexer/lexer.rs index cbdae322e81..7d4fcf3c0a9 100644 --- a/crates/noirc_frontend/src/lexer/lexer.rs +++ b/crates/noirc_frontend/src/lexer/lexer.rs @@ -33,7 +33,7 @@ impl<'a> Lexer<'a> { } pub fn from_file(source: File<'a>) -> Self { - let source_file = source.get_source(); + let source_file = source.source(); Lexer::new(source_file) } diff --git a/crates/noirc_frontend/src/lib.rs b/crates/noirc_frontend/src/lib.rs index 64b98c64eb3..2db27b62879 100644 --- a/crates/noirc_frontend/src/lib.rs +++ b/crates/noirc_frontend/src/lib.rs @@ -1,3 +1,4 @@ +#![forbid(unsafe_code)] pub mod ast; pub mod graph; pub mod lexer; diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index 6b05ebed6ec..35b3a7e7a52 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -18,7 +18,6 @@ use crate::hir_def::{ function::{FuncMeta, HirFunction}, stmt::HirStatement, }; -use crate::token::Attribute; use crate::{Shared, TypeBinding, TypeBindings, TypeVariableId}; #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] @@ -169,6 +168,11 @@ pub struct NodeInterner { language: Language, delayed_type_checks: Vec, + + // A map from a struct type and method name to a function id for the method + // along with any generic on the struct it may require. E.g. if the impl is + // only for `impl Foo` rather than all Foo, the generics will be `vec![String]`. + struct_methods: HashMap<(StructId, String), (Vec, FuncId)>, } type TypeCheckFn = Box Result<(), TypeCheckError>>; @@ -236,6 +240,7 @@ impl Default for NodeInterner { globals: HashMap::new(), language: Language::R1CS, delayed_type_checks: vec![], + struct_methods: HashMap::new(), }; // An empty block expression is used often, we add this into the `node` on startup @@ -554,7 +559,7 @@ impl NodeInterner { #[allow(deprecated)] pub fn foreign(&self, opcode: &str) -> bool { - let is_supported = acvm::default_is_blackbox_supported(self.language.clone()); + let is_supported = acvm::default_is_black_box_supported(self.language.clone()); let black_box_func = match acvm::acir::BlackBoxFunc::lookup(opcode) { Some(black_box_func) => black_box_func, None => return false, @@ -570,10 +575,26 @@ impl NodeInterner { std::mem::take(&mut self.delayed_type_checks) } - pub fn get_all_test_functions(&self) -> impl Iterator + '_ { - self.func_meta.iter().filter_map(|(id, meta)| { - let is_test = meta.attributes.as_ref()? == &Attribute::Test; - is_test.then_some(*id) - }) + /// Add a method to a type. + /// This will panic for non-struct types currently as we do not support methods + /// for primitives. We could allow this in the future however. + pub fn add_method( + &mut self, + self_type: &Type, + method_name: String, + method_id: FuncId, + ) -> Option { + match self_type { + Type::Struct(struct_type, generics) => { + let key = (struct_type.borrow().id, method_name); + self.struct_methods.insert(key, (generics.clone(), method_id)).map(|(_, id)| id) + } + other => unreachable!("Tried adding method to non-struct type '{}'", other), + } + } + + /// Search by name for a method on the given struct + pub fn lookup_method(&self, id: StructId, method_name: &str) -> Option { + self.struct_methods.get(&(id, method_name.to_owned())).map(|(_, id)| *id) } } diff --git a/crates/noirc_frontend/src/parser/errors.rs b/crates/noirc_frontend/src/parser/errors.rs index 9dd73e28531..7f19ef7f062 100644 --- a/crates/noirc_frontend/src/parser/errors.rs +++ b/crates/noirc_frontend/src/parser/errors.rs @@ -5,7 +5,6 @@ use crate::BinaryOp; use iter_extended::vecmap; use noirc_errors::CustomDiagnostic as Diagnostic; -use noirc_errors::DiagnosableError; use noirc_errors::Span; #[derive(Debug, Clone, PartialEq, Eq)] @@ -82,13 +81,13 @@ impl std::fmt::Display for ParserError { } } -impl DiagnosableError for ParserError { - fn to_diagnostic(&self) -> Diagnostic { - match &self.reason { - Some(reason) => Diagnostic::simple_error(reason.clone(), String::new(), self.span), +impl From for Diagnostic { + fn from(error: ParserError) -> Diagnostic { + match &error.reason { + Some(reason) => Diagnostic::simple_error(reason.clone(), String::new(), error.span), None => { - let primary = self.to_string(); - Diagnostic::simple_error(primary, String::new(), self.span) + let primary = error.to_string(); + Diagnostic::simple_error(primary, String::new(), error.span) } } } diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 686220c523c..56e21f59f3e 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -16,15 +16,15 @@ use crate::{ use chumsky::prelude::*; use iter_extended::vecmap; use noirc_abi::AbiVisibility; -use noirc_errors::{CustomDiagnostic, DiagnosableError, Span, Spanned}; +use noirc_errors::{CustomDiagnostic, Span, Spanned}; pub fn parse_program(source_program: &str) -> (ParsedModule, Vec) { let lexer = Lexer::new(source_program); let (tokens, lexing_errors) = lexer.lex(); - let mut errors = vecmap(&lexing_errors, DiagnosableError::to_diagnostic); + let mut errors = vecmap(lexing_errors, Into::into); let (module, parsing_errors) = program().parse_recovery_verbose(tokens); - errors.extend(parsing_errors.iter().map(DiagnosableError::to_diagnostic)); + errors.extend(parsing_errors.into_iter().map(Into::into)); (module.unwrap(), errors) } @@ -227,11 +227,14 @@ fn self_parameter() -> impl NoirParser<(Pattern, UnresolvedType, AbiVisibility)> fn implementation() -> impl NoirParser { keyword(Keyword::Impl) - .ignore_then(path()) + .ignore_then(generics()) + .then(parse_type().map_with_span(|typ, span| (typ, span))) .then_ignore(just(Token::LeftBrace)) .then(function_definition(true).repeated()) .then_ignore(just(Token::RightBrace)) - .map(|(type_path, methods)| TopLevelStatement::Impl(NoirImpl { type_path, methods })) + .map(|((generics, (object_type, type_span)), methods)| { + TopLevelStatement::Impl(NoirImpl { generics, object_type, type_span, methods }) + }) } fn block_expr<'a, P>(expr_parser: P) -> impl NoirParser + 'a @@ -898,7 +901,7 @@ fn literal() -> impl NoirParser { #[cfg(test)] mod test { - use noirc_errors::{CustomDiagnostic, DiagnosableError}; + use noirc_errors::CustomDiagnostic; use super::*; use crate::{ArrayLiteral, Literal}; @@ -910,12 +913,12 @@ mod test { let lexer = Lexer::new(program); let (tokens, lexer_errors) = lexer.lex(); if !lexer_errors.is_empty() { - return Err(vecmap(&lexer_errors, DiagnosableError::to_diagnostic)); + return Err(vecmap(lexer_errors, Into::into)); } parser .then_ignore(just(Token::EOF)) .parse(tokens) - .map_err(|errors| vecmap(&errors, DiagnosableError::to_diagnostic)) + .map_err(|errors| vecmap(errors, Into::into)) } fn parse_recover(parser: P, program: &str) -> (Option, Vec) @@ -926,8 +929,8 @@ mod test { let (tokens, lexer_errors) = lexer.lex(); let (opt, errs) = parser.then_ignore(force(just(Token::EOF))).parse_recovery(tokens); - let mut errors = vecmap(&lexer_errors, DiagnosableError::to_diagnostic); - errors.extend(errs.iter().map(DiagnosableError::to_diagnostic)); + let mut errors = vecmap(lexer_errors, Into::into); + errors.extend(errs.into_iter().map(Into::into)); (opt, errors) } diff --git a/crates/wasm/src/lib.rs b/crates/wasm/src/lib.rs index da1394f79cb..b66876ae682 100644 --- a/crates/wasm/src/lib.rs +++ b/crates/wasm/src/lib.rs @@ -1,3 +1,4 @@ +#![forbid(unsafe_code)] use acvm::acir::circuit::Circuit; use gloo_utils::format::JsValueSerdeExt; use std::path::PathBuf; diff --git a/noir_stdlib/src/array.nr b/noir_stdlib/src/array.nr index b31b967ec8e..b6256793c25 100644 --- a/noir_stdlib/src/array.nr +++ b/noir_stdlib/src/array.nr @@ -1,16 +1,5 @@ #[builtin(array_len)] fn len(_input : [T]) -> comptime Field {} -// insertion sort - n.b. it is a quadratic sort -fn sort(mut a: [T]) -> [T] { - for i in 1..len(a) { - for j in 0..i { - if(a[i] < a[j]) { - let c = a[j]; - a[j] = a[i]; - a[i]= c; - } - }; - }; - a -} +#[builtin(arraysort)] +fn sort(_a: [T; N]) -> [T; N] {}