From 8e8e97c68e48245a6c7de9b3a0fe9960a889c47a Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 9 Sep 2024 11:12:35 -0300 Subject: [PATCH 01/13] feat: let `nargo` and LSP work well in the stdlib (#5969) # Description ## Problem `nargo test` doesn't work in the standard library, and LSP gives lots of (incorrect) errors when opening the standard library. ## Summary The main issue is that when you run `nargo` against the standard library, a copy of it is also loaded as the standard library. Then we get duplicate definitions, etc. I tried several things to solve this and one that worked and ended up making a bit of sense is having a crate be the root and the standard library at the same time. Now with this PR we can run `nargo test`, LSP works well, and you can even run tests directly from the editor, which could speed up the stdlib development. ## Additional Context 1. 7 warnings still remain in the standard library (all unused functions) but I'd like to solve that in a separate PR. 2. If this gets merged we could eventually change CI to compile nargo, then run `nargo test`, instead of running stdlib tests as part of one crate's tests. ## Documentation Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- compiler/noirc_driver/src/lib.rs | 22 +++++++------- compiler/noirc_frontend/src/graph/mod.rs | 38 ++++++++++++++++++++++-- tooling/lsp/src/modules.rs | 2 +- tooling/lsp/src/notifications/mod.rs | 5 ++-- tooling/lsp/src/requests/hover.rs | 5 ++-- tooling/lsp/src/requests/mod.rs | 4 +-- tooling/lsp/src/requests/profile_run.rs | 6 ++-- tooling/lsp/src/requests/test_run.rs | 6 ++-- tooling/lsp/src/requests/tests.rs | 4 +-- tooling/nargo/src/workspace.rs | 18 +++++++++++ tooling/nargo_cli/src/cli/check_cmd.rs | 5 ++-- tooling/nargo_cli/src/cli/compile_cmd.rs | 4 +-- tooling/nargo_cli/src/cli/export_cmd.rs | 5 ++-- tooling/nargo_cli/src/cli/fmt_cmd.rs | 4 +-- tooling/nargo_cli/src/cli/test_cmd.rs | 6 ++-- 15 files changed, 89 insertions(+), 45 deletions(-) diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 31c279bc0f3..18a13517b75 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -212,23 +212,25 @@ fn add_debug_source_to_file_manager(file_manager: &mut FileManager) { /// Adds the file from the file system at `Path` to the crate graph as a root file /// -/// Note: This methods adds the stdlib as a dependency to the crate. -/// This assumes that the stdlib has already been added to the file manager. +/// Note: If the stdlib dependency has not been added yet, it's added. Otherwise +/// this method assumes the root crate is the stdlib (useful for running tests +/// in the stdlib, getting LSP stuff for the stdlib, etc.). pub fn prepare_crate(context: &mut Context, file_name: &Path) -> CrateId { let path_to_std_lib_file = Path::new(STD_CRATE_NAME).join("lib.nr"); - let std_file_id = context - .file_manager - .name_to_id(path_to_std_lib_file) - .expect("stdlib file id is expected to be present"); - let std_crate_id = context.crate_graph.add_stdlib(std_file_id); + let std_file_id = context.file_manager.name_to_id(path_to_std_lib_file); + let std_crate_id = std_file_id.map(|std_file_id| context.crate_graph.add_stdlib(std_file_id)); let root_file_id = context.file_manager.name_to_id(file_name.to_path_buf()).unwrap_or_else(|| panic!("files are expected to be added to the FileManager before reaching the compiler file_path: {file_name:?}")); - let root_crate_id = context.crate_graph.add_crate_root(root_file_id); + if let Some(std_crate_id) = std_crate_id { + let root_crate_id = context.crate_graph.add_crate_root(root_file_id); - add_dep(context, root_crate_id, std_crate_id, STD_CRATE_NAME.parse().unwrap()); + add_dep(context, root_crate_id, std_crate_id, STD_CRATE_NAME.parse().unwrap()); - root_crate_id + root_crate_id + } else { + context.crate_graph.add_crate_root_and_stdlib(root_file_id) + } } pub fn link_to_debug_crate(context: &mut Context, root_crate_id: CrateId) { diff --git a/compiler/noirc_frontend/src/graph/mod.rs b/compiler/noirc_frontend/src/graph/mod.rs index 452aef74b36..094594a50ac 100644 --- a/compiler/noirc_frontend/src/graph/mod.rs +++ b/compiler/noirc_frontend/src/graph/mod.rs @@ -16,6 +16,10 @@ pub enum CrateId { Root(usize), Crate(usize), Stdlib(usize), + /// The special case of running the compiler against the stdlib. + /// In that case there's only one crate, and it's both the root + /// crate and the stdlib crate. + RootAndStdlib(usize), Dummy, } @@ -25,11 +29,17 @@ impl CrateId { } pub fn is_stdlib(&self) -> bool { - matches!(self, CrateId::Stdlib(_)) + match self { + CrateId::Stdlib(_) | CrateId::RootAndStdlib(_) => true, + CrateId::Root(_) | CrateId::Crate(_) | CrateId::Dummy => false, + } } pub fn is_root(&self) -> bool { - matches!(self, CrateId::Root(_)) + match self { + CrateId::Root(_) | CrateId::RootAndStdlib(_) => true, + CrateId::Stdlib(_) | CrateId::Crate(_) | CrateId::Dummy => false, + } } } @@ -178,7 +188,7 @@ impl CrateGraph { Some((CrateId::Root(_), _)) => { panic!("ICE: Tried to re-add the root crate as a regular crate") } - Some((CrateId::Stdlib(_), _)) => { + Some((CrateId::Stdlib(_), _)) | Some((CrateId::RootAndStdlib(_), _)) => { panic!("ICE: Tried to re-add the stdlib crate as a regular crate") } Some((CrateId::Dummy, _)) => { @@ -212,6 +222,28 @@ impl CrateGraph { crate_id } + pub fn add_crate_root_and_stdlib(&mut self, file_id: FileId) -> CrateId { + for (crate_id, crate_data) in self.arena.iter() { + if crate_id.is_root() { + panic!("ICE: Cannot add two crate roots to a graph - use `add_crate` instead"); + } + + if crate_id.is_stdlib() { + panic!("ICE: Cannot add two stdlib crates to a graph - use `add_crate` instead"); + } + + if crate_data.root_file_id == file_id { + panic!("ICE: This FileId was already added to the CrateGraph") + } + } + + let data = CrateData { root_file_id: file_id, dependencies: Vec::new() }; + let crate_id = CrateId::RootAndStdlib(self.arena.len()); + let prev = self.arena.insert(crate_id, data); + assert!(prev.is_none()); + crate_id + } + pub fn iter_keys(&self) -> impl Iterator + '_ { self.arena.keys().copied() } diff --git a/tooling/lsp/src/modules.rs b/tooling/lsp/src/modules.rs index d78da15a8ff..cce7f324a2e 100644 --- a/tooling/lsp/src/modules.rs +++ b/tooling/lsp/src/modules.rs @@ -122,7 +122,7 @@ pub(crate) fn module_id_path( if !is_relative { // We don't record module attributes for the root module, // so we handle that case separately - if let CrateId::Root(_) = target_module_id.krate { + if target_module_id.krate.is_root() { segments.push("crate"); } } diff --git a/tooling/lsp/src/notifications/mod.rs b/tooling/lsp/src/notifications/mod.rs index 87e7bea8c3b..96e339ee212 100644 --- a/tooling/lsp/src/notifications/mod.rs +++ b/tooling/lsp/src/notifications/mod.rs @@ -7,7 +7,7 @@ use async_lsp::{ErrorCode, LanguageClient, ResponseError}; 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_driver::check_crate; use noirc_errors::{DiagnosticKind, FileDiagnostic}; use crate::types::{ @@ -120,7 +120,8 @@ pub(crate) fn process_workspace_for_noir_document( ResponseError::new(ErrorCode::REQUEST_FAILED, lsp_error.to_string()) })?; - let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); + let mut workspace_file_manager = workspace.new_file_manager(); + insert_all_files_for_workspace_into_file_manager( state, &workspace, diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index 20333e8b728..dab6ddd0fc6 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -5,7 +5,6 @@ use fm::FileMap; use lsp_types::{Hover, HoverContents, HoverParams, MarkupContent, MarkupKind}; use noirc_frontend::{ ast::Visibility, - graph::CrateId, hir::def_map::ModuleId, hir_def::{stmt::HirPattern, traits::Trait}, macros_api::{NodeInterner, StructId}, @@ -376,9 +375,9 @@ fn format_parent_module_from_module_id( } } - // We don't record module attriubtes for the root module, + // We don't record module attributes for the root module, // so we handle that case separately - if let CrateId::Root(_) = module.krate { + if module.krate.is_root() { segments.push(&args.crate_name); }; diff --git a/tooling/lsp/src/requests/mod.rs b/tooling/lsp/src/requests/mod.rs index af58396550d..fea758e0e52 100644 --- a/tooling/lsp/src/requests/mod.rs +++ b/tooling/lsp/src/requests/mod.rs @@ -15,7 +15,7 @@ use lsp_types::{ WorkDoneProgressOptions, }; use nargo_fmt::Config; -use noirc_driver::file_manager_with_stdlib; + use noirc_frontend::graph::CrateId; use noirc_frontend::hir::def_map::CrateDefMap; use noirc_frontend::{graph::Dependency, macros_api::NodeInterner}; @@ -432,7 +432,7 @@ where ResponseError::new(ErrorCode::REQUEST_FAILED, "Could not find package for file") })?; - let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); + let mut workspace_file_manager = workspace.new_file_manager(); insert_all_files_for_workspace_into_file_manager( state, &workspace, diff --git a/tooling/lsp/src/requests/profile_run.rs b/tooling/lsp/src/requests/profile_run.rs index d3b7743557a..a7362300adc 100644 --- a/tooling/lsp/src/requests/profile_run.rs +++ b/tooling/lsp/src/requests/profile_run.rs @@ -9,9 +9,7 @@ use async_lsp::{ErrorCode, ResponseError}; use nargo::ops::report_errors; use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_artifacts::debug::DebugArtifact; -use noirc_driver::{ - file_manager_with_stdlib, CompileOptions, DebugFile, NOIR_ARTIFACT_VERSION_STRING, -}; +use noirc_driver::{CompileOptions, DebugFile, NOIR_ARTIFACT_VERSION_STRING}; use noirc_errors::{debug_info::OpCodesCount, Location}; use crate::{ @@ -53,7 +51,7 @@ fn on_profile_run_request_inner( ResponseError::new(ErrorCode::REQUEST_FAILED, err) })?; - let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); + let mut workspace_file_manager = workspace.new_file_manager(); insert_all_files_for_workspace_into_file_manager( state, &workspace, diff --git a/tooling/lsp/src/requests/test_run.rs b/tooling/lsp/src/requests/test_run.rs index 5f4f9cd98d0..081eb815c8b 100644 --- a/tooling/lsp/src/requests/test_run.rs +++ b/tooling/lsp/src/requests/test_run.rs @@ -4,9 +4,7 @@ use crate::insert_all_files_for_workspace_into_file_manager; use async_lsp::{ErrorCode, ResponseError}; use nargo::ops::{run_test, TestStatus}; use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection}; -use noirc_driver::{ - check_crate, file_manager_with_stdlib, CompileOptions, NOIR_ARTIFACT_VERSION_STRING, -}; +use noirc_driver::{check_crate, CompileOptions, NOIR_ARTIFACT_VERSION_STRING}; use noirc_frontend::hir::FunctionNameMatch; use crate::{ @@ -48,7 +46,7 @@ fn on_test_run_request_inner( ResponseError::new(ErrorCode::REQUEST_FAILED, err) })?; - let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); + let mut workspace_file_manager = workspace.new_file_manager(); insert_all_files_for_workspace_into_file_manager( state, &workspace, diff --git a/tooling/lsp/src/requests/tests.rs b/tooling/lsp/src/requests/tests.rs index 7203aca7f09..81910bebedb 100644 --- a/tooling/lsp/src/requests/tests.rs +++ b/tooling/lsp/src/requests/tests.rs @@ -4,7 +4,7 @@ use crate::insert_all_files_for_workspace_into_file_manager; use async_lsp::{ErrorCode, LanguageClient, ResponseError}; use lsp_types::{LogMessageParams, MessageType}; use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection}; -use noirc_driver::{check_crate, file_manager_with_stdlib, NOIR_ARTIFACT_VERSION_STRING}; +use noirc_driver::{check_crate, NOIR_ARTIFACT_VERSION_STRING}; use crate::{ get_package_tests_in_crate, parse_diff, @@ -50,7 +50,7 @@ fn on_tests_request_inner( ResponseError::new(ErrorCode::REQUEST_FAILED, err) })?; - let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); + let mut workspace_file_manager = workspace.new_file_manager(); insert_all_files_for_workspace_into_file_manager( state, &workspace, diff --git a/tooling/nargo/src/workspace.rs b/tooling/nargo/src/workspace.rs index 0795ffd9304..810a9edb7e1 100644 --- a/tooling/nargo/src/workspace.rs +++ b/tooling/nargo/src/workspace.rs @@ -9,6 +9,9 @@ use std::{ slice, }; +use fm::FileManager; +use noirc_driver::file_manager_with_stdlib; + use crate::{ constants::{CONTRACT_DIR, EXPORT_DIR, PROOFS_DIR, TARGET_DIR}, package::Package, @@ -46,6 +49,21 @@ impl Workspace { pub fn export_directory_path(&self) -> PathBuf { self.root_dir.join(EXPORT_DIR) } + + /// Returns a new `FileManager` for the root directory of this workspace. + /// If the root directory is not the standard library, the standard library + /// is added to the returned `FileManager`. + pub fn new_file_manager(&self) -> FileManager { + if self.is_stdlib() { + FileManager::new(&self.root_dir) + } else { + file_manager_with_stdlib(&self.root_dir) + } + } + + fn is_stdlib(&self) -> bool { + self.members.len() == 1 && self.members[0].name.to_string() == "std" + } } pub enum IntoIter<'a, T> { diff --git a/tooling/nargo_cli/src/cli/check_cmd.rs b/tooling/nargo_cli/src/cli/check_cmd.rs index 5239070b4d2..65a09dcdd11 100644 --- a/tooling/nargo_cli/src/cli/check_cmd.rs +++ b/tooling/nargo_cli/src/cli/check_cmd.rs @@ -10,8 +10,7 @@ use nargo::{ use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_abi::{AbiParameter, AbiType, MAIN_RETURN_NAME}; use noirc_driver::{ - check_crate, compute_function_abi, file_manager_with_stdlib, CompileOptions, - NOIR_ARTIFACT_VERSION_STRING, + check_crate, compute_function_abi, CompileOptions, NOIR_ARTIFACT_VERSION_STRING, }; use noirc_frontend::{ graph::{CrateId, CrateName}, @@ -52,7 +51,7 @@ pub(crate) fn run(args: CheckCommand, config: NargoConfig) -> Result<(), CliErro Some(NOIR_ARTIFACT_VERSION_STRING.to_string()), )?; - let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); + let mut workspace_file_manager = workspace.new_file_manager(); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); let parsed_files = parse_all(&workspace_file_manager); diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index 85faf574a0a..ae96f6436e2 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -9,8 +9,8 @@ use nargo::package::Package; use nargo::workspace::Workspace; use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; +use noirc_driver::DEFAULT_EXPRESSION_WIDTH; use noirc_driver::NOIR_ARTIFACT_VERSION_STRING; -use noirc_driver::{file_manager_with_stdlib, DEFAULT_EXPRESSION_WIDTH}; use noirc_driver::{CompilationResult, CompileOptions, CompiledContract}; use noirc_frontend::graph::CrateName; @@ -114,7 +114,7 @@ pub(super) fn compile_workspace_full( workspace: &Workspace, compile_options: &CompileOptions, ) -> Result<(), CliError> { - let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); + let mut workspace_file_manager = workspace.new_file_manager(); insert_all_files_for_workspace_into_file_manager(workspace, &mut workspace_file_manager); let parsed_files = parse_all(&workspace_file_manager); diff --git a/tooling/nargo_cli/src/cli/export_cmd.rs b/tooling/nargo_cli/src/cli/export_cmd.rs index 19add7f30dc..c3752db7fbd 100644 --- a/tooling/nargo_cli/src/cli/export_cmd.rs +++ b/tooling/nargo_cli/src/cli/export_cmd.rs @@ -12,8 +12,7 @@ use nargo::workspace::Workspace; use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::{ - compile_no_check, file_manager_with_stdlib, CompileOptions, CompiledProgram, - NOIR_ARTIFACT_VERSION_STRING, + compile_no_check, CompileOptions, CompiledProgram, NOIR_ARTIFACT_VERSION_STRING, }; use noirc_frontend::graph::CrateName; @@ -54,7 +53,7 @@ pub(crate) fn run(args: ExportCommand, config: NargoConfig) -> Result<(), CliErr Some(NOIR_ARTIFACT_VERSION_STRING.to_owned()), )?; - let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); + let mut workspace_file_manager = workspace.new_file_manager(); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); let parsed_files = parse_all(&workspace_file_manager); diff --git a/tooling/nargo_cli/src/cli/fmt_cmd.rs b/tooling/nargo_cli/src/cli/fmt_cmd.rs index 8f66a0a328f..66496db517c 100644 --- a/tooling/nargo_cli/src/cli/fmt_cmd.rs +++ b/tooling/nargo_cli/src/cli/fmt_cmd.rs @@ -3,7 +3,7 @@ use std::{fs::DirEntry, path::Path}; use clap::Args; use nargo::{insert_all_files_for_workspace_into_file_manager, ops::report_errors}; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; -use noirc_driver::{file_manager_with_stdlib, NOIR_ARTIFACT_VERSION_STRING}; +use noirc_driver::NOIR_ARTIFACT_VERSION_STRING; use noirc_errors::CustomDiagnostic; use noirc_frontend::{hir::def_map::parse_file, parser::ParserError}; @@ -29,7 +29,7 @@ pub(crate) fn run(args: FormatCommand, config: NargoConfig) -> Result<(), CliErr Some(NOIR_ARTIFACT_VERSION_STRING.to_string()), )?; - let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); + let mut workspace_file_manager = workspace.new_file_manager(); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); let config = nargo_fmt::Config::read(&config.program_dir) diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index 2f9390d72e0..8b3c0e29c7d 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -9,9 +9,7 @@ use nargo::{ prepare_package, }; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; -use noirc_driver::{ - check_crate, file_manager_with_stdlib, CompileOptions, NOIR_ARTIFACT_VERSION_STRING, -}; +use noirc_driver::{check_crate, CompileOptions, NOIR_ARTIFACT_VERSION_STRING}; use noirc_frontend::{ graph::CrateName, hir::{FunctionNameMatch, ParsedFiles}, @@ -65,7 +63,7 @@ pub(crate) fn run(args: TestCommand, config: NargoConfig) -> Result<(), CliError Some(NOIR_ARTIFACT_VERSION_STRING.to_string()), )?; - let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); + let mut workspace_file_manager = workspace.new_file_manager(); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); let parsed_files = parse_all(&workspace_file_manager); From 5c746831e1b67b4148fd630150159cfde3f3e29e Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 9 Sep 2024 11:24:53 -0300 Subject: [PATCH 02/13] chore: remove 3 unused functions warnings in the stdlib (#5973) # Description ## Problem Two functions were unused, another one two but it was kind of a test so I marked it as such. ## Summary ## Additional Context After this 4 "unused" functions remain, but solving those is a bit more challenging so I'm leaving that for a separate PR. ## Documentation Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- noir_stdlib/src/hash/poseidon/bn254/consts.nr | 5 ----- noir_stdlib/src/hash/poseidon/mod.nr | 8 -------- noir_stdlib/src/meta/mod.nr | 1 + 3 files changed, 1 insertion(+), 13 deletions(-) diff --git a/noir_stdlib/src/hash/poseidon/bn254/consts.nr b/noir_stdlib/src/hash/poseidon/bn254/consts.nr index 81d78377ce8..3b28ced5835 100644 --- a/noir_stdlib/src/hash/poseidon/bn254/consts.nr +++ b/noir_stdlib/src/hash/poseidon/bn254/consts.nr @@ -5,11 +5,6 @@ // Consistent with https://github.com/iden3/circomlib/blob/master/circuits/poseidon.circom and https://github.com/iden3/circomlib/blob/master/circuits/poseidon_constants.circom use crate::hash::poseidon::PoseidonConfig; use crate::hash::poseidon::config; -// Number of full rounds -// Number of partial rounds -fn rp() -> [u8; 16] { - [56, 57, 56, 60, 60, 63, 64, 63, 60, 66, 60, 65, 70, 60, 64, 68] -} // S-box power fn alpha() -> Field { 5 diff --git a/noir_stdlib/src/hash/poseidon/mod.nr b/noir_stdlib/src/hash/poseidon/mod.nr index 963808f6053..cf9b6187c02 100644 --- a/noir_stdlib/src/hash/poseidon/mod.nr +++ b/noir_stdlib/src/hash/poseidon/mod.nr @@ -1,5 +1,4 @@ mod bn254; // Instantiations of Poseidon for prime field of the same order as BN254 -use crate::field::modulus_num_bits; use crate::hash::Hasher; use crate::default::Default; @@ -166,13 +165,6 @@ fn sigma(x: [Field; O]) -> [Field; O] { y } -// Check security of sponge instantiation -fn check_security(rate: Field, width: Field, security: Field) -> bool { - let n = modulus_num_bits(); - - ((n - 1) as Field * (width - rate) / 2) as u8 > security as u8 -} - struct PoseidonHasher{ _state: [Field], } diff --git a/noir_stdlib/src/meta/mod.nr b/noir_stdlib/src/meta/mod.nr index 9fc399ddbf9..1079cc6013a 100644 --- a/noir_stdlib/src/meta/mod.nr +++ b/noir_stdlib/src/meta/mod.nr @@ -122,6 +122,7 @@ mod tests { quote { 1 } } + #[test] fn returning_versus_macro_insertion() { comptime { From 65da5983ece16249fa939a493f197d13fbb1f9a4 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 9 Sep 2024 12:46:33 -0300 Subject: [PATCH 03/13] feat: add `Expr::as_let` (#5964) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description ## Problem Part of #5668 ## Summary Adds `Expr::as_let`, but also introduces a new `ExprValue` for patterns, their inlining code, etc. ## Additional Context I wonder if `Pattern` should be an `Expr` or something else, because it can't be converted to an expression like a statement or like an LValue 🤔 ## Documentation Check one: - [ ] No documentation needed. - [x] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- aztec_macros/src/utils/parse_utils.rs | 1 + compiler/noirc_frontend/src/ast/statement.rs | 48 +++++- compiler/noirc_frontend/src/ast/visitor.rs | 9 +- compiler/noirc_frontend/src/debug/mod.rs | 2 + .../noirc_frontend/src/elaborator/patterns.rs | 11 ++ .../noirc_frontend/src/hir/comptime/errors.rs | 20 ++- .../src/hir/comptime/interpreter/builtin.rs | 99 ++++++++--- .../interpreter/builtin/builtin_helpers.rs | 7 +- .../noirc_frontend/src/hir/comptime/value.rs | 156 ++++++++++++++++-- compiler/noirc_frontend/src/lexer/token.rs | 16 +- compiler/noirc_frontend/src/node_interner.rs | 16 ++ compiler/noirc_frontend/src/parser/parser.rs | 10 +- docs/docs/noir/standard_library/meta/expr.md | 9 +- noir_stdlib/src/meta/expr.nr | 26 +++ .../comptime_expr/src/main.nr | 75 ++++++--- tooling/lsp/src/requests/completion.rs | 3 +- 16 files changed, 437 insertions(+), 71 deletions(-) diff --git a/aztec_macros/src/utils/parse_utils.rs b/aztec_macros/src/utils/parse_utils.rs index 018307bfb86..dce3af1402b 100644 --- a/aztec_macros/src/utils/parse_utils.rs +++ b/aztec_macros/src/utils/parse_utils.rs @@ -401,6 +401,7 @@ fn empty_pattern(pattern: &mut Pattern) { empty_pattern(pattern); } } + Pattern::Interned(_, _) => (), } } diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index 30db8ad63fd..52c39a49e8a 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -7,13 +7,13 @@ use iter_extended::vecmap; use noirc_errors::{Span, Spanned}; use super::{ - BlockExpression, Expression, ExpressionKind, GenericTypeArgs, IndexExpression, ItemVisibility, - MemberAccessExpression, MethodCallExpression, UnresolvedType, + BlockExpression, ConstructorExpression, Expression, ExpressionKind, GenericTypeArgs, + IndexExpression, ItemVisibility, MemberAccessExpression, MethodCallExpression, UnresolvedType, }; use crate::elaborator::types::SELF_TYPE_NAME; use crate::lexer::token::SpannedToken; -use crate::macros_api::{SecondaryAttribute, UnresolvedTypeData}; -use crate::node_interner::{InternedExpressionKind, InternedStatementKind}; +use crate::macros_api::{NodeInterner, SecondaryAttribute, UnresolvedTypeData}; +use crate::node_interner::{InternedExpressionKind, InternedPattern, InternedStatementKind}; use crate::parser::{ParserError, ParserErrorReason}; use crate::token::Token; @@ -565,6 +565,7 @@ pub enum Pattern { Mutable(Box, Span, /*is_synthesized*/ bool), Tuple(Vec, Span), Struct(Path, Vec<(Ident, Pattern)>, Span), + Interned(InternedPattern, Span), } impl Pattern { @@ -577,7 +578,8 @@ impl Pattern { Pattern::Identifier(ident) => ident.span(), Pattern::Mutable(_, span, _) | Pattern::Tuple(_, span) - | Pattern::Struct(_, _, span) => *span, + | Pattern::Struct(_, _, span) + | Pattern::Interned(_, span) => *span, } } pub fn name_ident(&self) -> &Ident { @@ -595,6 +597,39 @@ impl Pattern { other => panic!("Pattern::into_ident called on {other} pattern with no identifier"), } } + + pub(crate) fn try_as_expression(&self, interner: &NodeInterner) -> Option { + match self { + Pattern::Identifier(ident) => Some(Expression { + kind: ExpressionKind::Variable(Path::from_ident(ident.clone())), + span: ident.span(), + }), + Pattern::Mutable(_, _, _) => None, + Pattern::Tuple(patterns, span) => { + let mut expressions = Vec::new(); + for pattern in patterns { + expressions.push(pattern.try_as_expression(interner)?); + } + Some(Expression { kind: ExpressionKind::Tuple(expressions), span: *span }) + } + Pattern::Struct(path, patterns, span) => { + let mut fields = Vec::new(); + for (field, pattern) in patterns { + let expression = pattern.try_as_expression(interner)?; + fields.push((field.clone(), expression)); + } + Some(Expression { + kind: ExpressionKind::Constructor(Box::new(ConstructorExpression { + type_name: path.clone(), + fields, + struct_type: None, + })), + span: *span, + }) + } + Pattern::Interned(id, _) => interner.get_pattern(*id).try_as_expression(interner), + } + } } impl Recoverable for Pattern { @@ -905,6 +940,9 @@ impl Display for Pattern { let fields = vecmap(fields, |(name, pattern)| format!("{name}: {pattern}")); write!(f, "{} {{ {} }}", typename, fields.join(", ")) } + Pattern::Interned(_, _) => { + write!(f, "?Interned") + } } } } diff --git a/compiler/noirc_frontend/src/ast/visitor.rs b/compiler/noirc_frontend/src/ast/visitor.rs index 6f54ad2e3b9..359e04e41a7 100644 --- a/compiler/noirc_frontend/src/ast/visitor.rs +++ b/compiler/noirc_frontend/src/ast/visitor.rs @@ -12,8 +12,8 @@ use crate::{ UseTreeKind, }, node_interner::{ - ExprId, InternedExpressionKind, InternedStatementKind, InternedUnresolvedTypeData, - QuotedTypeId, + ExprId, InternedExpressionKind, InternedPattern, InternedStatementKind, + InternedUnresolvedTypeData, QuotedTypeId, }, parser::{Item, ItemKind, ParsedSubModule}, token::{CustomAtrribute, SecondaryAttribute, Tokens}, @@ -440,6 +440,8 @@ pub trait Visitor { true } + fn visit_interned_pattern(&mut self, _: &InternedPattern, _: Span) {} + fn visit_secondary_attribute( &mut self, _: &SecondaryAttribute, @@ -1321,6 +1323,9 @@ impl Pattern { } } } + Pattern::Interned(id, span) => { + visitor.visit_interned_pattern(id, *span); + } } } } diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index 951c5220707..ed9265536f9 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -675,6 +675,7 @@ fn pattern_vars(pattern: &ast::Pattern) -> Vec<(ast::Ident, bool)> { stack.extend(pids.iter().map(|(_, pattern)| (pattern, is_mut))); vars.extend(pids.iter().map(|(id, _)| (id.clone(), false))); } + ast::Pattern::Interned(_, _) => (), } } vars @@ -701,6 +702,7 @@ fn pattern_to_string(pattern: &ast::Pattern) -> String { .join(", "), ) } + ast::Pattern::Interned(_, _) => "?Interned".to_string(), } } diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 06c153d4c10..f738657fd23 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -143,6 +143,17 @@ impl<'context> Elaborator<'context> { mutable, new_definitions, ), + Pattern::Interned(id, _) => { + let pattern = self.interner.get_pattern(id).clone(); + self.elaborate_pattern_mut( + pattern, + expected_type, + definition, + mutable, + new_definitions, + global_id, + ) + } } } diff --git a/compiler/noirc_frontend/src/hir/comptime/errors.rs b/compiler/noirc_frontend/src/hir/comptime/errors.rs index 70589973745..9c4761f3156 100644 --- a/compiler/noirc_frontend/src/hir/comptime/errors.rs +++ b/compiler/noirc_frontend/src/hir/comptime/errors.rs @@ -220,6 +220,14 @@ pub enum InterpreterError { duplicate_location: Location, existing_location: Location, }, + CannotResolveExpression { + location: Location, + expression: String, + }, + CannotSetFunctionBody { + location: Location, + expression: String, + }, // These cases are not errors, they are just used to prevent us from running more code // until the loop can be resumed properly. These cases will never be displayed to users. @@ -291,7 +299,9 @@ impl InterpreterError { | InterpreterError::InvalidAttribute { location, .. } | InterpreterError::GenericNameShouldBeAnIdent { location, .. } | InterpreterError::DuplicateGeneric { duplicate_location: location, .. } - | InterpreterError::TypeAnnotationsNeededForMethodCall { location } => *location, + | InterpreterError::TypeAnnotationsNeededForMethodCall { location } + | InterpreterError::CannotResolveExpression { location, .. } + | InterpreterError::CannotSetFunctionBody { location, .. } => *location, InterpreterError::FailedToParseMacro { error, file, .. } => { Location::new(error.span(), *file) @@ -626,6 +636,14 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic { ); error } + InterpreterError::CannotResolveExpression { location, expression } => { + let msg = format!("Cannot resolve expression `{expression}`"); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } + InterpreterError::CannotSetFunctionBody { location, expression } => { + let msg = format!("`{expression}` is not a valid function body"); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } } } } diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index c3aeac4aec4..899d62ecb61 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -22,8 +22,8 @@ use rustc_hash::FxHashMap as HashMap; use crate::{ ast::{ ArrayLiteral, BlockExpression, ConstrainKind, Expression, ExpressionKind, FunctionKind, - FunctionReturnType, IntegerBitSize, LValue, Literal, Statement, StatementKind, UnaryOp, - UnresolvedType, UnresolvedTypeData, Visibility, + FunctionReturnType, IntegerBitSize, LValue, Literal, Pattern, Statement, StatementKind, + UnaryOp, UnresolvedType, UnresolvedTypeData, Visibility, }, hir::def_collector::dc_crate::CollectedItems, hir::{ @@ -78,6 +78,7 @@ impl<'local, 'context> Interpreter<'local, 'context> { "expr_as_if" => expr_as_if(interner, arguments, return_type, location), "expr_as_index" => expr_as_index(interner, arguments, return_type, location), "expr_as_integer" => expr_as_integer(interner, arguments, return_type, location), + "expr_as_let" => expr_as_let(interner, arguments, return_type, location), "expr_as_member_access" => { expr_as_member_access(interner, arguments, return_type, location) } @@ -1500,6 +1501,41 @@ fn expr_as_integer( }) } +// fn as_let(self) -> Option<(Expr, Option, Expr)> +fn expr_as_let( + interner: &NodeInterner, + arguments: Vec<(Value, Location)>, + return_type: Type, + location: Location, +) -> IResult { + expr_as(interner, arguments, return_type.clone(), location, |expr| match expr { + ExprValue::Statement(StatementKind::Let(let_statement)) => { + let option_type = extract_option_generic_type(return_type); + let Type::Tuple(mut tuple_types) = option_type else { + panic!("Expected the return type option generic arg to be a tuple"); + }; + assert_eq!(tuple_types.len(), 3); + tuple_types.pop().unwrap(); + let option_type = tuple_types.pop().unwrap(); + + let typ = if let_statement.r#type.typ == UnresolvedTypeData::Unspecified { + None + } else { + Some(Value::UnresolvedType(let_statement.r#type.typ)) + }; + + let typ = option(option_type, typ).ok()?; + + Some(Value::Tuple(vec![ + Value::pattern(let_statement.pattern), + typ, + Value::expression(let_statement.expression.kind), + ])) + } + _ => None, + }) +} + // fn as_member_access(self) -> Option<(Expr, Quoted)> fn expr_as_member_access( interner: &NodeInterner, @@ -1777,27 +1813,33 @@ fn expr_resolve( interpreter.current_function }; - let value = - interpreter.elaborate_in_function(function_to_resolve_in, |elaborator| match expr_value { - ExprValue::Expression(expression_kind) => { - let expr = Expression { kind: expression_kind, span: self_argument_location.span }; - let (expr_id, _) = elaborator.elaborate_expression(expr); - Value::TypedExpr(TypedExpr::ExprId(expr_id)) - } - ExprValue::Statement(statement_kind) => { - let statement = - Statement { kind: statement_kind, span: self_argument_location.span }; - let (stmt_id, _) = elaborator.elaborate_statement(statement); - Value::TypedExpr(TypedExpr::StmtId(stmt_id)) - } - ExprValue::LValue(lvalue) => { - let expr = lvalue.as_expression(); - let (expr_id, _) = elaborator.elaborate_expression(expr); - Value::TypedExpr(TypedExpr::ExprId(expr_id)) + interpreter.elaborate_in_function(function_to_resolve_in, |elaborator| match expr_value { + ExprValue::Expression(expression_kind) => { + let expr = Expression { kind: expression_kind, span: self_argument_location.span }; + let (expr_id, _) = elaborator.elaborate_expression(expr); + Ok(Value::TypedExpr(TypedExpr::ExprId(expr_id))) + } + ExprValue::Statement(statement_kind) => { + let statement = Statement { kind: statement_kind, span: self_argument_location.span }; + let (stmt_id, _) = elaborator.elaborate_statement(statement); + Ok(Value::TypedExpr(TypedExpr::StmtId(stmt_id))) + } + ExprValue::LValue(lvalue) => { + let expr = lvalue.as_expression(); + let (expr_id, _) = elaborator.elaborate_expression(expr); + Ok(Value::TypedExpr(TypedExpr::ExprId(expr_id))) + } + ExprValue::Pattern(pattern) => { + if let Some(expression) = pattern.try_as_expression(elaborator.interner) { + let (expr_id, _) = elaborator.elaborate_expression(expression); + Ok(Value::TypedExpr(TypedExpr::ExprId(expr_id))) + } else { + let expression = Value::pattern(pattern).display(elaborator.interner).to_string(); + let location = self_argument_location; + Err(InterpreterError::CannotResolveExpression { location, expression }) } - }); - - Ok(value) + } + }) } fn unwrap_expr_value(interner: &NodeInterner, mut expr_value: ExprValue) -> ExprValue { @@ -1819,6 +1861,9 @@ fn unwrap_expr_value(interner: &NodeInterner, mut expr_value: ExprValue) -> Expr ExprValue::LValue(LValue::Interned(id, span)) => { expr_value = ExprValue::LValue(interner.get_lvalue(id, span).clone()); } + ExprValue::Pattern(Pattern::Interned(id, _)) => { + expr_value = ExprValue::Pattern(interner.get_pattern(id).clone()); + } _ => break, } } @@ -2031,6 +2076,16 @@ fn function_def_set_body( }), ExprValue::Statement(statement_kind) => statement_kind, ExprValue::LValue(lvalue) => StatementKind::Expression(lvalue.as_expression()), + ExprValue::Pattern(pattern) => { + if let Some(expression) = pattern.try_as_expression(interpreter.elaborator.interner) { + StatementKind::Expression(expression) + } else { + let expression = + Value::pattern(pattern).display(interpreter.elaborator.interner).to_string(); + let location = body_location; + return Err(InterpreterError::CannotSetFunctionBody { location, expression }); + } + } }; let statement = Statement { kind: statement_kind, span: body_location.span }; diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs index f90d50807b8..6e72866bec0 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs @@ -5,8 +5,8 @@ use noirc_errors::Location; use crate::{ ast::{ - BlockExpression, ExpressionKind, IntegerBitSize, LValue, Signedness, StatementKind, - UnresolvedTypeData, + BlockExpression, ExpressionKind, IntegerBitSize, LValue, Pattern, Signedness, + StatementKind, UnresolvedTypeData, }, elaborator::Elaborator, hir::{ @@ -191,6 +191,9 @@ pub(crate) fn get_expr( ExprValue::LValue(LValue::Interned(id, _)) => { Ok(ExprValue::LValue(interner.get_lvalue(id, location.span).clone())) } + ExprValue::Pattern(Pattern::Interned(id, _)) => { + Ok(ExprValue::Pattern(interner.get_pattern(id).clone())) + } _ => Ok(expr), }, value => type_mismatch(value, Type::Quoted(QuotedType::Expr), location), diff --git a/compiler/noirc_frontend/src/hir/comptime/value.rs b/compiler/noirc_frontend/src/hir/comptime/value.rs index 04c557552bd..f6450175955 100644 --- a/compiler/noirc_frontend/src/hir/comptime/value.rs +++ b/compiler/noirc_frontend/src/hir/comptime/value.rs @@ -1,4 +1,4 @@ -use std::{borrow::Cow, fmt::Display, rc::Rc}; +use std::{borrow::Cow, fmt::Display, rc::Rc, vec}; use acvm::{AcirField, FieldElement}; use chumsky::Parser; @@ -9,11 +9,11 @@ use strum_macros::Display; use crate::{ ast::{ - ArrayLiteral, AssignStatement, BlockExpression, CallExpression, CastExpression, - ConstrainStatement, ConstructorExpression, ForLoopStatement, ForRange, Ident, IfExpression, - IndexExpression, InfixExpression, IntegerBitSize, LValue, Lambda, LetStatement, - MemberAccessExpression, MethodCallExpression, PrefixExpression, Signedness, Statement, - StatementKind, UnresolvedTypeData, + ArrayLiteral, AsTraitPath, AssignStatement, BlockExpression, CallExpression, + CastExpression, ConstrainStatement, ConstructorExpression, ForLoopStatement, ForRange, + GenericTypeArgs, Ident, IfExpression, IndexExpression, InfixExpression, IntegerBitSize, + LValue, Lambda, LetStatement, MemberAccessExpression, MethodCallExpression, Pattern, + PrefixExpression, Signedness, Statement, StatementKind, UnresolvedType, UnresolvedTypeData, }, hir::{def_map::ModuleId, type_check::generics::TraitGenerics}, hir_def::{ @@ -78,6 +78,7 @@ pub enum ExprValue { Expression(ExpressionKind), Statement(StatementKind), LValue(LValue), + Pattern(Pattern), } #[derive(Debug, Clone, PartialEq, Eq, Display)] @@ -99,6 +100,10 @@ impl Value { Value::Expr(ExprValue::LValue(lvaue)) } + pub(crate) fn pattern(pattern: Pattern) -> Self { + Value::Expr(ExprValue::Pattern(pattern)) + } + pub(crate) fn get_type(&self) -> Cow { Cow::Owned(match self { Value::Unit => Type::Unit, @@ -273,7 +278,8 @@ impl Value { }) } Value::Expr(ExprValue::LValue(lvalue)) => lvalue.as_expression().kind, - Value::TypedExpr(..) + Value::Expr(ExprValue::Pattern(_)) + | Value::TypedExpr(..) | Value::Pointer(..) | Value::StructDefinition(_) | Value::TraitConstraint(..) @@ -443,6 +449,9 @@ impl Value { Value::Expr(ExprValue::LValue(lvalue)) => { Token::InternedLValue(interner.push_lvalue(lvalue)) } + Value::Expr(ExprValue::Pattern(pattern)) => { + Token::InternedPattern(interner.push_pattern(pattern)) + } Value::UnresolvedType(typ) => { Token::InternedUnresolvedTypeData(interner.push_unresolved_type_data(typ)) } @@ -671,6 +680,9 @@ impl<'value, 'interner> Display for ValuePrinter<'value, 'interner> { Value::Expr(ExprValue::LValue(lvalue)) => { write!(f, "{}", remove_interned_in_lvalue(self.interner, lvalue.clone())) } + Value::Expr(ExprValue::Pattern(pattern)) => { + write!(f, "{}", remove_interned_in_pattern(self.interner, pattern.clone())) + } Value::TypedExpr(TypedExpr::ExprId(id)) => { let hir_expr = self.interner.expression(id); let expr = hir_expr.to_display_ast(self.interner, Span::default()); @@ -682,12 +694,7 @@ impl<'value, 'interner> Display for ValuePrinter<'value, 'interner> { write!(f, "{}", stmt.kind) } Value::UnresolvedType(typ) => { - if let UnresolvedTypeData::Interned(id) = typ { - let typ = self.interner.get_unresolved_type_data(*id); - write!(f, "{}", typ) - } else { - write!(f, "{}", typ) - } + write!(f, "{}", remove_interned_in_unresolved_type_data(self.interner, typ.clone())) } } } @@ -729,6 +736,10 @@ impl<'token, 'interner> Display for TokenPrinter<'token, 'interner> { let value = Value::UnresolvedType(UnresolvedTypeData::Interned(*id)); value.display(self.interner).fmt(f) } + Token::InternedPattern(id) => { + let value = Value::pattern(Pattern::Interned(*id, Span::default())); + value.display(self.interner).fmt(f) + } Token::UnquoteMarker(id) => { let value = Value::TypedExpr(TypedExpr::ExprId(*id)); value.display(self.interner).fmt(f) @@ -901,7 +912,9 @@ fn remove_interned_in_statement_kind( ) -> StatementKind { match statement { StatementKind::Let(let_statement) => StatementKind::Let(LetStatement { + pattern: remove_interned_in_pattern(interner, let_statement.pattern), expression: remove_interned_in_expression(interner, let_statement.expression), + r#type: remove_interned_in_unresolved_type(interner, let_statement.r#type), ..let_statement }), StatementKind::Constrain(constrain) => StatementKind::Constrain(ConstrainStatement( @@ -966,3 +979,120 @@ fn remove_interned_in_lvalue(interner: &NodeInterner, lvalue: LValue) -> LValue } } } + +fn remove_interned_in_unresolved_type( + interner: &NodeInterner, + typ: UnresolvedType, +) -> UnresolvedType { + UnresolvedType { + typ: remove_interned_in_unresolved_type_data(interner, typ.typ), + span: typ.span, + } +} + +fn remove_interned_in_unresolved_type_data( + interner: &NodeInterner, + typ: UnresolvedTypeData, +) -> UnresolvedTypeData { + match typ { + UnresolvedTypeData::Array(expr, typ) => UnresolvedTypeData::Array( + expr, + Box::new(remove_interned_in_unresolved_type(interner, *typ)), + ), + UnresolvedTypeData::Slice(typ) => { + UnresolvedTypeData::Slice(Box::new(remove_interned_in_unresolved_type(interner, *typ))) + } + UnresolvedTypeData::FormatString(expr, typ) => UnresolvedTypeData::FormatString( + expr, + Box::new(remove_interned_in_unresolved_type(interner, *typ)), + ), + UnresolvedTypeData::Parenthesized(typ) => UnresolvedTypeData::Parenthesized(Box::new( + remove_interned_in_unresolved_type(interner, *typ), + )), + UnresolvedTypeData::Named(path, generic_type_args, is_synthesized) => { + UnresolvedTypeData::Named( + path, + remove_interned_in_generic_type_args(interner, generic_type_args), + is_synthesized, + ) + } + UnresolvedTypeData::TraitAsType(path, generic_type_args) => { + UnresolvedTypeData::TraitAsType( + path, + remove_interned_in_generic_type_args(interner, generic_type_args), + ) + } + UnresolvedTypeData::MutableReference(typ) => UnresolvedTypeData::MutableReference( + Box::new(remove_interned_in_unresolved_type(interner, *typ)), + ), + UnresolvedTypeData::Tuple(types) => UnresolvedTypeData::Tuple(vecmap(types, |typ| { + remove_interned_in_unresolved_type(interner, typ) + })), + UnresolvedTypeData::Function(arg_types, ret_type, env_type, unconstrained) => { + UnresolvedTypeData::Function( + vecmap(arg_types, |typ| remove_interned_in_unresolved_type(interner, typ)), + Box::new(remove_interned_in_unresolved_type(interner, *ret_type)), + Box::new(remove_interned_in_unresolved_type(interner, *env_type)), + unconstrained, + ) + } + UnresolvedTypeData::AsTraitPath(as_trait_path) => { + UnresolvedTypeData::AsTraitPath(Box::new(AsTraitPath { + typ: remove_interned_in_unresolved_type(interner, as_trait_path.typ), + trait_generics: remove_interned_in_generic_type_args( + interner, + as_trait_path.trait_generics, + ), + ..*as_trait_path + })) + } + UnresolvedTypeData::Interned(id) => interner.get_unresolved_type_data(id).clone(), + UnresolvedTypeData::FieldElement + | UnresolvedTypeData::Integer(_, _) + | UnresolvedTypeData::Bool + | UnresolvedTypeData::Unit + | UnresolvedTypeData::String(_) + | UnresolvedTypeData::Resolved(_) + | UnresolvedTypeData::Quoted(_) + | UnresolvedTypeData::Expression(_) + | UnresolvedTypeData::Unspecified + | UnresolvedTypeData::Error => typ, + } +} + +fn remove_interned_in_generic_type_args( + interner: &NodeInterner, + args: GenericTypeArgs, +) -> GenericTypeArgs { + GenericTypeArgs { + ordered_args: vecmap(args.ordered_args, |typ| { + remove_interned_in_unresolved_type(interner, typ) + }), + named_args: vecmap(args.named_args, |(name, typ)| { + (name, remove_interned_in_unresolved_type(interner, typ)) + }), + } +} + +// Returns a new Pattern where all Interned Patterns have been turned into Pattern. +fn remove_interned_in_pattern(interner: &NodeInterner, pattern: Pattern) -> Pattern { + match pattern { + Pattern::Identifier(_) => pattern, + Pattern::Mutable(pattern, span, is_synthesized) => Pattern::Mutable( + Box::new(remove_interned_in_pattern(interner, *pattern)), + span, + is_synthesized, + ), + Pattern::Tuple(patterns, span) => Pattern::Tuple( + vecmap(patterns, |pattern| remove_interned_in_pattern(interner, pattern)), + span, + ), + Pattern::Struct(path, patterns, span) => { + let patterns = vecmap(patterns, |(name, pattern)| { + (name, remove_interned_in_pattern(interner, pattern)) + }); + Pattern::Struct(path, patterns, span) + } + Pattern::Interned(id, _) => interner.get_pattern(id).clone(), + } +} diff --git a/compiler/noirc_frontend/src/lexer/token.rs b/compiler/noirc_frontend/src/lexer/token.rs index 5a932de83f4..f5f7f0458d7 100644 --- a/compiler/noirc_frontend/src/lexer/token.rs +++ b/compiler/noirc_frontend/src/lexer/token.rs @@ -5,8 +5,8 @@ use std::{fmt, iter::Map, vec::IntoIter}; use crate::{ lexer::errors::LexerErrorKind, node_interner::{ - ExprId, InternedExpressionKind, InternedStatementKind, InternedUnresolvedTypeData, - QuotedTypeId, + ExprId, InternedExpressionKind, InternedPattern, InternedStatementKind, + InternedUnresolvedTypeData, QuotedTypeId, }, }; @@ -36,6 +36,7 @@ pub enum BorrowedToken<'input> { InternedStatement(InternedStatementKind), InternedLValue(InternedExpressionKind), InternedUnresolvedTypeData(InternedUnresolvedTypeData), + InternedPattern(InternedPattern), /// < Less, /// <= @@ -151,6 +152,8 @@ pub enum Token { InternedLValue(InternedExpressionKind), /// A reference to an interned `UnresolvedTypeData`. InternedUnresolvedTypeData(InternedUnresolvedTypeData), + /// A reference to an interned `Patter`. + InternedPattern(InternedPattern), /// < Less, /// <= @@ -255,6 +258,7 @@ pub fn token_to_borrowed_token(token: &Token) -> BorrowedToken<'_> { Token::InternedStatement(id) => BorrowedToken::InternedStatement(*id), Token::InternedLValue(id) => BorrowedToken::InternedLValue(*id), Token::InternedUnresolvedTypeData(id) => BorrowedToken::InternedUnresolvedTypeData(*id), + Token::InternedPattern(id) => BorrowedToken::InternedPattern(*id), Token::IntType(ref i) => BorrowedToken::IntType(i.clone()), Token::Less => BorrowedToken::Less, Token::LessEqual => BorrowedToken::LessEqual, @@ -378,7 +382,10 @@ impl fmt::Display for Token { } // Quoted types and exprs only have an ID so there is nothing to display Token::QuotedType(_) => write!(f, "(type)"), - Token::InternedExpr(_) | Token::InternedStatement(_) | Token::InternedLValue(_) => { + Token::InternedExpr(_) + | Token::InternedStatement(_) + | Token::InternedLValue(_) + | Token::InternedPattern(_) => { write!(f, "(expr)") } Token::InternedUnresolvedTypeData(_) => write!(f, "(type)"), @@ -439,6 +446,7 @@ pub enum TokenKind { InternedStatement, InternedLValue, InternedUnresolvedTypeData, + InternedPattern, UnquoteMarker, OuterDocComment, InnerDocComment, @@ -459,6 +467,7 @@ impl fmt::Display for TokenKind { TokenKind::InternedStatement => write!(f, "interned statement"), TokenKind::InternedLValue => write!(f, "interned lvalue"), TokenKind::InternedUnresolvedTypeData => write!(f, "interned unresolved type"), + TokenKind::InternedPattern => write!(f, "interned pattern"), TokenKind::UnquoteMarker => write!(f, "macro result"), TokenKind::OuterDocComment => write!(f, "outer doc comment"), TokenKind::InnerDocComment => write!(f, "inner doc comment"), @@ -485,6 +494,7 @@ impl Token { Token::InternedStatement(_) => TokenKind::InternedStatement, Token::InternedLValue(_) => TokenKind::InternedLValue, Token::InternedUnresolvedTypeData(_) => TokenKind::InternedUnresolvedTypeData, + Token::InternedPattern(_) => TokenKind::InternedPattern, Token::LineComment(_, Some(DocStyle::Outer)) | Token::BlockComment(_, Some(DocStyle::Outer)) => TokenKind::OuterDocComment, Token::LineComment(_, Some(DocStyle::Inner)) diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 881c5e6251a..f298559e65c 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -16,6 +16,7 @@ use rustc_hash::FxHashMap as HashMap; use crate::ast::ExpressionKind; use crate::ast::Ident; use crate::ast::LValue; +use crate::ast::Pattern; use crate::ast::StatementKind; use crate::ast::UnresolvedTypeData; use crate::graph::CrateId; @@ -222,6 +223,9 @@ pub struct NodeInterner { // Interned `UnresolvedTypeData`s during comptime code. interned_unresolved_type_datas: noirc_arena::Arena, + // Interned `Pattern`s during comptime code. + interned_patterns: noirc_arena::Arena, + /// Determins whether to run in LSP mode. In LSP mode references are tracked. pub(crate) lsp_mode: bool, @@ -610,6 +614,9 @@ pub struct InternedStatementKind(noirc_arena::Index); #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct InternedUnresolvedTypeData(noirc_arena::Index); +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct InternedPattern(noirc_arena::Index); + impl Default for NodeInterner { fn default() -> Self { NodeInterner { @@ -650,6 +657,7 @@ impl Default for NodeInterner { interned_expression_kinds: Default::default(), interned_statement_kinds: Default::default(), interned_unresolved_type_datas: Default::default(), + interned_patterns: Default::default(), lsp_mode: false, location_indices: LocationIndices::default(), reference_graph: petgraph::graph::DiGraph::new(), @@ -2101,6 +2109,14 @@ impl NodeInterner { LValue::from_expression_kind(self.get_expression_kind(id).clone(), span) } + pub fn push_pattern(&mut self, pattern: Pattern) -> InternedPattern { + InternedPattern(self.interned_patterns.insert(pattern)) + } + + pub fn get_pattern(&self, id: InternedPattern) -> &Pattern { + &self.interned_patterns[id.0] + } + pub fn push_unresolved_type_data( &mut self, typ: UnresolvedTypeData, diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 317eb01f21e..0ffeb691d35 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -594,7 +594,15 @@ pub fn pattern() -> impl NoirParser { .delimited_by(just(Token::LeftParen), just(Token::RightParen)) .map_with_span(Pattern::Tuple); - choice((mut_pattern, tuple_pattern, struct_pattern, ident_pattern)) + let interned = + token_kind(TokenKind::InternedPattern).map_with_span(|token, span| match token { + Token::InternedPattern(id) => Pattern::Interned(id, span), + _ => unreachable!( + "token_kind(InternedPattern) guarantees we parse an interned pattern" + ), + }); + + choice((mut_pattern, tuple_pattern, struct_pattern, ident_pattern, interned)) }) .labelled(ParsingRuleLabel::Pattern) } diff --git a/docs/docs/noir/standard_library/meta/expr.md b/docs/docs/noir/standard_library/meta/expr.md index 3a3c61b41f5..7ee33027354 100644 --- a/docs/docs/noir/standard_library/meta/expr.md +++ b/docs/docs/noir/standard_library/meta/expr.md @@ -85,9 +85,16 @@ array and the index. #include_code as_integer noir_stdlib/src/meta/expr.nr rust -If this element is an integer literal, return the integer as a field +If this expression is an integer literal, return the integer as a field as well as whether the integer is negative (true) or not (false). +### as_let + +#include_code as_let noir_stdlib/src/meta/expr.nr rust + +If this expression is a let statement, returns the let pattern as an `Expr`, +the optional type annotation, and the assigned expression. + ### as_member_access #include_code as_member_access noir_stdlib/src/meta/expr.nr rust diff --git a/noir_stdlib/src/meta/expr.nr b/noir_stdlib/src/meta/expr.nr index 43638ad791b..96a83f954c9 100644 --- a/noir_stdlib/src/meta/expr.nr +++ b/noir_stdlib/src/meta/expr.nr @@ -66,6 +66,11 @@ impl Expr { fn as_index(self) -> Option<(Expr, Expr)> {} // docs:end:as_index + #[builtin(expr_as_let)] + // docs:start:as_let + fn as_let(self) -> Option<(Expr, Option, Expr)> {} + // docs:end:as_let + #[builtin(expr_as_member_access)] // docs:start:as_member_access fn as_member_access(self) -> Option<(Expr, Quoted)> {} @@ -134,6 +139,7 @@ impl Expr { let result = result.or_else(|| modify_comptime(self, f)); let result = result.or_else(|| modify_if(self, f)); let result = result.or_else(|| modify_index(self, f)); + let result = result.or_else(|| modify_let(self, f)); let result = result.or_else(|| modify_function_call(self, f)); let result = result.or_else(|| modify_member_access(self, f)); let result = result.or_else(|| modify_method_call(self, f)); @@ -280,6 +286,17 @@ fn modify_index(expr: Expr, f: fn[Env](Expr) -> Option) -> Option(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { + expr.as_let().map( + |expr: (Expr, Option, Expr)| { + let (pattern, typ, expr) = expr; + let pattern = pattern.modify(f); + let expr = expr.modify(f); + new_let(pattern, typ, expr) + } + ) +} + fn modify_member_access(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_member_access().map( |expr: (Expr, Quoted)| { @@ -423,6 +440,15 @@ fn new_index(object: Expr, index: Expr) -> Expr { quote { $object[$index] }.as_expr().unwrap() } +fn new_let(pattern: Expr, typ: Option, expr: Expr) -> Expr { + if typ.is_some() { + let typ = typ.unwrap(); + quote { let $pattern : $typ = $expr; }.as_expr().unwrap() + } else { + quote { let $pattern = $expr; }.as_expr().unwrap() + } +} + fn new_member_access(object: Expr, name: Quoted) -> Expr { quote { $object.$name }.as_expr().unwrap() } diff --git a/test_programs/noir_test_success/comptime_expr/src/main.nr b/test_programs/noir_test_success/comptime_expr/src/main.nr index c082c1dde33..c1f70e7acee 100644 --- a/test_programs/noir_test_success/comptime_expr/src/main.nr +++ b/test_programs/noir_test_success/comptime_expr/src/main.nr @@ -16,7 +16,7 @@ mod tests { } #[test] - fn test_expr_mutate_for_array() { + fn test_expr_modify_for_array() { comptime { let expr = quote { [1, 2, 4] }.as_expr().unwrap(); @@ -46,7 +46,7 @@ mod tests { } #[test] - fn test_expr_mutate_for_assert() { + fn test_expr_modify_for_assert() { comptime { let expr = quote { assert(1) }.as_expr().unwrap(); @@ -82,7 +82,7 @@ mod tests { } #[test] - fn test_expr_mutate_for_assert_eq() { + fn test_expr_modify_for_assert_eq() { comptime { let expr = quote { assert_eq(1, 2) }.as_expr().unwrap(); @@ -113,7 +113,7 @@ mod tests { } #[test] - fn test_expr_mutate_for_assign() { + fn test_expr_modify_for_assign() { comptime { let expr = quote { { a = 1; } }.as_expr().unwrap(); @@ -142,7 +142,7 @@ mod tests { } #[test] - fn test_expr_mutate_for_block() { + fn test_expr_modify_for_block() { comptime { let expr = quote { { 1; 4; 23 } }.as_expr().unwrap(); @@ -178,7 +178,7 @@ mod tests { } #[test] - fn test_expr_mutate_for_method_call() { + fn test_expr_modify_for_method_call() { comptime { let expr = quote { foo.bar(3, 4) }.as_expr().unwrap(); @@ -209,7 +209,7 @@ mod tests { } #[test] - fn test_expr_mutate_for_integer() { + fn test_expr_modify_for_integer() { comptime { let expr = quote { 1 }.as_expr().unwrap(); @@ -243,7 +243,7 @@ mod tests { } #[test] - fn test_expr_mutate_for_binary_op() { + fn test_expr_modify_for_binary_op() { comptime { let expr = quote { 3 + 4 }.as_expr().unwrap(); @@ -280,7 +280,7 @@ mod tests { } #[test] - fn test_expr_mutate_for_cast() { + fn test_expr_modify_for_cast() { comptime { let expr = quote { 1 as Field }.as_expr().unwrap(); @@ -302,7 +302,7 @@ mod tests { } #[test] - fn test_expr_mutate_for_comptime() { + fn test_expr_modify_for_comptime() { comptime { let expr = quote { comptime { 1; 4; 23 } }.as_expr().unwrap(); @@ -342,7 +342,7 @@ mod tests { // docs:end:as_expr_example #[test] - fn test_expr_mutate_for_function_call() { + fn test_expr_modify_for_function_call() { comptime { let expr = quote { foo(42) }.as_expr().unwrap(); @@ -368,7 +368,7 @@ mod tests { } #[test] - fn test_expr_mutate_for_if() { + fn test_expr_modify_for_if() { comptime { let expr = quote { if 1 { 2 } }.as_expr().unwrap(); @@ -400,7 +400,7 @@ mod tests { } #[test] - fn test_expr_mutate_for_index() { + fn test_expr_modify_for_index() { comptime { let expr = quote { 1[2] }.as_expr().unwrap(); @@ -422,7 +422,7 @@ mod tests { } #[test] - fn test_expr_mutate_for_member_access() { + fn test_expr_modify_for_member_access() { comptime { let expr = quote { 1.bar }.as_expr().unwrap(); @@ -457,7 +457,7 @@ mod tests { } #[test] - fn test_expr_mutate_for_repeated_element_array() { + fn test_expr_modify_for_repeated_element_array() { comptime { let expr = quote { [1; 3] }.as_expr().unwrap(); @@ -480,7 +480,7 @@ mod tests { } #[test] - fn test_expr_mutate_for_repeated_element_slice() { + fn test_expr_modify_for_repeated_element_slice() { comptime { let expr = quote { &[1; 3] }.as_expr().unwrap(); @@ -505,7 +505,7 @@ mod tests { } #[test] - fn test_expr_mutate_for_slice() { + fn test_expr_modify_for_slice() { comptime { let expr = quote { &[1, 3, 5] }.as_expr().unwrap(); @@ -529,7 +529,7 @@ mod tests { } #[test] - fn test_expr_mutate_for_tuple() { + fn test_expr_modify_for_tuple() { comptime { let expr = quote { (1, 2) }.as_expr().unwrap(); @@ -553,7 +553,7 @@ mod tests { } #[test] - fn test_expr_mutate_for_unary_op() { + fn test_expr_modify_for_unary_op() { comptime { let expr = quote { -(1) }.as_expr().unwrap(); @@ -575,7 +575,7 @@ mod tests { } #[test] - fn test_expr_mutate_for_unsafe() { + fn test_expr_modify_for_unsafe() { comptime { let expr = quote { unsafe { 1; 4; 23 } }.as_expr().unwrap(); @@ -606,6 +606,41 @@ mod tests { } } + #[test] + fn test_expr_as_let() { + comptime + { + let expr = quote { let x: Field = 1; }.as_expr().unwrap(); + let (_pattern, typ, expr) = expr.as_let().unwrap(); + assert(typ.unwrap().is_field()); + assert_eq(expr.as_integer().unwrap(), (1, false)); + } + } + + #[test] + fn test_expr_modify_for_let() { + comptime + { + let expr = quote { let x : Field = 1; }.as_expr().unwrap(); + let expr = expr.modify(times_two); + let (_pattern, typ, expr) = expr.as_let().unwrap(); + assert(typ.unwrap().is_field()); + assert_eq(expr.as_integer().unwrap(), (2, false)); + } + } + + #[test] + fn test_expr_modify_for_let_without_type() { + comptime + { + let expr = quote { let x = 1; }.as_expr().unwrap(); + let expr = expr.modify(times_two); + let (_pattern, typ, expr) = expr.as_let().unwrap(); + assert(typ.is_none()); + assert_eq(expr.as_integer().unwrap(), (2, false)); + } + } + #[test] fn test_automatically_unwraps_parenthesized_expression() { comptime diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index f861c8113df..de5c36bc59f 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -508,6 +508,7 @@ impl<'a> NodeFinder<'a> { self.collect_local_variables(pattern); } } + Pattern::Interned(..) => (), } } @@ -855,7 +856,7 @@ impl<'a> NodeFinder<'a> { } } Pattern::Mutable(pattern, ..) => self.try_set_self_type(pattern), - Pattern::Tuple(..) | Pattern::Struct(..) => (), + Pattern::Tuple(..) | Pattern::Struct(..) | Pattern::Interned(..) => (), } } From ec442d45e4e893c1b5ad30be3e9e2ed9bd048acf Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 9 Sep 2024 13:05:24 -0300 Subject: [PATCH 04/13] chore: document BoundedVec (#5974) # Description ## Problem Part of #5797 ## Summary This PR copies the docs from the `.md` file to the `.nr` file. ## Additional Context I'll be sending one PR per `.md` file. It looks like they are 36 in total so it will take some time to port all the docs, but not that much. And this way reviewing these PRs will be more manageable. For now examples are copied inline into the `.nr` file. Eventually we could maybe have `nargo test` run or at least type-check these example ## Documentation Check one: - [ ] No documentation needed. - [x] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- noir_stdlib/src/collections/bounded_vec.nr | 314 +++++++++++++++++- .../noir_test_success/bounded_vec/src/main.nr | 12 +- 2 files changed, 307 insertions(+), 19 deletions(-) diff --git a/noir_stdlib/src/collections/bounded_vec.nr b/noir_stdlib/src/collections/bounded_vec.nr index a4c0f642a82..fede1b17c07 100644 --- a/noir_stdlib/src/collections/bounded_vec.nr +++ b/noir_stdlib/src/collections/bounded_vec.nr @@ -1,45 +1,188 @@ use crate::{cmp::Eq, convert::From}; +/// A `BoundedVec` is a growable storage similar to a `Vec` except that it +/// is bounded with a maximum possible length. Unlike `Vec`, `BoundedVec` is not implemented +/// via slices and thus is not subject to the same restrictions slices are (notably, nested +/// slices - and thus nested vectors as well - are disallowed). +/// +/// Since a BoundedVec is backed by a normal array under the hood, growing the BoundedVec by +/// pushing an additional element is also more efficient - the length only needs to be increased +/// by one. +/// +/// For these reasons `BoundedVec` should generally be preferred over `Vec` when there +/// is a reasonable maximum bound that can be placed on the vector. +/// +/// Example: +/// +/// ```noir +/// let mut vector: BoundedVec = BoundedVec::new(); +/// for i in 0..5 { +/// vector.push(i); +/// } +/// assert(vector.len() == 5); +/// assert(vector.max_len() == 10); +/// ``` struct BoundedVec { storage: [T; MaxLen], len: u32, } impl BoundedVec { + /// Creates a new, empty vector of length zero. + /// + /// Since this container is backed by an array internally, it still needs an initial value + /// to give each element. To resolve this, each element is zeroed internally. This value + /// is guaranteed to be inaccessible unless `get_unchecked` is used. + /// + /// Example: + /// + /// ```noir + /// let empty_vector: BoundedVec = BoundedVec::new(); + /// assert(empty_vector.len() == 0); + /// ``` + /// + /// Note that whenever calling `new` the maximum length of the vector should always be specified + /// via a type signature: + /// + /// ```noir + /// fn good() -> BoundedVec { + /// // Ok! MaxLen is specified with a type annotation + /// let v1: BoundedVec = BoundedVec::new(); + /// let v2 = BoundedVec::new(); + /// + /// // Ok! MaxLen is known from the type of `good`'s return value + /// v2 + /// } + /// + /// fn bad() { + /// // Error: Type annotation needed + /// // The compiller can't infer `MaxLen` from the following code: + /// let mut v3 = BoundedVec::new(); + /// v3.push(5); + /// } + /// ``` + /// + /// This defaulting of `MaxLen` (and numeric generics in general) to zero may change in future noir versions + /// but for now make sure to use type annotations when using bounded vectors. Otherwise, you will receive a + /// constraint failure at runtime when the vec is pushed to. pub fn new() -> Self { let zeroed = crate::mem::zeroed(); BoundedVec { storage: [zeroed; MaxLen], len: 0 } } - /// Get an element from the vector at the given index. - /// Panics if the given index points beyond the end of the vector (`self.len()`). + /// Retrieves an element from the vector at the given index, starting from zero. + /// + /// If the given index is equal to or greater than the length of the vector, this + /// will issue a constraint failure. + /// + /// Example: + /// + /// ```noir + /// fn foo(v: BoundedVec) { + /// let first = v.get(0); + /// let last = v.get(v.len() - 1); + /// assert(first != last); + /// } + /// ``` pub fn get(self, index: u32) -> T { assert(index < self.len, "Attempted to read past end of BoundedVec"); self.get_unchecked(index) } - /// Get an element from the vector at the given index. - /// Responds with undefined data for `index` where `self.len < index < self.max_len()`. + /// Retrieves an element from the vector at the given index, starting from zero, without + /// performing a bounds check. + /// + /// Since this function does not perform a bounds check on length before accessing the element, + /// it is unsafe! Use at your own risk! + /// + /// Example: + /// + /// ```noir + /// fn sum_of_first_three(v: BoundedVec) -> u32 { + /// // Always ensure the length is larger than the largest + /// // index passed to get_unchecked + /// assert(v.len() > 2); + /// let first = v.get_unchecked(0); + /// let second = v.get_unchecked(1); + /// let third = v.get_unchecked(2); + /// first + second + third + /// } + /// ``` pub fn get_unchecked(self, index: u32) -> T { self.storage[index] } - /// Write an element to the vector at the given index. - /// Panics if the given index points beyond the end of the vector (`self.len()`). + /// Writes an element to the vector at the given index, starting from zero. + /// + /// If the given index is equal to or greater than the length of the vector, this will issue a constraint failure. + /// + /// Example: + /// + /// ```noir + /// fn foo(v: BoundedVec) { + /// let first = v.get(0); + /// assert(first != 42); + /// v.set(0, 42); + /// let new_first = v.get(0); + /// assert(new_first == 42); + /// } + /// ``` pub fn set(&mut self, index: u32, value: T) { assert(index < self.len, "Attempted to write past end of BoundedVec"); self.set_unchecked(index, value) } - /// Write an element to the vector at the given index. - /// Does not check whether the passed `index` is a valid index within the vector. - /// - /// Silently writes past the end of the vector for `index` where `self.len < index < self.max_len()` - /// Panics if the given index points beyond the maximum length of the vector (`self.max_len()`). + /// Writes an element to the vector at the given index, starting from zero, without performing a bounds check. + /// + /// Since this function does not perform a bounds check on length before accessing the element, it is unsafe! Use at your own risk! + /// + /// Example: + /// + /// ```noir + /// fn set_unchecked_example() { + /// let mut vec: BoundedVec = BoundedVec::new(); + /// vec.extend_from_array([1, 2]); + /// + /// // Here we're safely writing within the valid range of `vec` + /// // `vec` now has the value [42, 2] + /// vec.set_unchecked(0, 42); + /// + /// // We can then safely read this value back out of `vec`. + /// // Notice that we use the checked version of `get` which would prevent reading unsafe values. + /// assert_eq(vec.get(0), 42); + /// + /// // We've now written past the end of `vec`. + /// // As this index is still within the maximum potential length of `v`, + /// // it won't cause a constraint failure. + /// vec.set_unchecked(2, 42); + /// println(vec); + /// + /// // This will write past the end of the maximum potential length of `vec`, + /// // it will then trigger a constraint failure. + /// vec.set_unchecked(5, 42); + /// println(vec); + /// } + /// ``` pub fn set_unchecked(&mut self, index: u32, value: T) { self.storage[index] = value; } + /// Pushes an element to the end of the vector. This increases the length + /// of the vector by one. + /// + /// Panics if the new length of the vector will be greater than the max length. + /// + /// Example: + /// + /// ```noir + /// let mut v: BoundedVec = BoundedVec::new(); + /// + /// v.push(1); + /// v.push(2); + /// + /// // Panics with failed assertion "push out of bounds" + /// v.push(3); + /// ``` pub fn push(&mut self, elem: T) { assert(self.len < MaxLen, "push out of bounds"); @@ -47,20 +190,82 @@ impl BoundedVec { self.len += 1; } + /// Returns the current length of this vector + /// + /// Example: + /// + /// ```noir + /// let mut v: BoundedVec = BoundedVec::new(); + /// assert(v.len() == 0); + /// + /// v.push(100); + /// assert(v.len() == 1); + /// + /// v.push(200); + /// v.push(300); + /// v.push(400); + /// assert(v.len() == 4); + /// + /// let _ = v.pop(); + /// let _ = v.pop(); + /// assert(v.len() == 2); + /// ``` pub fn len(self) -> u32 { self.len } + /// Returns the maximum length of this vector. This is always + /// equal to the `MaxLen` parameter this vector was initialized with. + /// + /// Example: + /// + /// ```noir + /// let mut v: BoundedVec = BoundedVec::new(); + /// + /// assert(v.max_len() == 5); + /// v.push(10); + /// assert(v.max_len() == 5); + /// ``` pub fn max_len(_self: BoundedVec) -> u32 { MaxLen } - // This is a intermediate method, while we don't have an - // .extend method + /// Returns the internal array within this vector. + /// + /// Since arrays in Noir are immutable, mutating the returned storage array will not mutate + /// the storage held internally by this vector. + /// + /// Note that uninitialized elements may be zeroed out! + /// + /// Example: + /// + /// ```noir + /// let mut v: BoundedVec = BoundedVec::new(); + /// + /// assert(v.storage() == [0, 0, 0, 0, 0]); + /// + /// v.push(57); + /// assert(v.storage() == [57, 0, 0, 0, 0]); + /// ``` pub fn storage(self) -> [T; MaxLen] { self.storage } + /// Pushes each element from the given array to this vector. + /// + /// Panics if pushing each element would cause the length of this vector + /// to exceed the maximum length. + /// + /// Example: + /// + /// ```noir + /// let mut vec: BoundedVec = BoundedVec::new(); + /// vec.extend_from_array([2, 4]); + /// + /// assert(vec.len == 2); + /// assert(vec.get(0) == 2); + /// assert(vec.get(1) == 4); + /// ``` pub fn extend_from_array(&mut self, array: [T; Len]) { let new_len = self.len + array.len(); assert(new_len <= MaxLen, "extend_from_array out of bounds"); @@ -70,6 +275,21 @@ impl BoundedVec { self.len = new_len; } + /// Pushes each element from the given slice to this vector. + /// + /// Panics if pushing each element would cause the length of this vector + /// to exceed the maximum length. + /// + /// Example: + /// + /// ```noir + /// let mut vec: BoundedVec = BoundedVec::new(); + /// vec.extend_from_slice(&[2, 4]); + /// + /// assert(vec.len == 2); + /// assert(vec.get(0) == 2); + /// assert(vec.get(1) == 4); + /// ``` pub fn extend_from_slice(&mut self, slice: [T]) { let new_len = self.len + slice.len(); assert(new_len <= MaxLen, "extend_from_slice out of bounds"); @@ -79,6 +299,22 @@ impl BoundedVec { self.len = new_len; } + /// Pushes each element from the other vector to this vector. The length of + /// the other vector is left unchanged. + /// + /// Panics if pushing each element would cause the length of this vector + /// to exceed the maximum length. + /// + /// ```noir + /// let mut v1: BoundedVec = BoundedVec::new(); + /// let mut v2: BoundedVec = BoundedVec::new(); + /// + /// v2.extend_from_array([1, 2, 3]); + /// v1.extend_from_bounded_vec(v2); + /// + /// assert(v1.storage() == [1, 2, 3, 0, 0]); + /// assert(v2.storage() == [1, 2, 3, 0, 0, 0, 0]); + /// ``` pub fn extend_from_bounded_vec(&mut self, vec: BoundedVec) { let append_len = vec.len(); let new_len = self.len + append_len; @@ -94,6 +330,14 @@ impl BoundedVec { self.len = new_len; } + /// Creates a new vector, populating it with values derived from an array input. + /// The maximum length of the vector is determined based on the type signature. + /// + /// Example: + /// + /// ```noir + /// let bounded_vec: BoundedVec = BoundedVec::from_array([1, 2, 3]) + /// ``` pub fn from_array(array: [T; Len]) -> Self { assert(Len <= MaxLen, "from array out of bounds"); let mut vec: BoundedVec = BoundedVec::new(); @@ -101,6 +345,27 @@ impl BoundedVec { vec } + /// Pops the element at the end of the vector. This will decrease the length + /// of the vector by one. + /// + /// Panics if the vector is empty. + /// + /// Example: + /// + /// ```noir + /// let mut v: BoundedVec = BoundedVec::new(); + /// v.push(1); + /// v.push(2); + /// + /// let two = v.pop(); + /// let one = v.pop(); + /// + /// assert(two == 2); + /// assert(one == 1); + /// + /// // error: cannot pop from an empty vector + /// let _ = v.pop(); + /// ``` pub fn pop(&mut self) -> T { assert(self.len > 0); self.len -= 1; @@ -110,6 +375,18 @@ impl BoundedVec { elem } + /// Returns true if the given predicate returns true for any element + /// in this vector. + /// + /// Example: + /// + /// ```noir + /// let mut v: BoundedVec = BoundedVec::new(); + /// v.extend_from_array([2, 4, 6]); + /// + /// let all_even = !v.any(|elem: u32| elem % 2 != 0); + /// assert(all_even); + /// ``` pub fn any(self, predicate: fn[Env](T) -> bool) -> bool { let mut ret = false; let mut exceeded_len = false; @@ -122,6 +399,17 @@ impl BoundedVec { ret } + /// Creates a new vector of equal size by calling a closure on each element in this vector. + /// + /// Example: + /// + /// ```noir + /// let vec: BoundedVec = BoundedVec::from_array([1, 2, 3, 4]); + /// let result = vec.map(|value| value * 2); + /// + /// let expected = BoundedVec::from_array([2, 4, 6, 8]); + /// assert_eq(result, expected); + /// ``` pub fn map(self, f: fn[Env](T) -> U) -> BoundedVec { let mut ret = BoundedVec::new(); ret.len = self.len(); diff --git a/test_programs/noir_test_success/bounded_vec/src/main.nr b/test_programs/noir_test_success/bounded_vec/src/main.nr index e5aa5f88a94..7b3e63df072 100644 --- a/test_programs/noir_test_success/bounded_vec/src/main.nr +++ b/test_programs/noir_test_success/bounded_vec/src/main.nr @@ -1,6 +1,6 @@ #[test] -fn test_vec_new_foo() { - foo(); +fn test_vec_new_good() { + good(); } #[test(should_fail)] @@ -9,19 +9,19 @@ fn test_vec_new_bad() { } // docs:start:new_example -fn foo() -> BoundedVec { +fn good() -> BoundedVec { // Ok! MaxLen is specified with a type annotation let v1: BoundedVec = BoundedVec::new(); let v2 = BoundedVec::new(); - // Ok! MaxLen is known from the type of foo's return value + // Ok! MaxLen is known from the type of `good`'s return value v2 } fn bad() { + // Error: Type annotation needed + // The compiller can't infer `MaxLen` from this code. let mut v3 = BoundedVec::new(); - - // Not Ok! We don't know if v3's MaxLen is at least 1, and the compiler often infers 0 by default. v3.push(5); } // docs:end:new_example From ec24917bfda55746c7509dd28f8d808f97c948b8 Mon Sep 17 00:00:00 2001 From: jfecher Date: Mon, 9 Sep 2024 11:58:04 -0500 Subject: [PATCH 05/13] fix: Error when comptime functions are used in runtime code (#5976) # Description ## Problem\* Resolves ## Summary\* Adds an error during monomorphization if any comptime functions fall through the cracks and aren't executed at compile-time. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- .../noirc_frontend/src/monomorphization/errors.rs | 8 ++++++++ compiler/noirc_frontend/src/monomorphization/mod.rs | 12 ++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/monomorphization/errors.rs b/compiler/noirc_frontend/src/monomorphization/errors.rs index ce8ef3572e6..66db72eef55 100644 --- a/compiler/noirc_frontend/src/monomorphization/errors.rs +++ b/compiler/noirc_frontend/src/monomorphization/errors.rs @@ -8,6 +8,7 @@ pub enum MonomorphizationError { NoDefaultType { location: Location }, InternalError { message: &'static str, location: Location }, InterpreterError(InterpreterError), + ComptimeFnInRuntimeCode { name: String, location: Location }, } impl MonomorphizationError { @@ -15,6 +16,7 @@ impl MonomorphizationError { match self { MonomorphizationError::UnknownArrayLength { location, .. } | MonomorphizationError::InternalError { location, .. } + | MonomorphizationError::ComptimeFnInRuntimeCode { location, .. } | MonomorphizationError::NoDefaultType { location, .. } => *location, MonomorphizationError::InterpreterError(error) => error.get_location(), } @@ -43,6 +45,12 @@ impl MonomorphizationError { } MonomorphizationError::InterpreterError(error) => return error.into(), MonomorphizationError::InternalError { message, .. } => message.to_string(), + MonomorphizationError::ComptimeFnInRuntimeCode { name, location } => { + let message = format!("Comptime function {name} used in runtime code"); + let secondary = + "Comptime functions must be in a comptime block to be called".into(); + return CustomDiagnostic::simple_error(message, secondary, location.span); + } }; let location = self.location(); diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 87b55540bbd..9357cc65c14 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -133,7 +133,7 @@ pub fn monomorphize_debug( let impl_bindings = perform_impl_bindings(interner, trait_method, next_fn_id, location) .map_err(MonomorphizationError::InterpreterError)?; - monomorphizer.function(next_fn_id, new_id)?; + monomorphizer.function(next_fn_id, new_id, location)?; undo_instantiation_bindings(impl_bindings); undo_instantiation_bindings(bindings); } @@ -278,7 +278,10 @@ impl<'interner> Monomorphizer<'interner> { ) -> Result { let new_main_id = self.next_function_id(); assert_eq!(new_main_id, Program::main_id()); - self.function(main_id, new_main_id)?; + + let location = self.interner.function_meta(&main_id).location; + self.function(main_id, new_main_id, location)?; + self.return_location = self.interner.function(&main_id).block(self.interner).statements().last().and_then( |x| match self.interner.statement(x) { @@ -294,6 +297,7 @@ impl<'interner> Monomorphizer<'interner> { &mut self, f: node_interner::FuncId, id: FuncId, + location: Location, ) -> Result<(), MonomorphizationError> { if let Some((self_type, trait_id)) = self.interner.get_function_trait(&f) { let the_trait = self.interner.get_trait(trait_id); @@ -313,6 +317,10 @@ impl<'interner> Monomorphizer<'interner> { let modifiers = self.interner.function_modifiers(&f); let name = self.interner.function_name(&f).to_owned(); + if modifiers.is_comptime { + return Err(MonomorphizationError::ComptimeFnInRuntimeCode { name, location }); + } + let body_expr_id = self.interner.function(&f).as_expr(); let body_return_type = self.interner.id_type(body_expr_id); let return_type = match meta.return_type() { From a1b1346bf7525c508fd390393c307475cc2345d7 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Mon, 9 Sep 2024 13:38:34 -0400 Subject: [PATCH 06/13] fix: Restrict keccak256_injective test input to 8 bits (#5977) # Description ## Problem\* Resolves the keccak256_injective test being flakey ## Summary\* Makes the keccak256_injective test not flakey. ## Additional Context ## Documentation\* Check one: - [X] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [X] I have tested the changes locally. - [X] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- acvm-repo/acvm/tests/solver.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acvm-repo/acvm/tests/solver.rs b/acvm-repo/acvm/tests/solver.rs index 2a06e07f092..5a15586bf1b 100644 --- a/acvm-repo/acvm/tests/solver.rs +++ b/acvm-repo/acvm/tests/solver.rs @@ -1584,7 +1584,7 @@ proptest! { #[test] fn keccak256_injective(inputs_distinct_inputs in any_distinct_inputs(Some(8), 0, 32)) { let (inputs, distinct_inputs) = inputs_distinct_inputs; - let (result, message) = prop_assert_injective(inputs, distinct_inputs, 32, Some(32), keccak256_op); + let (result, message) = prop_assert_injective(inputs, distinct_inputs, 32, Some(8), keccak256_op); prop_assert!(result, "{}", message); } From 2adc24be8f191f6315c6357283501aef67f48a9b Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 9 Sep 2024 15:13:16 -0300 Subject: [PATCH 07/13] chore: document HashMap (#5984) # Description ## Problem Part of https://github.com/noir-lang/noir/issues/5797 ## Summary ## Additional Context ## Documentation Check one: - [ ] No documentation needed. - [x] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- .../standard_library/containers/hashmap.md | 5 - noir_stdlib/src/collections/map.nr | 293 ++++++++++++++++-- 2 files changed, 268 insertions(+), 30 deletions(-) diff --git a/docs/docs/noir/standard_library/containers/hashmap.md b/docs/docs/noir/standard_library/containers/hashmap.md index 651e7f5723b..70590ad30d8 100644 --- a/docs/docs/noir/standard_library/containers/hashmap.md +++ b/docs/docs/noir/standard_library/containers/hashmap.md @@ -11,11 +11,6 @@ Note that due to hash collisions, the actual maximum number of elements stored b hashmap is likely lower than `MaxLen`. This is true even with cryptographic hash functions since every hash value will be performed modulo `MaxLen`. -When creating `HashMap`s, the `MaxLen` generic should always be specified if it is not already -known. Otherwise, the compiler may infer a different value for `MaxLen` (such as zero), which -will likely change the result of the program. This behavior is set to become an error in future -versions instead. - Example: ```rust diff --git a/noir_stdlib/src/collections/map.nr b/noir_stdlib/src/collections/map.nr index 27a7d0d3550..e84103aafc5 100644 --- a/noir_stdlib/src/collections/map.nr +++ b/noir_stdlib/src/collections/map.nr @@ -10,14 +10,30 @@ use crate::collections::bounded_vec::BoundedVec; global MAX_LOAD_FACTOR_NUMERATOR = 3; global MAX_LOAD_FACTOR_DEN0MINATOR = 4; -// Hash table with open addressing and quadratic probing. -// Size of the underlying table must be known at compile time. -// It is advised to select capacity N as a power of two, or a prime number -// because utilized probing scheme is best tailored for it. +/// `HashMap` is used to efficiently store and look up key-value pairs. +/// +/// `HashMap` is a bounded type which can store anywhere from zero to `MaxLen` total elements. +/// Note that due to hash collisions, the actual maximum number of elements stored by any particular +/// hashmap is likely lower than `MaxLen`. This is true even with cryptographic hash functions since +/// every hash value will be performed modulo `MaxLen`. +/// +/// Example: +/// +/// ```noir +/// // Create a mapping from Fields to u32s with a maximum length of 12 +/// // using a poseidon2 hasher +/// use std::hash::poseidon2::Poseidon2Hasher; +/// let mut map: HashMap> = HashMap::default(); +/// +/// map.insert(1, 2); +/// map.insert(3, 4); +/// +/// let two = map.get(1).unwrap(); +/// ``` struct HashMap { _table: [Slot; N], - // Amount of valid elements in the map. + /// Amount of valid elements in the map. _len: u32, _build_hasher: B @@ -77,7 +93,16 @@ impl Slot { // that if we have went that far without finding desired, // it is very unlikely to be after - performance will be heavily degraded. impl HashMap { - // Creates a new instance of HashMap with specified BuildHasher. + /// Creates a hashmap with an existing `BuildHasher`. This can be used to ensure multiple + /// hashmaps are created with the same hasher instance. + /// + /// Example: + /// + /// ```noir + /// let my_hasher: BuildHasherDefault = Default::default(); + /// let hashmap: HashMap> = HashMap::with_hasher(my_hasher); + /// assert(hashmap.is_empty()); + /// ``` // docs:start:with_hasher pub fn with_hasher(_build_hasher: B) -> Self where @@ -88,7 +113,15 @@ impl HashMap { Self { _table, _len, _build_hasher } } - // Clears the map, removing all key-value entries. + /// Clears the hashmap, removing all key-value pairs from it. + /// + /// Example: + /// + /// ```noir + /// assert(!map.is_empty()); + /// map.clear(); + /// assert(map.is_empty()); + /// ``` // docs:start:clear pub fn clear(&mut self) { // docs:end:clear @@ -96,7 +129,19 @@ impl HashMap { self._len = 0; } - // Returns true if the map contains a value for the specified key. + /// Returns `true` if the hashmap contains the given key. Unlike `get`, this will not also return + /// the value associated with the key. + /// + /// Example: + /// + /// ```noir + /// if map.contains_key(7) { + /// let value = map.get(7); + /// assert(value.is_some()); + /// } else { + /// println("No value for key 7!"); + /// } + /// ``` // docs:start:contains_key pub fn contains_key( self, @@ -110,15 +155,43 @@ impl HashMap { self.get(key).is_some() } - // Returns true if the map contains no elements. + /// Returns `true` if the length of the hash map is empty. + /// + /// Example: + /// + /// ```noir + /// assert(map.is_empty()); + /// + /// map.insert(1, 2); + /// assert(!map.is_empty()); + /// + /// map.remove(1); + /// assert(map.is_empty()); + /// ``` // docs:start:is_empty pub fn is_empty(self) -> bool { // docs:end:is_empty self._len == 0 } - // Returns a BoundedVec of all valid entries in this HashMap. - // The length of the returned vector will always match the length of this HashMap. + /// Returns a vector of each key-value pair present in the hashmap. + /// + /// The length of the returned vector is always equal to the length of the hashmap. + /// + /// Example: + /// + /// ```noir + /// let entries = map.entries(); + /// + /// // The length of a hashmap may not be compile-time known, so we + /// // need to loop over its capacity instead + /// for i in 0..map.capacity() { + /// if i < entries.len() { + /// let (key, value) = entries.get(i); + /// println(f"{key} -> {value}"); + /// } + /// } + /// ``` // docs:start:entries pub fn entries(self) -> BoundedVec<(K, V), N> { // docs:end:entries @@ -138,8 +211,23 @@ impl HashMap { entries } - // Returns a BoundedVec containing all the keys within this HashMap. - // The length of the returned vector will always match the length of this HashMap. + /// Returns a vector of each key present in the hashmap. + /// + /// The length of the returned vector is always equal to the length of the hashmap. + /// + /// Example: + /// + /// ```noir + /// let keys = map.keys(); + /// + /// for i in 0..keys.max_len() { + /// if i < keys.len() { + /// let key = keys.get_unchecked(i); + /// let value = map.get(key).unwrap_unchecked(); + /// println(f"{key} -> {value}"); + /// } + /// } + /// ``` // docs:start:keys pub fn keys(self) -> BoundedVec { // docs:end:keys @@ -158,8 +246,22 @@ impl HashMap { keys } - // Returns a BoundedVec containing all the values within this HashMap. - // The length of the returned vector will always match the length of this HashMap. + /// Returns a vector of each value present in the hashmap. + /// + /// The length of the returned vector is always equal to the length of the hashmap. + /// + /// Example: + /// + /// ```noir + /// let values = map.values(); + /// + /// for i in 0..values.max_len() { + /// if i < values.len() { + /// let value = values.get_unchecked(i); + /// println(f"Found value {value}"); + /// } + /// } + /// ``` // docs:start:values pub fn values(self) -> BoundedVec { // docs:end:values @@ -180,7 +282,22 @@ impl HashMap { values } - // For each key-value entry applies mutator function. + /// Iterates through each key-value pair of the HashMap, setting each key-value pair to the + /// result returned from the given function. + /// + /// Note that since keys can be mutated, the HashMap needs to be rebuilt as it is iterated + /// through. If this is not desired, use `iter_values_mut` if only values need to be mutated, + /// or `entries` if neither keys nor values need to be mutated. + /// + /// The iteration order is left unspecified. As a result, if two keys are mutated to become + /// equal, which of the two values that will be present for the key in the resulting map is also unspecified. + /// + /// Example: + /// + /// ```noir + /// // Add 1 to each key in the map, and double the value associated with that key. + /// map.iter_mut(|k, v| (k + 1, v * 2)); + /// ``` // docs:start:iter_mut pub fn iter_mut( &mut self, @@ -205,7 +322,22 @@ impl HashMap { self._table = new_map._table; } - // For each key applies mutator function. + /// Iterates through the HashMap, mutating each key to the result returned from + /// the given function. + /// + /// Note that since keys can be mutated, the HashMap needs to be rebuilt as it is iterated + /// through. If only iteration is desired and the keys are not intended to be mutated, + /// prefer using `entries` instead. + /// + /// The iteration order is left unspecified. As a result, if two keys are mutated to become + /// equal, which of the two values that will be present for the key in the resulting map is also unspecified. + /// + /// Example: + /// + /// ```noir + /// // Double each key, leaving the value associated with that key untouched + /// map.iter_keys_mut(|k| k * 2); + /// ``` // docs:start:iter_keys_mut pub fn iter_keys_mut( &mut self, @@ -230,7 +362,16 @@ impl HashMap { self._table = new_map._table; } - // For each value applies mutator function. + /// Iterates through the HashMap, applying the given function to each value and mutating the + /// value to equal the result. This function is more efficient than `iter_mut` and `iter_keys_mut` + /// because the keys are untouched and the underlying hashmap thus does not need to be reordered. + /// + /// Example: + /// + /// ```noir + /// // Halve each value + /// map.iter_values_mut(|v| v / 2); + /// ``` // docs:start:iter_values_mut pub fn iter_values_mut(&mut self, f: fn(V) -> V) { // docs:end:iter_values_mut @@ -244,7 +385,14 @@ impl HashMap { } } - // Retains only the elements specified by the predicate. + /// Retains only the key-value pairs for which the given function returns true. + /// Any key-value pairs for which the function returns false will be removed from the map. + /// + /// Example: + /// + /// ```noir + /// map.retain(|k, v| (k != 0) & (v != 0)); + /// ``` // docs:start:retain pub fn retain(&mut self, f: fn(K, V) -> bool) { // docs:end:retain @@ -261,21 +409,67 @@ impl HashMap { } } - // Amount of active key-value entries. + /// Returns the current length of this hash map. + /// + /// Example: + /// + /// ```noir + /// // This is equivalent to checking map.is_empty() + /// assert(map.len() == 0); + /// + /// map.insert(1, 2); + /// map.insert(3, 4); + /// map.insert(5, 6); + /// assert(map.len() == 3); + /// + /// // 3 was already present as a key in the hash map, so the length is unchanged + /// map.insert(3, 7); + /// assert(map.len() == 3); + /// + /// map.remove(1); + /// assert(map.len() == 2); + /// ``` // docs:start:len pub fn len(self) -> u32 { // docs:end:len self._len } - // Get the compile-time map capacity. + /// Returns the maximum capacity of this hashmap. This is always equal to the capacity + /// specified in the hashmap's type. + /// + /// Unlike hashmaps in general purpose programming languages, hashmaps in Noir have a + /// static capacity that does not increase as the map grows larger. Thus, this capacity + /// is also the maximum possible element count that can be inserted into the hashmap. + /// Due to hash collisions (modulo the hashmap length), it is likely the actual maximum + /// element count will be lower than the full capacity. + /// + /// Example: + /// + /// ```noir + /// let empty_map: HashMap> = HashMap::default(); + /// assert(empty_map.len() == 0); + /// assert(empty_map.capacity() == 42); + /// ``` // docs:start:capacity pub fn capacity(_self: Self) -> u32 { // docs:end:capacity N } - // Get the value by key. If it does not exist, returns none(). + /// Retrieves a value from the hashmap, returning `Option::none()` if it was not found. + /// + /// Example: + /// + /// ```noir + /// fn get_example(map: HashMap>) { + /// let x = map.get(12); + /// + /// if x.is_some() { + /// assert(x.unwrap() == 42); + /// } + /// } + /// ``` // docs:start:get pub fn get( self, @@ -310,7 +504,16 @@ impl HashMap { result } - // Insert key-value entry. In case key was already present, value is overridden. + /// Inserts a new key-value pair into the map. If the key was already in the map, its + /// previous value will be overridden with the newly provided one. + /// + /// Example: + /// + /// ```noir + /// let mut map: HashMap> = HashMap::default(); + /// map.insert(12, 42); + /// assert(map.len() == 1); + /// ``` // docs:start:insert pub fn insert( &mut self, @@ -353,7 +556,23 @@ impl HashMap { } } - // Removes a key-value entry. If key is not present, HashMap remains unchanged. + /// Removes the given key-value pair from the map. If the key was not already present + /// in the map, this does nothing. + /// + /// Example: + /// + /// ```noir + /// let mut map: HashMap> = HashMap::default(); + /// map.insert(12, 42); + /// assert(!map.is_empty()); + /// + /// map.remove(12); + /// assert(map.is_empty()); + /// + /// // If a key was not present in the map, remove does nothing + /// map.remove(12); + /// assert(map.is_empty()); + /// ``` // docs:start:remove pub fn remove( &mut self, @@ -432,6 +651,22 @@ where B: BuildHasher, H: Hasher { + /// Checks if two HashMaps are equal. + /// + /// Example: + /// + /// ```noir + /// let mut map1: HashMap> = HashMap::default(); + /// let mut map2: HashMap> = HashMap::default(); + /// + /// map1.insert(1, 2); + /// map1.insert(3, 4); + /// + /// map2.insert(3, 4); + /// map2.insert(1, 2); + /// + /// assert(map1 == map2); + /// ``` fn eq(self, other: HashMap) -> bool { // docs:end:eq let mut equal = false; @@ -466,8 +701,16 @@ where B: BuildHasher + Default, H: Hasher + Default { + /// Constructs an empty HashMap. + /// + /// Example: + /// + /// ```noir + /// let hashmap: HashMap> = HashMap::default(); + /// assert(hashmap.is_empty()); + /// ``` fn default() -> Self { -// docs:end:default + // docs:end:default let _build_hasher = B::default(); let map: HashMap = HashMap::with_hasher(_build_hasher); map From cc30d88d85bb70248e452d9ec549d6dfe6be62ff Mon Sep 17 00:00:00 2001 From: jfecher Date: Mon, 9 Sep 2024 13:15:53 -0500 Subject: [PATCH 08/13] fix: Error when `quote` is used in runtime code (#5978) # Description ## Problem\* Resolves ## Summary\* Fixes a few small issues: - Error when `quote` is used in runtime code - Had to update some stdlib code for this - This could maybe be an issue in the future for something like `impl Append for Quoted` which relies on this. These impl methods have to be comptime now which we currently allow even though the parent trait says nothing about it. At least with https://github.com/noir-lang/noir/pull/5976 we'd get an error if one of these accidentally sneaks into runtime code. - `self.in_comptime_context()` wasn't being tracked accurately in the case of attributes and globals where the function context would only be of length 2 due to there being no outer function. I changed the check to make it more direct instead of relying on indirect changes. - Comptime globals weren't evaluated in their own function context before so we waited to resolve traits until the end of the program. This could lead to trait errors when interpreting them. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [ ] I have tested the changes locally. - [ ] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- .../noirc_frontend/src/elaborator/comptime.rs | 52 +++++++++++++++---- .../src/elaborator/expressions.rs | 31 ++++++----- compiler/noirc_frontend/src/elaborator/mod.rs | 31 +++++------ .../src/elaborator/statements.rs | 9 +--- .../src/hir/resolution/errors.rs | 9 ++++ compiler/noirc_frontend/src/tests.rs | 2 +- noir_stdlib/src/append.nr | 4 +- noir_stdlib/src/meta/expr.nr | 42 +++++++-------- noir_stdlib/src/meta/op.nr | 4 +- .../quote_at_runtime/Nargo.toml | 7 +++ .../quote_at_runtime/src/main.nr | 7 +++ .../attributes_struct/src/main.nr | 2 +- .../comptime_function_definition/src/main.nr | 2 +- .../comptime_global_using_trait/Nargo.toml | 7 +++ .../comptime_global_using_trait/src/main.nr | 3 ++ .../comptime_module/src/main.nr | 10 ++-- .../comptime_module/src/separate_module.nr | 3 +- .../inject_context_attribute/src/main.nr | 4 +- 18 files changed, 145 insertions(+), 84 deletions(-) create mode 100644 test_programs/compile_failure/quote_at_runtime/Nargo.toml create mode 100644 test_programs/compile_failure/quote_at_runtime/src/main.nr create mode 100644 test_programs/compile_success_empty/comptime_global_using_trait/Nargo.toml create mode 100644 test_programs/compile_success_empty/comptime_global_using_trait/src/main.nr diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index 240c48fd260..c2347adcbee 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -159,16 +159,18 @@ impl<'context> Elaborator<'context> { generated_items: &mut CollectedItems, ) { if let SecondaryAttribute::Custom(attribute) = attribute { - if let Err(error) = self.run_comptime_attribute_name_on_item( - &attribute.contents, - item.clone(), - span, - attribute.contents_span, - attribute_context, - generated_items, - ) { - self.errors.push(error); - } + self.elaborate_in_comptime_context(|this| { + if let Err(error) = this.run_comptime_attribute_name_on_item( + &attribute.contents, + item.clone(), + span, + attribute.contents_span, + attribute_context, + generated_items, + ) { + this.errors.push(error); + } + }); } } @@ -599,4 +601,34 @@ impl<'context> Elaborator<'context> { } } } + + /// Perform the given function in a comptime context. + /// This will set the `in_comptime_context` flag on `self` as well as + /// push a new function context to resolve any trait constraints early. + pub(super) fn elaborate_in_comptime_context(&mut self, f: impl FnOnce(&mut Self) -> T) -> T { + let old_comptime_value = std::mem::replace(&mut self.in_comptime_context, true); + // We have to push a new FunctionContext so that we can resolve any constraints + // in this comptime block early before the function as a whole finishes elaborating. + // Otherwise the interpreter below may find expressions for which the underlying trait + // call is not yet solved for. + self.function_context.push(Default::default()); + + let result = f(self); + + self.check_and_pop_function_context(); + self.in_comptime_context = old_comptime_value; + result + } + + /// True if we're currently within a `comptime` block, function, or global + pub(super) fn in_comptime_context(&self) -> bool { + self.in_comptime_context + || match self.current_item { + Some(DependencyId::Function(id)) => { + self.interner.function_modifiers(&id).is_comptime + } + Some(DependencyId::Global(id)) => self.interner.get_global_definition(id).comptime, + _ => false, + } + } } diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index beede7a3a30..15d6ed5506b 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -54,7 +54,7 @@ impl<'context> Elaborator<'context> { ExpressionKind::Tuple(tuple) => self.elaborate_tuple(tuple), ExpressionKind::Lambda(lambda) => self.elaborate_lambda(*lambda), ExpressionKind::Parenthesized(expr) => return self.elaborate_expression(*expr), - ExpressionKind::Quote(quote) => self.elaborate_quote(quote), + ExpressionKind::Quote(quote) => self.elaborate_quote(quote, expr.span), ExpressionKind::Comptime(comptime, _) => { return self.elaborate_comptime_block(comptime, expr.span) } @@ -307,7 +307,13 @@ impl<'context> Elaborator<'context> { let mut arguments = Vec::with_capacity(call.arguments.len()); let args = vecmap(call.arguments, |arg| { let span = arg.span; - let (arg, typ) = self.elaborate_expression(arg); + + let (arg, typ) = if call.is_macro_call { + self.elaborate_in_comptime_context(|this| this.elaborate_expression(arg)) + } else { + self.elaborate_expression(arg) + }; + arguments.push(arg); (typ, arg, span) }); @@ -735,20 +741,21 @@ impl<'context> Elaborator<'context> { (expr, Type::Function(arg_types, Box::new(body_type), Box::new(env_type), false)) } - fn elaborate_quote(&mut self, mut tokens: Tokens) -> (HirExpression, Type) { + fn elaborate_quote(&mut self, mut tokens: Tokens, span: Span) -> (HirExpression, Type) { tokens = self.find_unquoted_exprs_tokens(tokens); - (HirExpression::Quote(tokens), Type::Quoted(QuotedType::Quoted)) + + if self.in_comptime_context() { + (HirExpression::Quote(tokens), Type::Quoted(QuotedType::Quoted)) + } else { + self.push_err(ResolverError::QuoteInRuntimeCode { span }); + (HirExpression::Error, Type::Quoted(QuotedType::Quoted)) + } } fn elaborate_comptime_block(&mut self, block: BlockExpression, span: Span) -> (ExprId, Type) { - // We have to push a new FunctionContext so that we can resolve any constraints - // in this comptime block early before the function as a whole finishes elaborating. - // Otherwise the interpreter below may find expressions for which the underlying trait - // call is not yet solved for. - self.function_context.push(Default::default()); - let (block, _typ) = self.elaborate_block_expression(block); - - self.check_and_pop_function_context(); + let (block, _typ) = + self.elaborate_in_comptime_context(|this| this.elaborate_block_expression(block)); + let mut interpreter = self.setup_interpreter(); let value = interpreter.evaluate_block(block); let (id, typ) = self.inline_comptime_value(value, span); diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index dd8e985d3a2..6871152edb5 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -158,6 +158,10 @@ pub struct Elaborator<'context> { /// Initially empty, it is set whenever a new top-level item is resolved. local_module: LocalModuleId, + /// True if we're elaborating a comptime item such as a comptime function, + /// block, global, or attribute. + in_comptime_context: bool, + crate_id: CrateId, /// The scope of --debug-comptime, or None if unset @@ -215,6 +219,7 @@ impl<'context> Elaborator<'context> { unresolved_globals: BTreeMap::new(), current_trait: None, interpreter_call_stack, + in_comptime_context: false, } } @@ -1289,7 +1294,12 @@ impl<'context> Elaborator<'context> { let comptime = let_stmt.comptime; - let (let_statement, _typ) = self.elaborate_let(let_stmt, Some(global_id)); + let (let_statement, _typ) = if comptime { + self.elaborate_in_comptime_context(|this| this.elaborate_let(let_stmt, Some(global_id))) + } else { + self.elaborate_let(let_stmt, Some(global_id)) + }; + let statement_id = self.interner.get_global(global_id).let_statement; self.interner.replace_statement(statement_id, let_statement); @@ -1324,9 +1334,7 @@ impl<'context> Elaborator<'context> { .lookup_id(definition_id, location) .expect("The global should be defined since evaluate_let did not error"); - self.debug_comptime(location, |interner| { - interner.get_global(global_id).let_statement.to_display_ast(interner).kind - }); + self.debug_comptime(location, |interner| value.display(interner).to_string()); self.interner.get_global_mut(global_id).value = Some(value); } @@ -1423,21 +1431,6 @@ impl<'context> Elaborator<'context> { } } - /// True if we're currently within a `comptime` block, function, or global - fn in_comptime_context(&self) -> bool { - // The first context is the global context, followed by the function-specific context. - // Any context after that is a `comptime {}` block's. - if self.function_context.len() > 2 { - return true; - } - - match self.current_item { - Some(DependencyId::Function(id)) => self.interner.function_modifiers(&id).is_comptime, - Some(DependencyId::Global(id)) => self.interner.get_global_definition(id).comptime, - _ => false, - } - } - /// True if we're currently within a constrained function. /// Defaults to `true` if the current function is unknown. fn in_constrained_function(&self) -> bool { diff --git a/compiler/noirc_frontend/src/elaborator/statements.rs b/compiler/noirc_frontend/src/elaborator/statements.rs index d7d330f891a..9e29978a9d5 100644 --- a/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/compiler/noirc_frontend/src/elaborator/statements.rs @@ -441,14 +441,9 @@ impl<'context> Elaborator<'context> { } fn elaborate_comptime_statement(&mut self, statement: Statement) -> (HirStatement, Type) { - // We have to push a new FunctionContext so that we can resolve any constraints - // in this comptime block early before the function as a whole finishes elaborating. - // Otherwise the interpreter below may find expressions for which the underlying trait - // call is not yet solved for. - self.function_context.push(Default::default()); let span = statement.span; - let (hir_statement, _typ) = self.elaborate_statement(statement); - self.check_and_pop_function_context(); + let (hir_statement, _typ) = + self.elaborate_in_comptime_context(|this| this.elaborate_statement(statement)); let mut interpreter = self.setup_interpreter(); let value = interpreter.evaluate_statement(hir_statement); let (expr, typ) = self.inline_comptime_value(value, span); diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index e74468bdf18..5abc94b89a2 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -124,6 +124,8 @@ pub enum ResolverError { AssociatedConstantsMustBeNumeric { span: Span }, #[error("Overflow in `{lhs} {op} {rhs}`")] OverflowInType { lhs: u32, op: crate::BinaryTypeOperator, rhs: u32, span: Span }, + #[error("`quote` cannot be used in runtime code")] + QuoteInRuntimeCode { span: Span }, } impl ResolverError { @@ -504,6 +506,13 @@ impl<'a> From<&'a ResolverError> for Diagnostic { *span, ) } + ResolverError::QuoteInRuntimeCode { span } => { + Diagnostic::simple_error( + "`quote` cannot be used in runtime code".to_string(), + "Wrap this in a `comptime` block or function to use it".to_string(), + *span, + ) + }, } } } diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 1a0f086b484..64c9b7471b4 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -3373,7 +3373,7 @@ fn unquoted_integer_as_integer_token() { #[attr] pub fn foobar() {} - fn attr(_f: FunctionDefinition) -> Quoted { + comptime fn attr(_f: FunctionDefinition) -> Quoted { let serialized_len = 1; // We are testing that when we unquote $serialized_len, it's unquoted // as the token `1` and not as something else that later won't be parsed correctly diff --git a/noir_stdlib/src/append.nr b/noir_stdlib/src/append.nr index 4577ae199b8..22baa9205a0 100644 --- a/noir_stdlib/src/append.nr +++ b/noir_stdlib/src/append.nr @@ -25,11 +25,11 @@ impl Append for [T] { } impl Append for Quoted { - fn empty() -> Self { + comptime fn empty() -> Self { quote {} } - fn append(self, other: Self) -> Self { + comptime fn append(self, other: Self) -> Self { quote { $self $other } } } diff --git a/noir_stdlib/src/meta/expr.nr b/noir_stdlib/src/meta/expr.nr index 96a83f954c9..642dbecc36b 100644 --- a/noir_stdlib/src/meta/expr.nr +++ b/noir_stdlib/src/meta/expr.nr @@ -159,7 +159,7 @@ impl Expr { } // docs:start:quoted - fn quoted(self) -> Quoted { + comptime fn quoted(self) -> Quoted { // docs:end:quoted quote { $self } } @@ -381,12 +381,12 @@ fn modify_expressions(exprs: [Expr], f: fn[Env](Expr) -> Option) -> [ exprs.map(|expr: Expr| expr.modify(f)) } -fn new_array(exprs: [Expr]) -> Expr { +comptime fn new_array(exprs: [Expr]) -> Expr { let exprs = join_expressions(exprs, quote { , }); quote { [$exprs]}.as_expr().unwrap() } -fn new_assert(predicate: Expr, msg: Option) -> Expr { +comptime fn new_assert(predicate: Expr, msg: Option) -> Expr { if msg.is_some() { let msg = msg.unwrap(); quote { assert($predicate, $msg) }.as_expr().unwrap() @@ -395,7 +395,7 @@ fn new_assert(predicate: Expr, msg: Option) -> Expr { } } -fn new_assert_eq(lhs: Expr, rhs: Expr, msg: Option) -> Expr { +comptime fn new_assert_eq(lhs: Expr, rhs: Expr, msg: Option) -> Expr { if msg.is_some() { let msg = msg.unwrap(); quote { assert_eq($lhs, $rhs, $msg) }.as_expr().unwrap() @@ -404,30 +404,30 @@ fn new_assert_eq(lhs: Expr, rhs: Expr, msg: Option) -> Expr { } } -fn new_assign(lhs: Expr, rhs: Expr) -> Expr { +comptime fn new_assign(lhs: Expr, rhs: Expr) -> Expr { quote { $lhs = $rhs }.as_expr().unwrap() } -fn new_binary_op(lhs: Expr, op: BinaryOp, rhs: Expr) -> Expr { +comptime fn new_binary_op(lhs: Expr, op: BinaryOp, rhs: Expr) -> Expr { let op = op.quoted(); quote { ($lhs) $op ($rhs) }.as_expr().unwrap() } -fn new_block(exprs: [Expr]) -> Expr { +comptime fn new_block(exprs: [Expr]) -> Expr { let exprs = join_expressions(exprs, quote { ; }); quote { { $exprs }}.as_expr().unwrap() } -fn new_cast(expr: Expr, typ: UnresolvedType) -> Expr { +comptime fn new_cast(expr: Expr, typ: UnresolvedType) -> Expr { quote { ($expr) as $typ }.as_expr().unwrap() } -fn new_comptime(exprs: [Expr]) -> Expr { +comptime fn new_comptime(exprs: [Expr]) -> Expr { let exprs = join_expressions(exprs, quote { ; }); quote { comptime { $exprs }}.as_expr().unwrap() } -fn new_if(condition: Expr, consequence: Expr, alternative: Option) -> Expr { +comptime fn new_if(condition: Expr, consequence: Expr, alternative: Option) -> Expr { if alternative.is_some() { let alternative = alternative.unwrap(); quote { if $condition { $consequence } else { $alternative }}.as_expr().unwrap() @@ -436,11 +436,11 @@ fn new_if(condition: Expr, consequence: Expr, alternative: Option) -> Expr } } -fn new_index(object: Expr, index: Expr) -> Expr { +comptime fn new_index(object: Expr, index: Expr) -> Expr { quote { $object[$index] }.as_expr().unwrap() } -fn new_let(pattern: Expr, typ: Option, expr: Expr) -> Expr { +comptime fn new_let(pattern: Expr, typ: Option, expr: Expr) -> Expr { if typ.is_some() { let typ = typ.unwrap(); quote { let $pattern : $typ = $expr; }.as_expr().unwrap() @@ -449,17 +449,17 @@ fn new_let(pattern: Expr, typ: Option, expr: Expr) -> Expr { } } -fn new_member_access(object: Expr, name: Quoted) -> Expr { +comptime fn new_member_access(object: Expr, name: Quoted) -> Expr { quote { $object.$name }.as_expr().unwrap() } -fn new_function_call(function: Expr, arguments: [Expr]) -> Expr { +comptime fn new_function_call(function: Expr, arguments: [Expr]) -> Expr { let arguments = join_expressions(arguments, quote { , }); quote { $function($arguments) }.as_expr().unwrap() } -fn new_method_call(object: Expr, name: Quoted, generics: [UnresolvedType], arguments: [Expr]) -> Expr { +comptime fn new_method_call(object: Expr, name: Quoted, generics: [UnresolvedType], arguments: [Expr]) -> Expr { let arguments = join_expressions(arguments, quote { , }); if generics.len() == 0 { @@ -470,30 +470,30 @@ fn new_method_call(object: Expr, name: Quoted, generics: [UnresolvedType], argum } } -fn new_repeated_element_array(expr: Expr, length: Expr) -> Expr { +comptime fn new_repeated_element_array(expr: Expr, length: Expr) -> Expr { quote { [$expr; $length] }.as_expr().unwrap() } -fn new_repeated_element_slice(expr: Expr, length: Expr) -> Expr { +comptime fn new_repeated_element_slice(expr: Expr, length: Expr) -> Expr { quote { &[$expr; $length] }.as_expr().unwrap() } -fn new_slice(exprs: [Expr]) -> Expr { +comptime fn new_slice(exprs: [Expr]) -> Expr { let exprs = join_expressions(exprs, quote { , }); quote { &[$exprs]}.as_expr().unwrap() } -fn new_tuple(exprs: [Expr]) -> Expr { +comptime fn new_tuple(exprs: [Expr]) -> Expr { let exprs = join_expressions(exprs, quote { , }); quote { ($exprs) }.as_expr().unwrap() } -fn new_unary_op(op: UnaryOp, rhs: Expr) -> Expr { +comptime fn new_unary_op(op: UnaryOp, rhs: Expr) -> Expr { let op = op.quoted(); quote { $op($rhs) }.as_expr().unwrap() } -fn new_unsafe(exprs: [Expr]) -> Expr { +comptime fn new_unsafe(exprs: [Expr]) -> Expr { let exprs = join_expressions(exprs, quote { ; }); quote { unsafe { $exprs }}.as_expr().unwrap() } diff --git a/noir_stdlib/src/meta/op.nr b/noir_stdlib/src/meta/op.nr index 1917a04da59..4b104486201 100644 --- a/noir_stdlib/src/meta/op.nr +++ b/noir_stdlib/src/meta/op.nr @@ -28,7 +28,7 @@ impl UnaryOp { } // docs:start:unary_quoted - pub fn quoted(self) -> Quoted { + pub comptime fn quoted(self) -> Quoted { // docs:end:unary_quoted if self.is_minus() { quote { - } @@ -147,7 +147,7 @@ impl BinaryOp { } // docs:start:binary_quoted - pub fn quoted(self) -> Quoted { + pub comptime fn quoted(self) -> Quoted { // docs:end:binary_quoted if self.is_add() { quote { + } diff --git a/test_programs/compile_failure/quote_at_runtime/Nargo.toml b/test_programs/compile_failure/quote_at_runtime/Nargo.toml new file mode 100644 index 00000000000..3ce62412b32 --- /dev/null +++ b/test_programs/compile_failure/quote_at_runtime/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "quote_at_runtime" +type = "bin" +authors = [""] +compiler_version = ">=0.33.0" + +[dependencies] diff --git a/test_programs/compile_failure/quote_at_runtime/src/main.nr b/test_programs/compile_failure/quote_at_runtime/src/main.nr new file mode 100644 index 00000000000..5da409682f6 --- /dev/null +++ b/test_programs/compile_failure/quote_at_runtime/src/main.nr @@ -0,0 +1,7 @@ +fn main() { + foo(quote { test }) +} + +fn foo(q: Quoted) { + println(q); +} diff --git a/test_programs/compile_success_empty/attributes_struct/src/main.nr b/test_programs/compile_success_empty/attributes_struct/src/main.nr index 25c9402035c..79bf3e29533 100644 --- a/test_programs/compile_success_empty/attributes_struct/src/main.nr +++ b/test_programs/compile_success_empty/attributes_struct/src/main.nr @@ -14,7 +14,7 @@ struct Foo { } -fn add_attribute(s: StructDefinition) { +comptime fn add_attribute(s: StructDefinition) { assert(!s.has_named_attribute(quote { foo })); s.add_attribute("foo"); assert(s.has_named_attribute(quote { foo })); diff --git a/test_programs/compile_success_empty/comptime_function_definition/src/main.nr b/test_programs/compile_success_empty/comptime_function_definition/src/main.nr index 62f119cc0c0..6bfd8ef9699 100644 --- a/test_programs/compile_success_empty/comptime_function_definition/src/main.nr +++ b/test_programs/compile_success_empty/comptime_function_definition/src/main.nr @@ -74,7 +74,7 @@ mod foo { #[attr] pub fn some() {} - fn attr(f: FunctionDefinition) { + comptime fn attr(f: FunctionDefinition) { assert_eq(f.module().name(), quote { foo }); assert(!f.is_unconstrained()); diff --git a/test_programs/compile_success_empty/comptime_global_using_trait/Nargo.toml b/test_programs/compile_success_empty/comptime_global_using_trait/Nargo.toml new file mode 100644 index 00000000000..2de6e4d149e --- /dev/null +++ b/test_programs/compile_success_empty/comptime_global_using_trait/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "comptime_global_using_trait" +type = "bin" +authors = [""] +compiler_version = ">=0.33.0" + +[dependencies] diff --git a/test_programs/compile_success_empty/comptime_global_using_trait/src/main.nr b/test_programs/compile_success_empty/comptime_global_using_trait/src/main.nr new file mode 100644 index 00000000000..a1a2c4b125a --- /dev/null +++ b/test_programs/compile_success_empty/comptime_global_using_trait/src/main.nr @@ -0,0 +1,3 @@ +comptime global FOO: i32 = Default::default(); + +fn main() {} diff --git a/test_programs/compile_success_empty/comptime_module/src/main.nr b/test_programs/compile_success_empty/comptime_module/src/main.nr index baf45c517ed..c3ba7ba4643 100644 --- a/test_programs/compile_success_empty/comptime_module/src/main.nr +++ b/test_programs/compile_success_empty/comptime_module/src/main.nr @@ -21,23 +21,23 @@ mod separate_module; comptime mut global counter = 0; -fn increment_counter() { +comptime fn increment_counter() { counter += 1; } -fn outer_attribute_func(m: Module) -> Quoted { +comptime fn outer_attribute_func(m: Module) -> Quoted { assert_eq(m.name(), quote { yet_another_module }); increment_counter(); quote { pub fn generated_outer_function() {} } } -fn inner_attribute_func(m: Module) -> Quoted { +comptime fn inner_attribute_func(m: Module) -> Quoted { assert_eq(m.name(), quote { yet_another_module }); increment_counter(); quote { pub fn generated_inner_function() {} } } -fn outer_attribute_separate_module(m: Module) { +comptime fn outer_attribute_separate_module(m: Module) { assert_eq(m.name(), quote { separate_module }); increment_counter(); } @@ -49,7 +49,7 @@ mod add_to_me { fn add_to_me_function() {} } -fn add_function(m: Module) { +comptime fn add_function(m: Module) { m.add_item( quote { pub fn added_function() -> super::Foo { add_to_me_function(); diff --git a/test_programs/compile_success_empty/comptime_module/src/separate_module.nr b/test_programs/compile_success_empty/comptime_module/src/separate_module.nr index 53784101507..c1aeac4622c 100644 --- a/test_programs/compile_success_empty/comptime_module/src/separate_module.nr +++ b/test_programs/compile_success_empty/comptime_module/src/separate_module.nr @@ -1,5 +1,6 @@ #![inner_attribute_separate_module] -fn inner_attribute_separate_module(m: Module) { + +comptime fn inner_attribute_separate_module(m: Module) { assert_eq(m.name(), quote { separate_module }); super::increment_counter(); } diff --git a/test_programs/compile_success_empty/inject_context_attribute/src/main.nr b/test_programs/compile_success_empty/inject_context_attribute/src/main.nr index 26be3135133..9dc8cf1ed47 100644 --- a/test_programs/compile_success_empty/inject_context_attribute/src/main.nr +++ b/test_programs/compile_success_empty/inject_context_attribute/src/main.nr @@ -28,7 +28,7 @@ fn zero() -> Field { 0 } -fn inject_context(f: FunctionDefinition) { +comptime fn inject_context(f: FunctionDefinition) { // Add a `_context: Context` parameter to the function let parameters = f.parameters(); let parameters = parameters.push_front((quote { _context }, quote { Context }.as_type())); @@ -39,7 +39,7 @@ fn inject_context(f: FunctionDefinition) { f.set_body(body); } -fn mapping_function(expr: Expr, f: FunctionDefinition) -> Option { +comptime fn mapping_function(expr: Expr, f: FunctionDefinition) -> Option { expr.as_function_call().and_then( |func_call: (Expr, [Expr])| { let (name, arguments) = func_call; From d6f60d70dc41640ad84f7a968927b20818bcaf2a Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 9 Sep 2024 17:36:34 -0300 Subject: [PATCH 09/13] fix: suggest trait attributes in LSP (#5972) # Description ## Problem I noticed LSP didn't suggest trait attributes... because these weren't visited. I also noticed when an attribute related to a function was suggested, but the function only had one argument, the parentheses were included in the suggestion but they aren't needed. ## Summary Fixes the above problems. ![lsp-trait-attribute](https://github.com/user-attachments/assets/16cdd8ab-1d03-40d1-a1b1-19b26d88e322) ## Additional Context ## Documentation Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- compiler/noirc_frontend/src/ast/visitor.rs | 5 +++ .../lsp/src/requests/completion/builtins.rs | 2 +- .../requests/completion/completion_items.rs | 11 ++++-- tooling/lsp/src/requests/completion/tests.rs | 36 +++++++++++++++++++ 4 files changed, 50 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_frontend/src/ast/visitor.rs b/compiler/noirc_frontend/src/ast/visitor.rs index 359e04e41a7..fcb4e4c5dd1 100644 --- a/compiler/noirc_frontend/src/ast/visitor.rs +++ b/compiler/noirc_frontend/src/ast/visitor.rs @@ -30,6 +30,7 @@ use super::{ pub enum AttributeTarget { Module, Struct, + Trait, Function, } @@ -617,6 +618,10 @@ impl NoirTrait { } pub fn accept_children(&self, visitor: &mut impl Visitor) { + for attribute in &self.attributes { + attribute.accept(AttributeTarget::Trait, visitor); + } + for item in &self.items { item.item.accept(visitor); } diff --git a/tooling/lsp/src/requests/completion/builtins.rs b/tooling/lsp/src/requests/completion/builtins.rs index bca1061ff47..520c158d260 100644 --- a/tooling/lsp/src/requests/completion/builtins.rs +++ b/tooling/lsp/src/requests/completion/builtins.rs @@ -87,7 +87,7 @@ impl<'a> NodeFinder<'a> { pub(super) fn suggest_builtin_attributes(&mut self, prefix: &str, target: AttributeTarget) { match target { - AttributeTarget::Module => (), + AttributeTarget::Module | AttributeTarget::Trait => (), AttributeTarget::Struct => { self.suggest_one_argument_attributes(prefix, &["abi"]); } diff --git a/tooling/lsp/src/requests/completion/completion_items.rs b/tooling/lsp/src/requests/completion/completion_items.rs index aa959b226b0..7e7511cdaa3 100644 --- a/tooling/lsp/src/requests/completion/completion_items.rs +++ b/tooling/lsp/src/requests/completion/completion_items.rs @@ -49,6 +49,7 @@ impl<'a> NodeFinder<'a> { match target { AttributeTarget::Module => Some(Type::Quoted(QuotedType::Module)), AttributeTarget::Struct => Some(Type::Quoted(QuotedType::StructDefinition)), + AttributeTarget::Trait => Some(Type::Quoted(QuotedType::TraitDefinition)), AttributeTarget::Function => Some(Type::Quoted(QuotedType::FunctionDefinition)), } } else { @@ -226,10 +227,14 @@ impl<'a> NodeFinder<'a> { function_kind, skip_first_argument, ); - let label = if insert_text.ends_with("()") { - format!("{}()", name) + let (label, insert_text) = if insert_text.ends_with("()") { + if skip_first_argument { + (name.to_string(), insert_text.strip_suffix("()").unwrap().to_string()) + } else { + (format!("{}()", name), insert_text) + } } else { - format!("{}(…)", name) + (format!("{}(…)", name), insert_text) }; snippet_completion_item(label, kind, insert_text, Some(description)) diff --git a/tooling/lsp/src/requests/completion/tests.rs b/tooling/lsp/src/requests/completion/tests.rs index c449466fc5c..97cb145eb22 100644 --- a/tooling/lsp/src/requests/completion/tests.rs +++ b/tooling/lsp/src/requests/completion/tests.rs @@ -1952,4 +1952,40 @@ mod completion_tests { ) .await; } + + #[test] + async fn test_suggests_function_attribute_no_arguments() { + let src = r#" + #[some>|<] + fn foo() {} + + fn some_attr(f: FunctionDefinition) {} + "#; + + assert_completion_excluding_auto_import( + src, + vec![function_completion_item("some_attr", "some_attr", "fn(FunctionDefinition)")], + ) + .await; + } + + #[test] + async fn test_suggests_trait_attribute() { + let src = r#" + #[some>|<] + trait SomeTrait {} + + fn some_attr(t: TraitDefinition, x: Field) {} + "#; + + assert_completion_excluding_auto_import( + src, + vec![function_completion_item( + "some_attr(…)", + "some_attr(${1:x})", + "fn(TraitDefinition, Field)", + )], + ) + .await; + } } From 3d39196040aa01e64c8a7fe989e2979a5de80023 Mon Sep 17 00:00:00 2001 From: jfecher Date: Tue, 10 Sep 2024 09:19:41 -0500 Subject: [PATCH 10/13] fix: Error when comptime types are used in runtime code (#5987) # Description ## Problem\* Resolves https://github.com/noir-lang/noir/issues/5983 ## Summary\* Errors when a comptime-only type is used in runtime code. Currently this is just the various quoted types but in the future would also include user-defined `comptime` structs. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- .../noirc_frontend/src/elaborator/types.rs | 10 +- .../src/hir/resolution/errors.rs | 9 ++ .../src/monomorphization/errors.rs | 7 ++ .../src/monomorphization/mod.rs | 5 +- compiler/noirc_frontend/src/tests.rs | 11 +++ noir_stdlib/src/meta/expr.nr | 98 +++++++++---------- noir_stdlib/src/meta/format_string.nr | 2 +- noir_stdlib/src/meta/function_def.nr | 26 ++--- noir_stdlib/src/meta/module.nr | 10 +- noir_stdlib/src/meta/quoted.nr | 14 +-- noir_stdlib/src/meta/struct_def.nr | 18 ++-- noir_stdlib/src/meta/trait_constraint.nr | 8 +- noir_stdlib/src/meta/trait_def.nr | 10 +- noir_stdlib/src/meta/trait_impl.nr | 4 +- noir_stdlib/src/meta/typ.nr | 28 +++--- noir_stdlib/src/meta/typed_expr.nr | 2 +- noir_stdlib/src/meta/unresolved_type.nr | 2 +- .../comptime_fmt_strings/src/main.nr | 2 +- .../comptime_function_definition/src/main.nr | 2 +- 19 files changed, 153 insertions(+), 115 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 39ef4e0bb8e..46c779dc4ff 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -118,7 +118,15 @@ impl<'context> Elaborator<'context> { let fields = self.resolve_type_inner(*fields, kind); Type::FmtString(Box::new(resolved_size), Box::new(fields)) } - Quoted(quoted) => Type::Quoted(quoted), + Quoted(quoted) => { + let in_function = matches!(self.current_item, Some(DependencyId::Function(_))); + if in_function && !self.in_comptime_context() { + let span = typ.span; + let typ = quoted.to_string(); + self.push_err(ResolverError::ComptimeTypeInRuntimeCode { span, typ }); + } + Type::Quoted(quoted) + } Unit => Type::Unit, Unspecified => { let span = typ.span; diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index 5abc94b89a2..ec22c8f1986 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -126,6 +126,8 @@ pub enum ResolverError { OverflowInType { lhs: u32, op: crate::BinaryTypeOperator, rhs: u32, span: Span }, #[error("`quote` cannot be used in runtime code")] QuoteInRuntimeCode { span: Span }, + #[error("Comptime-only type `{typ}` cannot be used in runtime code")] + ComptimeTypeInRuntimeCode { typ: String, span: Span }, } impl ResolverError { @@ -513,6 +515,13 @@ impl<'a> From<&'a ResolverError> for Diagnostic { *span, ) }, + ResolverError::ComptimeTypeInRuntimeCode { typ, span } => { + Diagnostic::simple_error( + format!("Comptime-only type `{typ}` cannot be used in runtime code"), + "Comptime-only type used here".to_string(), + *span, + ) + }, } } } diff --git a/compiler/noirc_frontend/src/monomorphization/errors.rs b/compiler/noirc_frontend/src/monomorphization/errors.rs index 66db72eef55..7f4172017e2 100644 --- a/compiler/noirc_frontend/src/monomorphization/errors.rs +++ b/compiler/noirc_frontend/src/monomorphization/errors.rs @@ -9,6 +9,7 @@ pub enum MonomorphizationError { InternalError { message: &'static str, location: Location }, InterpreterError(InterpreterError), ComptimeFnInRuntimeCode { name: String, location: Location }, + ComptimeTypeInRuntimeCode { typ: String, location: Location }, } impl MonomorphizationError { @@ -17,6 +18,7 @@ impl MonomorphizationError { MonomorphizationError::UnknownArrayLength { location, .. } | MonomorphizationError::InternalError { location, .. } | MonomorphizationError::ComptimeFnInRuntimeCode { location, .. } + | MonomorphizationError::ComptimeTypeInRuntimeCode { location, .. } | MonomorphizationError::NoDefaultType { location, .. } => *location, MonomorphizationError::InterpreterError(error) => error.get_location(), } @@ -51,6 +53,11 @@ impl MonomorphizationError { "Comptime functions must be in a comptime block to be called".into(); return CustomDiagnostic::simple_error(message, secondary, location.span); } + MonomorphizationError::ComptimeTypeInRuntimeCode { typ, location } => { + let message = format!("Comptime-only type `{typ}` used in runtime code"); + let secondary = "Comptime type used here".into(); + return CustomDiagnostic::simple_error(message, secondary, location.span); + } }; let location = self.location(); diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 9357cc65c14..fd06a2b04a8 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -1056,7 +1056,10 @@ impl<'interner> Monomorphizer<'interner> { let message = "Unexpected Type::Error found during monomorphization"; return Err(MonomorphizationError::InternalError { message, location }); } - HirType::Quoted(_) => unreachable!("Tried to translate Code type into runtime code"), + HirType::Quoted(typ) => { + let typ = typ.to_string(); + return Err(MonomorphizationError::ComptimeTypeInRuntimeCode { typ, location }); + } }) } diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 64c9b7471b4..414bfa5fa5b 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -3451,3 +3451,14 @@ fn constrained_reference_to_unconstrained() { panic!("Expected an error about passing a constrained reference to unconstrained"); }; } + +#[test] +fn comptime_type_in_runtime_code() { + let source = "pub fn foo(_f: FunctionDefinition) {}"; + let errors = get_program_errors(source); + assert_eq!(errors.len(), 1); + assert!(matches!( + errors[0].0, + CompilationError::ResolverError(ResolverError::ComptimeTypeInRuntimeCode { .. }) + )); +} diff --git a/noir_stdlib/src/meta/expr.nr b/noir_stdlib/src/meta/expr.nr index 642dbecc36b..72e1a88cea8 100644 --- a/noir_stdlib/src/meta/expr.nr +++ b/noir_stdlib/src/meta/expr.nr @@ -5,129 +5,129 @@ use crate::meta::op::BinaryOp; impl Expr { #[builtin(expr_as_array)] // docs:start:as_array - fn as_array(self) -> Option<[Expr]> {} + comptime fn as_array(self) -> Option<[Expr]> {} // docs:end:as_array #[builtin(expr_as_assert)] // docs:start:as_assert - fn as_assert(self) -> Option<(Expr, Option)> {} + comptime fn as_assert(self) -> Option<(Expr, Option)> {} // docs:end:as_assert #[builtin(expr_as_assert_eq)] // docs:start:as_assert_eq - fn as_assert_eq(self) -> Option<(Expr, Expr, Option)> {} + comptime fn as_assert_eq(self) -> Option<(Expr, Expr, Option)> {} // docs:end:as_assert_eq #[builtin(expr_as_assign)] // docs:start:as_assign - fn as_assign(self) -> Option<(Expr, Expr)> {} + comptime fn as_assign(self) -> Option<(Expr, Expr)> {} // docs:end:as_assign #[builtin(expr_as_integer)] // docs:start:as_integer - fn as_integer(self) -> Option<(Field, bool)> {} + comptime fn as_integer(self) -> Option<(Field, bool)> {} // docs:end:as_integer #[builtin(expr_as_binary_op)] // docs:start:as_binary_op - fn as_binary_op(self) -> Option<(Expr, BinaryOp, Expr)> {} + comptime fn as_binary_op(self) -> Option<(Expr, BinaryOp, Expr)> {} // docs:end:as_binary_op #[builtin(expr_as_block)] // docs:start:as_block - fn as_block(self) -> Option<[Expr]> {} + comptime fn as_block(self) -> Option<[Expr]> {} // docs:end:as_block #[builtin(expr_as_bool)] // docs:start:as_bool - fn as_bool(self) -> Option {} + comptime fn as_bool(self) -> Option {} // docs:end:as_bool #[builtin(expr_as_cast)] - fn as_cast(self) -> Option<(Expr, UnresolvedType)> {} + comptime fn as_cast(self) -> Option<(Expr, UnresolvedType)> {} #[builtin(expr_as_comptime)] // docs:start:as_comptime - fn as_comptime(self) -> Option<[Expr]> {} + comptime fn as_comptime(self) -> Option<[Expr]> {} // docs:end:as_comptime #[builtin(expr_as_function_call)] // docs:start:as_function_call - fn as_function_call(self) -> Option<(Expr, [Expr])> {} + comptime fn as_function_call(self) -> Option<(Expr, [Expr])> {} // docs:end:as_function_call #[builtin(expr_as_if)] // docs:start:as_if - fn as_if(self) -> Option<(Expr, Expr, Option)> {} + comptime fn as_if(self) -> Option<(Expr, Expr, Option)> {} // docs:end:as_if #[builtin(expr_as_index)] // docs:start:as_index - fn as_index(self) -> Option<(Expr, Expr)> {} + comptime fn as_index(self) -> Option<(Expr, Expr)> {} // docs:end:as_index #[builtin(expr_as_let)] // docs:start:as_let - fn as_let(self) -> Option<(Expr, Option, Expr)> {} + comptime fn as_let(self) -> Option<(Expr, Option, Expr)> {} // docs:end:as_let #[builtin(expr_as_member_access)] // docs:start:as_member_access - fn as_member_access(self) -> Option<(Expr, Quoted)> {} + comptime fn as_member_access(self) -> Option<(Expr, Quoted)> {} // docs:end:as_member_access #[builtin(expr_as_method_call)] // docs:start:as_method_call - fn as_method_call(self) -> Option<(Expr, Quoted, [UnresolvedType], [Expr])> {} + comptime fn as_method_call(self) -> Option<(Expr, Quoted, [UnresolvedType], [Expr])> {} // docs:end:as_method_call #[builtin(expr_as_repeated_element_array)] // docs:start:as_repeated_element_array - fn as_repeated_element_array(self) -> Option<(Expr, Expr)> {} + comptime fn as_repeated_element_array(self) -> Option<(Expr, Expr)> {} // docs:end:as_repeated_element_array #[builtin(expr_as_repeated_element_slice)] // docs:start:as_repeated_element_slice - fn as_repeated_element_slice(self) -> Option<(Expr, Expr)> {} + comptime fn as_repeated_element_slice(self) -> Option<(Expr, Expr)> {} // docs:end:as_repeated_element_slice #[builtin(expr_as_slice)] // docs:start:as_slice - fn as_slice(self) -> Option<[Expr]> {} + comptime fn as_slice(self) -> Option<[Expr]> {} // docs:end:as_slice #[builtin(expr_as_tuple)] // docs:start:as_tuple - fn as_tuple(self) -> Option<[Expr]> {} + comptime fn as_tuple(self) -> Option<[Expr]> {} // docs:end:as_tuple #[builtin(expr_as_unary_op)] // docs:start:as_unary_op - fn as_unary_op(self) -> Option<(UnaryOp, Expr)> {} + comptime fn as_unary_op(self) -> Option<(UnaryOp, Expr)> {} // docs:end:as_unary_op #[builtin(expr_as_unsafe)] // docs:start:as_unsafe - fn as_unsafe(self) -> Option<[Expr]> {} + comptime fn as_unsafe(self) -> Option<[Expr]> {} // docs:end:as_unsafe #[builtin(expr_has_semicolon)] // docs:start:has_semicolon - fn has_semicolon(self) -> bool {} + comptime fn has_semicolon(self) -> bool {} // docs:end:has_semicolon #[builtin(expr_is_break)] // docs:start:is_break - fn is_break(self) -> bool {} + comptime fn is_break(self) -> bool {} // docs:end:is_break #[builtin(expr_is_continue)] // docs:start:is_continue - fn is_continue(self) -> bool {} + comptime fn is_continue(self) -> bool {} // docs:end:is_continue // docs:start:modify - fn modify(self, f: fn[Env](Expr) -> Option) -> Expr { + comptime fn modify(self, f: fn[Env](Expr) -> Option) -> Expr { // docs:end:modify let result = modify_array(self, f); let result = result.or_else(|| modify_assert(self, f)); @@ -166,11 +166,11 @@ impl Expr { #[builtin(expr_resolve)] // docs:start:resolve - fn resolve(self, in_function: Option) -> TypedExpr {} + comptime fn resolve(self, in_function: Option) -> TypedExpr {} // docs:end:resolve } -fn modify_array(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { +comptime fn modify_array(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_array().map( |exprs: [Expr]| { let exprs = modify_expressions(exprs, f); @@ -179,7 +179,7 @@ fn modify_array(expr: Expr, f: fn[Env](Expr) -> Option) -> Option(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { +comptime fn modify_assert(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_assert().map( |expr: (Expr, Option)| { let (predicate, msg) = expr; @@ -190,7 +190,7 @@ fn modify_assert(expr: Expr, f: fn[Env](Expr) -> Option) -> Option(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { +comptime fn modify_assert_eq(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_assert_eq().map( |expr: (Expr, Expr, Option)| { let (lhs, rhs, msg) = expr; @@ -202,7 +202,7 @@ fn modify_assert_eq(expr: Expr, f: fn[Env](Expr) -> Option) -> Option ) } -fn modify_assign(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { +comptime fn modify_assign(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_assign().map( |expr: (Expr, Expr)| { let (lhs, rhs) = expr; @@ -213,7 +213,7 @@ fn modify_assign(expr: Expr, f: fn[Env](Expr) -> Option) -> Option(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { +comptime fn modify_binary_op(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_binary_op().map( |expr: (Expr, BinaryOp, Expr)| { let (lhs, op, rhs) = expr; @@ -224,7 +224,7 @@ fn modify_binary_op(expr: Expr, f: fn[Env](Expr) -> Option) -> Option ) } -fn modify_block(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { +comptime fn modify_block(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_block().map( |exprs: [Expr]| { let exprs = modify_expressions(exprs, f); @@ -233,7 +233,7 @@ fn modify_block(expr: Expr, f: fn[Env](Expr) -> Option) -> Option(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { +comptime fn modify_cast(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_cast().map( |expr: (Expr, UnresolvedType)| { let (expr, typ) = expr; @@ -243,7 +243,7 @@ fn modify_cast(expr: Expr, f: fn[Env](Expr) -> Option) -> Option(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { +comptime fn modify_comptime(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_comptime().map( |exprs: [Expr]| { let exprs = exprs.map(|expr: Expr| expr.modify(f)); @@ -252,7 +252,7 @@ fn modify_comptime(expr: Expr, f: fn[Env](Expr) -> Option) -> Option< ) } -fn modify_function_call(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { +comptime fn modify_function_call(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_function_call().map( |expr: (Expr, [Expr])| { let (function, arguments) = expr; @@ -263,7 +263,7 @@ fn modify_function_call(expr: Expr, f: fn[Env](Expr) -> Option) -> Op ) } -fn modify_if(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { +comptime fn modify_if(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_if().map( |expr: (Expr, Expr, Option)| { let (condition, consequence, alternative) = expr; @@ -275,7 +275,7 @@ fn modify_if(expr: Expr, f: fn[Env](Expr) -> Option) -> Option ) } -fn modify_index(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { +comptime fn modify_index(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_index().map( |expr: (Expr, Expr)| { let (object, index) = expr; @@ -286,7 +286,7 @@ fn modify_index(expr: Expr, f: fn[Env](Expr) -> Option) -> Option(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { +comptime fn modify_let(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_let().map( |expr: (Expr, Option, Expr)| { let (pattern, typ, expr) = expr; @@ -297,7 +297,7 @@ fn modify_let(expr: Expr, f: fn[Env](Expr) -> Option) -> Option ) } -fn modify_member_access(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { +comptime fn modify_member_access(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_member_access().map( |expr: (Expr, Quoted)| { let (object, name) = expr; @@ -307,7 +307,7 @@ fn modify_member_access(expr: Expr, f: fn[Env](Expr) -> Option) -> Op ) } -fn modify_method_call(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { +comptime fn modify_method_call(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_method_call().map( |expr: (Expr, Quoted, [UnresolvedType], [Expr])| { let (object, name, generics, arguments) = expr; @@ -318,7 +318,7 @@ fn modify_method_call(expr: Expr, f: fn[Env](Expr) -> Option) -> Opti ) } -fn modify_repeated_element_array(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { +comptime fn modify_repeated_element_array(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_repeated_element_array().map( |expr: (Expr, Expr)| { let (expr, length) = expr; @@ -329,7 +329,7 @@ fn modify_repeated_element_array(expr: Expr, f: fn[Env](Expr) -> Option(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { +comptime fn modify_repeated_element_slice(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_repeated_element_slice().map( |expr: (Expr, Expr)| { let (expr, length) = expr; @@ -340,7 +340,7 @@ fn modify_repeated_element_slice(expr: Expr, f: fn[Env](Expr) -> Option(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { +comptime fn modify_slice(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_slice().map( |exprs: [Expr]| { let exprs = modify_expressions(exprs, f); @@ -349,7 +349,7 @@ fn modify_slice(expr: Expr, f: fn[Env](Expr) -> Option) -> Option(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { +comptime fn modify_tuple(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_tuple().map( |exprs: [Expr]| { let exprs = modify_expressions(exprs, f); @@ -358,7 +358,7 @@ fn modify_tuple(expr: Expr, f: fn[Env](Expr) -> Option) -> Option(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { +comptime fn modify_unary_op(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_unary_op().map( |expr: (UnaryOp, Expr)| { let (op, rhs) = expr; @@ -368,7 +368,7 @@ fn modify_unary_op(expr: Expr, f: fn[Env](Expr) -> Option) -> Option< ) } -fn modify_unsafe(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { +comptime fn modify_unsafe(expr: Expr, f: fn[Env](Expr) -> Option) -> Option { expr.as_unsafe().map( |exprs: [Expr]| { let exprs = exprs.map(|expr: Expr| expr.modify(f)); @@ -377,7 +377,7 @@ fn modify_unsafe(expr: Expr, f: fn[Env](Expr) -> Option) -> Option(exprs: [Expr], f: fn[Env](Expr) -> Option) -> [Expr] { +comptime fn modify_expressions(exprs: [Expr], f: fn[Env](Expr) -> Option) -> [Expr] { exprs.map(|expr: Expr| expr.modify(f)) } @@ -498,7 +498,7 @@ comptime fn new_unsafe(exprs: [Expr]) -> Expr { quote { unsafe { $exprs }}.as_expr().unwrap() } -fn join_expressions(exprs: [Expr], separator: Quoted) -> Quoted { +comptime fn join_expressions(exprs: [Expr], separator: Quoted) -> Quoted { exprs.map(|expr: Expr| expr.quoted()).join(separator) } diff --git a/noir_stdlib/src/meta/format_string.nr b/noir_stdlib/src/meta/format_string.nr index 44b69719efe..075a69fdafb 100644 --- a/noir_stdlib/src/meta/format_string.nr +++ b/noir_stdlib/src/meta/format_string.nr @@ -1,6 +1,6 @@ impl fmtstr { #[builtin(fmtstr_quoted_contents)] // docs:start:quoted_contents - fn quoted_contents(self) -> Quoted {} + comptime fn quoted_contents(self) -> Quoted {} // docs:end:quoted_contents } diff --git a/noir_stdlib/src/meta/function_def.nr b/noir_stdlib/src/meta/function_def.nr index 5ce3dbeabff..5d536d60ae1 100644 --- a/noir_stdlib/src/meta/function_def.nr +++ b/noir_stdlib/src/meta/function_def.nr @@ -1,66 +1,66 @@ impl FunctionDefinition { #[builtin(function_def_add_attribute)] // docs:start:add_attribute - fn add_attribute(self, attribute: str) {} + comptime fn add_attribute(self, attribute: str) {} // docs:end:add_attribute #[builtin(function_def_body)] // docs:start:body - fn body(self) -> Expr {} + comptime fn body(self) -> Expr {} // docs:end:body #[builtin(function_def_has_named_attribute)] // docs:start:has_named_attribute - fn has_named_attribute(self, name: Quoted) -> bool {} + comptime fn has_named_attribute(self, name: Quoted) -> bool {} // docs:end:has_named_attribute #[builtin(function_def_is_unconstrained)] // docs:start:is_unconstrained - fn is_unconstrained(self) -> bool {} + comptime fn is_unconstrained(self) -> bool {} // docs:end:is_unconstrained #[builtin(function_def_module)] // docs:start:module - fn module(self) -> Module {} + comptime fn module(self) -> Module {} // docs:end:module #[builtin(function_def_name)] // docs:start:name - fn name(self) -> Quoted {} + comptime fn name(self) -> Quoted {} // docs:end:name #[builtin(function_def_parameters)] // docs:start:parameters - fn parameters(self) -> [(Quoted, Type)] {} + comptime fn parameters(self) -> [(Quoted, Type)] {} // docs:end:parameters #[builtin(function_def_return_type)] // docs:start:return_type - fn return_type(self) -> Type {} + comptime fn return_type(self) -> Type {} // docs:end:return_type #[builtin(function_def_set_body)] // docs:start:set_body - fn set_body(self, body: Expr) {} + comptime fn set_body(self, body: Expr) {} // docs:end:set_body #[builtin(function_def_set_parameters)] // docs:start:set_parameters - fn set_parameters(self, parameters: [(Quoted, Type)]) {} + comptime fn set_parameters(self, parameters: [(Quoted, Type)]) {} // docs:end:set_parameters #[builtin(function_def_set_return_type)] // docs:start:set_return_type - fn set_return_type(self, return_type: Type) {} + comptime fn set_return_type(self, return_type: Type) {} // docs:end:set_return_type #[builtin(function_def_set_return_public)] // docs:start:set_return_public - fn set_return_public(self, public: bool) {} + comptime fn set_return_public(self, public: bool) {} // docs:end:set_return_public #[builtin(function_def_set_unconstrained)] // docs:start:set_unconstrained - fn set_unconstrained(self, value: bool) {} + comptime fn set_unconstrained(self, value: bool) {} // docs:end:set_unconstrained } diff --git a/noir_stdlib/src/meta/module.nr b/noir_stdlib/src/meta/module.nr index bee6612e1bf..6f936bf4c57 100644 --- a/noir_stdlib/src/meta/module.nr +++ b/noir_stdlib/src/meta/module.nr @@ -1,26 +1,26 @@ impl Module { #[builtin(module_add_item)] // docs:start:add_item - fn add_item(self, item: Quoted) {} + comptime fn add_item(self, item: Quoted) {} // docs:end:add_item #[builtin(module_has_named_attribute)] // docs:start:has_named_attribute - fn has_named_attribute(self, name: Quoted) -> bool {} + comptime fn has_named_attribute(self, name: Quoted) -> bool {} // docs:end:has_named_attribute #[builtin(module_is_contract)] // docs:start:is_contract - fn is_contract(self) -> bool {} + comptime fn is_contract(self) -> bool {} // docs:end:is_contract #[builtin(module_functions)] // docs:start:functions - fn functions(self) -> [FunctionDefinition] {} + comptime fn functions(self) -> [FunctionDefinition] {} // docs:end:functions #[builtin(module_name)] // docs:start:name - fn name(self) -> Quoted {} + comptime fn name(self) -> Quoted {} // docs:end:name } diff --git a/noir_stdlib/src/meta/quoted.nr b/noir_stdlib/src/meta/quoted.nr index 3fab43359c1..ff74580ce20 100644 --- a/noir_stdlib/src/meta/quoted.nr +++ b/noir_stdlib/src/meta/quoted.nr @@ -4,36 +4,36 @@ use crate::option::Option; impl Quoted { #[builtin(quoted_as_expr)] // docs:start:as_expr - fn as_expr(self) -> Option {} + comptime fn as_expr(self) -> Option {} // docs:end:as_expr #[builtin(quoted_as_module)] // docs:start:as_module - fn as_module(self) -> Option {} + comptime fn as_module(self) -> Option {} // docs:end:as_module #[builtin(quoted_as_trait_constraint)] // docs:start:as_trait_constraint - fn as_trait_constraint(self) -> TraitConstraint {} + comptime fn as_trait_constraint(self) -> TraitConstraint {} // docs:end:as_trait_constraint #[builtin(quoted_as_type)] // docs:start:as_type - fn as_type(self) -> Type {} + comptime fn as_type(self) -> Type {} // docs:end:as_type #[builtin(quoted_tokens)] // docs:start:tokens - fn tokens(self) -> [Quoted] {} + comptime fn tokens(self) -> [Quoted] {} // docs:end:tokens } impl Eq for Quoted { - fn eq(self, other: Quoted) -> bool { + comptime fn eq(self, other: Quoted) -> bool { quoted_eq(self, other) } } #[builtin(quoted_eq)] -fn quoted_eq(_first: Quoted, _second: Quoted) -> bool {} +comptime fn quoted_eq(_first: Quoted, _second: Quoted) -> bool {} diff --git a/noir_stdlib/src/meta/struct_def.nr b/noir_stdlib/src/meta/struct_def.nr index 5db720b91d3..e3621b3482e 100644 --- a/noir_stdlib/src/meta/struct_def.nr +++ b/noir_stdlib/src/meta/struct_def.nr @@ -1,47 +1,47 @@ impl StructDefinition { #[builtin(struct_def_add_attribute)] // docs:start:add_attribute - fn add_attribute(self, attribute: str) {} + comptime fn add_attribute(self, attribute: str) {} // docs:end:add_attribute #[builtin(struct_def_add_generic)] // docs:start:add_generic - fn add_generic(self, generic_name: str) -> Type {} + comptime fn add_generic(self, generic_name: str) -> Type {} // docs:end:add_generic /// Return a syntactic version of this struct definition as a type. /// For example, `as_type(quote { type Foo { ... } })` would return `Foo` #[builtin(struct_def_as_type)] // docs:start:as_type - fn as_type(self) -> Type {} + comptime fn as_type(self) -> Type {} // docs:end:as_type #[builtin(struct_def_has_named_attribute)] // docs:start:has_named_attribute - fn has_named_attribute(self, name: Quoted) -> bool {} + comptime fn has_named_attribute(self, name: Quoted) -> bool {} // docs:end:has_named_attribute /// Return each generic on this struct. #[builtin(struct_def_generics)] // docs:start:generics - fn generics(self) -> [Type] {} + comptime fn generics(self) -> [Type] {} // docs:end:generics /// Returns (name, type) pairs of each field in this struct. Each type is as-is /// with any generic arguments unchanged. #[builtin(struct_def_fields)] // docs:start:fields - fn fields(self) -> [(Quoted, Type)] {} + comptime fn fields(self) -> [(Quoted, Type)] {} // docs:end:fields #[builtin(struct_def_module)] // docs:start:module - fn module(self) -> Module {} + comptime fn module(self) -> Module {} // docs:end:module #[builtin(struct_def_name)] // docs:start:name - fn name(self) -> Quoted {} + comptime fn name(self) -> Quoted {} // docs:end:name /// Sets the fields of this struct to the given fields list. @@ -50,6 +50,6 @@ impl StructDefinition { /// Each name is expected to be a single identifier. #[builtin(struct_def_set_fields)] // docs:start:set_fields - fn set_fields(self, new_fields: [(Quoted, Type)]) {} + comptime fn set_fields(self, new_fields: [(Quoted, Type)]) {} // docs:end:set_fields } diff --git a/noir_stdlib/src/meta/trait_constraint.nr b/noir_stdlib/src/meta/trait_constraint.nr index f0276608974..b90f0b590d5 100644 --- a/noir_stdlib/src/meta/trait_constraint.nr +++ b/noir_stdlib/src/meta/trait_constraint.nr @@ -2,19 +2,19 @@ use crate::hash::{Hash, Hasher}; use crate::cmp::Eq; impl Eq for TraitConstraint { - fn eq(self, other: Self) -> bool { + comptime fn eq(self, other: Self) -> bool { constraint_eq(self, other) } } impl Hash for TraitConstraint { - fn hash(self, state: &mut H) where H: Hasher { + comptime fn hash(self, state: &mut H) where H: Hasher { state.write(constraint_hash(self)); } } #[builtin(trait_constraint_eq)] -fn constraint_eq(_first: TraitConstraint, _second: TraitConstraint) -> bool {} +comptime fn constraint_eq(_first: TraitConstraint, _second: TraitConstraint) -> bool {} #[builtin(trait_constraint_hash)] -fn constraint_hash(_constraint: TraitConstraint) -> Field {} +comptime fn constraint_hash(_constraint: TraitConstraint) -> Field {} diff --git a/noir_stdlib/src/meta/trait_def.nr b/noir_stdlib/src/meta/trait_def.nr index c26b571240b..51676efbc34 100644 --- a/noir_stdlib/src/meta/trait_def.nr +++ b/noir_stdlib/src/meta/trait_def.nr @@ -4,24 +4,24 @@ use crate::cmp::Eq; impl TraitDefinition { #[builtin(trait_def_as_trait_constraint)] // docs:start:as_trait_constraint - fn as_trait_constraint(_self: Self) -> TraitConstraint {} + comptime fn as_trait_constraint(_self: Self) -> TraitConstraint {} // docs:end:as_trait_constraint } impl Eq for TraitDefinition { - fn eq(self, other: Self) -> bool { + comptime fn eq(self, other: Self) -> bool { trait_def_eq(self, other) } } impl Hash for TraitDefinition { - fn hash(self, state: &mut H) where H: Hasher { + comptime fn hash(self, state: &mut H) where H: Hasher { state.write(trait_def_hash(self)); } } #[builtin(trait_def_eq)] -fn trait_def_eq(_first: TraitDefinition, _second: TraitDefinition) -> bool {} +comptime fn trait_def_eq(_first: TraitDefinition, _second: TraitDefinition) -> bool {} #[builtin(trait_def_hash)] -fn trait_def_hash(_def: TraitDefinition) -> Field {} +comptime fn trait_def_hash(_def: TraitDefinition) -> Field {} diff --git a/noir_stdlib/src/meta/trait_impl.nr b/noir_stdlib/src/meta/trait_impl.nr index 15b02eac6bd..6755a5c2031 100644 --- a/noir_stdlib/src/meta/trait_impl.nr +++ b/noir_stdlib/src/meta/trait_impl.nr @@ -1,11 +1,11 @@ impl TraitImpl { #[builtin(trait_impl_trait_generic_args)] // docs:start:trait_generic_args - fn trait_generic_args(self) -> [Type] {} + comptime fn trait_generic_args(self) -> [Type] {} // docs:end:trait_generic_args #[builtin(trait_impl_methods)] // docs:start:methods - fn methods(self) -> [FunctionDefinition] {} + comptime fn methods(self) -> [FunctionDefinition] {} // docs:end:methods } diff --git a/noir_stdlib/src/meta/typ.nr b/noir_stdlib/src/meta/typ.nr index 71bd6fd7f1c..d692f6e5a7e 100644 --- a/noir_stdlib/src/meta/typ.nr +++ b/noir_stdlib/src/meta/typ.nr @@ -3,71 +3,71 @@ use crate::option::Option; #[builtin(fresh_type_variable)] // docs:start:fresh_type_variable -pub fn fresh_type_variable() -> Type {} +pub comptime fn fresh_type_variable() -> Type {} // docs:end:fresh_type_variable impl Type { #[builtin(type_as_array)] // docs:start:as_array - fn as_array(self) -> Option<(Type, Type)> {} + comptime fn as_array(self) -> Option<(Type, Type)> {} // docs:end:as_array #[builtin(type_as_constant)] // docs:start:as_constant - fn as_constant(self) -> Option {} + comptime fn as_constant(self) -> Option {} // docs:end:as_constant #[builtin(type_as_integer)] // docs:start:as_integer - fn as_integer(self) -> Option<(bool, u8)> {} + comptime fn as_integer(self) -> Option<(bool, u8)> {} // docs:end:as_integer #[builtin(type_as_slice)] // docs:start:as_slice - fn as_slice(self) -> Option {} + comptime fn as_slice(self) -> Option {} // docs:end:as_slice #[builtin(type_as_str)] // docs:start:as_str - fn as_str(self) -> Option {} + comptime fn as_str(self) -> Option {} // docs:end:as_str #[builtin(type_as_struct)] // docs:start:as_struct - fn as_struct(self) -> Option<(StructDefinition, [Type])> {} + comptime fn as_struct(self) -> Option<(StructDefinition, [Type])> {} // docs:end:as_struct #[builtin(type_as_tuple)] // docs:start:as_tuple - fn as_tuple(self) -> Option<[Type]> {} + comptime fn as_tuple(self) -> Option<[Type]> {} // docs:end:as_tuple #[builtin(type_get_trait_impl)] // docs:start:get_trait_impl - fn get_trait_impl(self, constraint: TraitConstraint) -> Option {} + comptime fn get_trait_impl(self, constraint: TraitConstraint) -> Option {} // docs:end:get_trait_impl #[builtin(type_implements)] // docs:start:implements - fn implements(self, constraint: TraitConstraint) -> bool {} + comptime fn implements(self, constraint: TraitConstraint) -> bool {} // docs:end:implements #[builtin(type_is_bool)] // docs:start:is_bool - fn is_bool(self) -> bool {} + comptime fn is_bool(self) -> bool {} // docs:end:is_bool #[builtin(type_is_field)] // docs:start:is_field - fn is_field(self) -> bool {} + comptime fn is_field(self) -> bool {} // docs:end:is_field } impl Eq for Type { - fn eq(self, other: Self) -> bool { + comptime fn eq(self, other: Self) -> bool { type_eq(self, other) } } #[builtin(type_eq)] -fn type_eq(_first: Type, _second: Type) -> bool {} +comptime fn type_eq(_first: Type, _second: Type) -> bool {} diff --git a/noir_stdlib/src/meta/typed_expr.nr b/noir_stdlib/src/meta/typed_expr.nr index 8daede97438..e01c0d3af67 100644 --- a/noir_stdlib/src/meta/typed_expr.nr +++ b/noir_stdlib/src/meta/typed_expr.nr @@ -3,6 +3,6 @@ use crate::option::Option; impl TypedExpr { #[builtin(typed_expr_as_function_definition)] // docs:start:as_function_definition - fn as_function_definition(self) -> Option {} + comptime fn as_function_definition(self) -> Option {} // docs:end:as_function_definition } diff --git a/noir_stdlib/src/meta/unresolved_type.nr b/noir_stdlib/src/meta/unresolved_type.nr index 2589174ed64..f53635414cc 100644 --- a/noir_stdlib/src/meta/unresolved_type.nr +++ b/noir_stdlib/src/meta/unresolved_type.nr @@ -1,6 +1,6 @@ impl UnresolvedType { #[builtin(unresolved_type_is_field)] // docs:start:is_field - fn is_field(self) -> bool {} + comptime fn is_field(self) -> bool {} // docs:end:is_field } diff --git a/test_programs/compile_success_empty/comptime_fmt_strings/src/main.nr b/test_programs/compile_success_empty/comptime_fmt_strings/src/main.nr index 0e2d459a00f..ca337c822d8 100644 --- a/test_programs/compile_success_empty/comptime_fmt_strings/src/main.nr +++ b/test_programs/compile_success_empty/comptime_fmt_strings/src/main.nr @@ -17,7 +17,7 @@ fn main() { call!(glue(quote { hello }, quote { world })); } -fn glue(x: Quoted, y: Quoted) -> Quoted { +comptime fn glue(x: Quoted, y: Quoted) -> Quoted { f"{x}_{y}".quoted_contents() } diff --git a/test_programs/compile_success_empty/comptime_function_definition/src/main.nr b/test_programs/compile_success_empty/comptime_function_definition/src/main.nr index 6bfd8ef9699..e01ff4a71f1 100644 --- a/test_programs/compile_success_empty/comptime_function_definition/src/main.nr +++ b/test_programs/compile_success_empty/comptime_function_definition/src/main.nr @@ -66,7 +66,7 @@ contract some_contract { } } -fn set_pub_return(f: FunctionDefinition) { +comptime fn set_pub_return(f: FunctionDefinition) { f.set_return_public(true); } From 28415efd2fd8c7b836516b154ab54d65f15fbc23 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 10 Sep 2024 11:38:26 -0300 Subject: [PATCH 11/13] feat: better error message for misplaced doc comments (#5990) # Description ## Problem Resolves #5989 ## Summary Whenever there's a parsing error and we find a doc comment it means it's a misplaced doc comment. ## Additional Context ## Documentation Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- compiler/noirc_frontend/src/parser/errors.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/parser/errors.rs b/compiler/noirc_frontend/src/parser/errors.rs index ad6f6b928ab..6ba4cb68500 100644 --- a/compiler/noirc_frontend/src/parser/errors.rs +++ b/compiler/noirc_frontend/src/parser/errors.rs @@ -1,6 +1,7 @@ use crate::ast::{Expression, IntegerBitSize}; use crate::lexer::errors::LexerErrorKind; use crate::lexer::token::Token; +use crate::token::TokenKind; use small_ord_set::SmallOrdSet; use thiserror::Error; @@ -211,8 +212,17 @@ impl<'a> From<&'a ParserError> for Diagnostic { other => Diagnostic::simple_error(format!("{other}"), String::new(), error.span), }, None => { - let primary = error.to_string(); - Diagnostic::simple_error(primary, String::new(), error.span) + if matches!( + error.found.kind(), + TokenKind::InnerDocComment | TokenKind::OuterDocComment + ) { + let primary = "This doc comment doesn't document anything".to_string(); + let secondary = "Consider changing it to a regular `//` comment".to_string(); + Diagnostic::simple_error(primary, secondary, error.span) + } else { + let primary = error.to_string(); + Diagnostic::simple_error(primary, String::new(), error.span) + } } } } From 31f50c442b59eac4de2c5c530278e345bd2f149f Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 10 Sep 2024 14:59:47 -0300 Subject: [PATCH 12/13] feat: add `TypedExpr::get_type` (#5992) # Description ## Problem Part of #5668 ## Summary ## Additional Context ## Documentation Check one: - [ ] No documentation needed. - [x] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: jfecher --- .../src/hir/comptime/interpreter/builtin.rs | 26 +++++++++++++++++++ .../noir/standard_library/meta/typed_expr.md | 10 +++++-- noir_stdlib/src/meta/typed_expr.nr | 8 ++++++ .../comptime_typed_expr/Nargo.toml | 7 +++++ .../comptime_typed_expr/src/main.nr | 7 +++++ 5 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 test_programs/compile_success_empty/comptime_typed_expr/Nargo.toml create mode 100644 test_programs/compile_success_empty/comptime_typed_expr/src/main.nr diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index 899d62ecb61..166c6479b15 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -189,6 +189,9 @@ impl<'local, 'context> Interpreter<'local, 'context> { "typed_expr_as_function_definition" => { typed_expr_as_function_definition(interner, arguments, return_type, location) } + "typed_expr_get_type" => { + typed_expr_get_type(interner, arguments, return_type, location) + } "unresolved_type_is_field" => unresolved_type_is_field(interner, arguments, location), "zeroed" => zeroed(return_type), _ => { @@ -1070,6 +1073,7 @@ fn trait_impl_trait_generic_args( Ok(Value::Slice(trait_generics, slice_type)) } +// fn as_function_definition(self) -> Option fn typed_expr_as_function_definition( interner: &NodeInterner, arguments: Vec<(Value, Location)>, @@ -1087,6 +1091,28 @@ fn typed_expr_as_function_definition( option(return_type, option_value) } +// fn get_type(self) -> Option +fn typed_expr_get_type( + interner: &NodeInterner, + arguments: Vec<(Value, Location)>, + return_type: Type, + location: Location, +) -> IResult { + let self_argument = check_one_argument(arguments, location)?; + let typed_expr = get_typed_expr(self_argument)?; + let option_value = if let TypedExpr::ExprId(expr_id) = typed_expr { + let typ = interner.id_type(expr_id); + if typ == Type::Error { + None + } else { + Some(Value::Type(typ)) + } + } else { + None + }; + option(return_type, option_value) +} + // fn is_field(self) -> bool fn unresolved_type_is_field( interner: &NodeInterner, diff --git a/docs/docs/noir/standard_library/meta/typed_expr.md b/docs/docs/noir/standard_library/meta/typed_expr.md index eacfd9c1230..24b23f2ec19 100644 --- a/docs/docs/noir/standard_library/meta/typed_expr.md +++ b/docs/docs/noir/standard_library/meta/typed_expr.md @@ -6,8 +6,14 @@ title: TypedExpr ## Methods -### as_function_definition +### get_type #include_code as_function_definition noir_stdlib/src/meta/typed_expr.nr rust -If this expression refers to a function definitions, returns it. Otherwise returns `Option::none()`. \ No newline at end of file +If this expression refers to a function definitions, returns it. Otherwise returns `Option::none()`. + +### get_type + +#include_code get_type noir_stdlib/src/meta/typed_expr.nr rust + +Returns the type of the expression, or `Option::none()` if there were errors when the expression was previously resolved. \ No newline at end of file diff --git a/noir_stdlib/src/meta/typed_expr.nr b/noir_stdlib/src/meta/typed_expr.nr index e01c0d3af67..1d3b073b6da 100644 --- a/noir_stdlib/src/meta/typed_expr.nr +++ b/noir_stdlib/src/meta/typed_expr.nr @@ -1,8 +1,16 @@ +//! Contains methods on the built-in `TypedExpr` type for resolved and type-checked expressions. use crate::option::Option; impl TypedExpr { + /// If this expression refers to a function definitions, returns it. Otherwise returns `Option::none()`. #[builtin(typed_expr_as_function_definition)] // docs:start:as_function_definition comptime fn as_function_definition(self) -> Option {} // docs:end:as_function_definition + + /// Returns the type of the expression, if the expression could be resolved without errors. + #[builtin(typed_expr_get_type)] + // docs:start:get_type + comptime fn get_type(self) -> Option {} + // docs:end:get_type } diff --git a/test_programs/compile_success_empty/comptime_typed_expr/Nargo.toml b/test_programs/compile_success_empty/comptime_typed_expr/Nargo.toml new file mode 100644 index 00000000000..151b8b8e9d0 --- /dev/null +++ b/test_programs/compile_success_empty/comptime_typed_expr/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "comptime_typed_expr" +type = "bin" +authors = [""] +compiler_version = ">=0.31.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/compile_success_empty/comptime_typed_expr/src/main.nr b/test_programs/compile_success_empty/comptime_typed_expr/src/main.nr new file mode 100644 index 00000000000..ba2594f1da9 --- /dev/null +++ b/test_programs/compile_success_empty/comptime_typed_expr/src/main.nr @@ -0,0 +1,7 @@ +fn main() { + comptime + { + let expr = quote { [1, 3] }.as_expr().unwrap().resolve(Option::none()); + assert_eq(expr.get_type().unwrap(), quote { [Field; 2] }.as_type()); + } +} From e84f7d2e81c1f59e9af015f38c2d477607a9c558 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 10 Sep 2024 15:31:03 -0300 Subject: [PATCH 13/13] feat: LSP completion function detail (#5993) # Description ## Problem Just a tiny thing: when functions are suggested, Rust Analyzer shows the full function signature above the documentation (this is a completion item's `detail` property). It's useful because in the list the signature is usually trimmed. ## Summary Before: ![image](https://github.com/user-attachments/assets/173acca4-74fe-40ff-9c8c-6552296c05e4) After: ![image](https://github.com/user-attachments/assets/520fc7df-bf01-4ef2-887a-b4e651f4035c) ## Additional Context Also includes a bunch of small refactors to make it easier to test that something is a function completion item regardless of whether it ends up being a snippet completion or not, whether it triggers a command, etc. ## Documentation Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- .../requests/completion/completion_items.rs | 48 +++++++++---- tooling/lsp/src/requests/completion/tests.rs | 71 +++++++------------ 2 files changed, 59 insertions(+), 60 deletions(-) diff --git a/tooling/lsp/src/requests/completion/completion_items.rs b/tooling/lsp/src/requests/completion/completion_items.rs index 7e7511cdaa3..163a4e15d00 100644 --- a/tooling/lsp/src/requests/completion/completion_items.rs +++ b/tooling/lsp/src/requests/completion/completion_items.rs @@ -213,11 +213,14 @@ impl<'a> NodeFinder<'a> { let name = if self_prefix { format!("self.{}", name) } else { name.clone() }; let name = &name; let description = func_meta_type_to_string(func_meta, func_self_type.is_some()); + let mut has_arguments = false; let completion_item = match function_completion_kind { - FunctionCompletionKind::Name => { - simple_completion_item(name, CompletionItemKind::FUNCTION, Some(description)) - } + FunctionCompletionKind::Name => simple_completion_item( + name, + CompletionItemKind::FUNCTION, + Some(description.clone()), + ), FunctionCompletionKind::NameAndParameters => { let kind = CompletionItemKind::FUNCTION; let skip_first_argument = attribute_first_type.is_some(); @@ -227,20 +230,25 @@ impl<'a> NodeFinder<'a> { function_kind, skip_first_argument, ); - let (label, insert_text) = if insert_text.ends_with("()") { - if skip_first_argument { - (name.to_string(), insert_text.strip_suffix("()").unwrap().to_string()) - } else { - (format!("{}()", name), insert_text) - } - } else { - (format!("{}(…)", name), insert_text) - }; - snippet_completion_item(label, kind, insert_text, Some(description)) + if insert_text.ends_with("()") { + let label = + if skip_first_argument { name.to_string() } else { format!("{}()", name) }; + simple_completion_item(label, kind, Some(description.clone())) + } else { + has_arguments = true; + snippet_completion_item( + format!("{}(…)", name), + kind, + insert_text, + Some(description.clone()), + ) + } } }; + let completion_item = completion_item_with_detail(completion_item, description); + let completion_item = if is_operator { completion_item_with_sort_text(completion_item, operator_sort_text()) } else if function_kind == FunctionKind::Any && name == "new" { @@ -254,11 +262,16 @@ impl<'a> NodeFinder<'a> { let completion_item = match function_completion_kind { FunctionCompletionKind::Name => completion_item, FunctionCompletionKind::NameAndParameters => { - completion_item_with_trigger_parameter_hints_command(completion_item) + if has_arguments { + completion_item_with_trigger_parameter_hints_command(completion_item) + } else { + completion_item + } } }; let completion_item = self.completion_item_with_doc_comments(ReferenceId::Function(func_id), completion_item); + Some(completion_item) } @@ -494,3 +507,10 @@ pub(super) fn completion_item_with_trigger_parameter_hints_command( ..completion_item } } + +pub(super) fn completion_item_with_detail( + completion_item: CompletionItem, + detail: String, +) -> CompletionItem { + CompletionItem { detail: Some(detail), ..completion_item } +} diff --git a/tooling/lsp/src/requests/completion/tests.rs b/tooling/lsp/src/requests/completion/tests.rs index 97cb145eb22..a2eacea4b46 100644 --- a/tooling/lsp/src/requests/completion/tests.rs +++ b/tooling/lsp/src/requests/completion/tests.rs @@ -5,7 +5,7 @@ mod completion_tests { requests::{ completion::{ completion_items::{ - completion_item_with_sort_text, + completion_item_with_detail, completion_item_with_sort_text, completion_item_with_trigger_parameter_hints_command, module_completion_item, simple_completion_item, snippet_completion_item, }, @@ -107,12 +107,22 @@ mod completion_tests { insert_text: impl Into, description: impl Into, ) -> CompletionItem { - completion_item_with_trigger_parameter_hints_command(snippet_completion_item( - label, - CompletionItemKind::FUNCTION, - insert_text, - Some(description.into()), - )) + let insert_text: String = insert_text.into(); + let description: String = description.into(); + + let has_arguments = insert_text.ends_with(')') && !insert_text.ends_with("()"); + let completion_item = if has_arguments { + completion_item_with_trigger_parameter_hints_command(snippet_completion_item( + label, + CompletionItemKind::FUNCTION, + insert_text, + Some(description.clone()), + )) + } else { + simple_completion_item(label, CompletionItemKind::FUNCTION, Some(description.clone())) + }; + + completion_item_with_detail(completion_item, description) } fn field_completion_item(field: &str, typ: impl Into) -> CompletionItem { @@ -191,15 +201,8 @@ mod completion_tests { use foo::>|< "#; - assert_completion( - src, - vec![simple_completion_item( - "bar", - CompletionItemKind::FUNCTION, - Some("fn(i32) -> u64".to_string()), - )], - ) - .await; + assert_completion(src, vec![function_completion_item("bar", "bar()", "fn(i32) -> u64")]) + .await; } #[test] @@ -1672,11 +1675,7 @@ mod completion_tests { "#; assert_completion_excluding_auto_import( src, - vec![simple_completion_item( - "hello_world", - CompletionItemKind::FUNCTION, - Some("fn()".to_string()), - )], + vec![function_completion_item("hello_world", "hello_world()", "fn()")], ) .await; } @@ -1694,11 +1693,7 @@ mod completion_tests { "#; assert_completion_excluding_auto_import( src, - vec![simple_completion_item( - "bar", - CompletionItemKind::FUNCTION, - Some("fn()".to_string()), - )], + vec![function_completion_item("bar", "bar()", "fn()")], ) .await; } @@ -1716,11 +1711,7 @@ mod completion_tests { "#; assert_completion_excluding_auto_import( src, - vec![simple_completion_item( - "bar", - CompletionItemKind::FUNCTION, - Some("fn()".to_string()), - )], + vec![function_completion_item("bar", "bar()", "fn()")], ) .await; } @@ -1738,11 +1729,7 @@ mod completion_tests { "#; assert_completion_excluding_auto_import( src, - vec![simple_completion_item( - "bar", - CompletionItemKind::FUNCTION, - Some("fn()".to_string()), - )], + vec![function_completion_item("bar", "bar()", "fn()")], ) .await; } @@ -1762,11 +1749,7 @@ mod completion_tests { "#; assert_completion_excluding_auto_import( src, - vec![simple_completion_item( - "bar", - CompletionItemKind::FUNCTION, - Some("fn(self)".to_string()), - )], + vec![function_completion_item("bar", "bar()", "fn(self)")], ) .await; } @@ -1786,11 +1769,7 @@ mod completion_tests { "#; assert_completion_excluding_auto_import( src, - vec![simple_completion_item( - "bar", - CompletionItemKind::FUNCTION, - Some("fn(self)".to_string()), - )], + vec![function_completion_item("bar", "bar()", "fn(self)")], ) .await; }