diff --git a/.release-please-manifest.json b/.release-please-manifest.json index ea8ab2395df..73b991cb8af 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -1,4 +1,4 @@ { - ".": "0.19.2", - "acvm-repo": "0.33.0" + ".": "0.19.3", + "acvm-repo": "0.34.0" } \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index cacf629d818..d6077b06b05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,30 @@ # Changelog +## [0.19.3](https://github.com/noir-lang/noir/compare/v0.19.2...v0.19.3) (2023-11-22) + + +### Features + +* Add debugger commands to introspect (and modify) the current state ([#3391](https://github.com/noir-lang/noir/issues/3391)) ([9e1ad85](https://github.com/noir-lang/noir/commit/9e1ad858cf8a1d9aba0137abe6a749267498bfaf)) +* Add LSP command to profile opcodes in vscode ([#3496](https://github.com/noir-lang/noir/issues/3496)) ([6fbf77a](https://github.com/noir-lang/noir/commit/6fbf77ae2b87a55db92344f5066a82ccaf6c2086)) +* Add lsp formatting ([#3433](https://github.com/noir-lang/noir/issues/3433)) ([286c876](https://github.com/noir-lang/noir/commit/286c87694fda185f25b05cec5504142643bc207f)) +* Allow providing custom foreign call executors to `execute_circuit` ([#3506](https://github.com/noir-lang/noir/issues/3506)) ([d27db33](https://github.com/noir-lang/noir/commit/d27db332f8c320ffd9b5520bebbd83ae09e31de7)) +* Compile without a backend ([#3437](https://github.com/noir-lang/noir/issues/3437)) ([d69cf5d](https://github.com/noir-lang/noir/commit/d69cf5debcc430bb019b6cc95774aac084776dda)) +* Enable the `fmt` command in the help menu ([#3328](https://github.com/noir-lang/noir/issues/3328)) ([63d414c](https://github.com/noir-lang/noir/commit/63d414c06a399525601e3db11dc48b180e93c2d8)) +* Handle constant index operations on simple slices ([#3464](https://github.com/noir-lang/noir/issues/3464)) ([7ae12f8](https://github.com/noir-lang/noir/commit/7ae12f8c5243d31b2f410c246ed6b9e2fcea5d4c)) + + +### Bug Fixes + +* "Missing trait impl" error in trait dispatch ([#3440](https://github.com/noir-lang/noir/issues/3440)) ([52daaec](https://github.com/noir-lang/noir/commit/52daaec504101fe3c0caa30441c17f30a34af475)) +* Adding proving key initialization ([#3322](https://github.com/noir-lang/noir/issues/3322)) ([3383740](https://github.com/noir-lang/noir/commit/3383740f9a0004f2ee77c9686f81baed6cd1917c)) +* Allow `where` clause on all functions and improve error message ([#3465](https://github.com/noir-lang/noir/issues/3465)) ([1647e33](https://github.com/noir-lang/noir/commit/1647e33564bf56ab8721a365f5fc6bcb38901412)) +* Apply predicate to over/underflow checks ([#3494](https://github.com/noir-lang/noir/issues/3494)) ([fc3edf7](https://github.com/noir-lang/noir/commit/fc3edf7aa5da9074614fa900bbcb57e512e3d56b)) +* **debugger:** Step through foreign calls and breakpoints inside Brillig blocks ([#3511](https://github.com/noir-lang/noir/issues/3511)) ([5d77d7a](https://github.com/noir-lang/noir/commit/5d77d7ac82a4df6995ca151b2c8070044cb1fe9d)) +* Fix crash when using undeclared traits ([#3509](https://github.com/noir-lang/noir/issues/3509)) ([8bb095a](https://github.com/noir-lang/noir/commit/8bb095af77d3b4043855841f1ae5799d75ed94f0)) +* Match rust behaviour for left-shift overflow ([#3518](https://github.com/noir-lang/noir/issues/3518)) ([2d7ceb1](https://github.com/noir-lang/noir/commit/2d7ceb17edda1d9e70901cfd13f45cdc0df0d28d)) +* Verify impls arising from function calls exist ([#3472](https://github.com/noir-lang/noir/issues/3472)) ([d7f919d](https://github.com/noir-lang/noir/commit/d7f919dcc001080ed24616ebbc37426ef7ac7638)) + ## [0.19.2](https://github.com/noir-lang/noir/compare/v0.19.1...v0.19.2) (2023-11-07) diff --git a/Cargo.lock b/Cargo.lock index 648360c30a0..ee64cc13d92 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,7 +4,7 @@ version = 3 [[package]] name = "acir" -version = "0.33.0" +version = "0.34.0" dependencies = [ "acir_field", "base64", @@ -23,7 +23,7 @@ dependencies = [ [[package]] name = "acir_field" -version = "0.33.0" +version = "0.34.0" dependencies = [ "ark-bls12-381", "ark-bn254", @@ -37,7 +37,7 @@ dependencies = [ [[package]] name = "acvm" -version = "0.33.0" +version = "0.34.0" dependencies = [ "acir", "acvm_blackbox_solver", @@ -54,7 +54,7 @@ dependencies = [ [[package]] name = "acvm_blackbox_solver" -version = "0.33.0" +version = "0.34.0" dependencies = [ "acir", "blake2", @@ -67,7 +67,7 @@ dependencies = [ [[package]] name = "acvm_js" -version = "0.33.0" +version = "0.34.0" dependencies = [ "acvm", "barretenberg_blackbox_solver", @@ -88,7 +88,7 @@ dependencies = [ [[package]] name = "acvm_stdlib" -version = "0.33.0" +version = "0.34.0" dependencies = [ "acir", ] @@ -218,7 +218,7 @@ checksum = "a4668cab20f66d8d020e1fbc0ebe47217433c1b6c8f2040faf858554e394ace6" [[package]] name = "arena" -version = "0.19.2" +version = "0.19.3" dependencies = [ "generational-arena", ] @@ -450,7 +450,7 @@ dependencies = [ [[package]] name = "barretenberg_blackbox_solver" -version = "0.33.0" +version = "0.34.0" dependencies = [ "acir", "acvm_blackbox_solver", @@ -574,7 +574,7 @@ dependencies = [ [[package]] name = "brillig" -version = "0.33.0" +version = "0.34.0" dependencies = [ "acir_field", "serde", @@ -582,7 +582,7 @@ dependencies = [ [[package]] name = "brillig_vm" -version = "0.33.0" +version = "0.34.0" dependencies = [ "acir", "acvm_blackbox_solver", @@ -1612,7 +1612,7 @@ dependencies = [ [[package]] name = "fm" -version = "0.19.2" +version = "0.19.3" dependencies = [ "codespan-reporting", "iter-extended", @@ -2194,7 +2194,7 @@ dependencies = [ [[package]] name = "iter-extended" -version = "0.19.2" +version = "0.19.3" [[package]] name = "itertools" @@ -2422,7 +2422,7 @@ checksum = "7843ec2de400bcbc6a6328c958dc38e5359da6e93e72e37bc5246bf1ae776389" [[package]] name = "nargo" -version = "0.19.2" +version = "0.19.3" dependencies = [ "acvm", "codespan-reporting", @@ -2442,7 +2442,7 @@ dependencies = [ [[package]] name = "nargo_cli" -version = "0.19.2" +version = "0.19.3" dependencies = [ "acvm", "assert_cmd", @@ -2478,6 +2478,7 @@ dependencies = [ "rustc_version", "serde", "serde_json", + "similar-asserts", "tempfile", "termcolor", "test-binary", @@ -2490,7 +2491,7 @@ dependencies = [ [[package]] name = "nargo_fmt" -version = "0.19.2" +version = "0.19.3" dependencies = [ "bytecount", "noirc_frontend", @@ -2502,7 +2503,7 @@ dependencies = [ [[package]] name = "nargo_toml" -version = "0.19.2" +version = "0.19.3" dependencies = [ "dirs", "fm", @@ -2551,7 +2552,7 @@ dependencies = [ [[package]] name = "noir_debugger" -version = "0.19.2" +version = "0.19.3" dependencies = [ "acvm", "codespan-reporting", @@ -2565,7 +2566,7 @@ dependencies = [ [[package]] name = "noir_lsp" -version = "0.19.2" +version = "0.19.3" dependencies = [ "acvm", "async-lsp", @@ -2575,6 +2576,7 @@ dependencies = [ "fm", "lsp-types 0.94.1", "nargo", + "nargo_fmt", "nargo_toml", "noirc_driver", "noirc_errors", @@ -2589,7 +2591,7 @@ dependencies = [ [[package]] name = "noir_wasm" -version = "0.19.2" +version = "0.19.3" dependencies = [ "acvm", "build-data", @@ -2611,7 +2613,7 @@ dependencies = [ [[package]] name = "noirc_abi" -version = "0.19.2" +version = "0.19.3" dependencies = [ "acvm", "iter-extended", @@ -2628,7 +2630,7 @@ dependencies = [ [[package]] name = "noirc_abi_wasm" -version = "0.19.2" +version = "0.19.3" dependencies = [ "acvm", "build-data", @@ -2645,7 +2647,7 @@ dependencies = [ [[package]] name = "noirc_driver" -version = "0.19.2" +version = "0.19.3" dependencies = [ "acvm", "build-data", @@ -2662,7 +2664,7 @@ dependencies = [ [[package]] name = "noirc_errors" -version = "0.19.2" +version = "0.19.3" dependencies = [ "acvm", "chumsky", @@ -2675,13 +2677,12 @@ dependencies = [ [[package]] name = "noirc_evaluator" -version = "0.19.2" +version = "0.19.3" dependencies = [ "acvm", "fxhash", "im", "iter-extended", - "noirc_abi", "noirc_errors", "noirc_frontend", "num-bigint", @@ -2691,7 +2692,7 @@ dependencies = [ [[package]] name = "noirc_frontend" -version = "0.19.2" +version = "0.19.3" dependencies = [ "acvm", "arena", @@ -2713,7 +2714,7 @@ dependencies = [ [[package]] name = "noirc_printable_type" -version = "0.19.2" +version = "0.19.3" dependencies = [ "acvm", "iter-extended", diff --git a/Cargo.toml b/Cargo.toml index bc2c4e9f73e..b891aa7d935 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,7 +38,7 @@ resolver = "2" [workspace.package] # x-release-please-start-version -version = "0.19.2" +version = "0.19.3" # x-release-please-end authors = ["The Noir Team "] edition = "2021" @@ -118,8 +118,22 @@ hex = "0.4.2" const_format = "0.2.30" num-bigint = "0.4" num-traits = "0.2" +similar-asserts = "1.5.0" [profile.dev] # This is required to be able to run `cargo test` in acvm_js due to the `locals exceeds maximum` error. # See https://ritik-mishra.medium.com/resolving-the-wasm-pack-error-locals-exceed-maximum-ec3a9d96685b opt-level = 1 + + +[profile.size] +inherits = "release" +lto = true +opt-level = "z" + +[profile.size-aggressive] +inherits = "release" +strip = true +lto = true +panic = "abort" +opt-level = "z" diff --git a/acvm-repo/CHANGELOG.md b/acvm-repo/CHANGELOG.md index 3eb4e9146c6..9c55a1ad0c2 100644 --- a/acvm-repo/CHANGELOG.md +++ b/acvm-repo/CHANGELOG.md @@ -5,6 +5,54 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.34.0](https://github.com/noir-lang/noir/compare/v0.33.0...v0.34.0) (2023-11-22) + + +### ⚠ BREAKING CHANGES + +* Move circuit serialization circuit into acir ([#3345](https://github.com/noir-lang/noir/issues/3345)) +* expose pedersen hash in acir and bb solver ([#3269](https://github.com/noir-lang/noir/issues/3269)) +* Switch to new pedersen implementation ([#3151](https://github.com/noir-lang/noir/issues/3151)) +* Pass ACIR to ACVM by reference rather than passing ownership ([#2872](https://github.com/noir-lang/noir/issues/2872)) +* **wasm:** improve and simplify wasm compiler interface ([#2976](https://github.com/noir-lang/noir/issues/2976)) +* Maintain shape of foreign call arguments ([#2935](https://github.com/noir-lang/noir/issues/2935)) + +### Features + +* **acvm_js:** Export black box solver functions ([#2812](https://github.com/noir-lang/noir/issues/2812)) ([da8a98e](https://github.com/noir-lang/noir/commit/da8a98ed312fe69cb0bdb8f9d0a70ee7a981398f)) +* **acvm:** Separate ACVM optimizations and transformations ([#2979](https://github.com/noir-lang/noir/issues/2979)) ([5865d1a](https://github.com/noir-lang/noir/commit/5865d1a1bca16e1853663c71f893ff81fa3f7185)) +* Add ACIR serializer C++ codegen ([#2961](https://github.com/noir-lang/noir/issues/2961)) ([7556982](https://github.com/noir-lang/noir/commit/7556982dbebe25eaa17240abbe270b771b55de45)) +* Add conditional compilation of methods based on the underlying field being used ([#3045](https://github.com/noir-lang/noir/issues/3045)) ([2e008e2](https://github.com/noir-lang/noir/commit/2e008e2438795bbc41b0641e830378b76bf2e194)) +* Add debugger commands to introspect (and modify) the current state ([#3391](https://github.com/noir-lang/noir/issues/3391)) ([9e1ad85](https://github.com/noir-lang/noir/commit/9e1ad858cf8a1d9aba0137abe6a749267498bfaf)) +* Expose pedersen hash in acir and bb solver ([#3269](https://github.com/noir-lang/noir/issues/3269)) ([0108b6c](https://github.com/noir-lang/noir/commit/0108b6c1e8dc0dfc766ab3c4944deae9354dec36)) +* Extract Brillig VM to allow step debugging ([#3259](https://github.com/noir-lang/noir/issues/3259)) ([f6431f9](https://github.com/noir-lang/noir/commit/f6431f99711f15a96a4f7fed2f413daece94b5e1)) +* Implement euclidean division and signed division in terms of `AcirVar`s ([#3230](https://github.com/noir-lang/noir/issues/3230)) ([b8b7782](https://github.com/noir-lang/noir/commit/b8b77825410c0e1f95549259a51e2c40de1ec342)) +* Maintain shape of foreign call arguments ([#2935](https://github.com/noir-lang/noir/issues/2935)) ([f7869e6](https://github.com/noir-lang/noir/commit/f7869e6fb492b617e776e538ac4babfa56261d26)) +* Pass ACIR to ACVM by reference rather than passing ownership ([#2872](https://github.com/noir-lang/noir/issues/2872)) ([b3a9c34](https://github.com/noir-lang/noir/commit/b3a9c343993ce3207de62106bda6cb2b2ef3de50)) +* Pass brillig bytecode to VM by reference ([#3030](https://github.com/noir-lang/noir/issues/3030)) ([4ee290b](https://github.com/noir-lang/noir/commit/4ee290b8b6f75bc1974a5750248570eeca8d244e)) +* Refactor debugger and separate core from UI ([#3308](https://github.com/noir-lang/noir/issues/3308)) ([8466810](https://github.com/noir-lang/noir/commit/846681079ab7295b201480a5c8baebc45e858c6f)) +* Replace boolean range constraints with arithmetic opcodes ([#3234](https://github.com/noir-lang/noir/issues/3234)) ([949222c](https://github.com/noir-lang/noir/commit/949222c20d9e65152e3814d02da1c4c41ffc23a5)) +* Save Brillig execution state in ACVM ([#3026](https://github.com/noir-lang/noir/issues/3026)) ([88682da](https://github.com/noir-lang/noir/commit/88682da87ffc9e26da5c9e4b5a4d8e62a6ee43c6)) +* Solve `fixed_base_scalar_mul` black box functions in rust ([#3153](https://github.com/noir-lang/noir/issues/3153)) ([1c1afbc](https://github.com/noir-lang/noir/commit/1c1afbcddf0b5fdb39f00ad28ae90caf699d1265)) +* Switch to new pedersen implementation ([#3151](https://github.com/noir-lang/noir/issues/3151)) ([35fb3f7](https://github.com/noir-lang/noir/commit/35fb3f7076d52db7ca3bef0a70a3dbccaf82f58d)) +* **wasm:** Improve and simplify wasm compiler interface ([#2976](https://github.com/noir-lang/noir/issues/2976)) ([1b5124b](https://github.com/noir-lang/noir/commit/1b5124bc74f7ac5360db04b34d1b7b2284061fd3)) + + +### Bug Fixes + +* ACIR optimizer should update assertion messages ([#3010](https://github.com/noir-lang/noir/issues/3010)) ([758b6b6](https://github.com/noir-lang/noir/commit/758b6b62918907c1a39f3090a77419003551745e)) +* **acvm:** Return false rather than panicking on invalid ECDSA signatures ([#2783](https://github.com/noir-lang/noir/issues/2783)) ([155abc0](https://github.com/noir-lang/noir/commit/155abc0d99fff41c79163c16bf297d41e5dff0fa)) +* Determinism of fallback transformer ([#3100](https://github.com/noir-lang/noir/issues/3100)) ([12daad1](https://github.com/noir-lang/noir/commit/12daad19c902caf5ee9e2eb4b6847bde5a924353)) +* Fix method `program_counter`, change method signature ([#3012](https://github.com/noir-lang/noir/issues/3012)) ([5ea522b](https://github.com/noir-lang/noir/commit/5ea522b840ca0f6f90d02ca00f0de32f515d450f)) +* Minor problems with `aztec` publishing ([#3095](https://github.com/noir-lang/noir/issues/3095)) ([0fc8f20](https://github.com/noir-lang/noir/commit/0fc8f20b8b87d033d27ce18db039399c17f81837)) +* Prevent duplicated assert message transformation ([#3038](https://github.com/noir-lang/noir/issues/3038)) ([082a6d0](https://github.com/noir-lang/noir/commit/082a6d02dad67a25692bed15c340a16a848a320e)) +* Return error rather than panicking on unreadable circuits ([#3179](https://github.com/noir-lang/noir/issues/3179)) ([d4f61d3](https://github.com/noir-lang/noir/commit/d4f61d3d51d515e40a5fd02d35315889f841bf53)) + + +### Miscellaneous Chores + +* Move circuit serialization circuit into acir ([#3345](https://github.com/noir-lang/noir/issues/3345)) ([122119b](https://github.com/noir-lang/noir/commit/122119b7377cec1b7c42c586c64b69b3bdf4d539)) + ## [0.33.0](https://github.com/noir-lang/noir/compare/v0.32.0...v0.33.0) (2023-11-07) diff --git a/acvm-repo/acir/Cargo.toml b/acvm-repo/acir/Cargo.toml index 25d770e1f17..3bd07e56212 100644 --- a/acvm-repo/acir/Cargo.toml +++ b/acvm-repo/acir/Cargo.toml @@ -2,7 +2,7 @@ name = "acir" description = "ACIR is the IR that the VM processes, it is analogous to LLVM IR" # x-release-please-start-version -version = "0.33.0" +version = "0.34.0" # x-release-please-end authors.workspace = true edition.workspace = true diff --git a/acvm-repo/acir_field/Cargo.toml b/acvm-repo/acir_field/Cargo.toml index 2cd2801db10..c80271ce539 100644 --- a/acvm-repo/acir_field/Cargo.toml +++ b/acvm-repo/acir_field/Cargo.toml @@ -2,7 +2,7 @@ name = "acir_field" description = "The field implementation being used by ACIR." # x-release-please-start-version -version = "0.33.0" +version = "0.34.0" # x-release-please-end authors.workspace = true edition.workspace = true diff --git a/acvm-repo/acvm/Cargo.toml b/acvm-repo/acvm/Cargo.toml index 032d93230da..332399808a1 100644 --- a/acvm-repo/acvm/Cargo.toml +++ b/acvm-repo/acvm/Cargo.toml @@ -2,7 +2,7 @@ name = "acvm" description = "The virtual machine that processes ACIR given a backend/proof system." # x-release-please-start-version -version = "0.33.0" +version = "0.34.0" # x-release-please-end authors.workspace = true edition.workspace = true diff --git a/acvm-repo/acvm_js/Cargo.toml b/acvm-repo/acvm_js/Cargo.toml index e287ac9ed17..2efc618b3f5 100644 --- a/acvm-repo/acvm_js/Cargo.toml +++ b/acvm-repo/acvm_js/Cargo.toml @@ -2,7 +2,7 @@ name = "acvm_js" description = "Typescript wrapper around the ACVM allowing execution of ACIR code" # x-release-please-start-version -version = "0.33.0" +version = "0.34.0" # x-release-please-end authors.workspace = true edition.workspace = true diff --git a/acvm-repo/acvm_js/package.json b/acvm-repo/acvm_js/package.json index 19090022e25..55516059540 100644 --- a/acvm-repo/acvm_js/package.json +++ b/acvm-repo/acvm_js/package.json @@ -1,6 +1,6 @@ { "name": "@noir-lang/acvm_js", - "version": "0.33.0", + "version": "0.34.0", "repository": { "type": "git", "url": "https://github.com/noir-lang/acvm.git" diff --git a/acvm-repo/barretenberg_blackbox_solver/Cargo.toml b/acvm-repo/barretenberg_blackbox_solver/Cargo.toml index 6917e7fbf81..c2c5a4f98a1 100644 --- a/acvm-repo/barretenberg_blackbox_solver/Cargo.toml +++ b/acvm-repo/barretenberg_blackbox_solver/Cargo.toml @@ -2,7 +2,7 @@ name = "barretenberg_blackbox_solver" description = "A wrapper around a barretenberg WASM binary to execute black box functions for which there is no rust implementation" # x-release-please-start-version -version = "0.33.0" +version = "0.34.0" # x-release-please-end authors.workspace = true edition.workspace = true diff --git a/acvm-repo/blackbox_solver/Cargo.toml b/acvm-repo/blackbox_solver/Cargo.toml index f8b100890ca..60f6dedc766 100644 --- a/acvm-repo/blackbox_solver/Cargo.toml +++ b/acvm-repo/blackbox_solver/Cargo.toml @@ -2,7 +2,7 @@ name = "acvm_blackbox_solver" description = "A solver for the blackbox functions found in ACIR and Brillig" # x-release-please-start-version -version = "0.33.0" +version = "0.34.0" # x-release-please-end authors.workspace = true edition.workspace = true diff --git a/acvm-repo/brillig/Cargo.toml b/acvm-repo/brillig/Cargo.toml index 542839a5767..15b99f2f07d 100644 --- a/acvm-repo/brillig/Cargo.toml +++ b/acvm-repo/brillig/Cargo.toml @@ -2,7 +2,7 @@ name = "brillig" description = "Brillig is the bytecode ACIR uses for non-determinism." # x-release-please-start-version -version = "0.33.0" +version = "0.34.0" # x-release-please-end authors.workspace = true edition.workspace = true diff --git a/acvm-repo/brillig_vm/Cargo.toml b/acvm-repo/brillig_vm/Cargo.toml index 2d89aa7aa15..2c7b486ecea 100644 --- a/acvm-repo/brillig_vm/Cargo.toml +++ b/acvm-repo/brillig_vm/Cargo.toml @@ -2,7 +2,7 @@ name = "brillig_vm" description = "The virtual machine that processes Brillig bytecode, used to introduce non-determinism to the ACVM" # x-release-please-start-version -version = "0.33.0" +version = "0.34.0" # x-release-please-end authors.workspace = true edition.workspace = true diff --git a/acvm-repo/stdlib/Cargo.toml b/acvm-repo/stdlib/Cargo.toml index 63a8b71eb15..44c88954b7a 100644 --- a/acvm-repo/stdlib/Cargo.toml +++ b/acvm-repo/stdlib/Cargo.toml @@ -2,7 +2,7 @@ name = "acvm_stdlib" description = "The ACVM standard library." # x-release-please-start-version -version = "0.33.0" +version = "0.34.0" # x-release-please-end authors.workspace = true edition.workspace = true diff --git a/compiler/noirc_driver/src/abi_gen.rs b/compiler/noirc_driver/src/abi_gen.rs new file mode 100644 index 00000000000..1cad67ba5b7 --- /dev/null +++ b/compiler/noirc_driver/src/abi_gen.rs @@ -0,0 +1,141 @@ +use std::collections::BTreeMap; + +use acvm::acir::native_types::Witness; +use iter_extended::{btree_map, vecmap}; +use noirc_abi::{Abi, AbiParameter, AbiType}; +use noirc_frontend::{ + hir::Context, + hir_def::{function::Param, stmt::HirPattern}, + node_interner::{FuncId, NodeInterner}, +}; +use std::ops::Range; + +/// Arranges a function signature and a generated circuit's return witnesses into a +/// `noirc_abi::Abi`. +pub(super) fn gen_abi( + context: &Context, + func_id: &FuncId, + input_witnesses: Vec, + return_witnesses: Vec, +) -> Abi { + let (parameters, return_type) = compute_function_abi(context, func_id); + let param_witnesses = param_witnesses_from_abi_param(¶meters, input_witnesses); + Abi { parameters, return_type, param_witnesses, return_witnesses } +} + +pub(super) fn compute_function_abi( + context: &Context, + func_id: &FuncId, +) -> (Vec, Option) { + let func_meta = context.def_interner.function_meta(func_id); + + let (parameters, return_type) = func_meta.into_function_signature(); + let parameters = into_abi_params(context, parameters); + let return_type = return_type.map(|typ| AbiType::from_type(context, &typ)); + (parameters, return_type) +} + +/// Attempts to retrieve the name of this parameter. Returns None +/// if this parameter is a tuple or struct pattern. +fn get_param_name<'a>(pattern: &HirPattern, interner: &'a NodeInterner) -> Option<&'a str> { + match pattern { + HirPattern::Identifier(ident) => Some(interner.definition_name(ident.id)), + HirPattern::Mutable(pattern, _) => get_param_name(pattern, interner), + HirPattern::Tuple(_, _) => None, + HirPattern::Struct(_, _, _) => None, + } +} + +fn into_abi_params(context: &Context, params: Vec) -> Vec { + vecmap(params, |(pattern, typ, vis)| { + let param_name = get_param_name(&pattern, &context.def_interner) + .expect("Abi for tuple and struct parameters is unimplemented") + .to_owned(); + let as_abi = AbiType::from_type(context, &typ); + AbiParameter { name: param_name, typ: as_abi, visibility: vis.into() } + }) +} + +// Takes each abi parameter and shallowly maps to the expected witness range in which the +// parameter's constituent values live. +fn param_witnesses_from_abi_param( + abi_params: &Vec, + input_witnesses: Vec, +) -> BTreeMap>> { + let mut idx = 0_usize; + if input_witnesses.is_empty() { + return BTreeMap::new(); + } + + btree_map(abi_params, |param| { + let num_field_elements_needed = param.typ.field_count() as usize; + let param_witnesses = &input_witnesses[idx..idx + num_field_elements_needed]; + + // It's likely that `param_witnesses` will consist of mostly incrementing witness indices. + // We then want to collapse these into `Range`s to save space. + let param_witnesses = collapse_ranges(param_witnesses); + idx += num_field_elements_needed; + (param.name.clone(), param_witnesses) + }) +} + +/// Takes a vector of [`Witnesses`][`Witness`] and collapses it into a vector of [`Range`]s of [`Witnesses`][`Witness`]. +fn collapse_ranges(witnesses: &[Witness]) -> Vec> { + if witnesses.is_empty() { + return Vec::new(); + } + let mut wit = Vec::new(); + let mut last_wit: Witness = witnesses[0]; + + for (i, witness) in witnesses.iter().enumerate() { + if i == 0 { + continue; + }; + let witness_index = witness.witness_index(); + let prev_witness_index = witnesses[i - 1].witness_index(); + if witness_index != prev_witness_index + 1 { + wit.push(last_wit..Witness(prev_witness_index + 1)); + last_wit = *witness; + }; + } + + let last_witness = witnesses.last().unwrap().witness_index(); + wit.push(last_wit..Witness(last_witness + 1)); + + wit +} + +#[cfg(test)] +mod test { + use std::ops::Range; + + use acvm::acir::native_types::Witness; + + use super::collapse_ranges; + + #[test] + fn collapses_single_range() { + let witnesses: Vec<_> = vec![1, 2, 3].into_iter().map(Witness::from).collect(); + + let collapsed_witnesses = collapse_ranges(&witnesses); + + assert_eq!(collapsed_witnesses, vec![Range { start: Witness(1), end: Witness(4) },]); + } + + #[test] + fn collapse_ranges_correctly() { + let witnesses: Vec<_> = + vec![1, 2, 3, 5, 6, 2, 3, 4].into_iter().map(Witness::from).collect(); + + let collapsed_witnesses = collapse_ranges(&witnesses); + + assert_eq!( + collapsed_witnesses, + vec![ + Range { start: Witness(1), end: Witness(4) }, + Range { start: Witness(5), end: Witness(7) }, + Range { start: Witness(2), end: Witness(5) } + ] + ); + } +} diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index ec7a5091ffd..456c2c49609 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -8,8 +8,8 @@ use fm::FileId; use iter_extended::vecmap; use noirc_abi::{AbiParameter, AbiType, ContractEvent}; use noirc_errors::{CustomDiagnostic, FileDiagnostic}; +use noirc_evaluator::create_circuit; use noirc_evaluator::errors::RuntimeError; -use noirc_evaluator::{create_circuit, into_abi_params}; use noirc_frontend::graph::{CrateId, CrateName}; use noirc_frontend::hir::def_map::{Contract, CrateDefMap}; use noirc_frontend::hir::Context; @@ -18,6 +18,7 @@ use noirc_frontend::node_interner::FuncId; use serde::{Deserialize, Serialize}; use std::path::Path; +mod abi_gen; mod contract; mod debug; mod program; @@ -140,12 +141,7 @@ pub fn compute_function_abi( ) -> Option<(Vec, Option)> { let main_function = context.get_main_function(crate_id)?; - let func_meta = context.def_interner.function_meta(&main_function); - - let (parameters, return_type) = func_meta.into_function_signature(); - let parameters = into_abi_params(context, parameters); - let return_type = return_type.map(|typ| AbiType::from_type(context, &typ)); - Some((parameters, return_type)) + Some(abi_gen::compute_function_abi(context, &main_function)) } /// Run the frontend to check the crate for errors then compile the main function if there were none @@ -345,9 +341,10 @@ pub fn compile_no_check( return Ok(cached_program.expect("cache must exist for hashes to match")); } - let (circuit, debug, abi, warnings) = - create_circuit(context, program, options.show_ssa, options.show_brillig)?; + let (circuit, debug, input_witnesses, return_witnesses, warnings) = + create_circuit(program, options.show_ssa, options.show_brillig)?; + let abi = abi_gen::gen_abi(context, &main_function, input_witnesses, return_witnesses); let file_map = filter_relevant_files(&[debug.clone()], &context.file_manager); Ok(CompiledProgram { diff --git a/compiler/noirc_evaluator/Cargo.toml b/compiler/noirc_evaluator/Cargo.toml index c9f5f28478b..933ec2b300c 100644 --- a/compiler/noirc_evaluator/Cargo.toml +++ b/compiler/noirc_evaluator/Cargo.toml @@ -10,7 +10,6 @@ license.workspace = true [dependencies] noirc_frontend.workspace = true noirc_errors.workspace = true -noirc_abi.workspace = true acvm.workspace = true fxhash.workspace = true iter-extended.workspace = true diff --git a/compiler/noirc_evaluator/src/lib.rs b/compiler/noirc_evaluator/src/lib.rs index b5b697e3b65..70751d3e541 100644 --- a/compiler/noirc_evaluator/src/lib.rs +++ b/compiler/noirc_evaluator/src/lib.rs @@ -11,5 +11,4 @@ pub mod ssa; pub mod brillig; -pub use ssa::abi_gen::into_abi_params; pub use ssa::create_circuit; diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 24521b98bd1..8e1c62edc69 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -7,10 +7,7 @@ //! This module heavily borrows from Cranelift #![allow(dead_code)] -use std::{ - collections::{BTreeMap, BTreeSet}, - ops::Range, -}; +use std::collections::BTreeSet; use crate::{ brillig::Brillig, @@ -23,13 +20,12 @@ use acvm::acir::{ use noirc_errors::debug_info::DebugInfo; -use noirc_abi::Abi; - -use noirc_frontend::{hir::Context, monomorphization::ast::Program}; +use noirc_frontend::{ + hir_def::function::FunctionSignature, monomorphization::ast::Program, Visibility, +}; -use self::{abi_gen::gen_abi, acir_gen::GeneratedAcir, ssa_gen::Ssa}; +use self::{acir_gen::GeneratedAcir, ssa_gen::Ssa}; -pub mod abi_gen; mod acir_gen; pub(super) mod function_builder; pub mod ir; @@ -81,12 +77,12 @@ pub(crate) fn optimize_into_acir( /// Compiles the [`Program`] into [`ACIR`][acvm::acir::circuit::Circuit]. /// /// The output ACIR is is backend-agnostic and so must go through a transformation pass before usage in proof generation. +#[allow(clippy::type_complexity)] pub fn create_circuit( - context: &Context, program: Program, enable_ssa_logging: bool, enable_brillig_logging: bool, -) -> Result<(Circuit, DebugInfo, Abi, Vec), RuntimeError> { +) -> Result<(Circuit, DebugInfo, Vec, Vec, Vec), RuntimeError> { let func_sig = program.main_function_signature.clone(); let mut generated_acir = optimize_into_acir(program, enable_ssa_logging, enable_brillig_logging)?; @@ -101,15 +97,11 @@ pub fn create_circuit( .. } = generated_acir; - let abi = gen_abi(context, func_sig, input_witnesses, return_witnesses.clone()); - let public_abi = abi.clone().public_abi(); - - let public_parameters = PublicInputs(tree_to_set(&public_abi.param_witnesses)); + let (public_parameter_witnesses, private_parameters) = + split_public_and_private_inputs(&func_sig, &input_witnesses); - let all_parameters: BTreeSet = tree_to_set(&abi.param_witnesses); - let private_parameters = all_parameters.difference(&public_parameters.0).copied().collect(); - - let return_values = PublicInputs(return_witnesses.into_iter().collect()); + let public_parameters = PublicInputs(public_parameter_witnesses); + let return_values = PublicInputs(return_witnesses.iter().copied().collect()); let circuit = Circuit { current_witness_index, @@ -132,7 +124,41 @@ pub fn create_circuit( let (optimized_circuit, transformation_map) = acvm::compiler::optimize(circuit); debug_info.update_acir(transformation_map); - Ok((optimized_circuit, debug_info, abi, warnings)) + Ok((optimized_circuit, debug_info, input_witnesses, return_witnesses, warnings)) +} + +// Takes each function argument and partitions the circuit's inputs witnesses according to its visibility. +fn split_public_and_private_inputs( + func_sig: &FunctionSignature, + input_witnesses: &[Witness], +) -> (BTreeSet, BTreeSet) { + let mut idx = 0_usize; + if input_witnesses.is_empty() { + return (BTreeSet::new(), BTreeSet::new()); + } + + func_sig + .0 + .iter() + .map(|(_, typ, visibility)| { + let num_field_elements_needed = typ.field_count() as usize; + let witnesses = input_witnesses[idx..idx + num_field_elements_needed].to_vec(); + idx += num_field_elements_needed; + (visibility, witnesses) + }) + .fold((BTreeSet::new(), BTreeSet::new()), |mut acc, (vis, witnesses)| { + // Split witnesses into sets based on their visibility. + if *vis == Visibility::Public { + for witness in witnesses { + acc.0.insert(witness); + } + } else { + for witness in witnesses { + acc.1.insert(witness); + } + } + (acc.0, acc.1) + }) } // This is just a convenience object to bundle the ssa with `print_ssa_passes` for debug printing. @@ -178,15 +204,3 @@ impl SsaBuilder { self } } - -// Flatten the witnesses in the map into a BTreeSet -fn tree_to_set(input: &BTreeMap>>) -> BTreeSet { - let mut result = BTreeSet::new(); - for range in input.values().flatten() { - for i in range.start.witness_index()..range.end.witness_index() { - result.insert(Witness(i)); - } - } - - result -} diff --git a/compiler/noirc_evaluator/src/ssa/abi_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/abi_gen/mod.rs deleted file mode 100644 index 948fb7d5263..00000000000 --- a/compiler/noirc_evaluator/src/ssa/abi_gen/mod.rs +++ /dev/null @@ -1,86 +0,0 @@ -use std::collections::BTreeMap; - -use acvm::acir::native_types::Witness; -use iter_extended::{btree_map, vecmap}; -use noirc_abi::{Abi, AbiParameter, AbiType}; -use noirc_frontend::{ - hir::Context, - hir_def::{ - function::{FunctionSignature, Param}, - stmt::HirPattern, - }, - node_interner::NodeInterner, -}; -use std::ops::Range; - -/// Attempts to retrieve the name of this parameter. Returns None -/// if this parameter is a tuple or struct pattern. -fn get_param_name<'a>(pattern: &HirPattern, interner: &'a NodeInterner) -> Option<&'a str> { - match pattern { - HirPattern::Identifier(ident) => Some(interner.definition_name(ident.id)), - HirPattern::Mutable(pattern, _) => get_param_name(pattern, interner), - HirPattern::Tuple(_, _) => None, - HirPattern::Struct(_, _, _) => None, - } -} - -pub fn into_abi_params(context: &Context, params: Vec) -> Vec { - vecmap(params, |(pattern, typ, vis)| { - let param_name = get_param_name(&pattern, &context.def_interner) - .expect("Abi for tuple and struct parameters is unimplemented") - .to_owned(); - let as_abi = AbiType::from_type(context, &typ); - AbiParameter { name: param_name, typ: as_abi, visibility: vis.into() } - }) -} - -/// Arranges a function signature and a generated circuit's return witnesses into a -/// `noirc_abi::Abi`. -pub(crate) fn gen_abi( - context: &Context, - func_sig: FunctionSignature, - input_witnesses: Vec>, - return_witnesses: Vec, -) -> Abi { - let (parameters, return_type) = func_sig; - let parameters = into_abi_params(context, parameters); - let return_type = return_type.map(|typ| AbiType::from_type(context, &typ)); - let param_witnesses = param_witnesses_from_abi_param(¶meters, input_witnesses); - Abi { parameters, return_type, param_witnesses, return_witnesses } -} - -// Takes each abi parameter and shallowly maps to the expected witness range in which the -// parameter's constituent values live. -fn param_witnesses_from_abi_param( - abi_params: &Vec, - input_witnesses: Vec>, -) -> BTreeMap>> { - let mut idx = 0_usize; - if input_witnesses.is_empty() { - return BTreeMap::new(); - } - let mut processed_range = input_witnesses[idx].start.witness_index(); - - btree_map(abi_params, |param| { - let num_field_elements_needed = param.typ.field_count(); - let mut wit = Vec::new(); - let mut processed_fields = 0; - while processed_fields < num_field_elements_needed { - let end = input_witnesses[idx].end.witness_index(); - if num_field_elements_needed <= end - processed_range { - wit.push( - Witness(processed_range)..Witness(processed_range + num_field_elements_needed), - ); - processed_range += num_field_elements_needed; - processed_fields += num_field_elements_needed; - } else { - // consume the current range - wit.push(Witness(processed_range)..input_witnesses[idx].end); - processed_fields += end - processed_range; - idx += 1; - processed_range = input_witnesses[idx].start.witness_index(); - } - } - (param.name.clone(), wit) - }) -} diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index 4ee70ed9b22..c4b19379ecc 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -23,7 +23,6 @@ use acvm::{BlackBoxFunctionSolver, BlackBoxResolutionError}; use fxhash::FxHashMap as HashMap; use iter_extended::{try_vecmap, vecmap}; use num_bigint::BigUint; -use std::ops::RangeInclusive; use std::{borrow::Cow, hash::Hash}; #[derive(Clone, Debug, PartialEq, Eq, Hash)] @@ -115,7 +114,7 @@ impl AcirContext { self.acir_ir.current_witness_index() } - pub(crate) fn extract_witness(&self, inputs: &[AcirValue]) -> Vec { + pub(crate) fn extract_witness(&self, inputs: &[AcirValue]) -> Vec { inputs .iter() .flat_map(|value| value.clone().flatten()) @@ -126,7 +125,6 @@ impl AcirContext { .to_expression() .to_witness() .expect("ICE - cannot extract a witness") - .0 }) .collect() } @@ -1182,27 +1180,10 @@ impl AcirContext { /// Terminates the context and takes the resulting `GeneratedAcir` pub(crate) fn finish( mut self, - inputs: Vec>, + inputs: Vec, warnings: Vec, ) -> GeneratedAcir { - let mut current_range = 0..0; - for range in inputs { - if current_range.end == *range.start() { - current_range.end = range.end() + 1; - } else { - if current_range.end != 0 { - self.acir_ir - .input_witnesses - .push(Witness(current_range.start)..Witness(current_range.end)); - } - current_range = *range.start()..range.end() + 1; - } - } - if current_range.end != 0 { - self.acir_ir - .input_witnesses - .push(Witness(current_range.start)..Witness(current_range.end)); - } + self.acir_ir.input_witnesses = inputs; self.acir_ir.warnings = warnings; self.acir_ir } diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index 9bd83096adf..f29d3c9ec05 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -23,7 +23,6 @@ use acvm::{ }; use iter_extended::vecmap; use num_bigint::BigUint; -use std::ops::Range; #[derive(Debug, Default)] /// The output of the Acir-gen pass @@ -43,7 +42,7 @@ pub(crate) struct GeneratedAcir { pub(crate) return_witnesses: Vec, /// All witness indices which are inputs to the main function - pub(crate) input_witnesses: Vec>, + pub(crate) input_witnesses: Vec, /// Correspondence between an opcode index (in opcodes) and the source code call stack which generated it pub(crate) locations: BTreeMap, diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index e7aa54b9d91..d8ea9d9402b 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -3,7 +3,6 @@ mod acir_ir; use std::collections::HashSet; use std::fmt::Debug; -use std::ops::RangeInclusive; use self::acir_ir::acir_variable::{AcirContext, AcirType, AcirVar}; use super::ir::dfg::CallStack; @@ -26,6 +25,7 @@ use crate::brillig::{brillig_gen::brillig_fn::FunctionContext as BrilligFunction use crate::errors::{InternalError, InternalWarning, RuntimeError, SsaReport}; pub(crate) use acir_ir::generated_acir::GeneratedAcir; +use acvm::acir::native_types::Witness; use acvm::acir::BlackBoxFunc; use acvm::{ acir::{circuit::opcodes::BlockId, native_types::Expression}, @@ -226,7 +226,7 @@ impl Context { } warnings.extend(self.convert_ssa_return(entry_block.unwrap_terminator(), dfg)?); - Ok(self.acir_context.finish(vec![input_witness], warnings)) + Ok(self.acir_context.finish(input_witness, warnings)) } fn convert_brillig_main( @@ -262,8 +262,7 @@ impl Context { for acir_var in output_vars { self.acir_context.return_var(acir_var)?; } - let witnesses = vecmap(witness_inputs, |input| RangeInclusive::new(input, input)); - Ok(self.acir_context.finish(witnesses, Vec::new())) + Ok(self.acir_context.finish(witness_inputs, Vec::new())) } /// Adds and binds `AcirVar`s for each numeric block parameter or block parameter array element. @@ -271,7 +270,7 @@ impl Context { &mut self, params: &[ValueId], dfg: &DataFlowGraph, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { // The first witness (if any) is the next one let start_witness = self.acir_context.current_witness_index().0 + 1; for param_id in params { @@ -300,7 +299,8 @@ impl Context { self.ssa_values.insert(*param_id, value); } let end_witness = self.acir_context.current_witness_index().0; - Ok(start_witness..=end_witness) + let witnesses = (start_witness..=end_witness).map(Witness::from).collect(); + Ok(witnesses) } fn convert_ssa_block_param(&mut self, param_type: &Type) -> Result { diff --git a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index 3b20f3c3c80..a18ef746e1b 100644 --- a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs @@ -261,22 +261,6 @@ impl FunctionBuilder { arguments: Vec, result_types: Vec, ) -> Cow<[ValueId]> { - if let Value::Intrinsic(intrinsic) = &self.current_function.dfg[func] { - if intrinsic == &Intrinsic::WrappingShiftLeft { - let result_type = self.current_function.dfg.type_of_value(arguments[0]); - let bit_size = match result_type { - Type::Numeric(NumericType::Signed { bit_size }) - | Type::Numeric(NumericType::Unsigned { bit_size }) => bit_size, - _ => { - unreachable!("ICE: Truncation attempted on non-integer"); - } - }; - return self - .insert_wrapping_shift_left(arguments[0], arguments[1], bit_size) - .results(); - } - } - self.insert_instruction(Instruction::Call { func, arguments }, Some(result_types)).results() } @@ -291,12 +275,12 @@ impl FunctionBuilder { /// Insert ssa instructions which computes lhs << rhs by doing lhs*2^rhs /// and truncate the result to bit_size - fn insert_wrapping_shift_left( + pub(crate) fn insert_wrapping_shift_left( &mut self, lhs: ValueId, rhs: ValueId, bit_size: u32, - ) -> InsertInstructionResult { + ) -> ValueId { let base = self.field_constant(FieldElement::from(2_u128)); let typ = self.current_function.dfg.type_of_value(lhs); let (max_bit, pow) = if let Some(rhs_constant) = @@ -308,7 +292,7 @@ impl FunctionBuilder { 2_u32.overflowing_pow(rhs_constant.to_u128() as u32); if overflows { let zero = self.numeric_constant(FieldElement::zero(), typ); - return InsertInstructionResult::SimplifiedTo(zero); + return InsertInstructionResult::SimplifiedTo(zero).first(); } let pow = self.numeric_constant(FieldElement::from(rhs_bit_size_pow_2 as u128), typ); (bit_size + (rhs_constant.to_u128() as u32), pow) @@ -328,13 +312,14 @@ impl FunctionBuilder { let instruction = Instruction::Binary(Binary { lhs, rhs: pow, operator: BinaryOp::Mul }); if max_bit <= bit_size { - self.insert_instruction(instruction, None) + self.insert_instruction(instruction, None).first() } else { let result = self.insert_instruction(instruction, None).first(); self.insert_instruction( Instruction::Truncate { value: result, bit_size, max_bit_size: max_bit }, None, ) + .first() } } diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index d8e3a543848..63b32766f62 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -46,7 +46,6 @@ pub(crate) enum Intrinsic { BlackBox(BlackBoxFunc), FromField, AsField, - WrappingShiftLeft, } impl std::fmt::Display for Intrinsic { @@ -69,7 +68,6 @@ impl std::fmt::Display for Intrinsic { Intrinsic::BlackBox(function) => write!(f, "{function}"), Intrinsic::FromField => write!(f, "from_field"), Intrinsic::AsField => write!(f, "as_field"), - Intrinsic::WrappingShiftLeft => write!(f, "wrapping_shift_left"), } } } @@ -94,8 +92,7 @@ impl Intrinsic { | Intrinsic::ToBits(_) | Intrinsic::ToRadix(_) | Intrinsic::FromField - | Intrinsic::AsField - | Intrinsic::WrappingShiftLeft => false, + | Intrinsic::AsField => false, // Some black box functions have side-effects Intrinsic::BlackBox(func) => matches!(func, BlackBoxFunc::RecursiveAggregation), @@ -122,7 +119,6 @@ impl Intrinsic { "to_be_bits" => Some(Intrinsic::ToBits(Endian::Big)), "from_field" => Some(Intrinsic::FromField), "as_field" => Some(Intrinsic::AsField), - "wrapping_shift_left" => Some(Intrinsic::WrappingShiftLeft), other => BlackBoxFunc::lookup(other).map(Intrinsic::BlackBox), } } diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index da5544d7dc6..b07e2df7bd3 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -245,9 +245,6 @@ pub(super) fn simplify_call( let instruction = Instruction::Cast(arguments[0], ctrl_typevars.unwrap().remove(0)); SimplifyResult::SimplifiedToInstruction(instruction) } - Intrinsic::WrappingShiftLeft => { - unreachable!("ICE - wrapping shift left should have been proccessed before") - } } } diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs index b8ee9ec9707..9d27ffc60d8 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs @@ -364,22 +364,17 @@ impl<'a> FunctionContext<'a> { BinaryOpKind::ShiftLeft => "left shift", _ => unreachable!("operator {} should not overflow", operator), }; - let message = format!("attempt to {} with overflow", op_name); - let range_constraint = Instruction::RangeCheck { - value: result, - max_bit_size: bit_size, - assert_message: Some(message), - }; - self.builder.set_location(location).insert_instruction(range_constraint, None); + if operator == BinaryOpKind::ShiftLeft { - match result_type { - Type::Numeric(NumericType::Signed { bit_size }) - | Type::Numeric(NumericType::Unsigned { bit_size }) => { - self.builder.insert_truncate(result, bit_size, bit_size + 1) - } - _ => result, - } + self.check_left_shift_overflow(result, rhs, bit_size, location) } else { + let message = format!("attempt to {} with overflow", op_name); + let range_constraint = Instruction::RangeCheck { + value: result, + max_bit_size: bit_size, + assert_message: Some(message), + }; + self.builder.set_location(location).insert_instruction(range_constraint, None); result } } @@ -387,6 +382,30 @@ impl<'a> FunctionContext<'a> { } } + /// Overflow checks for shift-left + /// We use Rust behavior for shift left: + /// If rhs is more or equal than the bit size, then we overflow + /// If not, we do not overflow and shift left with 0 when bits are falling out of the bit size + fn check_left_shift_overflow( + &mut self, + result: ValueId, + rhs: ValueId, + bit_size: u32, + location: Location, + ) -> ValueId { + let max = self + .builder + .numeric_constant(FieldElement::from(bit_size as i128), Type::unsigned(bit_size)); + let overflow = self.builder.insert_binary(rhs, BinaryOp::Lt, max); + let one = self.builder.numeric_constant(FieldElement::one(), Type::bool()); + self.builder.set_location(location).insert_constrain( + overflow, + one, + Some("attempt to left shift with overflow".to_owned()), + ); + self.builder.insert_truncate(result, bit_size, bit_size + 1) + } + /// Insert constraints ensuring that the operation does not overflow the bit size of the result /// We assume that: /// lhs and rhs are signed integers of bit size bit_size @@ -487,7 +506,15 @@ impl<'a> FunctionContext<'a> { location: Location, ) -> Values { let mut result = match operator { - BinaryOpKind::ShiftLeft => self.builder.insert_shift_left(lhs, rhs), + BinaryOpKind::ShiftLeft => { + let result_type = self.builder.current_function.dfg.type_of_value(lhs); + let bit_size = match result_type { + Type::Numeric(NumericType::Signed { bit_size }) + | Type::Numeric(NumericType::Unsigned { bit_size }) => bit_size, + _ => unreachable!("ICE: Truncation attempted on non-integer"), + }; + self.builder.insert_wrapping_shift_left(lhs, rhs, bit_size) + } BinaryOpKind::ShiftRight => self.builder.insert_shift_right(lhs, rhs), BinaryOpKind::Equal | BinaryOpKind::NotEqual if matches!(self.builder.type_of_value(lhs), Type::Array(..)) => diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 8222f212190..74f076212fa 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -317,12 +317,19 @@ impl<'interner> TypeChecker<'interner> { match self.interner.lookup_trait_implementation(object_type, trait_id) { Ok(impl_kind) => self.interner.select_impl_for_ident(function_ident_id, impl_kind), Err(erroring_constraints) => { - let constraints = vecmap(erroring_constraints, |constraint| { - let r#trait = self.interner.get_trait(constraint.trait_id); - (constraint.typ, r#trait.name.to_string()) - }); + // Don't show any errors where try_get_trait returns None. + // This can happen if a trait is used that was never declared. + let constraints = erroring_constraints + .into_iter() + .map(|constraint| { + let r#trait = self.interner.try_get_trait(constraint.trait_id)?; + Some((constraint.typ, r#trait.name.to_string())) + }) + .collect::>>(); - self.errors.push(TypeCheckError::NoMatchingImplFound { constraints, span }); + if let Some(constraints) = constraints { + self.errors.push(TypeCheckError::NoMatchingImplFound { constraints, span }); + } } } } @@ -902,13 +909,15 @@ impl<'interner> TypeChecker<'interner> { for constraint in func_meta.trait_constraints { if *object_type == constraint.typ { - let the_trait = self.interner.get_trait(constraint.trait_id); - - for (method_index, method) in the_trait.methods.iter().enumerate() { - if method.name.0.contents == method_name { - let trait_method = - TraitMethodId { trait_id: constraint.trait_id, method_index }; - return Some(HirMethodReference::TraitMethodId(trait_method)); + if let Some(the_trait) = self.interner.try_get_trait(constraint.trait_id) { + for (method_index, method) in the_trait.methods.iter().enumerate() { + if method.name.0.contents == method_name { + let trait_method = TraitMethodId { + trait_id: constraint.trait_id, + method_index, + }; + return Some(HirMethodReference::TraitMethodId(trait_method)); + } } } } diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 956992a54d0..19d9d370377 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -58,10 +58,12 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec u32 { + match self { + Type::FieldElement | Type::Integer { .. } | Type::Bool => 1, + Type::Array(size, typ) => { + let length = size + .evaluate_to_u64() + .expect("Cannot have variable sized arrays as a parameter to main"); + let typ = typ.as_ref(); + (length as u32) * typ.field_count() + } + Type::Struct(ref def, args) => { + let struct_type = def.borrow(); + let fields = struct_type.get_fields(args); + fields.iter().fold(0, |acc, (_, field_type)| acc + field_type.field_count()) + } + Type::Tuple(fields) => { + fields.iter().fold(0, |acc, field_typ| acc + field_typ.field_count()) + } + Type::String(size) => { + let size = size + .evaluate_to_u64() + .expect("Cannot have variable sized strings as a parameter to main"); + size as u32 + } + Type::FmtString(_, _) + | Type::Unit + | Type::TypeVariable(_, _) + | Type::TraitAsType(_) + | Type::NamedGeneric(_, _) + | Type::Function(_, _, _) + | Type::MutableReference(_) + | Type::Forall(_, _) + | Type::Constant(_) + | Type::NotConstant + | Type::Error => unreachable!("This type cannot exist as a parameter to main"), + } + } +} + /// A list of TypeVariableIds to bind to a type. Storing the /// TypeVariable in addition to the matching TypeVariableId allows /// the binding to later be undone if needed. diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index e4532e2dceb..300a95f819c 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -845,6 +845,10 @@ impl NodeInterner { self.traits[&id].clone() } + pub fn try_get_trait(&self, id: TraitId) -> Option { + self.traits.get(&id).cloned() + } + pub fn get_type_alias(&self, id: TypeAliasId) -> &TypeAliasType { &self.type_aliases[id.0] } diff --git a/compiler/source-resolver/package.json b/compiler/source-resolver/package.json index fb214a822eb..23cb1c729a3 100644 --- a/compiler/source-resolver/package.json +++ b/compiler/source-resolver/package.json @@ -1,6 +1,6 @@ { "name": "@noir-lang/source-resolver", - "version": "0.19.2", + "version": "0.19.3", "license": "MIT", "main": "./lib-node/index_node.js", "types": "./types/index_node.d.ts", diff --git a/compiler/wasm/package.json b/compiler/wasm/package.json index 75b876e1db9..932dbb4a7b6 100644 --- a/compiler/wasm/package.json +++ b/compiler/wasm/package.json @@ -3,7 +3,7 @@ "collaborators": [ "The Noir Team " ], - "version": "0.19.2", + "version": "0.19.3", "license": "(MIT OR Apache-2.0)", "main": "./nodejs/noir_wasm.js", "types": "./web/noir_wasm.d.ts", diff --git a/flake.nix b/flake.nix index 0091a49f240..f9c458dc6ea 100644 --- a/flake.nix +++ b/flake.nix @@ -73,7 +73,7 @@ # Configuration shared between builds config = { # x-release-please-start-version - version = "0.19.2"; + version = "0.19.3"; # x-release-please-end src = pkgs.lib.cleanSourceWith { diff --git a/noir_stdlib/src/lib.nr b/noir_stdlib/src/lib.nr index 916e71cb490..8d878eecbb3 100644 --- a/noir_stdlib/src/lib.nr +++ b/noir_stdlib/src/lib.nr @@ -61,7 +61,3 @@ pub fn wrapping_sub(x: T, y: T) -> T { pub fn wrapping_mul(x: T, y: T) -> T { crate::from_field(crate::as_field(x) * crate::as_field(y)) } -/// Shift-left x by y bits -/// If the result overflow the bitsize; it does not fail and returns 0 instead -#[builtin(wrapping_shift_left)] -pub fn wrapping_shift_left(_x: T, _y: T) -> T {} diff --git a/noir_stdlib/src/sha256.nr b/noir_stdlib/src/sha256.nr index 4adb93f3848..8bb4a19f717 100644 --- a/noir_stdlib/src/sha256.nr +++ b/noir_stdlib/src/sha256.nr @@ -6,7 +6,7 @@ fn rotr32(a: u32, b: u32) -> u32 // 32-bit right rotation { // None of the bits overlap between `(a >> b)` and `(a << (32 - b))` // Addition is then equivalent to OR, with fewer constraints. - (a >> b) + (crate::wrapping_shift_left(a, 32 - b)) + (a >> b) + (a << (32 - b)) } fn ch(x: u32, y: u32, z: u32) -> u32 { diff --git a/noir_stdlib/src/sha512.nr b/noir_stdlib/src/sha512.nr index ad2926aea81..155ba593bba 100644 --- a/noir_stdlib/src/sha512.nr +++ b/noir_stdlib/src/sha512.nr @@ -6,7 +6,7 @@ fn rotr64(a: u64, b: u64) -> u64 // 64-bit right rotation { // None of the bits overlap between `(a >> b)` and `(a << (64 - b))` // Addition is then equivalent to OR, with fewer constraints. - (a >> b) + (crate::wrapping_shift_left(a, 64 - b)) + (a >> b) + (a << (64 - b)) } fn sha_ch(x: u64, y: u64, z: u64) -> u64 { diff --git a/tooling/backend_interface/src/lib.rs b/tooling/backend_interface/src/lib.rs index 6c91c181a92..36ebe5ebb91 100644 --- a/tooling/backend_interface/src/lib.rs +++ b/tooling/backend_interface/src/lib.rs @@ -153,6 +153,34 @@ impl BackendOpcodeSupport { } } } + + pub fn all() -> BackendOpcodeSupport { + BackendOpcodeSupport { + opcodes: HashSet::from([ + "arithmetic".to_string(), + "directive".to_string(), + "brillig".to_string(), + "memory_init".to_string(), + "memory_op".to_string(), + ]), + black_box_functions: HashSet::from([ + "sha256".to_string(), + "schnorr_verify".to_string(), + "blake2s".to_string(), + "pedersen".to_string(), + "pedersen_hash".to_string(), + "hash_to_field_128_security".to_string(), + "ecdsa_secp256k1".to_string(), + "fixed_base_scalar_mul".to_string(), + "and".to_string(), + "xor".to_string(), + "range".to_string(), + "keccak256".to_string(), + "recursive_aggregation".to_string(), + "ecdsa_secp256r1".to_string(), + ]), + } + } } #[cfg(test)] diff --git a/tooling/backend_interface/src/proof_system.rs b/tooling/backend_interface/src/proof_system.rs index 95da1462d4e..dcf1dad56b0 100644 --- a/tooling/backend_interface/src/proof_system.rs +++ b/tooling/backend_interface/src/proof_system.rs @@ -36,6 +36,18 @@ impl Backend { InfoCommand { crs_path: self.crs_directory() }.run(binary_path) } + /// If we cannot get a valid backend, returns the default backend which supports all the opcodes + /// and uses Plonk with width 3 + /// The function also prints a message saying we could not find a backend + pub fn get_backend_info_or_default(&self) -> (Language, BackendOpcodeSupport) { + if let Ok(backend_info) = self.get_backend_info() { + (backend_info.0, backend_info.1) + } else { + println!("No valid backend found, defaulting to Plonk with width 3 and all opcodes supported"); + (Language::PLONKCSat { width: 3 }, BackendOpcodeSupport::all()) + } + } + pub fn prove( &self, circuit: &Circuit, diff --git a/tooling/bb_abstraction_leaks/build.rs b/tooling/bb_abstraction_leaks/build.rs index fe6861a80b8..9c5e51a8115 100644 --- a/tooling/bb_abstraction_leaks/build.rs +++ b/tooling/bb_abstraction_leaks/build.rs @@ -10,7 +10,7 @@ use const_format::formatcp; const USERNAME: &str = "AztecProtocol"; const REPO: &str = "aztec-packages"; -const VERSION: &str = "0.12.0"; +const VERSION: &str = "0.15.1"; const TAG: &str = formatcp!("aztec-packages-v{}", VERSION); const API_URL: &str = @@ -30,8 +30,11 @@ fn main() -> Result<(), String> { }; // Arm builds of linux are not supported + // We do not panic because we allow users to run nargo without a backend. if let (Os::Linux, Arch::AARCH64) = (&os, &arch) { - panic!("ARM64 builds of linux are not supported") + println!( + "cargo:warning=ARM64 builds of linux are not supported for the barretenberg binary" + ); }; println!("cargo:rustc-env=BB_BINARY_URL={}", get_bb_download_url(arch, os)); diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 33862ed641f..40507aaed3a 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -120,7 +120,12 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { let foreign_call_result = self.foreign_call_executor.execute(&foreign_call); match foreign_call_result { Ok(foreign_call_result) => { - self.acvm.resolve_pending_foreign_call(foreign_call_result); + if let Some(mut solver) = self.brillig_solver.take() { + solver.resolve_pending_foreign_call(foreign_call_result); + self.brillig_solver = Some(solver); + } else { + self.acvm.resolve_pending_foreign_call(foreign_call_result); + } // TODO: should we retry executing the opcode somehow in this case? DebugCommandResult::Ok } @@ -168,13 +173,50 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { } } + fn currently_executing_brillig(&self) -> bool { + if self.brillig_solver.is_some() { + return true; + } + + match self.get_current_opcode_location() { + Some(OpcodeLocation::Brillig { .. }) => true, + Some(OpcodeLocation::Acir(acir_index)) => { + matches!(self.get_opcodes()[acir_index], Opcode::Brillig(_)) + } + _ => false, + } + } + + fn get_current_acir_index(&self) -> Option { + self.get_current_opcode_location().map(|opcode_location| match opcode_location { + OpcodeLocation::Acir(acir_index) => acir_index, + OpcodeLocation::Brillig { acir_index, .. } => acir_index, + }) + } + + fn step_out_of_brillig_opcode(&mut self) -> DebugCommandResult { + let Some(start_acir_index) = self.get_current_acir_index() else { + return DebugCommandResult::Done; + }; + loop { + let result = self.step_into_opcode(); + if !matches!(result, DebugCommandResult::Ok) { + return result; + } + let new_acir_index = self.get_current_acir_index().unwrap(); + if new_acir_index != start_acir_index { + return DebugCommandResult::Ok; + } + } + } + pub(super) fn step_acir_opcode(&mut self) -> DebugCommandResult { - let status = if let Some(solver) = self.brillig_solver.take() { - self.acvm.finish_brillig_with_solver(solver) + if self.currently_executing_brillig() { + self.step_out_of_brillig_opcode() } else { - self.acvm.solve_opcode() - }; - self.handle_acvm_status(status) + let status = self.acvm.solve_opcode(); + self.handle_acvm_status(status) + } } pub(super) fn next(&mut self) -> DebugCommandResult { @@ -276,3 +318,208 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { self.acvm.finalize() } } + +#[cfg(test)] +struct StubbedSolver; + +#[cfg(test)] +impl BlackBoxFunctionSolver for StubbedSolver { + fn schnorr_verify( + &self, + _public_key_x: &FieldElement, + _public_key_y: &FieldElement, + _signature: &[u8], + _message: &[u8], + ) -> Result { + unimplemented!(); + } + + fn pedersen_commitment( + &self, + _inputs: &[FieldElement], + _domain_separator: u32, + ) -> Result<(FieldElement, FieldElement), acvm::BlackBoxResolutionError> { + unimplemented!(); + } + + fn pedersen_hash( + &self, + _inputs: &[FieldElement], + _domain_separator: u32, + ) -> Result { + unimplemented!(); + } + + fn fixed_base_scalar_mul( + &self, + _low: &FieldElement, + _high: &FieldElement, + ) -> Result<(FieldElement, FieldElement), acvm::BlackBoxResolutionError> { + unimplemented!(); + } +} + +#[cfg(test)] +#[test] +fn test_resolve_foreign_calls_stepping_into_brillig() { + use std::collections::BTreeMap; + + use acvm::acir::{ + brillig::{Opcode as BrilligOpcode, RegisterIndex, RegisterOrMemory}, + circuit::brillig::{Brillig, BrilligInputs}, + native_types::Expression, + }; + + let fe_0 = FieldElement::zero(); + let fe_1 = FieldElement::one(); + let w_x = Witness(1); + + let blackbox_solver = &StubbedSolver; + + let brillig_opcodes = Brillig { + inputs: vec![BrilligInputs::Single(Expression { + linear_combinations: vec![(fe_1, w_x)], + ..Expression::default() + })], + outputs: vec![], + bytecode: vec![ + BrilligOpcode::Const { destination: RegisterIndex::from(1), value: Value::from(fe_0) }, + BrilligOpcode::ForeignCall { + function: "clear_mock".into(), + destinations: vec![], + inputs: vec![RegisterOrMemory::RegisterIndex(RegisterIndex::from(0))], + }, + BrilligOpcode::Stop, + ], + predicate: None, + }; + let opcodes = vec![Opcode::Brillig(brillig_opcodes)]; + let current_witness_index = 2; + let circuit = &Circuit { current_witness_index, opcodes, ..Circuit::default() }; + + let debug_symbols = vec![]; + let file_map = BTreeMap::new(); + let warnings = vec![]; + let debug_artifact = &DebugArtifact { debug_symbols, file_map, warnings }; + + let initial_witness = BTreeMap::from([(Witness(1), fe_1)]).into(); + + let mut context = DebugContext::new(blackbox_solver, circuit, debug_artifact, initial_witness); + + assert_eq!(context.get_current_opcode_location(), Some(OpcodeLocation::Acir(0))); + + // execute the first Brillig opcode (const) + let result = context.step_into_opcode(); + assert!(matches!(result, DebugCommandResult::Ok)); + assert_eq!( + context.get_current_opcode_location(), + Some(OpcodeLocation::Brillig { acir_index: 0, brillig_index: 1 }) + ); + + // try to execute the second Brillig opcode (and resolve the foreign call) + let result = context.step_into_opcode(); + assert!(matches!(result, DebugCommandResult::Ok)); + assert_eq!( + context.get_current_opcode_location(), + Some(OpcodeLocation::Brillig { acir_index: 0, brillig_index: 1 }) + ); + + // retry the second Brillig opcode (foreign call should be finished) + let result = context.step_into_opcode(); + assert!(matches!(result, DebugCommandResult::Ok)); + assert_eq!( + context.get_current_opcode_location(), + Some(OpcodeLocation::Brillig { acir_index: 0, brillig_index: 2 }) + ); + + // last Brillig opcode + let result = context.step_into_opcode(); + assert!(matches!(result, DebugCommandResult::Done)); + assert_eq!(context.get_current_opcode_location(), None); +} + +#[cfg(test)] +#[test] +fn test_break_brillig_block_while_stepping_acir_opcodes() { + use std::collections::BTreeMap; + + use acvm::acir::{ + brillig::{Opcode as BrilligOpcode, RegisterIndex}, + circuit::brillig::{Brillig, BrilligInputs, BrilligOutputs}, + native_types::Expression, + }; + use acvm::brillig_vm::brillig::BinaryFieldOp; + + let fe_0 = FieldElement::zero(); + let fe_1 = FieldElement::one(); + let w_x = Witness(1); + let w_y = Witness(2); + let w_z = Witness(3); + + let blackbox_solver = &StubbedSolver; + + // This Brillig block is equivalent to: z = x + y + let brillig_opcodes = Brillig { + inputs: vec![ + BrilligInputs::Single(Expression { + linear_combinations: vec![(fe_1, w_x)], + ..Expression::default() + }), + BrilligInputs::Single(Expression { + linear_combinations: vec![(fe_1, w_y)], + ..Expression::default() + }), + ], + outputs: vec![BrilligOutputs::Simple(w_z)], + bytecode: vec![ + BrilligOpcode::BinaryFieldOp { + destination: RegisterIndex::from(0), + op: BinaryFieldOp::Add, + lhs: RegisterIndex::from(0), + rhs: RegisterIndex::from(1), + }, + BrilligOpcode::Stop, + ], + predicate: None, + }; + let opcodes = vec![ + // z = x + y + Opcode::Brillig(brillig_opcodes), + // x + y - z = 0 + Opcode::Arithmetic(Expression { + mul_terms: vec![], + linear_combinations: vec![(fe_1, w_x), (fe_1, w_y), (-fe_1, w_z)], + q_c: fe_0, + }), + ]; + let current_witness_index = 3; + let circuit = &Circuit { current_witness_index, opcodes, ..Circuit::default() }; + + let debug_symbols = vec![]; + let file_map = BTreeMap::new(); + let warnings = vec![]; + let debug_artifact = &DebugArtifact { debug_symbols, file_map, warnings }; + + let initial_witness = BTreeMap::from([(Witness(1), fe_1), (Witness(2), fe_1)]).into(); + + let mut context = DebugContext::new(blackbox_solver, circuit, debug_artifact, initial_witness); + + // set breakpoint + let breakpoint_location = OpcodeLocation::Brillig { acir_index: 0, brillig_index: 1 }; + assert!(context.add_breakpoint(breakpoint_location.clone())); + + // execute the first ACIR opcode (Brillig block) -> should reach the breakpoint instead + let result = context.step_acir_opcode(); + assert!(matches!(result, DebugCommandResult::BreakpointReached(_))); + assert_eq!(context.get_current_opcode_location(), Some(breakpoint_location)); + + // continue execution to the next ACIR opcode + let result = context.step_acir_opcode(); + assert!(matches!(result, DebugCommandResult::Ok)); + assert_eq!(context.get_current_opcode_location(), Some(OpcodeLocation::Acir(1))); + + // last ACIR opcode + let result = context.step_acir_opcode(); + assert!(matches!(result, DebugCommandResult::Done)); + assert_eq!(context.get_current_opcode_location(), None); +} diff --git a/tooling/lsp/Cargo.toml b/tooling/lsp/Cargo.toml index 9e642d5fe9c..67778c744db 100644 --- a/tooling/lsp/Cargo.toml +++ b/tooling/lsp/Cargo.toml @@ -14,6 +14,7 @@ codespan-lsp.workspace = true codespan-reporting.workspace = true lsp-types.workspace = true nargo.workspace = true +nargo_fmt.workspace = true nargo_toml.workspace = true noirc_driver.workspace = true noirc_errors.workspace = true diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index 79fc544692a..d8a992155dd 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -4,6 +4,7 @@ #![cfg_attr(not(test), warn(unused_crate_dependencies, unused_extern_crates))] use std::{ + collections::HashMap, future::Future, ops::{self, ControlFlow}, path::{Path, PathBuf}, @@ -26,8 +27,8 @@ use notifications::{ on_did_open_text_document, on_did_save_text_document, on_exit, on_initialized, }; use requests::{ - on_code_lens_request, on_initialize, on_profile_run_request, on_shutdown, on_test_run_request, - on_tests_request, + on_code_lens_request, on_formatting, on_initialize, on_profile_run_request, on_shutdown, + on_test_run_request, on_tests_request, }; use serde_json::Value as JsonValue; use tower::Service; @@ -45,11 +46,17 @@ pub struct LspState { root_path: Option, client: ClientSocket, solver: WrapperSolver, + input_files: HashMap, } impl LspState { fn new(client: &ClientSocket, solver: impl BlackBoxFunctionSolver + 'static) -> Self { - Self { client: client.clone(), root_path: None, solver: WrapperSolver(Box::new(solver)) } + Self { + client: client.clone(), + root_path: None, + solver: WrapperSolver(Box::new(solver)), + input_files: HashMap::new(), + } } } @@ -63,6 +70,7 @@ impl NargoLspService { let mut router = Router::new(state); router .request::(on_initialize) + .request::(on_formatting) .request::(on_shutdown) .request::(on_code_lens_request) .request::(on_tests_request) diff --git a/tooling/lsp/src/notifications/mod.rs b/tooling/lsp/src/notifications/mod.rs index 93fa8baf6ac..f6484f49d48 100644 --- a/tooling/lsp/src/notifications/mod.rs +++ b/tooling/lsp/src/notifications/mod.rs @@ -30,23 +30,27 @@ pub(super) fn on_did_change_configuration( } pub(super) fn on_did_open_text_document( - _state: &mut LspState, - _params: DidOpenTextDocumentParams, + state: &mut LspState, + params: DidOpenTextDocumentParams, ) -> ControlFlow> { + state.input_files.insert(params.text_document.uri.to_string(), params.text_document.text); ControlFlow::Continue(()) } pub(super) fn on_did_change_text_document( - _state: &mut LspState, - _params: DidChangeTextDocumentParams, + state: &mut LspState, + params: DidChangeTextDocumentParams, ) -> ControlFlow> { + let text = params.content_changes.into_iter().next().unwrap().text; + state.input_files.insert(params.text_document.uri.to_string(), text); ControlFlow::Continue(()) } pub(super) fn on_did_close_text_document( - _state: &mut LspState, - _params: DidCloseTextDocumentParams, + state: &mut LspState, + params: DidCloseTextDocumentParams, ) -> ControlFlow> { + state.input_files.remove(¶ms.text_document.uri.to_string()); ControlFlow::Continue(()) } diff --git a/tooling/lsp/src/requests/mod.rs b/tooling/lsp/src/requests/mod.rs index b2be24e1187..a319f2593a4 100644 --- a/tooling/lsp/src/requests/mod.rs +++ b/tooling/lsp/src/requests/mod.rs @@ -1,7 +1,9 @@ use std::future::Future; -use crate::types::{CodeLensOptions, InitializeParams, TextDocumentSyncOptions}; +use crate::types::{CodeLensOptions, InitializeParams}; use async_lsp::ResponseError; +use lsp_types::{Position, TextDocumentSyncCapability, TextDocumentSyncKind}; +use nargo_fmt::Config; use crate::{ types::{InitializeResult, NargoCapability, NargoTestsOptions, ServerCapabilities}, @@ -35,8 +37,7 @@ pub(crate) fn on_initialize( state.root_path = params.root_uri.and_then(|root_uri| root_uri.to_file_path().ok()); async { - let text_document_sync = - TextDocumentSyncOptions { save: Some(true.into()), ..Default::default() }; + let text_document_sync = TextDocumentSyncCapability::Kind(TextDocumentSyncKind::FULL); let code_lens = CodeLensOptions { resolve_provider: Some(false) }; @@ -50,8 +51,9 @@ pub(crate) fn on_initialize( Ok(InitializeResult { capabilities: ServerCapabilities { - text_document_sync: Some(text_document_sync.into()), + text_document_sync: Some(text_document_sync), code_lens_provider: Some(code_lens), + document_formatting_provider: true, nargo: Some(nargo), }, server_info: None, @@ -59,6 +61,42 @@ pub(crate) fn on_initialize( } } +pub(crate) fn on_formatting( + state: &mut LspState, + params: lsp_types::DocumentFormattingParams, +) -> impl Future>, ResponseError>> { + std::future::ready(on_formatting_inner(state, params)) +} + +fn on_formatting_inner( + state: &LspState, + params: lsp_types::DocumentFormattingParams, +) -> Result>, ResponseError> { + let path = params.text_document.uri.to_string(); + + if let Some(source) = state.input_files.get(&path) { + let (module, errors) = noirc_frontend::parse_program(source); + if !errors.is_empty() { + return Ok(None); + } + + let new_text = nargo_fmt::format(source, module, &Config::default()); + + let start_position = Position { line: 0, character: 0 }; + let end_position = Position { + line: source.lines().count() as u32, + character: source.chars().count() as u32, + }; + + Ok(Some(vec![lsp_types::TextEdit { + range: lsp_types::Range::new(start_position, end_position), + new_text, + }])) + } else { + Ok(None) + } +} + pub(crate) fn on_shutdown( _state: &mut LspState, _params: (), @@ -70,7 +108,7 @@ pub(crate) fn on_shutdown( mod initialization { use async_lsp::ClientSocket; use lsp_types::{ - CodeLensOptions, InitializeParams, TextDocumentSyncCapability, TextDocumentSyncOptions, + CodeLensOptions, InitializeParams, TextDocumentSyncCapability, TextDocumentSyncKind, }; use tokio::test; @@ -88,10 +126,11 @@ mod initialization { assert!(matches!( response.capabilities, ServerCapabilities { - text_document_sync: Some(TextDocumentSyncCapability::Options( - TextDocumentSyncOptions { save: Some(_), .. } + text_document_sync: Some(TextDocumentSyncCapability::Kind( + TextDocumentSyncKind::FULL )), code_lens_provider: Some(CodeLensOptions { resolve_provider: Some(false) }), + document_formatting_provider: true, .. } )); diff --git a/tooling/lsp/src/types.rs b/tooling/lsp/src/types.rs index 7a50c538051..ba964cba0c1 100644 --- a/tooling/lsp/src/types.rs +++ b/tooling/lsp/src/types.rs @@ -12,7 +12,7 @@ pub(crate) use lsp_types::{ DidChangeConfigurationParams, DidChangeTextDocumentParams, DidCloseTextDocumentParams, DidOpenTextDocumentParams, DidSaveTextDocumentParams, InitializeParams, InitializedParams, LogMessageParams, MessageType, Position, PublishDiagnosticsParams, Range, ServerInfo, - TextDocumentSyncCapability, TextDocumentSyncOptions, Url, + TextDocumentSyncCapability, Url, }; pub(crate) mod request { @@ -24,7 +24,7 @@ pub(crate) mod request { }; // Re-providing lsp_types that we don't need to override - pub(crate) use lsp_types::request::{CodeLensRequest as CodeLens, Shutdown}; + pub(crate) use lsp_types::request::{CodeLensRequest as CodeLens, Formatting, Shutdown}; #[derive(Debug)] pub(crate) struct Initialize; @@ -112,6 +112,9 @@ pub(crate) struct ServerCapabilities { #[serde(skip_serializing_if = "Option::is_none")] pub(crate) code_lens_provider: Option, + /// The server provides document formatting. + pub(crate) document_formatting_provider: bool, + /// The server handles and provides custom nargo messages. #[serde(skip_serializing_if = "Option::is_none")] pub(crate) nargo: Option, diff --git a/tooling/nargo_cli/Cargo.toml b/tooling/nargo_cli/Cargo.toml index 1a08514dc5f..07298ae25d2 100644 --- a/tooling/nargo_cli/Cargo.toml +++ b/tooling/nargo_cli/Cargo.toml @@ -43,6 +43,7 @@ tower.workspace = true async-lsp = { workspace = true, features = ["client-monitor", "stdio", "tracing", "tokio"] } const_format.workspace = true hex.workspace = true +similar-asserts.workspace = true termcolor = "1.1.2" color-eyre = "0.6.2" tokio = { version = "1.0", features = ["io-std"] } diff --git a/tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs b/tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs index 59143526b24..02c83adb59a 100644 --- a/tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs +++ b/tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs @@ -6,8 +6,8 @@ use super::{ use crate::backends::Backend; use crate::errors::CliError; -use acvm::acir::circuit::Opcode; use acvm::Language; +use backend_interface::BackendOpcodeSupport; use bb_abstraction_leaks::ACVM_BACKEND_BARRETENBERG; use clap::Args; use nargo::package::Package; @@ -54,7 +54,7 @@ pub(crate) fn run( package, &args.compile_options, np_language, - &|opcode| opcode_support.is_opcode_supported(opcode), + &opcode_support, )?; let contract_dir = workspace.contracts_directory_path(package); @@ -74,15 +74,10 @@ fn smart_contract_for_package( package: &Package, compile_options: &CompileOptions, np_language: Language, - is_opcode_supported: &impl Fn(&Opcode) -> bool, + opcode_support: &BackendOpcodeSupport, ) -> Result { - let program = compile_bin_package( - workspace, - package, - compile_options, - np_language, - &is_opcode_supported, - )?; + let program = + compile_bin_package(workspace, package, compile_options, np_language, opcode_support)?; let mut smart_contract_string = backend.eth_contract(&program.circuit)?; diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index 9ffbc26828e..69533292bbd 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -76,7 +76,7 @@ pub(crate) fn run( .cloned() .partition(|package| package.is_binary()); - let (np_language, opcode_support) = backend.get_backend_info()?; + let (np_language, opcode_support) = backend.get_backend_info_or_default(); let (_, compiled_contracts) = compile_workspace( &workspace, &binary_packages, @@ -102,12 +102,11 @@ pub(super) fn compile_workspace( opcode_support: &BackendOpcodeSupport, compile_options: &CompileOptions, ) -> Result<(Vec, Vec), CliError> { - let is_opcode_supported = |opcode: &_| opcode_support.is_opcode_supported(opcode); - // Compile all of the packages in parallel. let program_results: Vec<(FileManager, CompilationResult)> = binary_packages .par_iter() .map(|package| { + let is_opcode_supported = |opcode: &_| opcode_support.is_opcode_supported(opcode); compile_program(workspace, package, compile_options, np_language, &is_opcode_supported) }) .collect(); @@ -115,6 +114,7 @@ pub(super) fn compile_workspace( contract_packages .par_iter() .map(|package| { + let is_opcode_supported = |opcode: &_| opcode_support.is_opcode_supported(opcode); compile_contract(package, compile_options, np_language, &is_opcode_supported) }) .collect(); @@ -151,14 +151,16 @@ pub(crate) fn compile_bin_package( package: &Package, compile_options: &CompileOptions, np_language: Language, - is_opcode_supported: &impl Fn(&Opcode) -> bool, + opcode_support: &BackendOpcodeSupport, ) -> Result { if package.is_library() { return Err(CompileError::LibraryCrate(package.name.clone()).into()); } let (file_manager, compilation_result) = - compile_program(workspace, package, compile_options, np_language, &is_opcode_supported); + compile_program(workspace, package, compile_options, np_language, &|opcode| { + opcode_support.is_opcode_supported(opcode) + }); let program = report_errors( compilation_result, diff --git a/tooling/nargo_cli/src/cli/debug_cmd.rs b/tooling/nargo_cli/src/cli/debug_cmd.rs index 1d344058312..0e7579b0721 100644 --- a/tooling/nargo_cli/src/cli/debug_cmd.rs +++ b/tooling/nargo_cli/src/cli/debug_cmd.rs @@ -56,10 +56,13 @@ pub(crate) fn run( return Ok(()); }; - let compiled_program = - compile_bin_package(&workspace, package, &args.compile_options, np_language, &|opcode| { - opcode_support.is_opcode_supported(opcode) - })?; + let compiled_program = compile_bin_package( + &workspace, + package, + &args.compile_options, + np_language, + &opcode_support, + )?; println!("[{}] Starting debugger", package.name); let (return_value, solved_witness) = diff --git a/tooling/nargo_cli/src/cli/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index 91e4b800453..2f69b4c7df7 100644 --- a/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -56,14 +56,14 @@ pub(crate) fn run( )?; let target_dir = &workspace.target_directory_path(); - let (np_language, opcode_support) = backend.get_backend_info()?; + let (np_language, opcode_support) = backend.get_backend_info_or_default(); for package in &workspace { let compiled_program = compile_bin_package( &workspace, package, &args.compile_options, np_language, - &|opcode| opcode_support.is_opcode_supported(opcode), + &opcode_support, )?; let (return_value, solved_witness) = diff --git a/tooling/nargo_cli/src/cli/fmt_cmd.rs b/tooling/nargo_cli/src/cli/fmt_cmd.rs index eaa66fff02b..ec3d373a483 100644 --- a/tooling/nargo_cli/src/cli/fmt_cmd.rs +++ b/tooling/nargo_cli/src/cli/fmt_cmd.rs @@ -11,10 +11,17 @@ use crate::errors::CliError; use super::NargoConfig; +/// Format the Noir files in a workspace #[derive(Debug, Clone, Args)] -pub(crate) struct FormatCommand {} +pub(crate) struct FormatCommand { + /// Run noirfmt in check mode + #[arg(long)] + check: bool, +} + +pub(crate) fn run(args: FormatCommand, config: NargoConfig) -> Result<(), CliError> { + let check_mode = args.check; -pub(crate) fn run(_args: FormatCommand, config: NargoConfig) -> Result<(), CliError> { let toml_path = get_package_manifest(&config.program_dir)?; let workspace = resolve_workspace_from_toml( &toml_path, @@ -25,6 +32,8 @@ pub(crate) fn run(_args: FormatCommand, config: NargoConfig) -> Result<(), CliEr let config = nargo_fmt::Config::read(&config.program_dir) .map_err(|err| CliError::Generic(err.to_string()))?; + let mut check_exit_code_one = false; + for package in &workspace { let mut file_manager = FileManager::new(&package.root_dir, Box::new(|path| std::fs::read_to_string(path))); @@ -52,16 +61,40 @@ pub(crate) fn run(_args: FormatCommand, config: NargoConfig) -> Result<(), CliEr return Ok(()); } - let source = nargo_fmt::format( - file_manager.fetch_file(file_id).source(), - parsed_module, - &config, - ); + let original = file_manager.fetch_file(file_id).source(); + let formatted = nargo_fmt::format(original, parsed_module, &config); + + if check_mode { + let diff = similar_asserts::SimpleDiff::from_str( + original, + &formatted, + "original", + "formatted", + ) + .to_string(); + + if !diff.lines().next().is_some_and(|line| line.contains("Invisible differences")) { + if !check_exit_code_one { + check_exit_code_one = true; + } - std::fs::write(entry.path(), source) + println!("{diff}"); + } + + Ok(()) + } else { + std::fs::write(entry.path(), formatted) + } }) .map_err(|error| CliError::Generic(error.to_string()))?; } + + if check_exit_code_one { + std::process::exit(1); + } else if check_mode { + println!("No formatting changes were detected"); + } + Ok(()) } diff --git a/tooling/nargo_cli/src/cli/info_cmd.rs b/tooling/nargo_cli/src/cli/info_cmd.rs index b1cd5f0b64f..b0f771bfc1c 100644 --- a/tooling/nargo_cli/src/cli/info_cmd.rs +++ b/tooling/nargo_cli/src/cli/info_cmd.rs @@ -67,7 +67,7 @@ pub(crate) fn run( .cloned() .partition(|package| package.is_binary()); - let (np_language, opcode_support) = backend.get_backend_info()?; + let (np_language, opcode_support) = backend.get_backend_info_or_default(); let (compiled_programs, compiled_contracts) = compile_workspace( &workspace, &binary_packages, diff --git a/tooling/nargo_cli/src/cli/mod.rs b/tooling/nargo_cli/src/cli/mod.rs index 8d22fb1b204..88c6b57a98c 100644 --- a/tooling/nargo_cli/src/cli/mod.rs +++ b/tooling/nargo_cli/src/cli/mod.rs @@ -60,7 +60,6 @@ pub(crate) struct NargoConfig { enum NargoCommand { Backend(backend_cmd::BackendCommand), Check(check_cmd::CheckCommand), - #[command(hide = true)] // Hidden while the feature has not been extensively tested Fmt(fmt_cmd::FormatCommand), CodegenVerifier(codegen_verifier_cmd::CodegenVerifierCommand), #[command(alias = "build")] diff --git a/tooling/nargo_cli/src/cli/prove_cmd.rs b/tooling/nargo_cli/src/cli/prove_cmd.rs index 3586e73ff2e..54b148ec3a2 100644 --- a/tooling/nargo_cli/src/cli/prove_cmd.rs +++ b/tooling/nargo_cli/src/cli/prove_cmd.rs @@ -64,7 +64,7 @@ pub(crate) fn run( package, &args.compile_options, np_language, - &|opcode| opcode_support.is_opcode_supported(opcode), + &opcode_support, )?; prove_package( diff --git a/tooling/nargo_cli/src/cli/verify_cmd.rs b/tooling/nargo_cli/src/cli/verify_cmd.rs index 8c6d92b3d2f..2f8a6efbba4 100644 --- a/tooling/nargo_cli/src/cli/verify_cmd.rs +++ b/tooling/nargo_cli/src/cli/verify_cmd.rs @@ -55,7 +55,7 @@ pub(crate) fn run( package, &args.compile_options, np_language, - &|opcode| opcode_support.is_opcode_supported(opcode), + &opcode_support, )?; verify_package(backend, &workspace, package, program, &args.verifier_name)?; diff --git a/tooling/nargo_cli/tests/acir_artifacts/array_dynamic/target/acir.gz b/tooling/nargo_cli/tests/acir_artifacts/array_dynamic/target/acir.gz index 376ae46ff4f..bb735d852d5 100644 Binary files a/tooling/nargo_cli/tests/acir_artifacts/array_dynamic/target/acir.gz and b/tooling/nargo_cli/tests/acir_artifacts/array_dynamic/target/acir.gz differ diff --git a/tooling/nargo_cli/tests/acir_artifacts/array_dynamic/target/witness.gz b/tooling/nargo_cli/tests/acir_artifacts/array_dynamic/target/witness.gz index 706a5784953..b80b33eecdf 100644 Binary files a/tooling/nargo_cli/tests/acir_artifacts/array_dynamic/target/witness.gz and b/tooling/nargo_cli/tests/acir_artifacts/array_dynamic/target/witness.gz differ diff --git a/tooling/nargo_cli/tests/acir_artifacts/bit_shifts_comptime/target/acir.gz b/tooling/nargo_cli/tests/acir_artifacts/bit_shifts_comptime/target/acir.gz index 4756ea6b632..27e71fce0a7 100644 Binary files a/tooling/nargo_cli/tests/acir_artifacts/bit_shifts_comptime/target/acir.gz and b/tooling/nargo_cli/tests/acir_artifacts/bit_shifts_comptime/target/acir.gz differ diff --git a/tooling/nargo_cli/tests/acir_artifacts/bit_shifts_comptime/target/witness.gz b/tooling/nargo_cli/tests/acir_artifacts/bit_shifts_comptime/target/witness.gz index 890e987dd74..3fadfc3d5be 100644 Binary files a/tooling/nargo_cli/tests/acir_artifacts/bit_shifts_comptime/target/witness.gz and b/tooling/nargo_cli/tests/acir_artifacts/bit_shifts_comptime/target/witness.gz differ diff --git a/tooling/nargo_cli/tests/acir_artifacts/bit_shifts_runtime/target/acir.gz b/tooling/nargo_cli/tests/acir_artifacts/bit_shifts_runtime/target/acir.gz index 45c1cc44c8e..1a800a63a57 100644 Binary files a/tooling/nargo_cli/tests/acir_artifacts/bit_shifts_runtime/target/acir.gz and b/tooling/nargo_cli/tests/acir_artifacts/bit_shifts_runtime/target/acir.gz differ diff --git a/tooling/nargo_cli/tests/acir_artifacts/bit_shifts_runtime/target/witness.gz b/tooling/nargo_cli/tests/acir_artifacts/bit_shifts_runtime/target/witness.gz index bdc4e70ba09..2af844993dd 100644 Binary files a/tooling/nargo_cli/tests/acir_artifacts/bit_shifts_runtime/target/witness.gz and b/tooling/nargo_cli/tests/acir_artifacts/bit_shifts_runtime/target/witness.gz differ diff --git a/tooling/nargo_cli/tests/acir_artifacts/conditional_1/target/acir.gz b/tooling/nargo_cli/tests/acir_artifacts/conditional_1/target/acir.gz index dff5ebe0a87..46a738a97e3 100644 Binary files a/tooling/nargo_cli/tests/acir_artifacts/conditional_1/target/acir.gz and b/tooling/nargo_cli/tests/acir_artifacts/conditional_1/target/acir.gz differ diff --git a/tooling/nargo_cli/tests/acir_artifacts/conditional_1/target/witness.gz b/tooling/nargo_cli/tests/acir_artifacts/conditional_1/target/witness.gz index acc00ece890..5bafffff0d8 100644 Binary files a/tooling/nargo_cli/tests/acir_artifacts/conditional_1/target/witness.gz and b/tooling/nargo_cli/tests/acir_artifacts/conditional_1/target/witness.gz differ diff --git a/tooling/nargo_cli/tests/acir_artifacts/conditional_regression_661/target/acir.gz b/tooling/nargo_cli/tests/acir_artifacts/conditional_regression_661/target/acir.gz index 7c336747f92..51e30b8bbc1 100644 Binary files a/tooling/nargo_cli/tests/acir_artifacts/conditional_regression_661/target/acir.gz and b/tooling/nargo_cli/tests/acir_artifacts/conditional_regression_661/target/acir.gz differ diff --git a/tooling/nargo_cli/tests/acir_artifacts/conditional_regression_661/target/witness.gz b/tooling/nargo_cli/tests/acir_artifacts/conditional_regression_661/target/witness.gz index 1efbfebfaad..2683a9ba4ae 100644 Binary files a/tooling/nargo_cli/tests/acir_artifacts/conditional_regression_661/target/witness.gz and b/tooling/nargo_cli/tests/acir_artifacts/conditional_regression_661/target/witness.gz differ diff --git a/tooling/nargo_cli/tests/acir_artifacts/conditional_regression_underflow/target/acir.gz b/tooling/nargo_cli/tests/acir_artifacts/conditional_regression_underflow/target/acir.gz new file mode 100644 index 00000000000..df762d9205e Binary files /dev/null and b/tooling/nargo_cli/tests/acir_artifacts/conditional_regression_underflow/target/acir.gz differ diff --git a/tooling/nargo_cli/tests/acir_artifacts/conditional_regression_underflow/target/witness.gz b/tooling/nargo_cli/tests/acir_artifacts/conditional_regression_underflow/target/witness.gz new file mode 100644 index 00000000000..939eb503b6f Binary files /dev/null and b/tooling/nargo_cli/tests/acir_artifacts/conditional_regression_underflow/target/witness.gz differ diff --git a/tooling/nargo_cli/tests/acir_artifacts/regression/target/acir.gz b/tooling/nargo_cli/tests/acir_artifacts/regression/target/acir.gz index 66b4abc42dc..f92aa2603b4 100644 Binary files a/tooling/nargo_cli/tests/acir_artifacts/regression/target/acir.gz and b/tooling/nargo_cli/tests/acir_artifacts/regression/target/acir.gz differ diff --git a/tooling/nargo_cli/tests/acir_artifacts/regression/target/witness.gz b/tooling/nargo_cli/tests/acir_artifacts/regression/target/witness.gz index ea06e69e18e..dc1f8e0e3f7 100644 Binary files a/tooling/nargo_cli/tests/acir_artifacts/regression/target/witness.gz and b/tooling/nargo_cli/tests/acir_artifacts/regression/target/witness.gz differ diff --git a/tooling/nargo_cli/tests/acir_artifacts/sha2_byte/target/acir.gz b/tooling/nargo_cli/tests/acir_artifacts/sha2_byte/target/acir.gz index 7f6715a12af..571fde25f2b 100644 Binary files a/tooling/nargo_cli/tests/acir_artifacts/sha2_byte/target/acir.gz and b/tooling/nargo_cli/tests/acir_artifacts/sha2_byte/target/acir.gz differ diff --git a/tooling/nargo_cli/tests/acir_artifacts/sha2_byte/target/witness.gz b/tooling/nargo_cli/tests/acir_artifacts/sha2_byte/target/witness.gz index c7be868682e..08ca373be7e 100644 Binary files a/tooling/nargo_cli/tests/acir_artifacts/sha2_byte/target/witness.gz and b/tooling/nargo_cli/tests/acir_artifacts/sha2_byte/target/witness.gz differ diff --git a/tooling/nargo_cli/tests/acir_artifacts/simple_shift_left_right/target/acir.gz b/tooling/nargo_cli/tests/acir_artifacts/simple_shift_left_right/target/acir.gz index 6bbdaef4a9b..bae747f46c6 100644 Binary files a/tooling/nargo_cli/tests/acir_artifacts/simple_shift_left_right/target/acir.gz and b/tooling/nargo_cli/tests/acir_artifacts/simple_shift_left_right/target/acir.gz differ diff --git a/tooling/nargo_cli/tests/acir_artifacts/simple_shift_left_right/target/witness.gz b/tooling/nargo_cli/tests/acir_artifacts/simple_shift_left_right/target/witness.gz index 6f15adc6525..6bc0b91e147 100644 Binary files a/tooling/nargo_cli/tests/acir_artifacts/simple_shift_left_right/target/witness.gz and b/tooling/nargo_cli/tests/acir_artifacts/simple_shift_left_right/target/witness.gz differ diff --git a/tooling/nargo_cli/tests/execution_success/bit_shifts_comptime/src/main.nr b/tooling/nargo_cli/tests/execution_success/bit_shifts_comptime/src/main.nr index 8afed4c1e1e..a03376d062a 100644 --- a/tooling/nargo_cli/tests/execution_success/bit_shifts_comptime/src/main.nr +++ b/tooling/nargo_cli/tests/execution_success/bit_shifts_comptime/src/main.nr @@ -10,6 +10,9 @@ fn main(x: u64) { assert(x >> 2 == 16); regression_2250(); + + //regression for 3481 + assert(x << 63 == 0); } fn regression_2250() { diff --git a/tooling/nargo_cli/tests/execution_success/conditional_regression_underflow/Nargo.toml b/tooling/nargo_cli/tests/execution_success/conditional_regression_underflow/Nargo.toml index 54a082081f7..75c4fb43b2f 100644 --- a/tooling/nargo_cli/tests/execution_success/conditional_regression_underflow/Nargo.toml +++ b/tooling/nargo_cli/tests/execution_success/conditional_regression_underflow/Nargo.toml @@ -1,5 +1,5 @@ [package] -name = "conditional_underflow" +name = "conditional_regression_underflow" type = "bin" authors = [""] diff --git a/tooling/nargo_fmt/Cargo.toml b/tooling/nargo_fmt/Cargo.toml index 921c9893ab5..374413ac9f2 100644 --- a/tooling/nargo_fmt/Cargo.toml +++ b/tooling/nargo_fmt/Cargo.toml @@ -13,4 +13,4 @@ toml.workspace = true thiserror.workspace = true [dev-dependencies] -similar-asserts = "1.5.0" +similar-asserts.workspace = true diff --git a/tooling/nargo_fmt/src/rewrite.rs b/tooling/nargo_fmt/src/rewrite.rs index c1ac585985e..5a9baf0aa05 100644 --- a/tooling/nargo_fmt/src/rewrite.rs +++ b/tooling/nargo_fmt/src/rewrite.rs @@ -1,5 +1,9 @@ mod array; +mod expr; mod infix; +mod parenthesized; pub(crate) use array::rewrite as array; +pub(crate) use expr::{rewrite as expr, rewrite_sub_expr as sub_expr}; pub(crate) use infix::rewrite as infix; +pub(crate) use parenthesized::rewrite as parenthesized; diff --git a/tooling/nargo_fmt/src/rewrite/array.rs b/tooling/nargo_fmt/src/rewrite/array.rs index 9c49d827528..f67ae5e75fe 100644 --- a/tooling/nargo_fmt/src/rewrite/array.rs +++ b/tooling/nargo_fmt/src/rewrite/array.rs @@ -26,7 +26,7 @@ pub(crate) fn rewrite(mut visitor: FmtVisitor, array: Vec, array_spa let end = item_span.start(); let leading = visitor.slice(start..end).trim_matches(pattern); - let item = visitor.format_sub_expr(item); + let item = super::sub_expr(&visitor, visitor.shape(), item); let next_start = items.peek().map_or(end_position, |expr| expr.span.start()); let trailing = visitor.slice(item_span.end()..next_start); let offset = trailing diff --git a/tooling/nargo_fmt/src/rewrite/expr.rs b/tooling/nargo_fmt/src/rewrite/expr.rs new file mode 100644 index 00000000000..4d7279815df --- /dev/null +++ b/tooling/nargo_fmt/src/rewrite/expr.rs @@ -0,0 +1,144 @@ +use noirc_frontend::{token::Token, ArrayLiteral, Expression, ExpressionKind, Literal, UnaryOp}; + +use crate::visitor::{ + expr::{format_brackets, format_parens}, + ExpressionType, FmtVisitor, Indent, Shape, +}; + +pub(crate) fn rewrite_sub_expr( + visitor: &FmtVisitor, + shape: Shape, + expression: Expression, +) -> String { + rewrite(visitor, expression, ExpressionType::SubExpression, shape) +} + +pub(crate) fn rewrite( + visitor: &FmtVisitor, + Expression { kind, span }: Expression, + expr_type: ExpressionType, + shape: Shape, +) -> String { + match kind { + ExpressionKind::Block(block) => { + let mut visitor = visitor.fork(); + visitor.visit_block(block, span); + visitor.finish() + } + ExpressionKind::Prefix(prefix) => { + let op = match prefix.operator { + UnaryOp::Minus => "-", + UnaryOp::Not => "!", + UnaryOp::MutableReference => "&mut ", + UnaryOp::Dereference { implicitly_added } => { + if implicitly_added { + "" + } else { + "*" + } + } + }; + + format!("{op}{}", rewrite_sub_expr(visitor, shape, prefix.rhs)) + } + ExpressionKind::Cast(cast) => { + format!("{} as {}", rewrite_sub_expr(visitor, shape, cast.lhs), cast.r#type) + } + kind @ ExpressionKind::Infix(_) => { + super::infix(visitor.fork(), Expression { kind, span }, shape) + } + ExpressionKind::Call(call_expr) => { + let args_span = + visitor.span_before(call_expr.func.span.end()..span.end(), Token::LeftParen); + + let callee = rewrite_sub_expr(visitor, shape, *call_expr.func); + let args = format_parens( + visitor.config.fn_call_width.into(), + visitor.fork(), + shape, + false, + call_expr.arguments, + args_span, + true, + ); + + format!("{callee}{args}") + } + ExpressionKind::MethodCall(method_call_expr) => { + let args_span = visitor.span_before( + method_call_expr.method_name.span().end()..span.end(), + Token::LeftParen, + ); + + let object = rewrite_sub_expr(visitor, shape, method_call_expr.object); + let method = method_call_expr.method_name.to_string(); + let args = format_parens( + visitor.config.fn_call_width.into(), + visitor.fork(), + shape, + false, + method_call_expr.arguments, + args_span, + true, + ); + + format!("{object}.{method}{args}") + } + ExpressionKind::MemberAccess(member_access_expr) => { + let lhs_str = rewrite_sub_expr(visitor, shape, member_access_expr.lhs); + format!("{}.{}", lhs_str, member_access_expr.rhs) + } + ExpressionKind::Index(index_expr) => { + let index_span = visitor + .span_before(index_expr.collection.span.end()..span.end(), Token::LeftBracket); + + let collection = rewrite_sub_expr(visitor, shape, index_expr.collection); + let index = format_brackets(visitor.fork(), false, vec![index_expr.index], index_span); + + format!("{collection}{index}") + } + ExpressionKind::Tuple(exprs) => { + format_parens(None, visitor.fork(), shape, exprs.len() == 1, exprs, span, false) + } + ExpressionKind::Literal(literal) => match literal { + Literal::Integer(_) | Literal::Bool(_) | Literal::Str(_) | Literal::FmtStr(_) => { + visitor.slice(span).to_string() + } + Literal::Array(ArrayLiteral::Repeated { repeated_element, length }) => { + let repeated = rewrite_sub_expr(visitor, shape, *repeated_element); + let length = rewrite_sub_expr(visitor, shape, *length); + + format!("[{repeated}; {length}]") + } + Literal::Array(ArrayLiteral::Standard(exprs)) => { + super::array(visitor.fork(), exprs, span) + } + Literal::Unit => "()".to_string(), + }, + ExpressionKind::Parenthesized(sub_expr) => { + super::parenthesized(visitor, shape, span, *sub_expr) + } + ExpressionKind::Constructor(constructor) => { + let type_name = visitor.slice(span.start()..constructor.type_name.span().end()); + let fields_span = visitor + .span_before(constructor.type_name.span().end()..span.end(), Token::LeftBrace); + + visitor.format_struct_lit(type_name, fields_span, *constructor) + } + ExpressionKind::If(if_expr) => { + let allow_single_line = expr_type == ExpressionType::SubExpression; + + if allow_single_line { + let mut visitor = visitor.fork(); + visitor.indent = Indent::default(); + if let Some(line) = visitor.format_if_single_line(*if_expr.clone()) { + return line; + } + } + + visitor.format_if(*if_expr) + } + ExpressionKind::Lambda(_) | ExpressionKind::Variable(_) => visitor.slice(span).to_string(), + ExpressionKind::Error => unreachable!(), + } +} diff --git a/tooling/nargo_fmt/src/rewrite/infix.rs b/tooling/nargo_fmt/src/rewrite/infix.rs index 0fbfa07a841..15f5fe23aae 100644 --- a/tooling/nargo_fmt/src/rewrite/infix.rs +++ b/tooling/nargo_fmt/src/rewrite/infix.rs @@ -3,8 +3,9 @@ use std::iter::zip; use noirc_frontend::{Expression, ExpressionKind}; use crate::{ + rewrite, utils::{first_line_width, is_single_line}, - visitor::{ExpressionType, FmtVisitor, Shape}, + visitor::{FmtVisitor, Shape}, }; pub(crate) fn rewrite(visitor: FmtVisitor, expr: Expression, shape: Shape) -> String { @@ -16,9 +17,9 @@ pub(crate) fn rewrite(visitor: FmtVisitor, expr: Expression, shape: Shape) -> St format!( "{} {} {}", - visitor.format_sub_expr(infix.lhs), + rewrite::sub_expr(&visitor, shape, infix.lhs), infix.operator.contents.as_string(), - visitor.format_sub_expr(infix.rhs) + rewrite::sub_expr(&visitor, shape, infix.rhs) ) } } @@ -87,10 +88,10 @@ pub(crate) fn flatten( } _ => { let rewrite = if result.is_empty() { - visitor.format_expr(node.clone(), ExpressionType::SubExpression) + rewrite::sub_expr(&visitor, visitor.shape(), node.clone()) } else { visitor.indent.block_indent(visitor.config); - visitor.format_expr(node.clone(), ExpressionType::SubExpression) + rewrite::sub_expr(&visitor, visitor.shape(), node.clone()) }; result.push(rewrite); diff --git a/tooling/nargo_fmt/src/rewrite/parenthesized.rs b/tooling/nargo_fmt/src/rewrite/parenthesized.rs new file mode 100644 index 00000000000..3926b52cb73 --- /dev/null +++ b/tooling/nargo_fmt/src/rewrite/parenthesized.rs @@ -0,0 +1,67 @@ +use noirc_frontend::{hir::resolution::errors::Span, Expression, ExpressionKind}; + +use crate::visitor::{FmtVisitor, Shape}; + +pub(crate) fn rewrite( + visitor: &FmtVisitor<'_>, + shape: Shape, + mut span: Span, + mut sub_expr: Expression, +) -> String { + let remove_nested_parens = visitor.config.remove_nested_parens; + + let mut leading; + let mut trailing; + + loop { + let leading_span = span.start() + 1..sub_expr.span.start(); + let trailing_span = sub_expr.span.end()..span.end() - 1; + + leading = visitor.format_comment(leading_span.into()); + trailing = visitor.format_comment(trailing_span.into()); + + if let ExpressionKind::Parenthesized(ref sub_sub_expr) = sub_expr.kind { + if remove_nested_parens && leading.is_empty() && trailing.is_empty() { + span = sub_expr.span; + sub_expr = *sub_sub_expr.clone(); + continue; + } + } + + break; + } + + if !leading.contains("//") && !trailing.contains("//") { + let sub_expr = super::sub_expr(visitor, shape, sub_expr); + format!("({leading}{sub_expr}{trailing})") + } else { + let mut visitor = visitor.fork(); + + let indent = visitor.indent.to_string_with_newline(); + visitor.indent.block_indent(visitor.config); + let nested_indent = visitor.indent.to_string_with_newline(); + + let sub_expr = super::sub_expr(&visitor, shape, sub_expr); + + let mut result = String::new(); + result.push('('); + + if !leading.is_empty() { + result.push_str(&nested_indent); + result.push_str(&leading); + } + + result.push_str(&nested_indent); + result.push_str(&sub_expr); + + if !trailing.is_empty() { + result.push_str(&nested_indent); + result.push_str(&trailing); + } + + result.push_str(&indent); + result.push(')'); + + result + } +} diff --git a/tooling/nargo_fmt/src/utils.rs b/tooling/nargo_fmt/src/utils.rs index 13938079e68..626795959a3 100644 --- a/tooling/nargo_fmt/src/utils.rs +++ b/tooling/nargo_fmt/src/utils.rs @@ -1,4 +1,5 @@ -use crate::visitor::FmtVisitor; +use crate::rewrite; +use crate::visitor::{FmtVisitor, Shape}; use noirc_frontend::hir::resolution::errors::Span; use noirc_frontend::lexer::Lexer; use noirc_frontend::token::Token; @@ -40,15 +41,22 @@ impl Expr { pub(crate) struct Exprs<'me, T> { pub(crate) visitor: &'me FmtVisitor<'me>, + shape: Shape, pub(crate) elements: std::iter::Peekable>, pub(crate) last_position: u32, pub(crate) end_position: u32, } impl<'me, T: Item> Exprs<'me, T> { - pub(crate) fn new(visitor: &'me FmtVisitor<'me>, span: Span, elements: Vec) -> Self { + pub(crate) fn new( + visitor: &'me FmtVisitor<'me>, + shape: Shape, + span: Span, + elements: Vec, + ) -> Self { Self { visitor, + shape, last_position: span.start() + 1, /*(*/ end_position: span.end() - 1, /*)*/ elements: elements.into_iter().peekable(), @@ -70,7 +78,7 @@ impl Iterator for Exprs<'_, T> { let next_start = self.elements.peek().map_or(self.end_position, |expr| expr.start()); let (leading, different_line) = self.leading(start, end); - let expr = element.format(self.visitor); + let expr = element.format(self.visitor, self.shape); let trailing = self.trailing(element_span.end(), next_start, is_last); Expr { leading, value: expr, trailing, different_line }.into() @@ -196,7 +204,7 @@ pub(crate) fn count_newlines(slice: &str) -> usize { pub(crate) trait Item { fn span(&self) -> Span; - fn format(self, visitor: &FmtVisitor) -> String; + fn format(self, visitor: &FmtVisitor, shape: Shape) -> String; fn start(&self) -> u32 { self.span().start() @@ -212,8 +220,8 @@ impl Item for Expression { self.span } - fn format(self, visitor: &FmtVisitor) -> String { - visitor.format_sub_expr(self) + fn format(self, visitor: &FmtVisitor, shape: Shape) -> String { + rewrite::sub_expr(visitor, shape, self) } } @@ -223,11 +231,11 @@ impl Item for (Ident, Expression) { (name.span().start()..value.span.end()).into() } - fn format(self, visitor: &FmtVisitor) -> String { + fn format(self, visitor: &FmtVisitor, shape: Shape) -> String { let (name, expr) = self; let name = name.0.contents; - let expr = visitor.format_sub_expr(expr); + let expr = rewrite::sub_expr(visitor, shape, expr); if name == expr { name @@ -242,7 +250,7 @@ impl Item for Param { self.span } - fn format(self, visitor: &FmtVisitor) -> String { + fn format(self, visitor: &FmtVisitor, _shape: Shape) -> String { let visibility = match self.visibility { Visibility::Public => "pub ", Visibility::Private => "", @@ -259,7 +267,7 @@ impl Item for Ident { self.span() } - fn format(self, visitor: &FmtVisitor) -> String { + fn format(self, visitor: &FmtVisitor, _shape: Shape) -> String { visitor.slice(self.span()).into() } } @@ -268,6 +276,10 @@ pub(crate) fn first_line_width(exprs: &str) -> usize { exprs.lines().next().map_or(0, |line: &str| line.chars().count()) } +pub(crate) fn last_line_width(s: &str) -> usize { + s.rsplit('\n').next().unwrap_or("").chars().count() +} + pub(crate) fn is_single_line(s: &str) -> bool { !s.chars().any(|c| c == '\n') } @@ -275,3 +287,11 @@ pub(crate) fn is_single_line(s: &str) -> bool { pub(crate) fn last_line_contains_single_line_comment(s: &str) -> bool { s.lines().last().map_or(false, |line| line.contains("//")) } + +pub(crate) fn last_line_used_width(s: &str, offset: usize) -> usize { + if s.contains('\n') { + last_line_width(s) + } else { + offset + s.chars().count() + } +} diff --git a/tooling/nargo_fmt/src/visitor.rs b/tooling/nargo_fmt/src/visitor.rs index cf3b3a41e8a..85989db79d8 100644 --- a/tooling/nargo_fmt/src/visitor.rs +++ b/tooling/nargo_fmt/src/visitor.rs @@ -30,12 +30,16 @@ impl<'me> FmtVisitor<'me> { } } + pub(crate) fn budget(&self, used_width: usize) -> usize { + self.config.max_width.saturating_sub(used_width) + } + pub(crate) fn slice(&self, span: impl Into) -> &'me str { let span = span.into(); &self.source[span.start() as usize..span.end() as usize] } - fn span_after(&self, span: impl Into, token: Token) -> Span { + pub(crate) fn span_after(&self, span: impl Into, token: Token) -> Span { let span = span.into(); let slice = self.slice(span); @@ -44,7 +48,7 @@ impl<'me> FmtVisitor<'me> { (span.start() + offset..span.end()).into() } - fn span_before(&self, span: impl Into, token: Token) -> Span { + pub(crate) fn span_before(&self, span: impl Into, token: Token) -> Span { let span = span.into(); let slice = self.slice(span); @@ -253,10 +257,12 @@ impl Indent { self.block_indent } + #[track_caller] pub(crate) fn block_indent(&mut self, config: &Config) { self.block_indent += config.tab_spaces; } + #[track_caller] pub(crate) fn block_unindent(&mut self, config: &Config) { self.block_indent -= config.tab_spaces; } diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index 8492fd5c05d..a5e5a1c7846 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -1,10 +1,9 @@ use noirc_frontend::{ - hir::resolution::errors::Span, lexer::Lexer, token::Token, ArrayLiteral, BlockExpression, - ConstructorExpression, Expression, ExpressionKind, IfExpression, Literal, Statement, - StatementKind, UnaryOp, + hir::resolution::errors::Span, lexer::Lexer, token::Token, BlockExpression, + ConstructorExpression, Expression, ExpressionKind, IfExpression, Statement, StatementKind, }; -use super::{ExpressionType, FmtVisitor, Indent, Shape}; +use super::{ExpressionType, FmtVisitor, Shape}; use crate::{ rewrite, utils::{self, first_line_width, Expr, FindToken, Item}, @@ -14,199 +13,14 @@ use crate::{ impl FmtVisitor<'_> { pub(crate) fn visit_expr(&mut self, expr: Expression, expr_type: ExpressionType) { let span = expr.span; - let rewrite = self.format_expr(expr, expr_type); + let rewrite = rewrite::expr(self, expr, expr_type, self.shape()); self.push_rewrite(rewrite, span); self.last_position = span.end(); } - pub(crate) fn format_sub_expr(&self, expression: Expression) -> String { - self.format_expr(expression, ExpressionType::SubExpression) - } - - pub(crate) fn format_expr( - &self, - Expression { kind, mut span }: Expression, - expr_type: ExpressionType, - ) -> String { - match kind { - ExpressionKind::Block(block) => { - let mut visitor = self.fork(); - visitor.visit_block(block, span); - visitor.buffer - } - ExpressionKind::Prefix(prefix) => { - let op = match prefix.operator { - UnaryOp::Minus => "-", - UnaryOp::Not => "!", - UnaryOp::MutableReference => "&mut ", - UnaryOp::Dereference { implicitly_added } => { - if implicitly_added { - "" - } else { - "*" - } - } - }; - - format!("{op}{}", self.format_sub_expr(prefix.rhs)) - } - ExpressionKind::Cast(cast) => { - format!("{} as {}", self.format_sub_expr(cast.lhs), cast.r#type) - } - kind @ ExpressionKind::Infix(_) => { - let shape = self.shape(); - rewrite::infix(self.fork(), Expression { kind, span }, shape) - } - ExpressionKind::Call(call_expr) => { - let args_span = - self.span_before(call_expr.func.span.end()..span.end(), Token::LeftParen); - - let callee = self.format_sub_expr(*call_expr.func); - let args = format_parens( - self.config.fn_call_width.into(), - self.fork(), - false, - call_expr.arguments, - args_span, - ); - - format!("{callee}{args}") - } - ExpressionKind::MethodCall(method_call_expr) => { - let args_span = self.span_before( - method_call_expr.method_name.span().end()..span.end(), - Token::LeftParen, - ); - - let object = self.format_sub_expr(method_call_expr.object); - let method = method_call_expr.method_name.to_string(); - let args = format_parens( - self.config.fn_call_width.into(), - self.fork(), - false, - method_call_expr.arguments, - args_span, - ); - - format!("{object}.{method}{args}") - } - ExpressionKind::MemberAccess(member_access_expr) => { - let lhs_str = self.format_sub_expr(member_access_expr.lhs); - format!("{}.{}", lhs_str, member_access_expr.rhs) - } - ExpressionKind::Index(index_expr) => { - let index_span = self - .span_before(index_expr.collection.span.end()..span.end(), Token::LeftBracket); - - let collection = self.format_sub_expr(index_expr.collection); - let index = format_brackets(self.fork(), false, vec![index_expr.index], index_span); - - format!("{collection}{index}") - } - ExpressionKind::Tuple(exprs) => { - format_parens(None, self.fork(), exprs.len() == 1, exprs, span) - } - ExpressionKind::Literal(literal) => match literal { - Literal::Integer(_) | Literal::Bool(_) | Literal::Str(_) | Literal::FmtStr(_) => { - self.slice(span).to_string() - } - Literal::Array(ArrayLiteral::Repeated { repeated_element, length }) => { - let repeated = self.format_sub_expr(*repeated_element); - let length = self.format_sub_expr(*length); - - format!("[{repeated}; {length}]") - } - Literal::Array(ArrayLiteral::Standard(exprs)) => { - rewrite::array(self.fork(), exprs, span) - } - Literal::Unit => "()".to_string(), - }, - ExpressionKind::Parenthesized(mut sub_expr) => { - let remove_nested_parens = self.config.remove_nested_parens; - - let mut leading; - let mut trailing; - - loop { - let leading_span = span.start() + 1..sub_expr.span.start(); - let trailing_span = sub_expr.span.end()..span.end() - 1; - - leading = self.format_comment(leading_span.into()); - trailing = self.format_comment(trailing_span.into()); - - if let ExpressionKind::Parenthesized(ref sub_sub_expr) = sub_expr.kind { - if remove_nested_parens && leading.is_empty() && trailing.is_empty() { - span = sub_expr.span; - sub_expr = sub_sub_expr.clone(); - continue; - } - } - - break; - } - - if !leading.contains("//") && !trailing.contains("//") { - let sub_expr = self.format_sub_expr(*sub_expr); - format!("({leading}{sub_expr}{trailing})") - } else { - let mut visitor = self.fork(); - - let indent = visitor.indent.to_string_with_newline(); - visitor.indent.block_indent(self.config); - let nested_indent = visitor.indent.to_string_with_newline(); - - let sub_expr = visitor.format_sub_expr(*sub_expr); - - let mut result = String::new(); - result.push('('); - - if !leading.is_empty() { - result.push_str(&nested_indent); - result.push_str(&leading); - } - - result.push_str(&nested_indent); - result.push_str(&sub_expr); - - if !trailing.is_empty() { - result.push_str(&nested_indent); - result.push_str(&trailing); - } - - result.push_str(&indent); - result.push(')'); - - result - } - } - ExpressionKind::Constructor(constructor) => { - let type_name = self.slice(span.start()..constructor.type_name.span().end()); - let fields_span = self - .span_before(constructor.type_name.span().end()..span.end(), Token::LeftBrace); - - self.format_struct_lit(type_name, fields_span, *constructor) - } - ExpressionKind::If(if_expr) => { - let allow_single_line = expr_type == ExpressionType::SubExpression; - - if allow_single_line { - let mut visitor = self.fork(); - visitor.indent = Indent::default(); - if let Some(line) = visitor.format_if_single_line(*if_expr.clone()) { - return line; - } - } - - self.format_if(*if_expr) - } - ExpressionKind::Lambda(_) | ExpressionKind::Variable(_) => self.slice(span).to_string(), - ExpressionKind::Error => unreachable!(), - } - } - - fn format_if(&self, if_expr: IfExpression) -> String { - let condition_str = self.format_sub_expr(if_expr.condition); - let consequence_str = self.format_sub_expr(if_expr.consequence); + pub(crate) fn format_if(&self, if_expr: IfExpression) -> String { + let condition_str = rewrite::sub_expr(self, self.shape(), if_expr.condition); + let consequence_str = rewrite::sub_expr(self, self.shape(), if_expr.consequence); let mut result = format!("if {condition_str} {consequence_str}"); @@ -216,7 +30,7 @@ impl FmtVisitor<'_> { { self.format_if(*if_expr) } else { - self.format_expr(alternative, ExpressionType::Statement) + rewrite::expr(self, alternative, ExpressionType::Statement, self.shape()) }; result.push_str(" else "); @@ -226,9 +40,10 @@ impl FmtVisitor<'_> { result } - fn format_if_single_line(&self, if_expr: IfExpression) -> Option { - let condition_str = self.format_sub_expr(if_expr.condition); - let consequence_str = self.format_sub_expr(extract_simple_expr(if_expr.consequence)?); + pub(crate) fn format_if_single_line(&self, if_expr: IfExpression) -> Option { + let condition_str = rewrite::sub_expr(self, self.shape(), if_expr.condition); + let consequence_str = + rewrite::sub_expr(self, self.shape(), extract_simple_expr(if_expr.consequence)?); let if_str = if let Some(alternative) = if_expr.alternative { let alternative_str = if let Some(ExpressionKind::If(_)) = @@ -236,7 +51,12 @@ impl FmtVisitor<'_> { { return None; } else { - self.format_expr(extract_simple_expr(alternative)?, ExpressionType::Statement) + rewrite::expr( + self, + extract_simple_expr(alternative)?, + ExpressionType::Statement, + self.shape(), + ) }; format!("if {} {{ {} }} else {{ {} }}", condition_str, consequence_str, alternative_str) @@ -247,7 +67,7 @@ impl FmtVisitor<'_> { (if_str.len() <= self.config.single_line_if_else_max_width).then_some(if_str) } - fn format_struct_lit( + pub(crate) fn format_struct_lit( &self, type_name: &str, fields_span: Span, @@ -261,7 +81,8 @@ impl FmtVisitor<'_> { let nested_indent = visitor.shape(); let exprs: Vec<_> = - utils::Exprs::new(&visitor, fields_span, constructor.fields).collect(); + utils::Exprs::new(&visitor, nested_indent, fields_span, constructor.fields) + .collect(); let exprs = format_exprs( visitor.config, Tactic::HorizontalVertical, @@ -367,27 +188,28 @@ impl FmtVisitor<'_> { #[allow(clippy::too_many_arguments)] pub(crate) fn format_seq( + shape: Shape, prefix: &str, suffix: &str, - mut visitor: FmtVisitor, + visitor: FmtVisitor, trailing_comma: bool, exprs: Vec, span: Span, tactic: Tactic, soft_newline: bool, ) -> String { - visitor.indent.block_indent(visitor.config); + let mut nested_indent = shape; + let shape = shape; - let nested_indent = visitor.shape(); - let exprs: Vec<_> = utils::Exprs::new(&visitor, span, exprs).collect(); - let exprs = format_exprs(visitor.config, tactic, trailing_comma, exprs, nested_indent); + nested_indent.indent.block_indent(visitor.config); - visitor.indent.block_unindent(visitor.config); + let exprs: Vec<_> = utils::Exprs::new(&visitor, nested_indent, span, exprs).collect(); + let exprs = format_exprs(visitor.config, tactic, trailing_comma, exprs, nested_indent); - wrap_exprs(prefix, suffix, exprs, nested_indent, visitor.shape(), soft_newline) + wrap_exprs(prefix, suffix, exprs, nested_indent, shape, soft_newline) } -fn format_brackets( +pub(crate) fn format_brackets( visitor: FmtVisitor, trailing_comma: bool, exprs: Vec, @@ -395,6 +217,7 @@ fn format_brackets( ) -> String { let array_width = visitor.config.array_width; format_seq( + visitor.shape(), "[", "]", visitor, @@ -406,15 +229,17 @@ fn format_brackets( ) } -fn format_parens( +pub(crate) fn format_parens( max_width: Option, visitor: FmtVisitor, + shape: Shape, trailing_comma: bool, exprs: Vec, span: Span, + soft_newline: bool, ) -> String { let tactic = max_width.map(Tactic::LimitedHorizontalVertical).unwrap_or(Tactic::Horizontal); - format_seq("(", ")", visitor, trailing_comma, exprs, span, tactic, false) + format_seq(shape, "(", ")", visitor, trailing_comma, exprs, span, tactic, soft_newline) } fn format_exprs( @@ -508,10 +333,10 @@ pub(crate) fn wrap_exprs( shape: Shape, soft_newline: bool, ) -> String { - let first_line_width = first_line_width(&exprs); - - let force_one_line = - if soft_newline { !exprs.contains('\n') } else { first_line_width <= shape.width }; + let mut force_one_line = first_line_width(&exprs) <= shape.width; + if soft_newline && force_one_line { + force_one_line = !exprs.contains('\n'); + } if force_one_line { let allow_trailing_newline = exprs diff --git a/tooling/nargo_fmt/src/visitor/item.rs b/tooling/nargo_fmt/src/visitor/item.rs index 7774618afea..af375515413 100644 --- a/tooling/nargo_fmt/src/visitor/item.rs +++ b/tooling/nargo_fmt/src/visitor/item.rs @@ -1,17 +1,22 @@ use noirc_frontend::{ + hir::resolution::errors::Span, parser::{Item, ItemKind}, - token::Token, + token::{Keyword, Token}, Distinctness, NoirFunction, ParsedModule, Visibility, }; -use crate::{utils::last_line_contains_single_line_comment, visitor::expr::format_seq}; +use crate::{ + utils::{last_line_contains_single_line_comment, last_line_used_width, FindToken}, + visitor::expr::format_seq, +}; -use super::expr::Tactic::LimitedHorizontalVertical; +use super::{ + expr::Tactic::{HorizontalVertical, LimitedHorizontalVertical}, + Shape, +}; impl super::FmtVisitor<'_> { fn format_fn_before_block(&self, func: NoirFunction, start: u32) -> (String, bool) { - let tactic = LimitedHorizontalVertical(self.config.max_width); - let name_span = func.name_ident().span(); let func_span = func.span(); @@ -30,6 +35,7 @@ impl super::FmtVisitor<'_> { let params_span = params_open..params_end; let return_type_span = func.return_type().span; + let return_type = self.format_return_type(return_type_span, &func, func_span, params_end); let parameters = func.def.parameters; if !func.def.generics.is_empty() { @@ -39,7 +45,17 @@ impl super::FmtVisitor<'_> { let generics = func.def.generics; let span = (start..end).into(); - let generics = format_seq("<", ">", self.fork(), false, generics, span, tactic, false); + let generics = format_seq( + self.shape(), + "<", + ">", + self.fork(), + false, + generics, + span, + HorizontalVertical, + false, + ); result.push_str(&generics); } @@ -47,10 +63,45 @@ impl super::FmtVisitor<'_> { let parameters = if parameters.is_empty() { self.slice(params_span).into() } else { - format_seq("(", ")", self.fork(), false, parameters, params_span.into(), tactic, true) + let fn_start = result.find_token(Token::Keyword(Keyword::Fn)).unwrap().start(); + let slice = self.slice(fn_start..result.len() as u32); + + let indent = self.indent; + let used_width = last_line_used_width(slice, indent.width()); + let one_line_budget = self.budget(used_width + return_type.len()); + let shape = Shape { width: one_line_budget, indent }; + + let tactic = LimitedHorizontalVertical(one_line_budget); + + format_seq( + shape, + "(", + ")", + self.fork(), + false, + parameters, + params_span.into(), + tactic, + true, + ) }; result.push_str(¶meters); + result.push_str(&return_type); + + let maybe_comment = self.slice(params_end..func_span.start()); + + (result.trim_end().to_string(), last_line_contains_single_line_comment(maybe_comment)) + } + + fn format_return_type( + &self, + return_type_span: Option, + func: &NoirFunction, + func_span: Span, + params_end: u32, + ) -> String { + let mut result = String::new(); if let Some(span) = return_type_span { result.push_str(" -> "); @@ -73,9 +124,7 @@ impl super::FmtVisitor<'_> { result.push_str(self.slice(params_end..func_span.start())); } - let maybe_comment = self.slice(params_end..func_span.start()); - - (result.trim_end().to_string(), last_line_contains_single_line_comment(maybe_comment)) + result } pub(crate) fn visit_file(&mut self, module: ParsedModule) { diff --git a/tooling/nargo_fmt/src/visitor/stmt.rs b/tooling/nargo_fmt/src/visitor/stmt.rs index b6dd67323fa..c27b7911d03 100644 --- a/tooling/nargo_fmt/src/visitor/stmt.rs +++ b/tooling/nargo_fmt/src/visitor/stmt.rs @@ -4,6 +4,8 @@ use noirc_frontend::{ ConstrainKind, ConstrainStatement, ExpressionKind, ForRange, Statement, StatementKind, }; +use crate::rewrite; + use super::ExpressionType; impl super::FmtVisitor<'_> { @@ -25,8 +27,8 @@ impl super::FmtVisitor<'_> { StatementKind::Let(let_stmt) => { let let_str = self.slice(span.start()..let_stmt.expression.span.start()).trim_end(); - let expr_str = - self.format_expr(let_stmt.expression, ExpressionType::SubExpression); + + let expr_str = rewrite::sub_expr(self, self.shape(), let_stmt.expression); self.push_rewrite(format!("{let_str} {expr_str};"), span); } @@ -35,14 +37,14 @@ impl super::FmtVisitor<'_> { message.map_or(String::new(), |message| format!(", \"{message}\"")); let constrain = match kind { ConstrainKind::Assert => { - let assertion = self.format_sub_expr(expr); + let assertion = rewrite::sub_expr(self, self.shape(), expr); format!("assert({assertion}{message});") } ConstrainKind::AssertEq => { if let ExpressionKind::Infix(infix) = expr.kind { - let lhs = self.format_sub_expr(infix.lhs); - let rhs = self.format_sub_expr(infix.rhs); + let lhs = rewrite::sub_expr(self, self.shape(), infix.lhs); + let rhs = rewrite::sub_expr(self, self.shape(), infix.rhs); format!("assert_eq({lhs}, {rhs}{message});") } else { @@ -50,7 +52,7 @@ impl super::FmtVisitor<'_> { } } ConstrainKind::Constrain => { - let expr = self.format_sub_expr(expr); + let expr = rewrite::sub_expr(self, self.shape(), expr); format!("constrain {expr};") } }; @@ -62,12 +64,12 @@ impl super::FmtVisitor<'_> { let range = match for_stmt.range { ForRange::Range(start, end) => format!( "{}..{}", - self.format_sub_expr(start), - self.format_sub_expr(end) + rewrite::sub_expr(self, self.shape(), start), + rewrite::sub_expr(self, self.shape(), end) ), - ForRange::Array(array) => self.format_sub_expr(array), + ForRange::Array(array) => rewrite::sub_expr(self, self.shape(), array), }; - let block = self.format_sub_expr(for_stmt.block); + let block = rewrite::sub_expr(self, self.shape(), for_stmt.block); let result = format!("for {identifier} in {range} {block}"); self.push_rewrite(result, span); diff --git a/tooling/nargo_fmt/tests/expected/call.nr b/tooling/nargo_fmt/tests/expected/call.nr index 3c9b36e8403..7824ba37089 100644 --- a/tooling/nargo_fmt/tests/expected/call.nr +++ b/tooling/nargo_fmt/tests/expected/call.nr @@ -1,25 +1,33 @@ fn foo() { my_function(10, some_value, another_func(20, 30)); - outer_function(some_function(), // Original inner function call + outer_function( + some_function(), // Original inner function call another_function() // Original inner function call ); - outer_function(some_function(), // Original inner function call + outer_function( + some_function(), // Original inner function call another_function() // Original inner function call ); - my_function(// Comment + my_function( + // Comment some_value, /* Multiline Comment */ - another_func(20, 30)); + another_func(20, 30) + ); - my_function(some_function(10, "arg1", another_function()), - another_func(20, some_function(), 30)); + my_function( + some_function(10, "arg1", another_function()), + another_func(20, some_function(), 30) + ); - outer_function(some_function(), - another_function(some_function(), some_value)); + outer_function( + some_function(), + another_function(some_function(), some_value) + ); assert_eq(x, y); @@ -31,6 +39,10 @@ fn foo() { assert(x == y); - assert(p4_affine.eq(Gaffine::new(6890855772600357754907169075114257697580319025794532037257385534741338397365, - 4338620300185947561074059802482547481416142213883829469920100239455078257889))); + assert(p4_affine.eq( + Gaffine::new( + 6890855772600357754907169075114257697580319025794532037257385534741338397365, + 4338620300185947561074059802482547481416142213883829469920100239455078257889 + ) + )); } diff --git a/tooling/nargo_fmt/tests/expected/contract.nr b/tooling/nargo_fmt/tests/expected/contract.nr index 0d9bd2f100d..d288b1af7eb 100644 --- a/tooling/nargo_fmt/tests/expected/contract.nr +++ b/tooling/nargo_fmt/tests/expected/contract.nr @@ -57,9 +57,11 @@ contract Benchmarking { fn increment_balance(owner: Field, value: Field) { let current = storage.balances.at(owner).read(); storage.balances.at(owner).write(current + value); - let _callStackItem1 = context.call_public_function(context.this_address(), + let _callStackItem1 = context.call_public_function( + context.this_address(), compute_selector("broadcast(Field)"), - [owner]); + [owner] + ); } // Est ultricies integer quis auctor elit sed. In nibh mauris cursus mattis molestie a iaculis. @@ -68,7 +70,12 @@ contract Benchmarking { emit_unencrypted_log(&mut context, storage.balances.at(owner).read()); } - unconstrained fn compute_note_hash_and_nullifier(contract_address: Field, nonce: Field, storage_slot: Field, preimage: [Field; VALUE_NOTE_LEN]) -> [Field; 4] { + unconstrained fn compute_note_hash_and_nullifier( + contract_address: Field, + nonce: Field, + storage_slot: Field, + preimage: [Field; VALUE_NOTE_LEN] + ) -> [Field; 4] { let note_header = NoteHeader::new(contract_address, nonce, storage_slot); note_utils::compute_note_hash_and_nullifier(ValueNoteMethods, note_header, preimage) } diff --git a/tooling/nargo_fmt/tests/expected/fn.nr b/tooling/nargo_fmt/tests/expected/fn.nr index 5bca2dd8bb1..0e61483398c 100644 --- a/tooling/nargo_fmt/tests/expected/fn.nr +++ b/tooling/nargo_fmt/tests/expected/fn.nr @@ -28,6 +28,12 @@ fn main( initial_call_stack_pointer: u64 ) -> pub ExecutionResult {} -fn apply_binary_field_op(lhs: RegisterIndex, rhs: RegisterIndex, result: RegisterIndex, op: u8, registers: &mut Registers) -> bool {} +fn apply_binary_field_op( + lhs: RegisterIndex, + rhs: RegisterIndex, + result: RegisterIndex, + op: u8, + registers: &mut Registers +) -> bool {} fn main() -> distinct pub [Field;2] {} diff --git a/tooling/nargo_fmt/tests/expected/let.nr b/tooling/nargo_fmt/tests/expected/let.nr index b60ae644790..c57801155a0 100644 --- a/tooling/nargo_fmt/tests/expected/let.nr +++ b/tooling/nargo_fmt/tests/expected/let.nr @@ -1,7 +1,9 @@ //@error_on_lost_comment=false fn let_() { - let fn_call = my_function(some_function(10, "arg1", another_function()), - another_func(20, some_function(), 30)); + let fn_call = my_function( + some_function(10, "arg1", another_function()), + another_func(20, some_function(), 30) + ); let array = [[[1, 2, 3], [4, 5, 6]], [[7, 8, 9], [10, 11, 12]], [[13, 14, 15], [16, 17, 18]]]; let padded_sha256_hash: [u8; 259] = [ @@ -15,7 +17,11 @@ fn let_() { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 ]; - let a = BigUint56 { limbs: [1, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] }; + let a = BigUint56 { + limbs: [ + 1, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 + ] + }; let person = Person { first_name: "John", diff --git a/tooling/noir_codegen/package.json b/tooling/noir_codegen/package.json index 17772461dff..14f9bad6df6 100644 --- a/tooling/noir_codegen/package.json +++ b/tooling/noir_codegen/package.json @@ -3,7 +3,7 @@ "collaborators": [ "The Noir Team " ], - "version": "0.19.2", + "version": "0.19.3", "packageManager": "yarn@3.5.1", "license": "(MIT OR Apache-2.0)", "type": "module", diff --git a/tooling/noir_js/package.json b/tooling/noir_js/package.json index 51bc7550200..440bd8dec63 100644 --- a/tooling/noir_js/package.json +++ b/tooling/noir_js/package.json @@ -3,7 +3,7 @@ "collaborators": [ "The Noir Team " ], - "version": "0.19.2", + "version": "0.19.3", "packageManager": "yarn@3.5.1", "license": "(MIT OR Apache-2.0)", "type": "module", diff --git a/tooling/noir_js_backend_barretenberg/package.json b/tooling/noir_js_backend_barretenberg/package.json index 13c64dc1d65..360b3e70ec0 100644 --- a/tooling/noir_js_backend_barretenberg/package.json +++ b/tooling/noir_js_backend_barretenberg/package.json @@ -3,7 +3,7 @@ "collaborators": [ "The Noir Team " ], - "version": "0.19.2", + "version": "0.19.3", "packageManager": "yarn@3.5.1", "license": "(MIT OR Apache-2.0)", "type": "module", @@ -32,7 +32,7 @@ "lint": "NODE_NO_WARNINGS=1 eslint . --ext .ts --ignore-path ./.eslintignore --max-warnings 0" }, "dependencies": { - "@aztec/bb.js": "0.12.0", + "@aztec/bb.js": "0.15.1", "@noir-lang/types": "workspace:*", "fflate": "^0.8.0" }, diff --git a/tooling/noir_js_types/package.json b/tooling/noir_js_types/package.json index 3d74d335162..57bb2f050b7 100644 --- a/tooling/noir_js_types/package.json +++ b/tooling/noir_js_types/package.json @@ -4,7 +4,7 @@ "The Noir Team " ], "packageManager": "yarn@3.5.1", - "version": "0.19.2", + "version": "0.19.3", "license": "(MIT OR Apache-2.0)", "files": [ "lib", diff --git a/tooling/noirc_abi_wasm/package.json b/tooling/noirc_abi_wasm/package.json index d307a186a7d..d679ec37194 100644 --- a/tooling/noirc_abi_wasm/package.json +++ b/tooling/noirc_abi_wasm/package.json @@ -3,7 +3,7 @@ "collaborators": [ "The Noir Team " ], - "version": "0.19.2", + "version": "0.19.3", "license": "(MIT OR Apache-2.0)", "files": [ "nodejs", diff --git a/yarn.lock b/yarn.lock index 87713e1f915..9c6a447d718 100644 --- a/yarn.lock +++ b/yarn.lock @@ -221,9 +221,9 @@ __metadata: languageName: node linkType: hard -"@aztec/bb.js@npm:0.12.0": - version: 0.12.0 - resolution: "@aztec/bb.js@npm:0.12.0" +"@aztec/bb.js@npm:0.15.1": + version: 0.15.1 + resolution: "@aztec/bb.js@npm:0.15.1" dependencies: comlink: ^4.4.1 commander: ^10.0.1 @@ -231,7 +231,7 @@ __metadata: tslib: ^2.4.0 bin: bb.js: dest/node/main.js - checksum: d9d57b893b9b1c61949cb9bd911d4b7e1ece34965ccb9e122b39cd4e2edac9f06858abbe05c23f66141c9a74ff4861a354bfc5d62e5dcf772cfe8d1c783e2562 + checksum: b3d94eb6db1d1579fa7266486d4b1c6ddc408f1d36bd2585b50e623aa90222d273e56464284b94677979840c1119c5385aa961462d3a1af6cb9a2ba1cf9655f9 languageName: node linkType: hard @@ -3434,7 +3434,7 @@ __metadata: version: 0.0.0-use.local resolution: "@noir-lang/backend_barretenberg@workspace:tooling/noir_js_backend_barretenberg" dependencies: - "@aztec/bb.js": 0.12.0 + "@aztec/bb.js": 0.15.1 "@noir-lang/types": "workspace:*" "@types/node": ^20.6.2 "@types/prettier": ^3