diff --git a/compiler/noirc_frontend/src/elaborator/enums.rs b/compiler/noirc_frontend/src/elaborator/enums.rs index d8faabf0e76..56a82cfe029 100644 --- a/compiler/noirc_frontend/src/elaborator/enums.rs +++ b/compiler/noirc_frontend/src/elaborator/enums.rs @@ -27,7 +27,7 @@ use crate::{ token::Attributes, }; -use super::Elaborator; +use super::{Elaborator, path_resolution::PathResolutionTarget}; const WILDCARD_PATTERN: &str = "_"; @@ -419,7 +419,7 @@ impl Elaborator<'_> { // user is trying to resolve to a non-local item. let shadow_existing = path.is_ident().then_some(last_ident); - match self.resolve_path_or_error(path) { + match self.resolve_path_or_error(path, PathResolutionTarget::Value) { Ok(resolution) => self.path_resolution_to_constructor( resolution, shadow_existing, @@ -588,7 +588,7 @@ impl Elaborator<'_> { ExpressionKind::Variable(path) => { let location = path.location; - match self.resolve_path_or_error(path) { + match self.resolve_path_or_error(path, PathResolutionTarget::Value) { // Use None for `name` here - we don't want to define a variable if this // resolves to an existing item. Ok(resolution) => self.path_resolution_to_constructor( diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 00f404a5552..99ae4e755b7 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -70,7 +70,9 @@ use noirc_errors::{Located, Location}; pub(crate) use options::ElaboratorOptions; pub use options::{FrontendOptions, UnstableFeature}; pub use path_resolution::Turbofish; -use path_resolution::{PathResolution, PathResolutionItem, PathResolutionMode}; +use path_resolution::{ + PathResolution, PathResolutionItem, PathResolutionMode, PathResolutionTarget, +}; use types::bind_ordered_generics; use self::traits::check_trait_impl_method_matches_declaration; @@ -788,16 +790,18 @@ impl<'context> Elaborator<'context> { } pub fn resolve_module_by_path(&mut self, path: Path) -> Option { - match self.resolve_path(path.clone()) { - Ok(PathResolution { item: PathResolutionItem::Module(module_id), errors }) => { - if errors.is_empty() { Some(module_id) } else { None } + match self.resolve_path_as_type(path.clone()) { + Ok(PathResolution { item: PathResolutionItem::Module(module_id), errors }) + if errors.is_empty() => + { + Some(module_id) } _ => None, } } fn resolve_trait_by_path(&mut self, path: Path) -> Option { - let error = match self.resolve_path(path.clone()) { + let error = match self.resolve_path_as_type(path.clone()) { Ok(PathResolution { item: PathResolutionItem::Trait(trait_id), errors }) => { for error in errors { self.push_err(error); @@ -863,11 +867,9 @@ impl<'context> Elaborator<'context> { ) -> Vec { let mut added_generics = Vec::new(); - let Ok(item) = self.resolve_path_or_error(bound.trait_path.clone()) else { - return Vec::new(); - }; - - let PathResolutionItem::Trait(trait_id) = item else { + let Ok(PathResolutionItem::Trait(trait_id)) = + self.resolve_path_or_error(bound.trait_path.clone(), PathResolutionTarget::Type) + else { return Vec::new(); }; diff --git a/compiler/noirc_frontend/src/elaborator/path_resolution.rs b/compiler/noirc_frontend/src/elaborator/path_resolution.rs index f1428b09dbc..25f7f63279e 100644 --- a/compiler/noirc_frontend/src/elaborator/path_resolution.rs +++ b/compiler/noirc_frontend/src/elaborator/path_resolution.rs @@ -24,12 +24,15 @@ pub(crate) struct PathResolution { /// All possible items that result from resolving a Path. /// Note that this item doesn't include the last turbofish in a Path, /// only intermediate ones, if any. -#[derive(Debug, Clone)] -pub enum PathResolutionItem { +#[derive(Debug)] +pub(crate) enum PathResolutionItem { + // These are types Module(ModuleId), Type(TypeId), TypeAlias(TypeAliasId), Trait(TraitId), + + // These are values Global(GlobalId), ModuleFunction(FuncId), Method(TypeId, Option, FuncId), @@ -39,7 +42,7 @@ pub enum PathResolutionItem { } impl PathResolutionItem { - pub fn function_id(&self) -> Option { + pub(crate) fn function_id(&self) -> Option { match self { PathResolutionItem::ModuleFunction(func_id) | PathResolutionItem::Method(_, _, func_id) @@ -54,14 +57,7 @@ impl PathResolutionItem { } } - pub fn module_id(&self) -> Option { - match self { - Self::Module(module_id) => Some(*module_id), - _ => None, - } - } - - pub fn description(&self) -> &'static str { + pub(crate) fn description(&self) -> &'static str { match self { PathResolutionItem::Module(..) => "module", PathResolutionItem::Type(..) => "type", @@ -84,7 +80,7 @@ pub struct Turbofish { } /// Any item that can appear before the last segment in a path. -#[derive(Debug)] +#[derive(Debug, Clone)] enum IntermediatePathResolutionItem { SelfType, Module, @@ -142,27 +138,43 @@ pub(super) enum PathResolutionMode { MarkAsUsed, } +/// Depenending on where a path appears in the source code it should either resolve to a type +/// or a value. For example, in `let x: Foo::Bar = Foo::Bar {}` both `Foo::Bar` should resolve to +/// types, never values. On the other hand, in `Foo::Bar()` `Foo::Bar` should resolve to a value, +/// typically a function. +/// +/// When using any of the `resolve` methods in this module, items in the target namespace +/// will be returned first if another one exists in the other namespace. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub(super) enum PathResolutionTarget { + Type, + Value, +} + impl Elaborator<'_> { pub(super) fn resolve_path_or_error( &mut self, path: Path, + target: PathResolutionTarget, ) -> Result { - self.resolve_path_or_error_inner(path, PathResolutionMode::MarkAsReferenced) + self.resolve_path_or_error_inner(path, target, PathResolutionMode::MarkAsReferenced) } pub(super) fn use_path_or_error( &mut self, path: Path, + target: PathResolutionTarget, ) -> Result { - self.resolve_path_or_error_inner(path, PathResolutionMode::MarkAsUsed) + self.resolve_path_or_error_inner(path, target, PathResolutionMode::MarkAsUsed) } pub(super) fn resolve_path_or_error_inner( &mut self, path: Path, + target: PathResolutionTarget, mode: PathResolutionMode, ) -> Result { - let path_resolution = self.resolve_path_inner(path, mode)?; + let path_resolution = self.resolve_path_inner(path, target, mode)?; for error in path_resolution.errors { self.push_err(error); @@ -171,12 +183,16 @@ impl Elaborator<'_> { Ok(path_resolution.item) } - pub(super) fn resolve_path(&mut self, path: Path) -> PathResolutionResult { - self.resolve_path_inner(path, PathResolutionMode::MarkAsReferenced) + pub(super) fn resolve_path_as_type(&mut self, path: Path) -> PathResolutionResult { + self.resolve_path_inner( + path, + PathResolutionTarget::Type, + PathResolutionMode::MarkAsReferenced, + ) } - pub(super) fn use_path(&mut self, path: Path) -> PathResolutionResult { - self.resolve_path_inner(path, PathResolutionMode::MarkAsUsed) + pub(super) fn use_path_as_type(&mut self, path: Path) -> PathResolutionResult { + self.resolve_path_inner(path, PathResolutionTarget::Type, PathResolutionMode::MarkAsUsed) } /// Resolves a path in the current module. @@ -186,6 +202,7 @@ impl Elaborator<'_> { pub(super) fn resolve_path_inner( &mut self, mut path: Path, + target: PathResolutionTarget, mode: PathResolutionMode, ) -> PathResolutionResult { let mut module_id = self.module_id(); @@ -207,7 +224,7 @@ impl Elaborator<'_> { } } - self.resolve_path_in_module(path, module_id, intermediate_item, mode) + self.resolve_path_in_module(path, module_id, intermediate_item, target, mode) } /// Resolves a path in `current_module`. @@ -217,6 +234,7 @@ impl Elaborator<'_> { path: Path, importing_module: ModuleId, intermediate_item: IntermediatePathResolutionItem, + target: PathResolutionTarget, mode: PathResolutionMode, ) -> PathResolutionResult { let references_tracker = if self.interner.is_in_lsp_mode() { @@ -226,7 +244,14 @@ impl Elaborator<'_> { }; let (path, module_id, _) = resolve_path_kind(path, importing_module, self.def_maps, references_tracker)?; - self.resolve_name_in_module(path, module_id, importing_module, intermediate_item, mode) + self.resolve_name_in_module( + path, + module_id, + importing_module, + intermediate_item, + target, + mode, + ) } /// Resolves a Path assuming we are inside `starting_module`. @@ -237,6 +262,7 @@ impl Elaborator<'_> { starting_module: ModuleId, importing_module: ModuleId, mut intermediate_item: IntermediatePathResolutionItem, + target: PathResolutionTarget, mode: PathResolutionMode, ) -> PathResolutionResult { // There is a possibility that the import path is empty. In that case, early return. @@ -403,9 +429,51 @@ impl Elaborator<'_> { current_ns = found_ns; } - let (module_def_id, visibility, _) = - current_ns.values.or(current_ns.types).expect("Found empty namespace"); + let (target_ns, fallback_ns) = match target { + PathResolutionTarget::Type => (current_ns.types, current_ns.values), + PathResolutionTarget::Value => (current_ns.values, current_ns.types), + }; + let item = target_ns + .map(|(module_def_id, visibility, ..)| { + self.per_ns_item_to_path_resolution_item( + path.clone(), + importing_module, + intermediate_item.clone(), + current_module_id, + &mut errors, + module_def_id, + visibility, + ) + }) + .unwrap_or_else(|| { + let (module_def_id, visibility, ..) = + fallback_ns.expect("A namespace should never be empty"); + self.per_ns_item_to_path_resolution_item( + path.clone(), + importing_module, + intermediate_item, + current_module_id, + &mut errors, + module_def_id, + visibility, + ) + }); + + Ok(PathResolution { item, errors }) + } + + #[allow(clippy::too_many_arguments)] + fn per_ns_item_to_path_resolution_item( + &mut self, + path: Path, + importing_module: ModuleId, + intermediate_item: IntermediatePathResolutionItem, + current_module_id: ModuleId, + errors: &mut Vec, + module_def_id: ModuleDefId, + visibility: crate::ast::ItemVisibility, + ) -> PathResolutionItem { let name = path.last_ident(); let is_self_type = name.is_self_type_name(); let location = name.location(); @@ -427,7 +495,7 @@ impl Elaborator<'_> { errors.push(PathResolutionError::Private(name.clone())); } - Ok(PathResolution { item, errors }) + item } fn self_type_module_id(&self) -> Option { diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 1e29d8db9bc..181f787ce08 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -874,7 +874,7 @@ impl Elaborator<'_> { } } - pub fn get_ident_from_path( + pub(crate) fn get_ident_from_path( &mut self, path: Path, ) -> ((HirIdent, usize), Option) { diff --git a/compiler/noirc_frontend/src/elaborator/scope.rs b/compiler/noirc_frontend/src/elaborator/scope.rs index 942785964c3..49aa170426c 100644 --- a/compiler/noirc_frontend/src/elaborator/scope.rs +++ b/compiler/noirc_frontend/src/elaborator/scope.rs @@ -15,7 +15,7 @@ use crate::{Type, TypeAlias}; use super::path_resolution::{PathResolutionItem, PathResolutionMode}; use super::types::SELF_TYPE_NAME; -use super::{Elaborator, ResolverMeta}; +use super::{Elaborator, PathResolutionTarget, ResolverMeta}; type Scope = GenericScope; type ScopeTree = GenericScopeTree; @@ -82,7 +82,7 @@ impl Elaborator<'_> { path: Path, ) -> Result<(DefinitionId, PathResolutionItem), ResolverError> { let location = path.location; - let item = self.use_path_or_error(path)?; + let item = self.use_path_or_error(path, PathResolutionTarget::Value)?; if let Some(function) = item.function_id() { return Ok((self.interner.function_definition_id(function), item)); @@ -137,7 +137,7 @@ impl Elaborator<'_> { /// Lookup a given trait by name/path. pub fn lookup_trait_or_error(&mut self, path: Path) -> Option<&mut Trait> { let location = path.location; - match self.resolve_path_or_error(path) { + match self.resolve_path_or_error(path, PathResolutionTarget::Type) { Ok(item) => { if let PathResolutionItem::Trait(trait_id) = item { Some(self.get_trait_mut(trait_id)) @@ -164,7 +164,7 @@ impl Elaborator<'_> { mode: PathResolutionMode, ) -> Option> { let location = path.location; - match self.resolve_path_or_error_inner(path, mode) { + match self.resolve_path_or_error_inner(path, PathResolutionTarget::Type, mode) { Ok(item) => { if let PathResolutionItem::Type(struct_id) = item { Some(self.get_type(struct_id)) @@ -195,7 +195,7 @@ impl Elaborator<'_> { } let location = path.location; - match self.use_path_or_error(path) { + match self.use_path_or_error(path, PathResolutionTarget::Type) { Ok(PathResolutionItem::Type(struct_id)) => { let struct_type = self.get_type(struct_id); let generics = struct_type.borrow().instantiate(self.interner); @@ -226,7 +226,7 @@ impl Elaborator<'_> { path: Path, mode: PathResolutionMode, ) -> Option> { - match self.resolve_path_or_error_inner(path, mode) { + match self.resolve_path_or_error_inner(path, PathResolutionTarget::Type, mode) { Ok(PathResolutionItem::TypeAlias(type_alias_id)) => { Some(self.interner.get_type_alias(type_alias_id)) } diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 5695a309c19..315785c56b5 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -42,7 +42,7 @@ use crate::{ }; use super::{ - Elaborator, FunctionContext, UnsafeBlockStatus, lints, + Elaborator, FunctionContext, PathResolutionTarget, UnsafeBlockStatus, lints, path_resolution::{PathResolutionItem, PathResolutionMode}, }; @@ -506,7 +506,7 @@ impl Elaborator<'_> { } // If we cannot find a local generic of the same name, try to look up a global - match self.resolve_path_or_error_inner(path.clone(), mode) { + match self.resolve_path_or_error_inner(path.clone(), PathResolutionTarget::Value, mode) { Ok(PathResolutionItem::Global(id)) => { if let Some(current_item) = self.current_item { self.interner.add_global_dependency(current_item, id); @@ -718,7 +718,7 @@ impl Elaborator<'_> { // Returns the trait method, trait constraint, and whether the impl is assumed to exist by a where clause or not // E.g. `t.method()` with `where T: Foo` in scope will return `(Foo::method, T, vec![Bar])` fn resolve_trait_static_method(&mut self, path: &Path) -> Option { - let path_resolution = self.use_path(path.clone()).ok()?; + let path_resolution = self.use_path_as_type(path.clone()).ok()?; let func_id = path_resolution.item.function_id()?; let meta = self.interner.try_function_meta(&func_id)?; let the_trait = self.interner.get_trait(meta.trait_id?); @@ -775,7 +775,7 @@ impl Elaborator<'_> { let last_segment = path.pop(); let before_last_segment = path.last_segment(); - let path_resolution = self.use_path(path).ok()?; + let path_resolution = self.use_path_as_type(path).ok()?; let PathResolutionItem::Type(type_id) = path_resolution.item else { return None; }; diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index eef4ad54dc5..1bb7c4c41a3 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -4675,3 +4675,43 @@ fn attempt_to_divide_by_zero_at_comptime() { "#; check_errors!(src); } + +#[named] +#[test] +fn same_name_in_types_and_values_namespace_works() { + let src = " + struct foo {} + + fn foo(x: foo) -> foo { + x + } + + fn main() { + let x: foo = foo {}; + let _ = foo(x); + } + "; + assert_no_errors!(src); +} + +#[named] +#[test] +fn only_one_private_error_when_name_in_types_and_values_namespace_collides() { + let src = " + mod moo { + struct foo {} + + fn foo() {} + } + + fn main() { + let _ = moo::foo {}; + ^^^ foo is private and not visible from the current module + ~~~ foo is private + x + ^ cannot find `x` in this scope + ~ not found in this scope + } + "; + check_errors!(src); +} diff --git a/test_programs/compile_failure/noirc_frontend_tests_only_one_private_error_when_name_in_types_and_values_namespace_collides/Nargo.toml b/test_programs/compile_failure/noirc_frontend_tests_only_one_private_error_when_name_in_types_and_values_namespace_collides/Nargo.toml new file mode 100644 index 00000000000..fc26df9d0be --- /dev/null +++ b/test_programs/compile_failure/noirc_frontend_tests_only_one_private_error_when_name_in_types_and_values_namespace_collides/Nargo.toml @@ -0,0 +1,7 @@ + + [package] + name = "noirc_frontend_tests_only_one_private_error_when_name_in_types_and_values_namespace_collides" + type = "bin" + authors = [""] + + [dependencies] \ No newline at end of file diff --git a/test_programs/compile_failure/noirc_frontend_tests_only_one_private_error_when_name_in_types_and_values_namespace_collides/src/main.nr b/test_programs/compile_failure/noirc_frontend_tests_only_one_private_error_when_name_in_types_and_values_namespace_collides/src/main.nr new file mode 100644 index 00000000000..9385de2ffac --- /dev/null +++ b/test_programs/compile_failure/noirc_frontend_tests_only_one_private_error_when_name_in_types_and_values_namespace_collides/src/main.nr @@ -0,0 +1,12 @@ + + mod moo { + struct foo {} + + fn foo() {} + } + + fn main() { + let _ = moo::foo {}; + x + } + \ No newline at end of file diff --git a/test_programs/compile_failure/noirc_frontend_tests_only_one_private_error_when_name_in_types_and_values_namespace_collides/src_hash.txt b/test_programs/compile_failure/noirc_frontend_tests_only_one_private_error_when_name_in_types_and_values_namespace_collides/src_hash.txt new file mode 100644 index 00000000000..3fa26b26993 --- /dev/null +++ b/test_programs/compile_failure/noirc_frontend_tests_only_one_private_error_when_name_in_types_and_values_namespace_collides/src_hash.txt @@ -0,0 +1 @@ +2073315907866401059 \ No newline at end of file diff --git a/test_programs/compile_failure/noirc_frontend_tests_only_one_private_error_when_name_in_types_and_values_namespace_collides/stderr.txt b/test_programs/compile_failure/noirc_frontend_tests_only_one_private_error_when_name_in_types_and_values_namespace_collides/stderr.txt new file mode 100644 index 00000000000..12bc15b8f0a --- /dev/null +++ b/test_programs/compile_failure/noirc_frontend_tests_only_one_private_error_when_name_in_types_and_values_namespace_collides/stderr.txt @@ -0,0 +1,15 @@ +warning: foo is private and not visible from the current module + ┌─ src/main.nr:9:22 + │ +9 │ let _ = moo::foo {}; + │ --- foo is private + │ + +error: cannot find `x` in this scope + ┌─ src/main.nr:10:9 + │ +10 │ x + │ - not found in this scope + │ + +Aborting due to 1 previous error diff --git a/test_programs/compile_success_no_bug/noirc_frontend_tests_same_name_in_types_and_values_namespace_works/Nargo.toml b/test_programs/compile_success_no_bug/noirc_frontend_tests_same_name_in_types_and_values_namespace_works/Nargo.toml new file mode 100644 index 00000000000..0c9c6b1f904 --- /dev/null +++ b/test_programs/compile_success_no_bug/noirc_frontend_tests_same_name_in_types_and_values_namespace_works/Nargo.toml @@ -0,0 +1,7 @@ + + [package] + name = "noirc_frontend_tests_same_name_in_types_and_values_namespace_works" + type = "bin" + authors = [""] + + [dependencies] \ No newline at end of file diff --git a/test_programs/compile_success_no_bug/noirc_frontend_tests_same_name_in_types_and_values_namespace_works/src/main.nr b/test_programs/compile_success_no_bug/noirc_frontend_tests_same_name_in_types_and_values_namespace_works/src/main.nr new file mode 100644 index 00000000000..9429943e48e --- /dev/null +++ b/test_programs/compile_success_no_bug/noirc_frontend_tests_same_name_in_types_and_values_namespace_works/src/main.nr @@ -0,0 +1,12 @@ + + struct foo {} + + fn foo(x: foo) -> foo { + x + } + + fn main() { + let x: foo = foo {}; + let _ = foo(x); + } + \ No newline at end of file diff --git a/test_programs/compile_success_no_bug/noirc_frontend_tests_same_name_in_types_and_values_namespace_works/src_hash.txt b/test_programs/compile_success_no_bug/noirc_frontend_tests_same_name_in_types_and_values_namespace_works/src_hash.txt new file mode 100644 index 00000000000..cf3bdad5a13 --- /dev/null +++ b/test_programs/compile_success_no_bug/noirc_frontend_tests_same_name_in_types_and_values_namespace_works/src_hash.txt @@ -0,0 +1 @@ +17640324439011329444 \ No newline at end of file diff --git a/tooling/nargo_cli/tests/snapshots/expand/compile_success_no_bug/noirc_frontend_tests_same_name_in_types_and_values_namespace_works/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/expand/compile_success_no_bug/noirc_frontend_tests_same_name_in_types_and_values_namespace_works/execute__tests__expanded.snap new file mode 100644 index 00000000000..10bf08fea12 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/expand/compile_success_no_bug/noirc_frontend_tests_same_name_in_types_and_values_namespace_works/execute__tests__expanded.snap @@ -0,0 +1,14 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: expanded_code +--- +struct foo {} + +fn foo(x: foo) -> foo { + x +} + +fn main() { + let x: foo = foo {}; + let _: foo = foo(x); +}