From 2ac64ca691db55f37a3d5703cbc51c9d9afec5f1 Mon Sep 17 00:00:00 2001 From: Tom French Date: Thu, 25 Jan 2024 22:31:09 +0000 Subject: [PATCH 01/10] feat: prevent declarations of blackbox functions outside of the stdlib --- .../noirc_frontend/src/hir/def_collector/dc_mod.rs | 10 ++++++++++ .../noirc_frontend/src/hir/def_collector/errors.rs | 7 +++++++ .../low_level_function_declaration/Nargo.toml | 7 +++++++ .../low_level_function_declaration/src/main.nr | 10 ++++++++++ 4 files changed, 34 insertions(+) create mode 100644 test_programs/compile_failure/low_level_function_declaration/Nargo.toml create mode 100644 test_programs/compile_failure/low_level_function_declaration/src/main.nr diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 3cd60c33b8b..8e1aa628287 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -209,6 +209,7 @@ impl<'a> ModCollector<'a> { let mut errors = vec![]; let module = ModuleId { krate, local_id: self.module_id }; + let in_stdlib = *context.stdlib_crate_id() == krate; for function in functions { // check if optional field attribute is compatible with native field @@ -218,6 +219,15 @@ impl<'a> ModCollector<'a> { } } + let is_foreign_function = + function.attributes().function.as_ref().map_or(false, |func| func.is_foreign()); + if !in_stdlib && is_foreign_function { + let error = DefCollectorErrorKind::LowLevelFunctionOutsideOfStdlib { + span: function.span(), + }; + errors.push((error.into(), self.file_id)); + } + let name = function.name_ident().clone(); let func_id = context.def_interner.push_empty_fn(); diff --git a/compiler/noirc_frontend/src/hir/def_collector/errors.rs b/compiler/noirc_frontend/src/hir/def_collector/errors.rs index de45be48c4e..510f95c9ae4 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/errors.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/errors.rs @@ -73,6 +73,8 @@ pub enum DefCollectorErrorKind { "Either the type or the trait must be from the same crate as the trait implementation" )] TraitImplOrphaned { span: Span }, + #[error("Usage of the `#[foreign]` function attribute is not allowed outside of the Noir standard library")] + LowLevelFunctionOutsideOfStdlib { span: Span }, #[error("macro error : {0:?}")] MacroError(MacroError), } @@ -246,6 +248,11 @@ impl From for Diagnostic { "Either the type or the trait must be from the same crate as the trait implementation".into(), span, ), + DefCollectorErrorKind::LowLevelFunctionOutsideOfStdlib { span } => Diagnostic::simple_error( + "Definition of low-level function outside of standard library".into(), + "Usage of the `#[foreign]` function attribute is not allowed outside of the Noir standard library".into(), + span, + ), DefCollectorErrorKind::MacroError(macro_error) => { Diagnostic::simple_error(macro_error.primary_message, macro_error.secondary_message.unwrap_or_default(), macro_error.span.unwrap_or_default()) }, diff --git a/test_programs/compile_failure/low_level_function_declaration/Nargo.toml b/test_programs/compile_failure/low_level_function_declaration/Nargo.toml new file mode 100644 index 00000000000..39a5f41d804 --- /dev/null +++ b/test_programs/compile_failure/low_level_function_declaration/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "low_level_function_declaration" +type = "bin" +authors = [""] +compiler_version = ">=0.23.0" + +[dependencies] diff --git a/test_programs/compile_failure/low_level_function_declaration/src/main.nr b/test_programs/compile_failure/low_level_function_declaration/src/main.nr new file mode 100644 index 00000000000..6273067f6a7 --- /dev/null +++ b/test_programs/compile_failure/low_level_function_declaration/src/main.nr @@ -0,0 +1,10 @@ +// This test prevents users from trying to create their own blackbox functions as these should only exist in the stdlib. + +// This would otherwise be a perfectly valid definition of the `pedersen_hash` black box function, +// however executing the circuit results in an unhelpful ICE. +#[foreign(pedersen_hash)] +fn my_pedersen_hash(_input: [Field; N]) -> Field {} + +fn main() -> pub Field { + my_pedersen_hash([1]) +} From b6f608631091bb72155c3454bc6dadf71887a601 Mon Sep 17 00:00:00 2001 From: Tom French Date: Thu, 25 Jan 2024 22:45:05 +0000 Subject: [PATCH 02/10] feat: disallow builtin function definitions as well --- .../noirc_frontend/src/hir/def_collector/dc_mod.rs | 6 +++--- .../noirc_frontend/src/hir/def_collector/errors.rs | 4 ++-- .../Nargo.toml | 2 +- .../builtin_function_declaration/Prover.toml | 0 .../builtin_function_declaration/src/main.nr | 10 ++++++++++ .../foreign_function_declaration/Nargo.toml | 7 +++++++ .../src/main.nr | 0 7 files changed, 23 insertions(+), 6 deletions(-) rename test_programs/compile_failure/{low_level_function_declaration => builtin_function_declaration}/Nargo.toml (67%) create mode 100644 test_programs/compile_failure/builtin_function_declaration/Prover.toml create mode 100644 test_programs/compile_failure/builtin_function_declaration/src/main.nr create mode 100644 test_programs/compile_failure/foreign_function_declaration/Nargo.toml rename test_programs/compile_failure/{low_level_function_declaration => foreign_function_declaration}/src/main.nr (100%) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 8e1aa628287..4f581c3d5f9 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -219,9 +219,9 @@ impl<'a> ModCollector<'a> { } } - let is_foreign_function = - function.attributes().function.as_ref().map_or(false, |func| func.is_foreign()); - if !in_stdlib && is_foreign_function { + let is_low_level_function = + function.attributes().function.as_ref().map_or(false, |func| func.is_low_level()); + if !in_stdlib && is_low_level_function { let error = DefCollectorErrorKind::LowLevelFunctionOutsideOfStdlib { span: function.span(), }; diff --git a/compiler/noirc_frontend/src/hir/def_collector/errors.rs b/compiler/noirc_frontend/src/hir/def_collector/errors.rs index 510f95c9ae4..7cd2f117254 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/errors.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/errors.rs @@ -73,7 +73,7 @@ pub enum DefCollectorErrorKind { "Either the type or the trait must be from the same crate as the trait implementation" )] TraitImplOrphaned { span: Span }, - #[error("Usage of the `#[foreign]` function attribute is not allowed outside of the Noir standard library")] + #[error("Usage of the `#[foreign]` or `#[builtin] function attributes are not allowed outside of the Noir standard library")] LowLevelFunctionOutsideOfStdlib { span: Span }, #[error("macro error : {0:?}")] MacroError(MacroError), @@ -250,7 +250,7 @@ impl From for Diagnostic { ), DefCollectorErrorKind::LowLevelFunctionOutsideOfStdlib { span } => Diagnostic::simple_error( "Definition of low-level function outside of standard library".into(), - "Usage of the `#[foreign]` function attribute is not allowed outside of the Noir standard library".into(), + "Usage of the `#[foreign]` or `#[builtin] function attributes are not allowed outside of the Noir standard library".into(), span, ), DefCollectorErrorKind::MacroError(macro_error) => { diff --git a/test_programs/compile_failure/low_level_function_declaration/Nargo.toml b/test_programs/compile_failure/builtin_function_declaration/Nargo.toml similarity index 67% rename from test_programs/compile_failure/low_level_function_declaration/Nargo.toml rename to test_programs/compile_failure/builtin_function_declaration/Nargo.toml index 39a5f41d804..3835292a6ba 100644 --- a/test_programs/compile_failure/low_level_function_declaration/Nargo.toml +++ b/test_programs/compile_failure/builtin_function_declaration/Nargo.toml @@ -1,5 +1,5 @@ [package] -name = "low_level_function_declaration" +name = "builtin_function_declaration" type = "bin" authors = [""] compiler_version = ">=0.23.0" diff --git a/test_programs/compile_failure/builtin_function_declaration/Prover.toml b/test_programs/compile_failure/builtin_function_declaration/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test_programs/compile_failure/builtin_function_declaration/src/main.nr b/test_programs/compile_failure/builtin_function_declaration/src/main.nr new file mode 100644 index 00000000000..ed376557371 --- /dev/null +++ b/test_programs/compile_failure/builtin_function_declaration/src/main.nr @@ -0,0 +1,10 @@ +// This test prevents users from trying to create their own builtin functions as these should only exist in the stdlib. + +// This would otherwise be a perfectly valid declaration of the `to_le_bits` builtin function +#[builtin(to_le_bits)] +fn to_le_bits(_x: Field, _bit_size: u32) -> [u1] {} + +fn main(x: Field) -> pub u1 { + let bits = to_le_bits(x, 100); + bits[0] +} diff --git a/test_programs/compile_failure/foreign_function_declaration/Nargo.toml b/test_programs/compile_failure/foreign_function_declaration/Nargo.toml new file mode 100644 index 00000000000..951658d7fb8 --- /dev/null +++ b/test_programs/compile_failure/foreign_function_declaration/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "foreign_function_declaration" +type = "bin" +authors = [""] +compiler_version = ">=0.23.0" + +[dependencies] diff --git a/test_programs/compile_failure/low_level_function_declaration/src/main.nr b/test_programs/compile_failure/foreign_function_declaration/src/main.nr similarity index 100% rename from test_programs/compile_failure/low_level_function_declaration/src/main.nr rename to test_programs/compile_failure/foreign_function_declaration/src/main.nr From 8d3893cd8ce5dc6f508cccf803ef603cbe071cc6 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Thu, 25 Jan 2024 22:47:02 +0000 Subject: [PATCH 03/10] Apply suggestions from code review --- compiler/noirc_frontend/src/hir/def_collector/errors.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/errors.rs b/compiler/noirc_frontend/src/hir/def_collector/errors.rs index 7cd2f117254..123f9c0ccbe 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/errors.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/errors.rs @@ -73,7 +73,7 @@ pub enum DefCollectorErrorKind { "Either the type or the trait must be from the same crate as the trait implementation" )] TraitImplOrphaned { span: Span }, - #[error("Usage of the `#[foreign]` or `#[builtin] function attributes are not allowed outside of the Noir standard library")] + #[error("Usage of the `#[foreign]` or `#[builtin]` function attributes are not allowed outside of the Noir standard library")] LowLevelFunctionOutsideOfStdlib { span: Span }, #[error("macro error : {0:?}")] MacroError(MacroError), @@ -250,7 +250,7 @@ impl From for Diagnostic { ), DefCollectorErrorKind::LowLevelFunctionOutsideOfStdlib { span } => Diagnostic::simple_error( "Definition of low-level function outside of standard library".into(), - "Usage of the `#[foreign]` or `#[builtin] function attributes are not allowed outside of the Noir standard library".into(), + "Usage of the `#[foreign]` or `#[builtin]` function attributes are not allowed outside of the Noir standard library".into(), span, ), DefCollectorErrorKind::MacroError(macro_error) => { From df192063ef5ec2057c3ff7903afdacf85f791cb2 Mon Sep 17 00:00:00 2001 From: Tom French Date: Fri, 26 Jan 2024 09:26:57 +0000 Subject: [PATCH 04/10] chore: add dummy stdlib in tests --- compiler/noirc_frontend/src/tests.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 9ccbddab9ec..59b1a553821 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -6,6 +6,7 @@ mod test { use core::panic; use std::collections::BTreeMap; + use std::path::Path; use fm::FileId; @@ -51,11 +52,19 @@ mod test { src: &str, ) -> (ParsedModule, Context, Vec<(CompilationError, FileId)>) { let root = std::path::Path::new("/"); - let fm = FileManager::new(root); + let mut fm = FileManager::new(root); + let entrypoint = Path::new("main.nr"); + let root_file_id = fm.add_file_with_source(entrypoint, src.to_string()).unwrap(); + let path_to_std_lib_file = Path::new("std").join("lib.nr"); + let std_file_id = + fm.add_file_with_source_canonical_path(&path_to_std_lib_file, "".to_string()).unwrap(); + let mut context = Context::new(fm, Default::default()); context.def_interner.populate_dummy_operator_traits(); - let root_file_id = FileId::dummy(); + let root_crate_id = context.crate_graph.add_crate_root(root_file_id); + context.crate_graph.add_stdlib(std_file_id); + let (program, parser_errors) = parse_program(src); let mut errors = vecmap(parser_errors, |e| (e.into(), root_file_id)); remove_experimental_warnings(&mut errors); From 571e403ee7d70062d0b8c273b614e4f19bafe337 Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 30 Jan 2024 17:31:23 +0000 Subject: [PATCH 05/10] chore: move check into function resolver --- .../noirc_frontend/src/hir/def_collector/dc_mod.rs | 10 ---------- .../noirc_frontend/src/hir/resolution/errors.rs | 7 +++++++ .../noirc_frontend/src/hir/resolution/functions.rs | 2 +- compiler/noirc_frontend/src/hir/resolution/impls.rs | 2 +- .../noirc_frontend/src/hir/resolution/resolver.rs | 13 +++++++++++++ .../noirc_frontend/src/hir/resolution/traits.rs | 9 +++++++-- 6 files changed, 29 insertions(+), 14 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 4f581c3d5f9..3cd60c33b8b 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -209,7 +209,6 @@ impl<'a> ModCollector<'a> { let mut errors = vec![]; let module = ModuleId { krate, local_id: self.module_id }; - let in_stdlib = *context.stdlib_crate_id() == krate; for function in functions { // check if optional field attribute is compatible with native field @@ -219,15 +218,6 @@ impl<'a> ModCollector<'a> { } } - let is_low_level_function = - function.attributes().function.as_ref().map_or(false, |func| func.is_low_level()); - if !in_stdlib && is_low_level_function { - let error = DefCollectorErrorKind::LowLevelFunctionOutsideOfStdlib { - span: function.span(), - }; - errors.push((error.into(), self.file_id)); - } - let name = function.name_ident().clone(); let func_id = context.def_interner.push_empty_fn(); diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index 390807afd17..7bd4de77e84 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -84,6 +84,8 @@ pub enum ResolverError { InvalidTypeForEntryPoint { span: Span }, #[error("Nested slices are not supported")] NestedSlices { span: Span }, + #[error("Usage of the `#[foreign]` or `#[builtin]` function attributes are not allowed outside of the Noir standard library")] + LowLevelFunctionOutsideOfStdlib { ident: Ident }, } impl ResolverError { @@ -311,6 +313,11 @@ impl From for Diagnostic { "Try to use a constant sized array instead".into(), span, ), + ResolverError::LowLevelFunctionOutsideOfStdlib { ident } => Diagnostic::simple_error( + "Definition of low-level function outside of standard library".into(), + "Usage of the `#[foreign]` or `#[builtin]` function attributes are not allowed outside of the Noir standard library".into(), + ident.span(), + ), } } } diff --git a/compiler/noirc_frontend/src/hir/resolution/functions.rs b/compiler/noirc_frontend/src/hir/resolution/functions.rs index e63de9b9173..b2eab9e97ca 100644 --- a/compiler/noirc_frontend/src/hir/resolution/functions.rs +++ b/compiler/noirc_frontend/src/hir/resolution/functions.rs @@ -37,7 +37,7 @@ pub(crate) fn resolve_function_set( let module_id = ModuleId { krate: crate_id, local_id: mod_id }; let path_resolver = StandardPathResolver::new(module_id); - let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file_id); + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, crate_id, file_id); // Must use set_generics here to ensure we re-use the same generics from when // the impl was originally collected. Otherwise the function will be using different // TypeVariables for the same generic, causing it to instantiate incorrectly. diff --git a/compiler/noirc_frontend/src/hir/resolution/impls.rs b/compiler/noirc_frontend/src/hir/resolution/impls.rs index 4aa70f00cfc..80a3f2427f9 100644 --- a/compiler/noirc_frontend/src/hir/resolution/impls.rs +++ b/compiler/noirc_frontend/src/hir/resolution/impls.rs @@ -98,7 +98,7 @@ pub(crate) fn resolve_impls( let file = def_maps[&crate_id].file_id(module_id); for (generics, _, functions) in methods { - let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, crate_id, file); resolver.add_generics(&generics); let generics = resolver.get_generics().to_vec(); let self_type = resolver.resolve_type(unresolved_type.clone()); diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index f3e3b221036..33cc39cad5e 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -101,6 +101,9 @@ pub struct Resolver<'a> { /// for these so we can still resolve them in the parent module without them being in a contract. in_contract: bool, + /// True if the current crate is the Noir standard library. + in_stdlib: bool, + /// Contains a mapping of the current struct or functions's generics to /// unique type variables if we're resolving a struct. Empty otherwise. /// This is a Vec rather than a map to preserve the order a functions generics @@ -131,10 +134,12 @@ impl<'a> Resolver<'a> { interner: &'a mut NodeInterner, path_resolver: &'a dyn PathResolver, def_maps: &'a BTreeMap, + crate_id: CrateId, file: FileId, ) -> Resolver<'a> { let module_id = path_resolver.module_id(); let in_contract = module_id.module(def_maps).is_contract; + let in_stdlib = crate_id.is_stdlib(); Self { path_resolver, @@ -150,6 +155,7 @@ impl<'a> Resolver<'a> { current_trait_impl: None, file, in_contract, + in_stdlib, } } @@ -900,6 +906,13 @@ impl<'a> Resolver<'a> { position: PubPosition::ReturnType, }); } + let is_low_level_function = + func.attributes().function.as_ref().map_or(false, |func| func.is_low_level()); + if !self.in_stdlib && is_low_level_function { + let error = + ResolverError::LowLevelFunctionOutsideOfStdlib { ident: func.name_ident().clone() }; + self.push_err(error); + } // 'pub' is required on return types for entry point functions if self.is_entry_point_function(func) diff --git a/compiler/noirc_frontend/src/hir/resolution/traits.rs b/compiler/noirc_frontend/src/hir/resolution/traits.rs index 8f966be312b..87d7bf02106 100644 --- a/compiler/noirc_frontend/src/hir/resolution/traits.rs +++ b/compiler/noirc_frontend/src/hir/resolution/traits.rs @@ -382,8 +382,13 @@ pub(crate) fn resolve_trait_impls( let self_type_span = unresolved_type.span; - let mut resolver = - Resolver::new(interner, &path_resolver, &context.def_maps, trait_impl.file_id); + let mut resolver = Resolver::new( + interner, + &path_resolver, + &context.def_maps, + crate_id, + trait_impl.file_id, + ); resolver.add_generics(&trait_impl.generics); let trait_generics = From d358abbb2e104ee25ef2ec52d3d3646a96194db0 Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 30 Jan 2024 17:35:41 +0000 Subject: [PATCH 06/10] chore: fix missing instances of `crate_id` --- .../noirc_frontend/src/hir/resolution/globals.rs | 1 + .../noirc_frontend/src/hir/resolution/impls.rs | 2 +- .../noirc_frontend/src/hir/resolution/structs.rs | 2 +- .../noirc_frontend/src/hir/resolution/traits.rs | 15 ++++++++++----- .../src/hir/resolution/type_aliases.rs | 11 ++++++++--- compiler/noirc_frontend/src/hir/type_check/mod.rs | 3 ++- 6 files changed, 23 insertions(+), 11 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/resolution/globals.rs b/compiler/noirc_frontend/src/hir/resolution/globals.rs index b5aec212dbf..9d78e5da4e5 100644 --- a/compiler/noirc_frontend/src/hir/resolution/globals.rs +++ b/compiler/noirc_frontend/src/hir/resolution/globals.rs @@ -37,6 +37,7 @@ pub(crate) fn resolve_globals( &mut context.def_interner, &path_resolver, &context.def_maps, + crate_id, global.file_id, ); diff --git a/compiler/noirc_frontend/src/hir/resolution/impls.rs b/compiler/noirc_frontend/src/hir/resolution/impls.rs index 80a3f2427f9..08540c1a963 100644 --- a/compiler/noirc_frontend/src/hir/resolution/impls.rs +++ b/compiler/noirc_frontend/src/hir/resolution/impls.rs @@ -39,7 +39,7 @@ pub(crate) fn collect_impls( let file = def_maps[&crate_id].file_id(*module_id); for (generics, span, unresolved) in methods { - let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, crate_id, file); resolver.add_generics(generics); let typ = resolver.resolve_type(unresolved_type.clone()); diff --git a/compiler/noirc_frontend/src/hir/resolution/structs.rs b/compiler/noirc_frontend/src/hir/resolution/structs.rs index cf3e3436c88..93d0b5137d9 100644 --- a/compiler/noirc_frontend/src/hir/resolution/structs.rs +++ b/compiler/noirc_frontend/src/hir/resolution/structs.rs @@ -73,7 +73,7 @@ fn resolve_struct_fields( StandardPathResolver::new(ModuleId { local_id: unresolved.module_id, krate }); let file_id = unresolved.file_id; let (generics, fields, errors) = - Resolver::new(&mut context.def_interner, &path_resolver, &context.def_maps, file_id) + Resolver::new(&mut context.def_interner, &path_resolver, &context.def_maps, krate, file_id) .resolve_struct_fields(unresolved.struct_def); (generics, fields, errors) diff --git a/compiler/noirc_frontend/src/hir/resolution/traits.rs b/compiler/noirc_frontend/src/hir/resolution/traits.rs index 87d7bf02106..ea379f5506d 100644 --- a/compiler/noirc_frontend/src/hir/resolution/traits.rs +++ b/compiler/noirc_frontend/src/hir/resolution/traits.rs @@ -122,7 +122,7 @@ fn resolve_trait_methods( let self_type = Type::TypeVariable(self_typevar.clone(), TypeVariableKind::Normal); let name_span = the_trait.name.span(); - let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, crate_id, file); resolver.add_generics(generics); resolver.add_existing_generics(&unresolved_trait.trait_def.generics, trait_generics); resolver.add_existing_generic("Self", name_span, self_typevar); @@ -288,7 +288,7 @@ fn collect_trait_impl( let path_resolver = StandardPathResolver::new(module); let file = def_maps[&crate_id].file_id(trait_impl.module_id); - let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, crate_id, file); resolver.add_generics(&trait_impl.generics); let typ = resolver.resolve_type(unresolved_type); errors.extend(take_errors(trait_impl.file_id, resolver)); @@ -334,7 +334,7 @@ fn check_trait_impl_crate_coherence( let module = ModuleId { krate: current_crate, local_id: trait_impl.module_id }; let file = def_maps[¤t_crate].file_id(trait_impl.module_id); let path_resolver = StandardPathResolver::new(module); - let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, current_crate, file); let object_crate = match resolver.resolve_type(trait_impl.object_type.clone()) { Type::Struct(struct_type, _) => struct_type.borrow().id.krate(), @@ -422,8 +422,13 @@ pub(crate) fn resolve_trait_impls( errors.push((error.into(), trait_impl.file_id)); } - let mut new_resolver = - Resolver::new(interner, &path_resolver, &context.def_maps, trait_impl.file_id); + let mut new_resolver = Resolver::new( + interner, + &path_resolver, + &context.def_maps, + crate_id, + trait_impl.file_id, + ); new_resolver.set_generics(impl_generics.clone()); new_resolver.set_self_type(Some(self_type.clone())); diff --git a/compiler/noirc_frontend/src/hir/resolution/type_aliases.rs b/compiler/noirc_frontend/src/hir/resolution/type_aliases.rs index f66f6c8dfa7..3f8d4cf41f2 100644 --- a/compiler/noirc_frontend/src/hir/resolution/type_aliases.rs +++ b/compiler/noirc_frontend/src/hir/resolution/type_aliases.rs @@ -23,9 +23,14 @@ pub(crate) fn resolve_type_aliases( krate: crate_id, }); let file = unresolved_typ.file_id; - let (typ, generics, resolver_errors) = - Resolver::new(&mut context.def_interner, &path_resolver, &context.def_maps, file) - .resolve_type_aliases(unresolved_typ.type_alias_def); + let (typ, generics, resolver_errors) = Resolver::new( + &mut context.def_interner, + &path_resolver, + &context.def_maps, + crate_id, + file, + ) + .resolve_type_aliases(unresolved_typ.type_alias_def); errors.extend(resolver_errors.iter().cloned().map(|e| (e.into(), file))); context.def_interner.set_type_alias(type_id, typ, generics); } diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 3c2a970ee84..bfd7bbb9c97 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -520,7 +520,8 @@ mod test { ); let func_meta = vecmap(program.into_sorted().functions, |nf| { - let resolver = Resolver::new(&mut interner, &path_resolver, &def_maps, file); + let resolver = + Resolver::new(&mut interner, &path_resolver, &def_maps, CrateId::dummy_id(), file); let (hir_func, func_meta, resolver_errors) = resolver.resolve_function(nf, main_id); assert_eq!(resolver_errors, vec![]); (hir_func, func_meta) From c8d7009a8cc88bf9db26e082ee1530dfa4f14a6a Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 30 Jan 2024 17:36:55 +0000 Subject: [PATCH 07/10] chore: remove unused error --- compiler/noirc_frontend/src/hir/def_collector/errors.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/errors.rs b/compiler/noirc_frontend/src/hir/def_collector/errors.rs index 123f9c0ccbe..de45be48c4e 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/errors.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/errors.rs @@ -73,8 +73,6 @@ pub enum DefCollectorErrorKind { "Either the type or the trait must be from the same crate as the trait implementation" )] TraitImplOrphaned { span: Span }, - #[error("Usage of the `#[foreign]` or `#[builtin]` function attributes are not allowed outside of the Noir standard library")] - LowLevelFunctionOutsideOfStdlib { span: Span }, #[error("macro error : {0:?}")] MacroError(MacroError), } @@ -248,11 +246,6 @@ impl From for Diagnostic { "Either the type or the trait must be from the same crate as the trait implementation".into(), span, ), - DefCollectorErrorKind::LowLevelFunctionOutsideOfStdlib { span } => Diagnostic::simple_error( - "Definition of low-level function outside of standard library".into(), - "Usage of the `#[foreign]` or `#[builtin]` function attributes are not allowed outside of the Noir standard library".into(), - span, - ), DefCollectorErrorKind::MacroError(macro_error) => { Diagnostic::simple_error(macro_error.primary_message, macro_error.secondary_message.unwrap_or_default(), macro_error.span.unwrap_or_default()) }, From 37d4354e6273d3d8da6931fdcfdab83612a40ba2 Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 30 Jan 2024 17:38:56 +0000 Subject: [PATCH 08/10] chore: revert changes to tests --- compiler/noirc_frontend/src/tests.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 59b1a553821..a4246a9fe7d 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -6,7 +6,6 @@ mod test { use core::panic; use std::collections::BTreeMap; - use std::path::Path; use fm::FileId; @@ -52,18 +51,12 @@ mod test { src: &str, ) -> (ParsedModule, Context, Vec<(CompilationError, FileId)>) { let root = std::path::Path::new("/"); - let mut fm = FileManager::new(root); - let entrypoint = Path::new("main.nr"); - let root_file_id = fm.add_file_with_source(entrypoint, src.to_string()).unwrap(); - let path_to_std_lib_file = Path::new("std").join("lib.nr"); - let std_file_id = - fm.add_file_with_source_canonical_path(&path_to_std_lib_file, "".to_string()).unwrap(); + let fm = FileManager::new(root); let mut context = Context::new(fm, Default::default()); context.def_interner.populate_dummy_operator_traits(); - + let root_file_id = FileId::dummy(); let root_crate_id = context.crate_graph.add_crate_root(root_file_id); - context.crate_graph.add_stdlib(std_file_id); let (program, parser_errors) = parse_program(src); let mut errors = vecmap(parser_errors, |e| (e.into(), root_file_id)); From 22e310817c4daf54214b71c06189a602d5415bbd Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Tue, 30 Jan 2024 17:39:47 +0000 Subject: [PATCH 09/10] Delete test_programs/compile_failure/builtin_function_declaration/Prover.toml --- .../compile_failure/builtin_function_declaration/Prover.toml | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 test_programs/compile_failure/builtin_function_declaration/Prover.toml diff --git a/test_programs/compile_failure/builtin_function_declaration/Prover.toml b/test_programs/compile_failure/builtin_function_declaration/Prover.toml deleted file mode 100644 index e69de29bb2d..00000000000 From 5c1e86fd5e275ee3333db0e97c5e54752c575a6e Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 30 Jan 2024 18:09:20 +0000 Subject: [PATCH 10/10] chore: remove `crate_id` argument from `Resolver::new()` --- .../src/hir/resolution/functions.rs | 2 +- .../src/hir/resolution/globals.rs | 1 - .../src/hir/resolution/impls.rs | 4 ++-- .../src/hir/resolution/resolver.rs | 8 +------ .../src/hir/resolution/structs.rs | 2 +- .../src/hir/resolution/traits.rs | 24 ++++++------------- .../src/hir/resolution/type_aliases.rs | 11 +++------ .../noirc_frontend/src/hir/type_check/mod.rs | 3 +-- 8 files changed, 16 insertions(+), 39 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/resolution/functions.rs b/compiler/noirc_frontend/src/hir/resolution/functions.rs index b2eab9e97ca..e63de9b9173 100644 --- a/compiler/noirc_frontend/src/hir/resolution/functions.rs +++ b/compiler/noirc_frontend/src/hir/resolution/functions.rs @@ -37,7 +37,7 @@ pub(crate) fn resolve_function_set( let module_id = ModuleId { krate: crate_id, local_id: mod_id }; let path_resolver = StandardPathResolver::new(module_id); - let mut resolver = Resolver::new(interner, &path_resolver, def_maps, crate_id, file_id); + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file_id); // Must use set_generics here to ensure we re-use the same generics from when // the impl was originally collected. Otherwise the function will be using different // TypeVariables for the same generic, causing it to instantiate incorrectly. diff --git a/compiler/noirc_frontend/src/hir/resolution/globals.rs b/compiler/noirc_frontend/src/hir/resolution/globals.rs index 9d78e5da4e5..b5aec212dbf 100644 --- a/compiler/noirc_frontend/src/hir/resolution/globals.rs +++ b/compiler/noirc_frontend/src/hir/resolution/globals.rs @@ -37,7 +37,6 @@ pub(crate) fn resolve_globals( &mut context.def_interner, &path_resolver, &context.def_maps, - crate_id, global.file_id, ); diff --git a/compiler/noirc_frontend/src/hir/resolution/impls.rs b/compiler/noirc_frontend/src/hir/resolution/impls.rs index 08540c1a963..4aa70f00cfc 100644 --- a/compiler/noirc_frontend/src/hir/resolution/impls.rs +++ b/compiler/noirc_frontend/src/hir/resolution/impls.rs @@ -39,7 +39,7 @@ pub(crate) fn collect_impls( let file = def_maps[&crate_id].file_id(*module_id); for (generics, span, unresolved) in methods { - let mut resolver = Resolver::new(interner, &path_resolver, def_maps, crate_id, file); + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); resolver.add_generics(generics); let typ = resolver.resolve_type(unresolved_type.clone()); @@ -98,7 +98,7 @@ pub(crate) fn resolve_impls( let file = def_maps[&crate_id].file_id(module_id); for (generics, _, functions) in methods { - let mut resolver = Resolver::new(interner, &path_resolver, def_maps, crate_id, file); + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); resolver.add_generics(&generics); let generics = resolver.get_generics().to_vec(); let self_type = resolver.resolve_type(unresolved_type.clone()); diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 33cc39cad5e..07aa0ac7eb4 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -101,9 +101,6 @@ pub struct Resolver<'a> { /// for these so we can still resolve them in the parent module without them being in a contract. in_contract: bool, - /// True if the current crate is the Noir standard library. - in_stdlib: bool, - /// Contains a mapping of the current struct or functions's generics to /// unique type variables if we're resolving a struct. Empty otherwise. /// This is a Vec rather than a map to preserve the order a functions generics @@ -134,12 +131,10 @@ impl<'a> Resolver<'a> { interner: &'a mut NodeInterner, path_resolver: &'a dyn PathResolver, def_maps: &'a BTreeMap, - crate_id: CrateId, file: FileId, ) -> Resolver<'a> { let module_id = path_resolver.module_id(); let in_contract = module_id.module(def_maps).is_contract; - let in_stdlib = crate_id.is_stdlib(); Self { path_resolver, @@ -155,7 +150,6 @@ impl<'a> Resolver<'a> { current_trait_impl: None, file, in_contract, - in_stdlib, } } @@ -908,7 +902,7 @@ impl<'a> Resolver<'a> { } let is_low_level_function = func.attributes().function.as_ref().map_or(false, |func| func.is_low_level()); - if !self.in_stdlib && is_low_level_function { + if !self.path_resolver.module_id().krate.is_stdlib() && is_low_level_function { let error = ResolverError::LowLevelFunctionOutsideOfStdlib { ident: func.name_ident().clone() }; self.push_err(error); diff --git a/compiler/noirc_frontend/src/hir/resolution/structs.rs b/compiler/noirc_frontend/src/hir/resolution/structs.rs index 93d0b5137d9..cf3e3436c88 100644 --- a/compiler/noirc_frontend/src/hir/resolution/structs.rs +++ b/compiler/noirc_frontend/src/hir/resolution/structs.rs @@ -73,7 +73,7 @@ fn resolve_struct_fields( StandardPathResolver::new(ModuleId { local_id: unresolved.module_id, krate }); let file_id = unresolved.file_id; let (generics, fields, errors) = - Resolver::new(&mut context.def_interner, &path_resolver, &context.def_maps, krate, file_id) + Resolver::new(&mut context.def_interner, &path_resolver, &context.def_maps, file_id) .resolve_struct_fields(unresolved.struct_def); (generics, fields, errors) diff --git a/compiler/noirc_frontend/src/hir/resolution/traits.rs b/compiler/noirc_frontend/src/hir/resolution/traits.rs index ea379f5506d..8f966be312b 100644 --- a/compiler/noirc_frontend/src/hir/resolution/traits.rs +++ b/compiler/noirc_frontend/src/hir/resolution/traits.rs @@ -122,7 +122,7 @@ fn resolve_trait_methods( let self_type = Type::TypeVariable(self_typevar.clone(), TypeVariableKind::Normal); let name_span = the_trait.name.span(); - let mut resolver = Resolver::new(interner, &path_resolver, def_maps, crate_id, file); + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); resolver.add_generics(generics); resolver.add_existing_generics(&unresolved_trait.trait_def.generics, trait_generics); resolver.add_existing_generic("Self", name_span, self_typevar); @@ -288,7 +288,7 @@ fn collect_trait_impl( let path_resolver = StandardPathResolver::new(module); let file = def_maps[&crate_id].file_id(trait_impl.module_id); - let mut resolver = Resolver::new(interner, &path_resolver, def_maps, crate_id, file); + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); resolver.add_generics(&trait_impl.generics); let typ = resolver.resolve_type(unresolved_type); errors.extend(take_errors(trait_impl.file_id, resolver)); @@ -334,7 +334,7 @@ fn check_trait_impl_crate_coherence( let module = ModuleId { krate: current_crate, local_id: trait_impl.module_id }; let file = def_maps[¤t_crate].file_id(trait_impl.module_id); let path_resolver = StandardPathResolver::new(module); - let mut resolver = Resolver::new(interner, &path_resolver, def_maps, current_crate, file); + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); let object_crate = match resolver.resolve_type(trait_impl.object_type.clone()) { Type::Struct(struct_type, _) => struct_type.borrow().id.krate(), @@ -382,13 +382,8 @@ pub(crate) fn resolve_trait_impls( let self_type_span = unresolved_type.span; - let mut resolver = Resolver::new( - interner, - &path_resolver, - &context.def_maps, - crate_id, - trait_impl.file_id, - ); + let mut resolver = + Resolver::new(interner, &path_resolver, &context.def_maps, trait_impl.file_id); resolver.add_generics(&trait_impl.generics); let trait_generics = @@ -422,13 +417,8 @@ pub(crate) fn resolve_trait_impls( errors.push((error.into(), trait_impl.file_id)); } - let mut new_resolver = Resolver::new( - interner, - &path_resolver, - &context.def_maps, - crate_id, - trait_impl.file_id, - ); + let mut new_resolver = + Resolver::new(interner, &path_resolver, &context.def_maps, trait_impl.file_id); new_resolver.set_generics(impl_generics.clone()); new_resolver.set_self_type(Some(self_type.clone())); diff --git a/compiler/noirc_frontend/src/hir/resolution/type_aliases.rs b/compiler/noirc_frontend/src/hir/resolution/type_aliases.rs index 3f8d4cf41f2..f66f6c8dfa7 100644 --- a/compiler/noirc_frontend/src/hir/resolution/type_aliases.rs +++ b/compiler/noirc_frontend/src/hir/resolution/type_aliases.rs @@ -23,14 +23,9 @@ pub(crate) fn resolve_type_aliases( krate: crate_id, }); let file = unresolved_typ.file_id; - let (typ, generics, resolver_errors) = Resolver::new( - &mut context.def_interner, - &path_resolver, - &context.def_maps, - crate_id, - file, - ) - .resolve_type_aliases(unresolved_typ.type_alias_def); + let (typ, generics, resolver_errors) = + Resolver::new(&mut context.def_interner, &path_resolver, &context.def_maps, file) + .resolve_type_aliases(unresolved_typ.type_alias_def); errors.extend(resolver_errors.iter().cloned().map(|e| (e.into(), file))); context.def_interner.set_type_alias(type_id, typ, generics); } diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index bfd7bbb9c97..3c2a970ee84 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -520,8 +520,7 @@ mod test { ); let func_meta = vecmap(program.into_sorted().functions, |nf| { - let resolver = - Resolver::new(&mut interner, &path_resolver, &def_maps, CrateId::dummy_id(), file); + let resolver = Resolver::new(&mut interner, &path_resolver, &def_maps, file); let (hir_func, func_meta, resolver_errors) = resolver.resolve_function(nf, main_id); assert_eq!(resolver_errors, vec![]); (hir_func, func_meta)