diff --git a/.noir-sync-commit b/.noir-sync-commit index c253290bf18..f58ed005d42 100644 --- a/.noir-sync-commit +++ b/.noir-sync-commit @@ -1 +1 @@ -45344bfe1148a2f592c2e432744d3fb3d46340cc +44cf9a2140bc06b550d4b46966f1637598ac11a7 diff --git a/noir/noir-repo/.github/workflows/release.yml b/noir/noir-repo/.github/workflows/release.yml index d27fac0e039..5124592a3fe 100644 --- a/noir/noir-repo/.github/workflows/release.yml +++ b/noir/noir-repo/.github/workflows/release.yml @@ -54,8 +54,8 @@ jobs: - name: Configure git run: | - git config user.name kevaundray - git config user.email kevtheappdev@gmail.com + git config user.name noirwhal + git config user.email tomfrench@aztecprotocol.com - name: Commit updates run: | @@ -100,8 +100,8 @@ jobs: - name: Configure git run: | - git config --local user.name 'kevaundray' - git config --local user.email 'kevtheappdev@gmail.com' + git config --local user.name noirwhal + git config --local user.email tomfrench@aztecprotocol.com - name: Commit new documentation version run: | diff --git a/noir/noir-repo/Cargo.lock b/noir/noir-repo/Cargo.lock index cd936e4bca2..3f56f5b6965 100644 --- a/noir/noir-repo/Cargo.lock +++ b/noir/noir-repo/Cargo.lock @@ -7,7 +7,7 @@ name = "acir" version = "0.49.0" dependencies = [ "acir_field", - "base64 0.21.2", + "base64 0.21.7", "bincode", "brillig", "criterion", @@ -488,9 +488,9 @@ checksum = "9e1b586273c5702936fe7b7d6896644d8be71e6314cfe09d3167c95f712589e8" [[package]] name = "base64" -version = "0.21.2" +version = "0.21.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "604178f6c5c21f02dc555784810edfb88d34ac2c73b2eae109655649ee73ce3d" +checksum = "9d297deb1925b89f2ccc13d7635fa0714f12c87adce1c75356b39ca9b7178567" [[package]] name = "base64ct" @@ -665,7 +665,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6798148dccfbff0fae41c7574d2fa8f1ef3492fba0face179de5d8d447d67b05" dependencies = [ "memchr", - "regex-automata 0.3.3", + "regex-automata 0.3.9", "serde", ] @@ -771,9 +771,9 @@ checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" [[package]] name = "chrono" -version = "0.4.37" +version = "0.4.38" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a0d04d43504c61aa6c7531f1871dd0d418d91130162063b789da00fd7057a5e" +checksum = "a21f936df1771bf62b77f047b726c4625ff2e8aa607c01ec06e5a05bd8463401" dependencies = [ "android-tzdata", "iana-time-zone", @@ -781,7 +781,7 @@ dependencies = [ "num-traits", "serde", "wasm-bindgen", - "windows-targets 0.52.4", + "windows-targets 0.52.6", ] [[package]] @@ -1123,7 +1123,7 @@ dependencies = [ "autocfg", "cfg-if 1.0.0", "crossbeam-utils", - "memoffset 0.9.0", + "memoffset 0.9.1", "scopeguard", ] @@ -1454,12 +1454,12 @@ checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" [[package]] name = "errno" -version = "0.3.5" +version = "0.3.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ac3e13f66a2f95e32a39eaa81f6b95d42878ca0e1db0c7543723dfe12557e860" +checksum = "534c5cf6194dfab3db3242765c03bbe257cf92f22b38f6bc0c58d59108a820ba" dependencies = [ "libc", - "windows-sys 0.48.0", + "windows-sys 0.52.0", ] [[package]] @@ -2428,7 +2428,7 @@ version = "0.20.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "507460a910eb7b32ee961886ff48539633b788a36b65692b95f225b844c82553" dependencies = [ - "regex-automata 0.4.5", + "regex-automata 0.4.7", ] [[package]] @@ -2471,9 +2471,9 @@ dependencies = [ [[package]] name = "linux-raw-sys" -version = "0.4.3" +version = "0.4.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "09fc20d2ca12cb9f044c93e3bd6d32d523e6e2ec3db4f7b2939cd99026ecd3f0" +checksum = "78b3ae25bc7c8c38cec158d1f2757ee79e9b3740fbc7ccf0e59e4b08d793fa89" [[package]] name = "lock_api" @@ -2567,9 +2567,9 @@ dependencies = [ [[package]] name = "memoffset" -version = "0.9.0" +version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5a634b1c61a95585bd15607c6ab0c4e5b226e695ff2800ba0cdccddf208c406c" +checksum = "488016bfae457b036d996092f6cb448677611ce4449e970ceaf42695203f218a" dependencies = [ "autocfg", ] @@ -2582,9 +2582,9 @@ checksum = "2145869435ace5ea6ea3d35f59be559317ec9a0d04e1812d5f185a87b6d36f1a" [[package]] name = "miniz_oxide" -version = "0.7.1" +version = "0.7.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e7810e0be55b428ada41041c41f32c9f1a42817901b4ccf45fa3d4b6561e74c7" +checksum = "b8a240ddb74feaf34a79a7add65a741f3167852fba007066dcac1ca548d89c08" dependencies = [ "adler", ] @@ -2986,7 +2986,7 @@ name = "noirc_errors" version = "0.33.0" dependencies = [ "acvm", - "base64 0.21.2", + "base64 0.21.7", "chumsky", "codespan", "codespan-reporting", @@ -3027,7 +3027,7 @@ name = "noirc_frontend" version = "0.33.0" dependencies = [ "acvm", - "base64 0.21.2", + "base64 0.21.7", "bn254_blackbox_solver", "cfg-if 1.0.0", "chumsky", @@ -3272,7 +3272,7 @@ dependencies = [ "libc", "redox_syscall 0.3.5", "smallvec", - "windows-targets 0.48.1", + "windows-targets 0.48.5", ] [[package]] @@ -3743,9 +3743,9 @@ checksum = "977b1e897f9d764566891689e642653e5ed90c6895106acd005eb4c1d0203991" [[package]] name = "rayon" -version = "1.8.0" +version = "1.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c27db03db7734835b3f53954b534c91069375ce6ccaa2e065441e07d9b6cdb1" +checksum = "b418a60154510ca1a002a752ca9714984e21e4241e804d32555251faf8b78ffa" dependencies = [ "either", "rayon-core", @@ -3753,9 +3753,9 @@ dependencies = [ [[package]] name = "rayon-core" -version = "1.12.0" +version = "1.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5ce3fb6ad83f861aac485e76e1985cd109d9a3713802152be56c3b1f0e0658ed" +checksum = "1465873a3dfdaa8ae7cb14b4383657caab0b3e8a0aa9ae8e04b044854c8dfce2" dependencies = [ "crossbeam-deque", "crossbeam-utils", @@ -3813,7 +3813,7 @@ checksum = "b62dbe01f0b06f9d8dc7d49e05a0785f153b00b2c227856282f671e0318c9b15" dependencies = [ "aho-corasick", "memchr", - "regex-automata 0.4.5", + "regex-automata 0.4.7", "regex-syntax 0.8.2", ] @@ -3828,15 +3828,15 @@ dependencies = [ [[package]] name = "regex-automata" -version = "0.3.3" +version = "0.3.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "39354c10dd07468c2e73926b23bb9c2caca74c5501e38a35da70406f1d923310" +checksum = "59b23e92ee4318893fa3fe3e6fb365258efbfe6ac6ab30f090cdcbb7aa37efa9" [[package]] name = "regex-automata" -version = "0.4.5" +version = "0.4.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5bb987efffd3c6d0d8f5f89510bb458559eab11e4f869acb20bf845e016259cd" +checksum = "38caf58cc5ef2fed281f89292ef23f6365465ed9a41b7a7754eb4e26496c92df" dependencies = [ "aho-corasick", "memchr", @@ -3945,15 +3945,15 @@ dependencies = [ [[package]] name = "rustix" -version = "0.38.4" +version = "0.38.34" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0a962918ea88d644592894bc6dc55acc6c0956488adcebbfb6e273506b7fd6e5" +checksum = "70dc5ec042f7a43c4a73241207cecc9873a06d45debb38b329f8541d85c2730f" dependencies = [ "bitflags 2.5.0", "errno", "libc", "linux-raw-sys", - "windows-sys 0.48.0", + "windows-sys 0.52.0", ] [[package]] @@ -4113,9 +4113,9 @@ dependencies = [ [[package]] name = "serde" -version = "1.0.202" +version = "1.0.209" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "226b61a0d411b2ba5ff6d7f73a476ac4f8bb900373459cd00fab8512828ba395" +checksum = "99fce0ffe7310761ca6bf9faf5115afbc19688edd00171d81b1bb1b116c63e09" dependencies = [ "serde_derive", ] @@ -4156,9 +4156,9 @@ dependencies = [ [[package]] name = "serde_derive" -version = "1.0.202" +version = "1.0.209" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6048858004bcff69094cd972ed40a32500f153bd3be9f716b2eed2e8217c4838" +checksum = "a5831b979fd7b5439637af1752d535ff49f4860c0f341d1baeb6faf0f4242170" dependencies = [ "proc-macro2", "quote", @@ -4202,7 +4202,7 @@ version = "3.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1402f54f9a3b9e2efe71c1cea24e648acce55887983553eeb858cf3115acfd49" dependencies = [ - "base64 0.21.2", + "base64 0.21.7", "chrono", "hex", "indexmap 1.9.3", @@ -4670,9 +4670,9 @@ checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" [[package]] name = "tokio" -version = "1.36.0" +version = "1.38.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "61285f6515fa018fb2d1e46eb21223fff441ee8db5d0f1435e8ab4f5cdb80931" +checksum = "eb2caba9f80616f438e09748d5acda951967e1ea58508ef53d9c6402485a46df" dependencies = [ "backtrace", "bytes", @@ -4687,9 +4687,9 @@ dependencies = [ [[package]] name = "tokio-macros" -version = "2.2.0" +version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b8a1e28f2deaa14e508979454cb3a223b10b938b45af148bc0986de36f1923b" +checksum = "5f5ae998a069d4b5aba8ee9dad856af7d520c3699e6159b185c2acd48155d39a" dependencies = [ "proc-macro2", "quote", @@ -5215,7 +5215,7 @@ version = "0.50.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "af6041b3f84485c21b57acdc0fee4f4f0c93f426053dc05fa5d6fc262537bbff" dependencies = [ - "windows-targets 0.48.1", + "windows-targets 0.48.5", ] [[package]] @@ -5224,7 +5224,7 @@ version = "0.48.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "677d2418bec65e3338edb076e806bc1ec15693c5d0104683f2efe857f61056a9" dependencies = [ - "windows-targets 0.48.1", + "windows-targets 0.48.5", ] [[package]] @@ -5233,122 +5233,129 @@ version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d" dependencies = [ - "windows-targets 0.52.4", + "windows-targets 0.52.6", ] [[package]] name = "windows-targets" -version = "0.48.1" +version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "05d4b17490f70499f20b9e791dcf6a299785ce8af4d709018206dc5b4953e95f" +checksum = "9a2fa6e2155d7247be68c096456083145c183cbbbc2764150dda45a87197940c" dependencies = [ - "windows_aarch64_gnullvm 0.48.0", - "windows_aarch64_msvc 0.48.0", - "windows_i686_gnu 0.48.0", - "windows_i686_msvc 0.48.0", - "windows_x86_64_gnu 0.48.0", - "windows_x86_64_gnullvm 0.48.0", - "windows_x86_64_msvc 0.48.0", + "windows_aarch64_gnullvm 0.48.5", + "windows_aarch64_msvc 0.48.5", + "windows_i686_gnu 0.48.5", + "windows_i686_msvc 0.48.5", + "windows_x86_64_gnu 0.48.5", + "windows_x86_64_gnullvm 0.48.5", + "windows_x86_64_msvc 0.48.5", ] [[package]] name = "windows-targets" -version = "0.52.4" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7dd37b7e5ab9018759f893a1952c9420d060016fc19a472b4bb20d1bdd694d1b" +checksum = "9b724f72796e036ab90c1021d4780d4d3d648aca59e491e6b98e725b84e99973" dependencies = [ - "windows_aarch64_gnullvm 0.52.4", - "windows_aarch64_msvc 0.52.4", - "windows_i686_gnu 0.52.4", - "windows_i686_msvc 0.52.4", - "windows_x86_64_gnu 0.52.4", - "windows_x86_64_gnullvm 0.52.4", - "windows_x86_64_msvc 0.52.4", + "windows_aarch64_gnullvm 0.52.6", + "windows_aarch64_msvc 0.52.6", + "windows_i686_gnu 0.52.6", + "windows_i686_gnullvm", + "windows_i686_msvc 0.52.6", + "windows_x86_64_gnu 0.52.6", + "windows_x86_64_gnullvm 0.52.6", + "windows_x86_64_msvc 0.52.6", ] [[package]] name = "windows_aarch64_gnullvm" -version = "0.48.0" +version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "91ae572e1b79dba883e0d315474df7305d12f569b400fcf90581b06062f7e1bc" +checksum = "2b38e32f0abccf9987a4e3079dfb67dcd799fb61361e53e2882c3cbaf0d905d8" [[package]] name = "windows_aarch64_gnullvm" -version = "0.52.4" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bcf46cf4c365c6f2d1cc93ce535f2c8b244591df96ceee75d8e83deb70a9cac9" +checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" [[package]] name = "windows_aarch64_msvc" -version = "0.48.0" +version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b2ef27e0d7bdfcfc7b868b317c1d32c641a6fe4629c171b8928c7b08d98d7cf3" +checksum = "dc35310971f3b2dbbf3f0690a219f40e2d9afcf64f9ab7cc1be722937c26b4bc" [[package]] name = "windows_aarch64_msvc" -version = "0.52.4" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da9f259dd3bcf6990b55bffd094c4f7235817ba4ceebde8e6d11cd0c5633b675" +checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" [[package]] name = "windows_i686_gnu" -version = "0.48.0" +version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "622a1962a7db830d6fd0a69683c80a18fda201879f0f447f065a3b7467daa241" +checksum = "a75915e7def60c94dcef72200b9a8e58e5091744960da64ec734a6c6e9b3743e" [[package]] name = "windows_i686_gnu" -version = "0.52.4" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b474d8268f99e0995f25b9f095bc7434632601028cf86590aea5c8a5cb7801d3" +checksum = "8e9b5ad5ab802e97eb8e295ac6720e509ee4c243f69d781394014ebfe8bbfa0b" + +[[package]] +name = "windows_i686_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" [[package]] name = "windows_i686_msvc" -version = "0.48.0" +version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4542c6e364ce21bf45d69fdd2a8e455fa38d316158cfd43b3ac1c5b1b19f8e00" +checksum = "8f55c233f70c4b27f66c523580f78f1004e8b5a8b659e05a4eb49d4166cca406" [[package]] name = "windows_i686_msvc" -version = "0.52.4" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1515e9a29e5bed743cb4415a9ecf5dfca648ce85ee42e15873c3cd8610ff8e02" +checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" [[package]] name = "windows_x86_64_gnu" -version = "0.48.0" +version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ca2b8a661f7628cbd23440e50b05d705db3686f894fc9580820623656af974b1" +checksum = "53d40abd2583d23e4718fddf1ebec84dbff8381c07cae67ff7768bbf19c6718e" [[package]] name = "windows_x86_64_gnu" -version = "0.52.4" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5eee091590e89cc02ad514ffe3ead9eb6b660aedca2183455434b93546371a03" +checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" [[package]] name = "windows_x86_64_gnullvm" -version = "0.48.0" +version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7896dbc1f41e08872e9d5e8f8baa8fdd2677f29468c4e156210174edc7f7b953" +checksum = "0b7b52767868a23d5bab768e390dc5f5c55825b6d30b86c844ff2dc7414044cc" [[package]] name = "windows_x86_64_gnullvm" -version = "0.52.4" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "77ca79f2451b49fa9e2af39f0747fe999fcda4f5e241b2898624dca97a1f2177" +checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" [[package]] name = "windows_x86_64_msvc" -version = "0.48.0" +version = "0.48.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a515f5799fe4961cb532f983ce2b23082366b898e52ffbce459c86f67c8378a" +checksum = "ed94fce61571a4006852b7389a063ab983c02eb1bb37b47f8272ce92d06d9538" [[package]] name = "windows_x86_64_msvc" -version = "0.52.4" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32b752e52a2da0ddfbdbcc6fceadfeede4c939ed16d13e648833a61dfb611ed8" +checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" [[package]] name = "winnow" diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 55794c2b7dd..a38d8ef582d 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -270,16 +270,40 @@ impl<'block> BrilligBlock<'block> { self.convert_ssa_binary(binary, dfg, result_var); } Instruction::Constrain(lhs, rhs, assert_message) => { - let condition = SingleAddrVariable { - address: self.brillig_context.allocate_register(), - bit_size: 1, + let (condition, deallocate) = match ( + dfg.get_numeric_constant_with_type(*lhs), + dfg.get_numeric_constant_with_type(*rhs), + ) { + // If the constraint is of the form `x == u1 1` then we can simply constrain `x` directly + ( + Some((constant, Type::Numeric(NumericType::Unsigned { bit_size: 1 }))), + None, + ) if constant == FieldElement::one() => { + (self.convert_ssa_single_addr_value(*rhs, dfg), false) + } + ( + None, + Some((constant, Type::Numeric(NumericType::Unsigned { bit_size: 1 }))), + ) if constant == FieldElement::one() => { + (self.convert_ssa_single_addr_value(*lhs, dfg), false) + } + + // Otherwise we need to perform the equality explicitly. + _ => { + let condition = SingleAddrVariable { + address: self.brillig_context.allocate_register(), + bit_size: 1, + }; + self.convert_ssa_binary( + &Binary { lhs: *lhs, rhs: *rhs, operator: BinaryOp::Eq }, + dfg, + condition, + ); + + (condition, true) + } }; - self.convert_ssa_binary( - &Binary { lhs: *lhs, rhs: *rhs, operator: BinaryOp::Eq }, - dfg, - condition, - ); match assert_message { Some(ConstrainError::Dynamic(selector, values)) => { let payload_values = @@ -302,7 +326,9 @@ impl<'block> BrilligBlock<'block> { self.brillig_context.codegen_constrain(condition, None); } } - self.brillig_context.deallocate_single_addr(condition); + if deallocate { + self.brillig_context.deallocate_single_addr(condition); + } } Instruction::Allocate => { let result_value = dfg.instruction_results(instruction_id)[0]; diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/errors.rs b/noir/noir-repo/compiler/noirc_evaluator/src/errors.rs index 2c7ec0f8e1a..c4f56d032f9 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/errors.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/errors.rs @@ -44,7 +44,7 @@ pub enum RuntimeError { StaticAssertDynamicPredicate { call_stack: CallStack }, #[error("Argument is false")] StaticAssertFailed { call_stack: CallStack }, - #[error("Nested slices are not supported")] + #[error("Nested slices, i.e. slices within an array or slice, are not supported")] NestedSlice { call_stack: CallStack }, #[error("Big Integer modulus do no match")] BigIntModulus { call_stack: CallStack }, diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs index 57bd76d4f78..ad6645df228 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs @@ -82,6 +82,7 @@ pub(crate) fn optimize_into_acir( ) -> Result { let ssa_gen_span = span!(Level::TRACE, "ssa_generation"); let ssa_gen_span_guard = ssa_gen_span.enter(); + let mut ssa = SsaBuilder::new( program, options.enable_ssa_logging, @@ -418,8 +419,9 @@ impl SsaBuilder { Ok(self.print(msg)) } - fn print(self, msg: &str) -> Self { + fn print(mut self, msg: &str) -> Self { if self.print_ssa_passes { + self.ssa.normalize_ids(); println!("{msg}\n{}", self.ssa); } self diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index a2b9e46a15a..0360b15d950 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -770,10 +770,12 @@ impl<'a> Context<'a> { .map(|result_id| dfg.type_of_value(*result_id).flattened_size()) .sum(); - let acir_function_id = ssa - .entry_point_to_generated_index - .get(id) - .expect("ICE: should have an associated final index"); + let Some(acir_function_id) = + ssa.entry_point_to_generated_index.get(id) + else { + unreachable!("Expected an associated final index for call to acir function {id} with args {arguments:?}"); + }; + let output_vars = self.acir_context.call_acir_function( AcirFunctionId(*acir_function_id), inputs, diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs index 26eab290d4b..aa5f4c8df95 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs @@ -244,7 +244,7 @@ impl Context { } }, Value::ForeignFunction(..) => { - panic!("Should not be able to reach foreign function from non-brillig functions"); + panic!("Should not be able to reach foreign function from non-brillig functions, {func_id} in function {}", function.name()); } Value::Array { .. } | Value::Instruction { .. } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/function.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/function.rs index bae9f82e4f1..65a616ef612 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/function.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/function.rs @@ -72,6 +72,13 @@ impl Function { Self { name: another.name.clone(), id, entry_block, dfg, runtime: another.runtime } } + /// Takes the signature (function name & runtime) from a function but does not copy the body. + pub(crate) fn clone_signature(id: FunctionId, another: &Function) -> Self { + let mut new_function = Function::new(another.name.clone(), id); + new_function.runtime = another.runtime; + new_function + } + /// The name of the function. /// Used exclusively for debugging purposes. pub(crate) fn name(&self) -> &str { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/map.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/map.rs index 769d52e6e65..23f5380f030 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/map.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/map.rs @@ -1,6 +1,7 @@ use fxhash::FxHashMap as HashMap; use serde::{Deserialize, Serialize}; use std::{ + collections::BTreeMap, hash::Hash, str::FromStr, sync::atomic::{AtomicUsize, Ordering}, @@ -240,7 +241,7 @@ impl std::ops::IndexMut> for DenseMap { /// call to .remove(). #[derive(Debug)] pub(crate) struct SparseMap { - storage: HashMap, T>, + storage: BTreeMap, T>, } impl SparseMap { @@ -271,11 +272,16 @@ impl SparseMap { pub(crate) fn remove(&mut self, id: Id) -> Option { self.storage.remove(&id) } + + /// Unwraps the inner storage of this map + pub(crate) fn into_btree(self) -> BTreeMap, T> { + self.storage + } } impl Default for SparseMap { fn default() -> Self { - Self { storage: HashMap::default() } + Self { storage: Default::default() } } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index 1ff593a1531..7843c55da65 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -2,7 +2,7 @@ //! The purpose of this pass is to inline the instructions of each function call //! within the function caller. If all function calls are known, there will only //! be a single function remaining when the pass finishes. -use std::collections::{BTreeSet, HashSet}; +use std::collections::{BTreeSet, HashSet, VecDeque}; use acvm::acir::AcirField; use iter_extended::{btree_map, vecmap}; @@ -372,14 +372,14 @@ impl<'function> PerFunctionContext<'function> { fn translate_block( &mut self, source_block: BasicBlockId, - block_queue: &mut Vec, + block_queue: &mut VecDeque, ) -> BasicBlockId { if let Some(block) = self.blocks.get(&source_block) { return *block; } // The block is not yet inlined, queue it - block_queue.push(source_block); + block_queue.push_back(source_block); // The block is not already present in the function being inlined into so we must create it. // The block's instructions are not copied over as they will be copied later in inlining. @@ -415,13 +415,14 @@ impl<'function> PerFunctionContext<'function> { /// Inline all reachable blocks within the source_function into the destination function. fn inline_blocks(&mut self, ssa: &Ssa) -> Vec { let mut seen_blocks = HashSet::new(); - let mut block_queue = vec![self.source_function.entry_block()]; + let mut block_queue = VecDeque::new(); + block_queue.push_back(self.source_function.entry_block()); // This Vec will contain each block with a Return instruction along with the // returned values of that block. let mut function_returns = vec![]; - while let Some(source_block_id) = block_queue.pop() { + while let Some(source_block_id) = block_queue.pop_front() { if seen_blocks.contains(&source_block_id) { continue; } @@ -609,7 +610,7 @@ impl<'function> PerFunctionContext<'function> { fn handle_terminator_instruction( &mut self, block_id: BasicBlockId, - block_queue: &mut Vec, + block_queue: &mut VecDeque, ) -> Option<(BasicBlockId, Vec)> { match self.source_function.dfg[block_id].unwrap_terminator() { TerminatorInstruction::Jmp { destination, arguments, call_stack } => { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs index 9d6582c0db7..3d98f4126cf 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs @@ -116,7 +116,7 @@ struct PerFunctionContext<'f> { /// Track a value's last load across all blocks. /// If a value is not used in anymore loads we can remove the last store to that value. - last_loads: HashMap, + last_loads: HashMap, } impl<'f> PerFunctionContext<'f> { @@ -152,9 +152,31 @@ impl<'f> PerFunctionContext<'f> { // This rule does not apply to reference parameters, which we must also check for before removing these stores. for (block_id, block) in self.blocks.iter() { let block_params = self.inserter.function.dfg.block_parameters(*block_id); - for (value, store_instruction) in block.last_stores.iter() { - let is_reference_param = block_params.contains(value); - if self.last_loads.get(value).is_none() && !is_reference_param { + for (store_address, store_instruction) in block.last_stores.iter() { + let is_reference_param = block_params.contains(store_address); + let terminator = self.inserter.function.dfg[*block_id].unwrap_terminator(); + + let is_return = matches!(terminator, TerminatorInstruction::Return { .. }); + let remove_load = if is_return { + // Determine whether the last store is used in the return value + let mut is_return_value = false; + terminator.for_each_value(|return_value| { + is_return_value = return_value == *store_address || is_return_value; + }); + + // If the last load of a store is not part of the block with a return terminator, + // we can safely remove this store. + let last_load_not_in_return = self + .last_loads + .get(store_address) + .map(|(_, last_load_block)| *last_load_block != *block_id) + .unwrap_or(true); + !is_return_value && last_load_not_in_return + } else { + self.last_loads.get(store_address).is_none() + }; + + if remove_load && !is_reference_param { self.instructions_to_remove.insert(*store_instruction); } } @@ -259,7 +281,7 @@ impl<'f> PerFunctionContext<'f> { } else { references.mark_value_used(address, self.inserter.function); - self.last_loads.insert(address, instruction); + self.last_loads.insert(address, (instruction, block_id)); } } Instruction::Store { address, value } => { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs index 4e5fa262696..bd9d0baff97 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs @@ -13,6 +13,7 @@ mod die; pub(crate) mod flatten_cfg; mod inlining; mod mem2reg; +mod normalize_value_ids; mod rc; mod remove_bit_shifts; mod remove_enable_side_effects; diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs new file mode 100644 index 00000000000..f11b310494b --- /dev/null +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs @@ -0,0 +1,194 @@ +use std::collections::BTreeMap; + +use crate::ssa::{ + ir::{ + basic_block::BasicBlockId, + function::{Function, FunctionId}, + map::SparseMap, + post_order::PostOrder, + value::{Value, ValueId}, + }, + ssa_gen::Ssa, +}; +use fxhash::FxHashMap as HashMap; +use iter_extended::vecmap; + +impl Ssa { + /// This is a debugging pass which re-inserts each instruction + /// and block in a fresh DFG context for each function so that ValueIds, + /// BasicBlockIds, and FunctionIds are always identical for the same SSA code. + /// + /// During normal compilation this is often not the case since prior passes + /// may increase the ID counter so that later passes start at different offsets, + /// even if they contain the same SSA code. + pub(crate) fn normalize_ids(&mut self) { + let mut context = Context::default(); + context.populate_functions(&self.functions); + for function in self.functions.values_mut() { + context.normalize_ids(function); + } + self.functions = context.functions.into_btree(); + } +} + +#[derive(Default)] +struct Context { + functions: SparseMap, + + new_ids: IdMaps, +} + +/// Maps from old ids to new ones. +/// Separate from the rest of Context so we can call mutable methods on it +/// while Context gives out mutable references to functions within. +#[derive(Default)] +struct IdMaps { + // Maps old function id -> new function id + function_ids: HashMap, + + // Maps old block id -> new block id + // Cleared in between each function. + blocks: HashMap, + + // Maps old value id -> new value id + // Cleared in between each function. + values: HashMap, +} + +impl Context { + fn populate_functions(&mut self, functions: &BTreeMap) { + for (id, function) in functions { + self.functions.insert_with_id(|new_id| { + self.new_ids.function_ids.insert(*id, new_id); + Function::clone_signature(new_id, function) + }); + } + } + + fn normalize_ids(&mut self, old_function: &mut Function) { + self.new_ids.blocks.clear(); + self.new_ids.values.clear(); + + let new_function_id = self.new_ids.function_ids[&old_function.id()]; + let new_function = &mut self.functions[new_function_id]; + + let mut reachable_blocks = PostOrder::with_function(old_function).into_vec(); + reachable_blocks.reverse(); + + self.new_ids.populate_blocks(&reachable_blocks, old_function, new_function); + + // Map each parameter, instruction, and terminator + for old_block_id in reachable_blocks { + let new_block_id = self.new_ids.blocks[&old_block_id]; + + let old_block = &mut old_function.dfg[old_block_id]; + for old_instruction_id in old_block.take_instructions() { + let instruction = old_function.dfg[old_instruction_id] + .map_values(|value| self.new_ids.map_value(new_function, old_function, value)); + + let call_stack = old_function.dfg.get_call_stack(old_instruction_id); + let old_results = old_function.dfg.instruction_results(old_instruction_id); + + let ctrl_typevars = instruction + .requires_ctrl_typevars() + .then(|| vecmap(old_results, |result| old_function.dfg.type_of_value(*result))); + + let new_results = new_function.dfg.insert_instruction_and_results( + instruction, + new_block_id, + ctrl_typevars, + call_stack, + ); + + assert_eq!(old_results.len(), new_results.len()); + for (old_result, new_result) in old_results.iter().zip(new_results.results().iter()) + { + let old_result = old_function.dfg.resolve(*old_result); + self.new_ids.values.insert(old_result, *new_result); + } + } + + let old_block = &mut old_function.dfg[old_block_id]; + let mut terminator = old_block + .take_terminator() + .map_values(|value| self.new_ids.map_value(new_function, old_function, value)); + terminator.mutate_blocks(|old_block| self.new_ids.blocks[&old_block]); + new_function.dfg.set_block_terminator(new_block_id, terminator); + } + } +} + +impl IdMaps { + fn populate_blocks( + &mut self, + reachable_blocks: &[BasicBlockId], + old_function: &mut Function, + new_function: &mut Function, + ) { + let old_entry = old_function.entry_block(); + self.blocks.insert(old_entry, new_function.entry_block()); + + for old_id in reachable_blocks { + if *old_id != old_entry { + let new_id = new_function.dfg.make_block(); + self.blocks.insert(*old_id, new_id); + } + + let new_id = self.blocks[old_id]; + let old_block = &mut old_function.dfg[*old_id]; + for old_parameter in old_block.take_parameters() { + let old_parameter = old_function.dfg.resolve(old_parameter); + let typ = old_function.dfg.type_of_value(old_parameter); + let new_parameter = new_function.dfg.add_block_parameter(new_id, typ); + self.values.insert(old_parameter, new_parameter); + } + } + } + + fn map_value( + &mut self, + new_function: &mut Function, + old_function: &Function, + old_value: ValueId, + ) -> ValueId { + let old_value = old_function.dfg.resolve(old_value); + match &old_function.dfg[old_value] { + value @ Value::Instruction { instruction, .. } => { + *self.values.get(&old_value).unwrap_or_else(|| { + let instruction = &old_function.dfg[*instruction]; + unreachable!("Unmapped value with id {old_value}: {value:?}\n from instruction: {instruction:?}, SSA: {old_function}") + }) + } + + value @ Value::Param { .. } => { + *self.values.get(&old_value).unwrap_or_else(|| { + unreachable!("Unmapped value with id {old_value}: {value:?}") + }) + } + + Value::Function(id) => { + let new_id = self.function_ids[id]; + new_function.dfg.import_function(new_id) + } + + Value::NumericConstant { constant, typ } => { + new_function.dfg.make_constant(*constant, typ.clone()) + } + Value::Array { array, typ } => { + if let Some(value) = self.values.get(&old_value) { + return *value; + } + + let array = array + .iter() + .map(|value| self.map_value(new_function, old_function, *value)) + .collect(); + let new_value = new_function.dfg.make_array(array, typ.clone()); + self.values.insert(old_value, new_value); + new_value + } + Value::Intrinsic(intrinsic) => new_function.dfg.import_intrinsic(*intrinsic), + Value::ForeignFunction(name) => new_function.dfg.import_foreign_function(name), + } + } +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 4980045c68d..d8e62b66eca 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -1653,10 +1653,20 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { assert_eq!(arguments.len(), 2); let print_newline = arguments[0].0 == Value::Bool(true); - if print_newline { - println!("{}", arguments[1].0.display(self.elaborator.interner)); + let contents = arguments[1].0.display(self.elaborator.interner); + if self.elaborator.interner.is_in_lsp_mode() { + // If we `println!` in LSP it gets mixed with the protocol stream and leads to crashing + // the connection. If we use `eprintln!` not only it doesn't crash, but the output + // appears in the "Noir Language Server" output window in case you want to see it. + if print_newline { + eprintln!("{}", contents); + } else { + eprint!("{}", contents); + } + } else if print_newline { + println!("{}", contents); } else { - print!("{}", arguments[1].0.display(self.elaborator.interner)); + print!("{}", contents); } Ok(Value::Unit) diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index e5b098b41ed..070749e45ba 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -39,7 +39,7 @@ use crate::{ QuotedType, Shared, Type, }; -use self::builtin_helpers::{get_array, get_u8}; +use self::builtin_helpers::{get_array, get_str, get_u8}; use super::Interpreter; pub(crate) mod builtin_helpers; @@ -248,25 +248,15 @@ fn str_as_bytes( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let (string, string_location) = check_one_argument(arguments, location)?; + let string = check_one_argument(arguments, location)?; + let string = get_str(interner, string)?; - match string { - Value::String(string) => { - let string_as_bytes = string.as_bytes(); - let bytes_vector: Vec = string_as_bytes.iter().cloned().map(Value::U8).collect(); - let byte_array_type = Type::Array( - Box::new(Type::Constant(string_as_bytes.len() as u32)), - Box::new(Type::Integer(Signedness::Unsigned, IntegerBitSize::Eight)), - ); - Ok(Value::Array(bytes_vector.into(), byte_array_type)) - } - value => { - let type_var = Box::new(interner.next_type_variable()); - let expected = Type::Array(type_var.clone(), type_var); - let actual = value.get_type().into_owned(); - Err(InterpreterError::TypeMismatch { expected, actual, location: string_location }) - } - } + let bytes: im::Vector = string.bytes().map(Value::U8).collect(); + let byte_array_type = Type::Array( + Box::new(Type::Constant(bytes.len() as u32)), + Box::new(Type::Integer(Signedness::Unsigned, IntegerBitSize::Eight)), + ); + Ok(Value::Array(bytes, byte_array_type)) } /// fn as_type(self) -> Type diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs index dd9ea51961e..14a0e177544 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs @@ -104,6 +104,19 @@ pub(crate) fn get_slice( } } +pub(crate) fn get_str( + interner: &NodeInterner, + (value, location): (Value, Location), +) -> IResult> { + match value { + Value::String(string) => Ok(string), + value => { + let expected = Type::String(Box::new(interner.next_type_variable())); + type_mismatch(value, expected, location) + } + } +} + pub(crate) fn get_tuple( interner: &NodeInterner, (value, location): (Value, Location), diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/value.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/value.rs index b96c4852931..c5818c20c57 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/value.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/value.rs @@ -567,11 +567,33 @@ impl<'value, 'interner> Display for ValuePrinter<'value, 'interner> { Value::Quoted(tokens) => { write!(f, "quote {{")?; for token in tokens.iter() { + write!(f, " ")?; + match token { Token::QuotedType(id) => { - write!(f, " {}", self.interner.get_quoted_type(*id))?; + write!(f, "{}", self.interner.get_quoted_type(*id))?; + } + Token::InternedExpr(id) => { + let value = Value::expression(ExpressionKind::Interned(*id)); + value.display(self.interner).fmt(f)?; + } + Token::InternedStatement(id) => { + let value = Value::statement(StatementKind::Interned(*id)); + value.display(self.interner).fmt(f)?; + } + Token::InternedLValue(id) => { + let value = Value::lvalue(LValue::Interned(*id, Span::default())); + value.display(self.interner).fmt(f)?; } - other => write!(f, " {other}")?, + Token::InternedUnresolvedTypeData(id) => { + let value = Value::UnresolvedType(UnresolvedTypeData::Interned(*id)); + value.display(self.interner).fmt(f)?; + } + Token::UnquoteMarker(id) => { + let value = Value::TypedExpr(TypedExpr::ExprId(*id)); + value.display(self.interner).fmt(f)?; + } + other => write!(f, "{other}")?, } } write!(f, " }}") @@ -632,7 +654,16 @@ impl<'value, 'interner> Display for ValuePrinter<'value, 'interner> { Value::Expr(ExprValue::LValue(lvalue)) => { write!(f, "{}", remove_interned_in_lvalue(self.interner, lvalue.clone())) } - Value::TypedExpr(_) => write!(f, "(typed expr)"), + Value::TypedExpr(TypedExpr::ExprId(id)) => { + let hir_expr = self.interner.expression(id); + let expr = hir_expr.to_display_ast(self.interner, Span::default()); + write!(f, "{}", expr.kind) + } + Value::TypedExpr(TypedExpr::StmtId(id)) => { + let hir_statement = self.interner.statement(id); + let stmt = hir_statement.to_display_ast(self.interner, Span::default()); + write!(f, "{}", stmt.kind) + } Value::UnresolvedType(typ) => { if let UnresolvedTypeData::Interned(id) = typ { let typ = self.interner.get_unresolved_type_data(*id); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs index cede04dd582..c2038c646b5 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -72,7 +72,7 @@ pub enum ResolverError { NumericConstantInFormatString { name: String, span: Span }, #[error("Closure environment must be a tuple or unit type")] InvalidClosureEnvironment { typ: Type, span: Span }, - #[error("Nested slices are not supported")] + #[error("Nested slices, i.e. slices within an array or slice, are not supported")] NestedSlices { span: Span }, #[error("#[recursive] attribute is only allowed on entry points to a program")] MisplacedRecursiveAttribute { ident: Ident }, @@ -323,8 +323,8 @@ impl<'a> From<&'a ResolverError> for Diagnostic { format!("{typ} is not a valid closure environment type"), "Closure environment must be a tuple or unit type".to_string(), *span), ResolverError::NestedSlices { span } => Diagnostic::simple_error( - "Nested slices are not supported".into(), - "Try to use a constant sized array instead".into(), + "Nested slices, i.e. slices within an array or slice, are not supported".into(), + "Try to use a constant sized array or BoundedVec instead".into(), *span, ), ResolverError::MisplacedRecursiveAttribute { ident } => { diff --git a/noir/noir-repo/tooling/lsp/src/lib.rs b/noir/noir-repo/tooling/lsp/src/lib.rs index 4a764f4268b..6557975743c 100644 --- a/noir/noir-repo/tooling/lsp/src/lib.rs +++ b/noir/noir-repo/tooling/lsp/src/lib.rs @@ -4,7 +4,7 @@ #![cfg_attr(not(test), warn(unused_crate_dependencies, unused_extern_crates))] use std::{ - collections::{BTreeMap, HashMap}, + collections::{BTreeMap, HashMap, HashSet}, future::Future, ops::{self, ControlFlow}, path::{Path, PathBuf}, @@ -91,10 +91,13 @@ pub struct LspState { open_documents_count: usize, input_files: HashMap, cached_lenses: HashMap>, - cached_definitions: HashMap, + cached_definitions: HashMap, cached_parsed_files: HashMap))>, - cached_def_maps: HashMap>, + cached_def_maps: HashMap>, options: LspInitializationOptions, + + // Tracks files that currently have errors, by package root. + files_with_errors: HashMap>, } impl LspState { @@ -113,6 +116,8 @@ impl LspState { cached_parsed_files: HashMap::new(), cached_def_maps: HashMap::new(), options: Default::default(), + + files_with_errors: HashMap::new(), } } } diff --git a/noir/noir-repo/tooling/lsp/src/notifications/mod.rs b/noir/noir-repo/tooling/lsp/src/notifications/mod.rs index d1ffdb55066..87e7bea8c3b 100644 --- a/noir/noir-repo/tooling/lsp/src/notifications/mod.rs +++ b/noir/noir-repo/tooling/lsp/src/notifications/mod.rs @@ -1,8 +1,12 @@ +use std::collections::HashSet; use std::ops::ControlFlow; +use std::path::PathBuf; use crate::insert_all_files_for_workspace_into_file_manager; use async_lsp::{ErrorCode, LanguageClient, ResponseError}; -use lsp_types::DiagnosticTag; +use fm::{FileManager, FileMap}; +use fxhash::FxHashMap as HashMap; +use lsp_types::{DiagnosticTag, Url}; use noirc_driver::{check_crate, file_manager_with_stdlib}; use noirc_errors::{DiagnosticKind, FileDiagnostic}; @@ -105,7 +109,7 @@ pub(super) fn on_did_save_text_document( // caching code lenses and type definitions, and notifying about compilation errors. pub(crate) fn process_workspace_for_noir_document( state: &mut LspState, - document_uri: lsp_types::Url, + document_uri: Url, output_diagnostics: bool, ) -> Result<(), async_lsp::Error> { let file_path = document_uri.to_file_path().map_err(|_| { @@ -125,100 +129,123 @@ pub(crate) fn process_workspace_for_noir_document( let parsed_files = parse_diff(&workspace_file_manager, state); - let diagnostics: Vec<_> = workspace - .into_iter() - .flat_map(|package| -> Vec { - let package_root_dir: String = package.root_dir.as_os_str().to_string_lossy().into(); - - let (mut context, crate_id) = - crate::prepare_package(&workspace_file_manager, &parsed_files, package); - - let file_diagnostics = match check_crate(&mut context, crate_id, &Default::default()) { - Ok(((), warnings)) => warnings, - Err(errors_and_warnings) => errors_and_warnings, - }; - - // We don't add test headings for a package if it contains no `#[test]` functions - if let Some(tests) = get_package_tests_in_crate(&context, &crate_id, &package.name) { - let _ = state.client.notify::(NargoPackageTests { - package: package.name.to_string(), - tests, - }); - } - - let collected_lenses = crate::requests::collect_lenses_for_package( - &context, - crate_id, - &workspace, - package, - Some(&file_path), - ); - state.cached_lenses.insert(document_uri.to_string(), collected_lenses); - state.cached_definitions.insert(package_root_dir.clone(), context.def_interner); - state.cached_def_maps.insert(package_root_dir.clone(), context.def_maps); - - let fm = &context.file_manager; - let files = fm.as_file_map(); - - if output_diagnostics { - file_diagnostics - .into_iter() - .filter_map(|FileDiagnostic { file_id, diagnostic, call_stack: _ }| { - // Ignore diagnostics for any file that wasn't the file we saved - // TODO: In the future, we could create "related" diagnostics for these files - if fm.path(file_id).expect("file must exist to have emitted diagnostic") - != file_path - { - return None; - } - - // TODO: Should this be the first item in secondaries? Should we bail when we find a range? - let range = diagnostic - .secondaries - .into_iter() - .filter_map(|sec| byte_span_to_range(files, file_id, sec.span.into())) - .last() - .unwrap_or_default(); - - let severity = match diagnostic.kind { - DiagnosticKind::Error => DiagnosticSeverity::ERROR, - DiagnosticKind::Warning => DiagnosticSeverity::WARNING, - DiagnosticKind::Info => DiagnosticSeverity::INFORMATION, - DiagnosticKind::Bug => DiagnosticSeverity::WARNING, - }; - - let mut tags = Vec::new(); - if diagnostic.unnecessary { - tags.push(DiagnosticTag::UNNECESSARY); - } - if diagnostic.deprecated { - tags.push(DiagnosticTag::DEPRECATED); - } - - Some(Diagnostic { - range, - severity: Some(severity), - message: diagnostic.message, - tags: if tags.is_empty() { None } else { Some(tags) }, - ..Default::default() - }) - }) - .collect() - } else { - vec![] - } - }) - .collect(); - - if output_diagnostics { + for package in workspace.into_iter() { + let (mut context, crate_id) = + crate::prepare_package(&workspace_file_manager, &parsed_files, package); + + let file_diagnostics = match check_crate(&mut context, crate_id, &Default::default()) { + Ok(((), warnings)) => warnings, + Err(errors_and_warnings) => errors_and_warnings, + }; + + // We don't add test headings for a package if it contains no `#[test]` functions + if let Some(tests) = get_package_tests_in_crate(&context, &crate_id, &package.name) { + let _ = state.client.notify::(NargoPackageTests { + package: package.name.to_string(), + tests, + }); + } + + let collected_lenses = crate::requests::collect_lenses_for_package( + &context, + crate_id, + &workspace, + package, + Some(&file_path), + ); + state.cached_lenses.insert(document_uri.to_string(), collected_lenses); + state.cached_definitions.insert(package.root_dir.clone(), context.def_interner); + state.cached_def_maps.insert(package.root_dir.clone(), context.def_maps); + + let fm = &context.file_manager; + let files = fm.as_file_map(); + + if output_diagnostics { + publish_diagnostics(state, &package.root_dir, files, fm, file_diagnostics); + } + } + + Ok(()) +} + +fn publish_diagnostics( + state: &mut LspState, + package_root_dir: &PathBuf, + files: &FileMap, + fm: &FileManager, + file_diagnostics: Vec, +) { + let mut diagnostics_per_url: HashMap> = HashMap::default(); + + for file_diagnostic in file_diagnostics.into_iter() { + let file_id = file_diagnostic.file_id; + let diagnostic = file_diagnostic_to_diagnostic(file_diagnostic, files); + + let path = fm.path(file_id).expect("file must exist to have emitted diagnostic"); + if let Ok(uri) = Url::from_file_path(path) { + diagnostics_per_url.entry(uri).or_default().push(diagnostic); + } + } + + let new_files_with_errors: HashSet<_> = diagnostics_per_url.keys().cloned().collect(); + + for (uri, diagnostics) in diagnostics_per_url { let _ = state.client.publish_diagnostics(PublishDiagnosticsParams { - uri: document_uri, + uri, version: None, diagnostics, }); } - Ok(()) + // For files that previously had errors but no longer have errors we still need to publish empty diagnostics + if let Some(old_files_with_errors) = state.files_with_errors.get(package_root_dir) { + for uri in old_files_with_errors.difference(&new_files_with_errors) { + let _ = state.client.publish_diagnostics(PublishDiagnosticsParams { + uri: uri.clone(), + version: None, + diagnostics: vec![], + }); + } + } + + // Remember which files currently have errors, for next time + state.files_with_errors.insert(package_root_dir.clone(), new_files_with_errors); +} + +fn file_diagnostic_to_diagnostic(file_diagnostic: FileDiagnostic, files: &FileMap) -> Diagnostic { + let file_id = file_diagnostic.file_id; + let diagnostic = file_diagnostic.diagnostic; + + // TODO: Should this be the first item in secondaries? Should we bail when we find a range? + let range = diagnostic + .secondaries + .into_iter() + .filter_map(|sec| byte_span_to_range(files, file_id, sec.span.into())) + .last() + .unwrap_or_default(); + + let severity = match diagnostic.kind { + DiagnosticKind::Error => DiagnosticSeverity::ERROR, + DiagnosticKind::Warning => DiagnosticSeverity::WARNING, + DiagnosticKind::Info => DiagnosticSeverity::INFORMATION, + DiagnosticKind::Bug => DiagnosticSeverity::WARNING, + }; + + let mut tags = Vec::new(); + if diagnostic.unnecessary { + tags.push(DiagnosticTag::UNNECESSARY); + } + if diagnostic.deprecated { + tags.push(DiagnosticTag::DEPRECATED); + } + + Diagnostic { + range, + severity: Some(severity), + message: diagnostic.message, + tags: if tags.is_empty() { None } else { Some(tags) }, + ..Default::default() + } } pub(super) fn on_exit( diff --git a/noir/noir-repo/tooling/lsp/src/requests/code_action.rs b/noir/noir-repo/tooling/lsp/src/requests/code_action.rs index 8e153bb0b46..95cdc0b88b4 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/code_action.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/code_action.rs @@ -7,26 +7,26 @@ use async_lsp::ResponseError; use fm::{FileId, FileMap, PathString}; use lsp_types::{ CodeAction, CodeActionKind, CodeActionOrCommand, CodeActionParams, CodeActionResponse, - Position, Range, TextDocumentPositionParams, TextEdit, Url, WorkspaceEdit, + TextDocumentPositionParams, TextEdit, Url, WorkspaceEdit, }; -use noirc_errors::{Location, Span}; +use noirc_errors::Span; use noirc_frontend::{ - ast::{Ident, Path, Visitor}, + ast::{ConstructorExpression, Path, Visitor}, graph::CrateId, hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}, - macros_api::{ModuleDefId, NodeInterner}, + macros_api::NodeInterner, +}; +use noirc_frontend::{ parser::{Item, ItemKind, ParsedSubModule}, ParsedModule, }; -use crate::{ - byte_span_to_range, - modules::{get_parent_module_id, module_full_path, module_id_path}, - utils, LspState, -}; +use crate::{utils, LspState}; use super::{process_request, to_lsp_location}; +mod fill_struct_fields; +mod import_or_qualify; #[cfg(test)] mod tests; @@ -68,6 +68,7 @@ struct CodeActionFinder<'a> { uri: Url, files: &'a FileMap, file: FileId, + source: &'a str, lines: Vec<&'a str>, byte_index: usize, /// The module ID in scope. This might change as we traverse the AST @@ -108,6 +109,7 @@ impl<'a> CodeActionFinder<'a> { uri, files, file, + source, lines: source.lines().collect(), byte_index, module_id, @@ -137,46 +139,7 @@ impl<'a> CodeActionFinder<'a> { Some(code_actions) } - fn push_import_code_action(&mut self, full_path: &str) { - let line = self.auto_import_line as u32; - let character = (self.nesting * 4) as u32; - let indent = " ".repeat(self.nesting * 4); - let mut newlines = "\n"; - - // If the line we are inserting into is not an empty line, insert an extra line to make some room - if let Some(line_text) = self.lines.get(line as usize) { - if !line_text.trim().is_empty() { - newlines = "\n\n"; - } - } - - let title = format!("Import {}", full_path); - let text_edit = TextEdit { - range: Range { start: Position { line, character }, end: Position { line, character } }, - new_text: format!("use {};{}{}", full_path, newlines, indent), - }; - - let code_action = self.new_quick_fix(title, text_edit); - self.code_actions.push(CodeActionOrCommand::CodeAction(code_action)); - } - - fn push_qualify_code_action(&mut self, ident: &Ident, prefix: &str, full_path: &str) { - let Some(range) = byte_span_to_range( - self.files, - self.file, - ident.span().start() as usize..ident.span().start() as usize, - ) else { - return; - }; - - let title = format!("Qualify as {}", full_path); - let text_edit = TextEdit { range, new_text: format!("{}::", prefix) }; - - let code_action = self.new_quick_fix(title, text_edit); - self.code_actions.push(CodeActionOrCommand::CodeAction(code_action)); - } - - fn new_quick_fix(&self, title: String, text_edit: TextEdit) -> CodeAction { + fn new_quick_fix(&self, title: String, text_edit: TextEdit) -> CodeActionOrCommand { let mut changes = HashMap::new(); changes.insert(self.uri.clone(), vec![text_edit]); @@ -186,7 +149,7 @@ impl<'a> CodeActionFinder<'a> { change_annotations: None, }; - CodeAction { + CodeActionOrCommand::CodeAction(CodeAction { title, kind: Some(CodeActionKind::QUICKFIX), diagnostics: None, @@ -195,7 +158,7 @@ impl<'a> CodeActionFinder<'a> { is_preferred: None, disabled: None, data: None, - } + }) } fn includes_span(&self, span: Span) -> bool { @@ -244,69 +207,16 @@ impl<'a> Visitor for CodeActionFinder<'a> { } fn visit_path(&mut self, path: &Path) { - if path.segments.len() != 1 { - return; - } - - let ident = &path.segments[0].ident; - if !self.includes_span(ident.span()) { - return; - } - - let location = Location::new(ident.span(), self.file); - if self.interner.find_referenced(location).is_some() { - return; - } - - let current_module_parent_id = get_parent_module_id(self.def_maps, self.module_id); - - // The Path doesn't resolve to anything so it means it's an error and maybe we - // can suggest an import or to fully-qualify the path. - for (name, entries) in self.interner.get_auto_import_names() { - if name != &ident.0.contents { - continue; - } - - for (module_def_id, visibility, defining_module) in entries { - let module_full_path = if let Some(defining_module) = defining_module { - module_id_path( - *defining_module, - &self.module_id, - current_module_parent_id, - self.interner, - ) - } else { - let Some(module_full_path) = module_full_path( - *module_def_id, - *visibility, - self.module_id, - current_module_parent_id, - self.interner, - ) else { - continue; - }; - module_full_path - }; - - let full_path = if defining_module.is_some() - || !matches!(module_def_id, ModuleDefId::ModuleId(..)) - { - format!("{}::{}", module_full_path, name) - } else { - module_full_path.clone() - }; + self.import_or_qualify(path); + } - let qualify_prefix = if let ModuleDefId::ModuleId(..) = module_def_id { - let mut segments: Vec<_> = module_full_path.split("::").collect(); - segments.pop(); - segments.join("::") - } else { - module_full_path - }; + fn visit_constructor_expression( + &mut self, + constructor: &ConstructorExpression, + span: Span, + ) -> bool { + self.fill_struct_fields(constructor, span); - self.push_import_code_action(&full_path); - self.push_qualify_code_action(ident, &qualify_prefix, &full_path); - } - } + true } } diff --git a/noir/noir-repo/tooling/lsp/src/requests/code_action/fill_struct_fields.rs b/noir/noir-repo/tooling/lsp/src/requests/code_action/fill_struct_fields.rs new file mode 100644 index 00000000000..f57fbc652ad --- /dev/null +++ b/noir/noir-repo/tooling/lsp/src/requests/code_action/fill_struct_fields.rs @@ -0,0 +1,307 @@ +use lsp_types::TextEdit; +use noirc_errors::{Location, Span}; +use noirc_frontend::{ast::ConstructorExpression, node_interner::ReferenceId}; + +use crate::byte_span_to_range; + +use super::CodeActionFinder; + +impl<'a> CodeActionFinder<'a> { + pub(super) fn fill_struct_fields(&mut self, constructor: &ConstructorExpression, span: Span) { + if !self.includes_span(span) { + return; + } + + // Find out which struct this is + let location = Location::new(constructor.type_name.last_ident().span(), self.file); + let Some(ReferenceId::Struct(struct_id)) = self.interner.find_referenced(location) else { + return; + }; + + let struct_type = self.interner.get_struct(struct_id); + let struct_type = struct_type.borrow(); + + // First get all of the struct's fields + let mut fields = struct_type.get_fields_as_written(); + + // Remove the ones that already exists in the constructor + for (field, _) in &constructor.fields { + fields.retain(|(name, _)| name != &field.0.contents); + } + + if fields.is_empty() { + return; + } + + // Some fields are missing. Let's suggest a quick fix that adds them. + let bytes = self.source.as_bytes(); + let right_brace_index = span.end() as usize - 1; + let mut index = right_brace_index - 1; + while bytes[index].is_ascii_whitespace() { + index -= 1; + } + + let char_before_right_brace = bytes[index] as char; + + index += 1; + + let Some(range) = byte_span_to_range(self.files, self.file, index..index) else { + return; + }; + + // If the constructor spans multiple lines, we'll add the new fields in new lines too. + // Otherwise we'll add all the fields in a single line. + let constructor_range = + byte_span_to_range(self.files, self.file, span.start() as usize..span.end() as usize); + + // If it's multiline, find out the indent of the beginning line: we'll add new fields + // with that indent "plus one" (4 more spaces). + let line_indent = if let Some(constructor_range) = constructor_range { + if constructor_range.start.line == constructor_range.end.line { + None + } else { + let line = self.lines[constructor_range.start.line as usize]; + let whitespace_bytes = + line.bytes().take_while(|byte| byte.is_ascii_whitespace()).count(); + Some(whitespace_bytes) + } + } else { + None + }; + let line_indent = line_indent.map(|indent| " ".repeat(indent + 4)); + + let on_whitespace = bytes[index].is_ascii_whitespace(); + + let mut new_text = String::new(); + + // Add a comma if there's not a trailing one (if there are existing fields) + if !constructor.fields.is_empty() && char_before_right_brace != ',' { + new_text.push(','); + } + + // Add space or newline depending on whether it's multiline or not + if let Some(line_indent) = &line_indent { + new_text.push('\n'); + new_text.push_str(line_indent); + } else if !on_whitespace || constructor.fields.is_empty() { + new_text.push(' '); + } + + for (index, (name, _)) in fields.iter().enumerate() { + if index > 0 { + new_text.push(','); + if let Some(line_indent) = &line_indent { + new_text.push('\n'); + new_text.push_str(line_indent); + } else { + new_text.push(' '); + } + } + new_text.push_str(name); + new_text.push_str(": ()"); + } + + if !bytes[right_brace_index - 1].is_ascii_whitespace() { + new_text.push(' '); + } + + let title = "Fill struct fields".to_string(); + let text_edit = TextEdit { range, new_text }; + let code_action = self.new_quick_fix(title, text_edit); + self.code_actions.push(code_action); + } +} + +#[cfg(test)] +mod tests { + use tokio::test; + + use crate::requests::code_action::tests::assert_code_action; + + #[test] + async fn test_fill_struct_fields_code_action_no_space() { + let title = "Fill struct fields"; + + let src = r#" + struct Foo { + one: Field, + two: Field, + } + + fn main() { + Foo {>|<} + } + "#; + + let expected = r#" + struct Foo { + one: Field, + two: Field, + } + + fn main() { + Foo { one: (), two: () } + } + "#; + + assert_code_action(title, src, expected).await; + } + + #[test] + async fn test_fill_struct_fields_code_action_space() { + let title = "Fill struct fields"; + + let src = r#" + struct Foo { + one: Field, + two: Field, + } + + fn main() { + Foo { >|<} + } + "#; + + let expected = r#" + struct Foo { + one: Field, + two: Field, + } + + fn main() { + Foo { one: (), two: () } + } + "#; + + assert_code_action(title, src, expected).await; + } + + #[test] + async fn test_fill_struct_fields_code_action_some_fields() { + let title = "Fill struct fields"; + + let src = r#" + struct Foo { + one: Field, + two: Field, + three: Field, + } + + fn main() { + Foo { two: 1>|<} + } + "#; + + let expected = r#" + struct Foo { + one: Field, + two: Field, + three: Field, + } + + fn main() { + Foo { two: 1, one: (), three: () } + } + "#; + + assert_code_action(title, src, expected).await; + } + + #[test] + async fn test_fill_struct_fields_code_action_some_fields_trailing_comma() { + let title = "Fill struct fields"; + + let src = r#" + struct Foo { + one: Field, + two: Field, + three: Field, + } + + fn main() { + Foo { two: 1,>|<} + } + "#; + + let expected = r#" + struct Foo { + one: Field, + two: Field, + three: Field, + } + + fn main() { + Foo { two: 1, one: (), three: () } + } + "#; + + assert_code_action(title, src, expected).await; + } + + #[test] + async fn test_fill_struct_fields_code_action_multiline_empty() { + let title = "Fill struct fields"; + + let src = r#" + struct Foo { + one: Field, + two: Field, + } + + fn main() { + Foo {>|< + } + } + "#; + + let expected = r#" + struct Foo { + one: Field, + two: Field, + } + + fn main() { + Foo { + one: (), + two: () + } + } + "#; + + assert_code_action(title, src, expected).await; + } + + #[test] + async fn test_fill_struct_fields_code_action_multiline_some_fields() { + let title = "Fill struct fields"; + + let src = r#" + struct Foo { + one: Field, + two: Field, + } + + fn main() { + Foo {>|< + one: 1, + } + } + "#; + + let expected = r#" + struct Foo { + one: Field, + two: Field, + } + + fn main() { + Foo { + one: 1, + two: () + } + } + "#; + + assert_code_action(title, src, expected).await; + } +} diff --git a/noir/noir-repo/tooling/lsp/src/requests/code_action/import_or_qualify.rs b/noir/noir-repo/tooling/lsp/src/requests/code_action/import_or_qualify.rs new file mode 100644 index 00000000000..d07d117a317 --- /dev/null +++ b/noir/noir-repo/tooling/lsp/src/requests/code_action/import_or_qualify.rs @@ -0,0 +1,240 @@ +use lsp_types::{Position, Range, TextEdit}; +use noirc_errors::Location; +use noirc_frontend::{ + ast::{Ident, Path}, + macros_api::ModuleDefId, +}; + +use crate::{ + byte_span_to_range, + modules::{get_parent_module_id, module_full_path, module_id_path}, +}; + +use super::CodeActionFinder; + +impl<'a> CodeActionFinder<'a> { + pub(super) fn import_or_qualify(&mut self, path: &Path) { + if path.segments.len() != 1 { + return; + } + + let ident = &path.segments[0].ident; + if !self.includes_span(ident.span()) { + return; + } + + let location = Location::new(ident.span(), self.file); + if self.interner.find_referenced(location).is_some() { + return; + } + + let current_module_parent_id = get_parent_module_id(self.def_maps, self.module_id); + + // The Path doesn't resolve to anything so it means it's an error and maybe we + // can suggest an import or to fully-qualify the path. + for (name, entries) in self.interner.get_auto_import_names() { + if name != &ident.0.contents { + continue; + } + + for (module_def_id, visibility, defining_module) in entries { + let module_full_path = if let Some(defining_module) = defining_module { + module_id_path( + *defining_module, + &self.module_id, + current_module_parent_id, + self.interner, + ) + } else { + let Some(module_full_path) = module_full_path( + *module_def_id, + *visibility, + self.module_id, + current_module_parent_id, + self.interner, + ) else { + continue; + }; + module_full_path + }; + + let full_path = if defining_module.is_some() + || !matches!(module_def_id, ModuleDefId::ModuleId(..)) + { + format!("{}::{}", module_full_path, name) + } else { + module_full_path.clone() + }; + + let qualify_prefix = if let ModuleDefId::ModuleId(..) = module_def_id { + let mut segments: Vec<_> = module_full_path.split("::").collect(); + segments.pop(); + segments.join("::") + } else { + module_full_path + }; + + self.push_import_code_action(&full_path); + self.push_qualify_code_action(ident, &qualify_prefix, &full_path); + } + } + } + + fn push_import_code_action(&mut self, full_path: &str) { + let line = self.auto_import_line as u32; + let character = (self.nesting * 4) as u32; + let indent = " ".repeat(self.nesting * 4); + let mut newlines = "\n"; + + // If the line we are inserting into is not an empty line, insert an extra line to make some room + if let Some(line_text) = self.lines.get(line as usize) { + if !line_text.trim().is_empty() { + newlines = "\n\n"; + } + } + + let title = format!("Import {}", full_path); + let text_edit = TextEdit { + range: Range { start: Position { line, character }, end: Position { line, character } }, + new_text: format!("use {};{}{}", full_path, newlines, indent), + }; + + let code_action = self.new_quick_fix(title, text_edit); + self.code_actions.push(code_action); + } + + fn push_qualify_code_action(&mut self, ident: &Ident, prefix: &str, full_path: &str) { + let Some(range) = byte_span_to_range( + self.files, + self.file, + ident.span().start() as usize..ident.span().start() as usize, + ) else { + return; + }; + + let title = format!("Qualify as {}", full_path); + let text_edit = TextEdit { range, new_text: format!("{}::", prefix) }; + + let code_action = self.new_quick_fix(title, text_edit); + self.code_actions.push(code_action); + } +} + +#[cfg(test)] +mod tests { + use tokio::test; + + use crate::requests::code_action::tests::assert_code_action; + + #[test] + async fn test_qualify_code_action_for_struct() { + let title = "Qualify as foo::bar::SomeTypeInBar"; + + let src = r#" + mod foo { + mod bar { + struct SomeTypeInBar {} + } + } + + fn foo(x: SomeType>|||| CodeActionResponse { .unwrap() } -async fn assert_code_action(title: &str, src: &str, expected: &str) { +pub(crate) async fn assert_code_action(title: &str, src: &str, expected: &str) { let actions = get_code_action(src).await; let action = actions .iter() @@ -87,150 +86,3 @@ fn apply_text_edit(src: &str, text_edit: &TextEdit) -> String { lines[text_edit.range.start.line as usize] = &line; lines.join("\n") } - -#[test] -async fn test_qualify_code_action_for_struct() { - let title = "Qualify as foo::bar::SomeTypeInBar"; - - let src = r#" - mod foo { - mod bar { - struct SomeTypeInBar {} - } - } - - fn foo(x: SomeType>||||| { location: noirc_errors::Location, files: &'a FileMap, interner: &'a NodeInterner, - interners: &'a HashMap, + interners: &'a HashMap, crate_id: CrateId, crate_name: String, dependencies: &'a Vec, @@ -432,8 +432,6 @@ where ResponseError::new(ErrorCode::REQUEST_FAILED, "Could not find package for file") })?; - let package_root_path: String = package.root_dir.as_os_str().to_string_lossy().into(); - let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager( state, @@ -447,9 +445,9 @@ where let interner; let def_maps; - if let Some(def_interner) = state.cached_definitions.get(&package_root_path) { + if let Some(def_interner) = state.cached_definitions.get(&package.root_dir) { interner = def_interner; - def_maps = state.cached_def_maps.get(&package_root_path).unwrap(); + def_maps = state.cached_def_maps.get(&package.root_dir).unwrap(); } else { // We ignore the warnings and errors produced by compilation while resolving the definition let _ = noirc_driver::check_crate(&mut context, crate_id, &Default::default()); @@ -479,7 +477,7 @@ where pub(crate) fn find_all_references_in_workspace( location: noirc_errors::Location, interner: &NodeInterner, - cached_interners: &HashMap, + cached_interners: &HashMap, files: &FileMap, include_declaration: bool, include_self_type_name: bool, diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/lsp_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/lsp_cmd.rs index 9ff7a42e5f5..bfaa913b33a 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/lsp_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/lsp_cmd.rs @@ -35,8 +35,6 @@ pub(crate) fn run(_args: LspCommand, _config: NargoConfig) -> Result<(), CliErro .service(router) }); - eprintln!("LSP starting..."); - // Prefer truly asynchronous piped stdin/stdout without blocking tasks. #[cfg(unix)] let (stdin, stdout) = (