From d512a991ebd3d615f8813fa0d2f1631db9b80f38 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 29 Apr 2025 17:44:08 -0300 Subject: [PATCH 01/16] fix: allow names to collide in the values/types namespaces --- .../noirc_frontend/src/elaborator/enums.rs | 20 +- compiler/noirc_frontend/src/elaborator/mod.rs | 27 ++- .../src/elaborator/path_resolution.rs | 228 +++++++++++++----- .../noirc_frontend/src/elaborator/patterns.rs | 30 +-- .../noirc_frontend/src/elaborator/scope.rs | 62 ++--- .../noirc_frontend/src/elaborator/types.rs | 20 +- compiler/noirc_frontend/src/tests.rs | 18 ++ .../Nargo.toml | 7 + .../src/main.nr | 12 + .../src_hash.txt | 1 + 10 files changed, 302 insertions(+), 123 deletions(-) create mode 100644 test_programs/compile_success_no_bug/noirc_frontend_tests_types_and_values_namespace/Nargo.toml create mode 100644 test_programs/compile_success_no_bug/noirc_frontend_tests_types_and_values_namespace/src/main.nr create mode 100644 test_programs/compile_success_no_bug/noirc_frontend_tests_types_and_values_namespace/src_hash.txt diff --git a/compiler/noirc_frontend/src/elaborator/enums.rs b/compiler/noirc_frontend/src/elaborator/enums.rs index 9917460dc87..2dbe5a071f3 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::PathResolutionValueItem}; const WILDCARD_PATTERN: &str = "_"; @@ -644,8 +644,8 @@ impl Elaborator<'_> { location: Location, variables_defined: &mut Vec, ) -> Pattern { - let (actual_type, expected_arg_types, variant_index) = match &resolution { - PathResolutionItem::Global(id) => { + let (actual_type, expected_arg_types, variant_index) = match resolution.as_value() { + Some(PathResolutionValueItem::Global(id)) => { // variant constant self.elaborate_global_if_unresolved(id); let global = self.interner.get_global(*id); @@ -669,7 +669,8 @@ impl Elaborator<'_> { let actual_type = global_type.instantiate(self.interner).0; (actual_type, Vec::new(), variant_index) } - PathResolutionItem::Method(_, _, func_id) | PathResolutionItem::SelfMethod(func_id) => { + Some(PathResolutionValueItem::Method(_, _, func_id)) + | Some(PathResolutionValueItem::SelfMethod(func_id)) => { // TODO(#7430): Take type_turbofish into account when instantiating the function's type let meta = self.interner.function_meta(func_id); let Some(variant_index) = meta.enum_variant_index else { @@ -686,13 +687,10 @@ impl Elaborator<'_> { (actual_type, expected_arg_types, variant_index) } - PathResolutionItem::Module(_) - | PathResolutionItem::Type(_) - | PathResolutionItem::TypeAlias(_) - | PathResolutionItem::Trait(_) - | PathResolutionItem::ModuleFunction(_) - | PathResolutionItem::TypeAliasFunction(_, _, _) - | PathResolutionItem::TraitFunction(_, _, _) => { + Some(PathResolutionValueItem::ModuleFunction(_)) + | Some(PathResolutionValueItem::TypeAliasFunction(_, _, _)) + | Some(PathResolutionValueItem::TraitFunction(_, _, _)) + | None => { // This variable refers to an existing item if let Some(name) = name { // If name is set, shadow the existing item diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index cf648560db7..e4b880d2b75 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -70,7 +70,7 @@ 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::{PathResolutionMode, PathResolutionTypeItem}; use types::bind_ordered_generics; use self::traits::check_trait_impl_method_matches_declaration; @@ -789,8 +789,12 @@ 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 } + Ok(resolution) if resolution.errors.is_empty() => { + if let Some(PathResolutionTypeItem::Module(module_id)) = resolution.item.as_type() { + Some(*module_id) + } else { + None + } } _ => None, } @@ -798,13 +802,16 @@ impl<'context> Elaborator<'context> { fn resolve_trait_by_path(&mut self, path: Path) -> Option { let error = match self.resolve_path(path.clone()) { - Ok(PathResolution { item: PathResolutionItem::Trait(trait_id), errors }) => { - for error in errors { - self.push_err(error); + Ok(resolution) => { + if let Some(PathResolutionTypeItem::Trait(trait_id)) = resolution.item.as_type() { + for error in resolution.errors { + self.push_err(error); + } + return Some(*trait_id); + } else { + DefCollectorErrorKind::NotATrait { not_a_trait_name: path } } - return Some(trait_id); } - Ok(_) => DefCollectorErrorKind::NotATrait { not_a_trait_name: path }, Err(_) => DefCollectorErrorKind::TraitNotFound { trait_path: path }, }; self.push_err(error); @@ -867,11 +874,11 @@ impl<'context> Elaborator<'context> { return Vec::new(); }; - let PathResolutionItem::Trait(trait_id) = item else { + let Some(PathResolutionTypeItem::Trait(trait_id)) = item.as_type() else { return Vec::new(); }; - let the_trait = self.get_trait_mut(trait_id); + let the_trait = self.get_trait_mut(*trait_id); if the_trait.associated_types.len() > bound.trait_generics.named_args.len() { let trait_name = the_trait.name.to_string(); diff --git a/compiler/noirc_frontend/src/elaborator/path_resolution.rs b/compiler/noirc_frontend/src/elaborator/path_resolution.rs index f1428b09dbc..ca7ece21072 100644 --- a/compiler/noirc_frontend/src/elaborator/path_resolution.rs +++ b/compiler/noirc_frontend/src/elaborator/path_resolution.rs @@ -21,15 +21,79 @@ pub(crate) struct PathResolution { pub(crate) errors: Vec, } -/// 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)] +pub(crate) enum PathResolutionItem { + Type(PathResolutionTypeItem), + Value(PathResolutionValueItem), + TypeAndValue(PathResolutionTypeItem, PathResolutionValueItem), +} + +impl PathResolutionItem { + pub(crate) fn into_type(self) -> Option { + match self { + Self::Type(typ) | Self::TypeAndValue(typ, _) => Some(typ), + _ => None, + } + } + + pub(crate) fn as_type(&self) -> Option<&PathResolutionTypeItem> { + match self { + Self::Type(typ) | Self::TypeAndValue(typ, _) => Some(typ), + _ => None, + } + } + + pub(crate) fn into_value(self) -> Option { + match self { + Self::Value(value) | Self::TypeAndValue(_, value) => Some(value), + _ => None, + } + } + + pub(crate) fn as_value(&self) -> Option<&PathResolutionValueItem> { + match self { + Self::Value(value) | Self::TypeAndValue(_, value) => Some(value), + _ => None, + } + } + + pub(crate) fn function_id(&self) -> Option { + self.as_value().and_then(|item| item.function_id()) + } + + pub(crate) fn description(&self) -> &'static str { + match self { + Self::Type(item) => item.description(), + Self::Value(item) => item.description(), + Self::TypeAndValue(item, _) => { + // Any of the two items can be used to describe the resolution. + item.description() + } + } + } +} + #[derive(Debug, Clone)] -pub enum PathResolutionItem { +pub(crate) enum PathResolutionTypeItem { Module(ModuleId), Type(TypeId), TypeAlias(TypeAliasId), Trait(TraitId), +} + +impl PathResolutionTypeItem { + pub(crate) fn description(&self) -> &'static str { + match self { + Self::Module(..) => "module", + Self::Type(..) => "type", + Self::TypeAlias(..) => "type alias", + Self::Trait(..) => "trait", + } + } +} + +#[derive(Debug, Clone)] +pub(crate) enum PathResolutionValueItem { Global(GlobalId), ModuleFunction(FuncId), Method(TypeId, Option, FuncId), @@ -38,41 +102,26 @@ pub enum PathResolutionItem { TraitFunction(TraitId, Option, FuncId), } -impl PathResolutionItem { - pub fn function_id(&self) -> Option { - match self { - PathResolutionItem::ModuleFunction(func_id) - | PathResolutionItem::Method(_, _, func_id) - | PathResolutionItem::SelfMethod(func_id) - | PathResolutionItem::TypeAliasFunction(_, _, func_id) - | PathResolutionItem::TraitFunction(_, _, func_id) => Some(*func_id), - PathResolutionItem::Module(..) - | PathResolutionItem::Type(..) - | PathResolutionItem::TypeAlias(..) - | PathResolutionItem::Trait(..) - | PathResolutionItem::Global(..) => None, - } - } - - pub fn module_id(&self) -> Option { +impl PathResolutionValueItem { + pub(crate) fn function_id(&self) -> Option { match self { - Self::Module(module_id) => Some(*module_id), - _ => None, + Self::ModuleFunction(func_id) + | Self::Method(_, _, func_id) + | Self::SelfMethod(func_id) + | Self::TypeAliasFunction(_, _, func_id) + | Self::TraitFunction(_, _, func_id) => Some(*func_id), + Self::Global(..) => None, } } - pub fn description(&self) -> &'static str { + pub(crate) fn description(&self) -> &'static str { match self { - PathResolutionItem::Module(..) => "module", - PathResolutionItem::Type(..) => "type", - PathResolutionItem::TypeAlias(..) => "type alias", - PathResolutionItem::Trait(..) => "trait", - PathResolutionItem::Global(..) => "global", - PathResolutionItem::ModuleFunction(..) - | PathResolutionItem::Method(..) - | PathResolutionItem::SelfMethod(..) - | PathResolutionItem::TypeAliasFunction(..) - | PathResolutionItem::TraitFunction(..) => "function", + Self::Global(..) => "global", + Self::ModuleFunction(..) + | Self::Method(..) + | Self::SelfMethod(..) + | Self::TypeAliasFunction(..) + | Self::TraitFunction(..) => "function", } } } @@ -84,7 +133,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, @@ -196,7 +245,7 @@ impl Elaborator<'_> { let datatype = datatype.borrow(); if path.segments.len() == 1 { return Ok(PathResolution { - item: PathResolutionItem::Type(datatype.id), + item: PathResolutionItem::Type(PathResolutionTypeItem::Type(datatype.id)), errors: Vec::new(), }); } @@ -242,7 +291,7 @@ impl Elaborator<'_> { // There is a possibility that the import path is empty. In that case, early return. if path.segments.is_empty() { return Ok(PathResolution { - item: PathResolutionItem::Module(starting_module), + item: PathResolutionItem::Type(PathResolutionTypeItem::Module(starting_module)), errors: Vec::new(), }); } @@ -403,9 +452,65 @@ impl Elaborator<'_> { current_ns = found_ns; } - let (module_def_id, visibility, _) = - current_ns.values.or(current_ns.types).expect("Found empty namespace"); + let type_item = current_ns.types.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, + ) + }); + + let value_item = current_ns.values.map(|(module_def_id, visibility, ..)| { + self.per_ns_item_to_path_resolution_item( + path, + importing_module, + intermediate_item, + current_module_id, + &mut errors, + module_def_id, + visibility, + ) + }); + + let type_item = type_item.map(|type_item| match type_item { + PathResolutionItem::Type(item) | PathResolutionItem::TypeAndValue(item, _) => item, + PathResolutionItem::Value(_) => { + unreachable!("A type should have been produced from the type namespace") + } + }); + let value_item = value_item.map(|value_item| match value_item { + PathResolutionItem::Value(item) | PathResolutionItem::TypeAndValue(_, item) => item, + PathResolutionItem::Type(_) => { + unreachable!("A value should have been produced from the value namespace") + } + }); + let item = match (type_item, value_item) { + (Some(type_item), None) => PathResolutionItem::Type(type_item), + (None, Some(value_item)) => PathResolutionItem::Value(value_item), + (Some(type_item), Some(value_item)) => { + PathResolutionItem::TypeAndValue(type_item, value_item) + } + (None, None) => unreachable!("Found an empty namespace"), + }; + + 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(); @@ -426,8 +531,7 @@ impl Elaborator<'_> { { errors.push(PathResolutionError::Private(name.clone())); } - - Ok(PathResolution { item, errors }) + item } fn self_type_module_id(&self) -> Option { @@ -500,23 +604,39 @@ fn merge_intermediate_path_resolution_item_with_module_def_id( module_def_id: ModuleDefId, ) -> PathResolutionItem { match module_def_id { - ModuleDefId::ModuleId(module_id) => PathResolutionItem::Module(module_id), - ModuleDefId::TypeId(type_id) => PathResolutionItem::Type(type_id), - ModuleDefId::TypeAliasId(type_alias_id) => PathResolutionItem::TypeAlias(type_alias_id), - ModuleDefId::TraitId(trait_id) => PathResolutionItem::Trait(trait_id), - ModuleDefId::GlobalId(global_id) => PathResolutionItem::Global(global_id), + ModuleDefId::ModuleId(module_id) => { + PathResolutionItem::Type(PathResolutionTypeItem::Module(module_id)) + } + ModuleDefId::TypeId(type_id) => { + PathResolutionItem::Type(PathResolutionTypeItem::Type(type_id)) + } + ModuleDefId::TypeAliasId(type_alias_id) => { + PathResolutionItem::Type(PathResolutionTypeItem::TypeAlias(type_alias_id)) + } + ModuleDefId::TraitId(trait_id) => { + PathResolutionItem::Type(PathResolutionTypeItem::Trait(trait_id)) + } + ModuleDefId::GlobalId(global_id) => { + PathResolutionItem::Value(PathResolutionValueItem::Global(global_id)) + } ModuleDefId::FunctionId(func_id) => match intermediate_item { - IntermediatePathResolutionItem::SelfType => PathResolutionItem::SelfMethod(func_id), - IntermediatePathResolutionItem::Module => PathResolutionItem::ModuleFunction(func_id), - IntermediatePathResolutionItem::Type(type_id, generics) => { - PathResolutionItem::Method(type_id, generics, func_id) + IntermediatePathResolutionItem::SelfType => { + PathResolutionItem::Value(PathResolutionValueItem::SelfMethod(func_id)) } - IntermediatePathResolutionItem::TypeAlias(alias_id, generics) => { - PathResolutionItem::TypeAliasFunction(alias_id, generics, func_id) + IntermediatePathResolutionItem::Module => { + PathResolutionItem::Value(PathResolutionValueItem::ModuleFunction(func_id)) } - IntermediatePathResolutionItem::Trait(trait_id, generics) => { - PathResolutionItem::TraitFunction(trait_id, generics, func_id) + IntermediatePathResolutionItem::Type(type_id, generics) => PathResolutionItem::Value( + PathResolutionValueItem::Method(type_id, generics, func_id), + ), + IntermediatePathResolutionItem::TypeAlias(alias_id, generics) => { + PathResolutionItem::Value(PathResolutionValueItem::TypeAliasFunction( + alias_id, generics, func_id, + )) } + IntermediatePathResolutionItem::Trait(trait_id, generics) => PathResolutionItem::Value( + PathResolutionValueItem::TraitFunction(trait_id, generics, func_id), + ), }, } } diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 1e29d8db9bc..9663d550cc3 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -22,7 +22,10 @@ use crate::{ }, }; -use super::{Elaborator, ResolverMeta, path_resolution::PathResolutionItem}; +use super::{ + Elaborator, ResolverMeta, + path_resolution::{PathResolutionItem, PathResolutionValueItem}, +}; impl Elaborator<'_> { pub(super) fn elaborate_pattern( @@ -620,8 +623,8 @@ impl Elaborator<'_> { /// ``` /// Solve `` above fn resolve_item_turbofish(&mut self, item: PathResolutionItem) -> Vec { - match item { - PathResolutionItem::Method(struct_id, Some(generics), _func_id) => { + match item.into_value() { + Some(PathResolutionValueItem::Method(struct_id, Some(generics), _func_id)) => { let struct_type = self.interner.get_type(struct_id); let struct_type = struct_type.borrow(); let struct_generics = struct_type.instantiate(self.interner); @@ -632,14 +635,14 @@ impl Elaborator<'_> { generics.location, ) } - PathResolutionItem::SelfMethod(_) => { + Some(PathResolutionValueItem::SelfMethod(_)) => { if let Some(Type::DataType(_, generics)) = &self.self_type { generics.clone() } else { Vec::new() } } - PathResolutionItem::TypeAliasFunction(type_alias_id, generics, _func_id) => { + Some(PathResolutionValueItem::TypeAliasFunction(type_alias_id, generics, _func_id)) => { let type_alias = self.interner.get_type_alias(type_alias_id); let type_alias = type_alias.borrow(); let alias_generics = vecmap(&type_alias.generics, |generic| { @@ -664,7 +667,7 @@ impl Elaborator<'_> { // type Alias = Struct; get_type_alias_generics(&type_alias, &generics) } - PathResolutionItem::TraitFunction(trait_id, Some(generics), _func_id) => { + Some(PathResolutionValueItem::TraitFunction(trait_id, Some(generics), _func_id)) => { let trait_ = self.interner.get_trait(trait_id); let kinds = vecmap(&trait_.generics, |generic| generic.kind()); let trait_generics = @@ -678,14 +681,11 @@ impl Elaborator<'_> { generics.location, ) } - PathResolutionItem::Method(_, None, _) - | PathResolutionItem::TraitFunction(_, None, _) - | PathResolutionItem::Module(..) - | PathResolutionItem::Type(..) - | PathResolutionItem::TypeAlias(..) - | PathResolutionItem::Trait(..) - | PathResolutionItem::Global(..) - | PathResolutionItem::ModuleFunction(..) => Vec::new(), + Some(PathResolutionValueItem::Method(_, None, _)) + | Some(PathResolutionValueItem::TraitFunction(_, None, _)) + | Some(PathResolutionValueItem::ModuleFunction(..)) + | Some(PathResolutionValueItem::Global(..)) + | None => Vec::new(), } } @@ -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..ed6bd5ddc25 100644 --- a/compiler/noirc_frontend/src/elaborator/scope.rs +++ b/compiler/noirc_frontend/src/elaborator/scope.rs @@ -13,7 +13,9 @@ use crate::{ }; use crate::{Type, TypeAlias}; -use super::path_resolution::{PathResolutionItem, PathResolutionMode}; +use super::path_resolution::{ + PathResolutionItem, PathResolutionMode, PathResolutionTypeItem, PathResolutionValueItem, +}; use super::types::SELF_TYPE_NAME; use super::{Elaborator, ResolverMeta}; @@ -88,8 +90,8 @@ impl Elaborator<'_> { return Ok((self.interner.function_definition_id(function), item)); } - if let PathResolutionItem::Global(global) = item { - let global = self.interner.get_global(global); + if let Some(PathResolutionValueItem::Global(global)) = item.as_value() { + let global = self.interner.get_global(*global); return Ok((global.definition_id, item)); } @@ -139,8 +141,8 @@ impl Elaborator<'_> { let location = path.location; match self.resolve_path_or_error(path) { Ok(item) => { - if let PathResolutionItem::Trait(trait_id) = item { - Some(self.get_trait_mut(trait_id)) + if let Some(PathResolutionTypeItem::Trait(trait_id)) = item.as_type() { + Some(self.get_trait_mut(*trait_id)) } else { self.push_err(ResolverError::Expected { expected: "trait", @@ -166,8 +168,8 @@ impl Elaborator<'_> { let location = path.location; match self.resolve_path_or_error_inner(path, mode) { Ok(item) => { - if let PathResolutionItem::Type(struct_id) = item { - Some(self.get_type(struct_id)) + if let Some(PathResolutionTypeItem::Type(struct_id)) = item.as_type() { + Some(self.get_type(*struct_id)) } else { self.push_err(ResolverError::Expected { expected: "type", @@ -196,24 +198,26 @@ impl Elaborator<'_> { let location = path.location; match self.use_path_or_error(path) { - Ok(PathResolutionItem::Type(struct_id)) => { - let struct_type = self.get_type(struct_id); - let generics = struct_type.borrow().instantiate(self.interner); - Some(Type::DataType(struct_type, generics)) - } - Ok(PathResolutionItem::TypeAlias(alias_id)) => { - let alias = self.interner.get_type_alias(alias_id); - let alias = alias.borrow(); - Some(alias.instantiate(self.interner)) - } - Ok(other) => { - self.push_err(ResolverError::Expected { - expected: "type", - got: other.description(), - location, - }); - None - } + Ok(item) => match item.as_type() { + Some(PathResolutionTypeItem::Type(struct_id)) => { + let struct_type = self.get_type(*struct_id); + let generics = struct_type.borrow().instantiate(self.interner); + Some(Type::DataType(struct_type, generics)) + } + Some(PathResolutionTypeItem::TypeAlias(alias_id)) => { + let alias = self.interner.get_type_alias(*alias_id); + let alias = alias.borrow(); + Some(alias.instantiate(self.interner)) + } + _ => { + self.push_err(ResolverError::Expected { + expected: "type", + got: item.description(), + location, + }); + None + } + }, Err(error) => { self.push_err(error); None @@ -227,8 +231,12 @@ impl Elaborator<'_> { mode: PathResolutionMode, ) -> Option> { match self.resolve_path_or_error_inner(path, mode) { - Ok(PathResolutionItem::TypeAlias(type_alias_id)) => { - Some(self.interner.get_type_alias(type_alias_id)) + Ok(item) => { + if let Some(PathResolutionTypeItem::TypeAlias(type_alias_id)) = item.into_type() { + Some(self.interner.get_type_alias(type_alias_id)) + } else { + None + } } _ => None, } diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 5695a309c19..9deb14682eb 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -43,7 +43,9 @@ use crate::{ use super::{ Elaborator, FunctionContext, UnsafeBlockStatus, lints, - path_resolution::{PathResolutionItem, PathResolutionMode}, + path_resolution::{ + PathResolutionItem, PathResolutionMode, PathResolutionTypeItem, PathResolutionValueItem, + }, }; pub const SELF_TYPE_NAME: &str = "Self"; @@ -506,8 +508,12 @@ 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) { - Ok(PathResolutionItem::Global(id)) => { + match self + .resolve_path_or_error_inner(path.clone(), mode) + .ok() + .and_then(|item| item.into_value()) + { + Some(PathResolutionValueItem::Global(id)) => { if let Some(current_item) = self.current_item { self.interner.add_global_dependency(current_item, id); } @@ -776,11 +782,11 @@ impl Elaborator<'_> { let before_last_segment = path.last_segment(); let path_resolution = self.use_path(path).ok()?; - let PathResolutionItem::Type(type_id) = path_resolution.item else { + let Some(PathResolutionTypeItem::Type(type_id)) = path_resolution.item.as_type() else { return None; }; - let datatype = self.get_type(type_id); + let datatype = self.get_type(*type_id); let generics = datatype.borrow().instantiate(self.interner); let typ = Type::DataType(datatype, generics); let method_name = last_segment.ident.as_str(); @@ -810,7 +816,9 @@ impl Elaborator<'_> { let method = TraitMethod { method_id: trait_method_id, constraint, assumed: false }; let turbofish = before_last_segment.turbofish(); - let item = PathResolutionItem::TraitFunction(trait_id, turbofish, func_id); + let item = PathResolutionItem::Value(PathResolutionValueItem::TraitFunction( + trait_id, turbofish, func_id, + )); let mut errors = path_resolution.errors; if let Some(error) = error { errors.push(error); diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index c4668b4f77a..177a6858c9e 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -4673,3 +4673,21 @@ fn attempt_to_divide_by_zero_at_comptime() { "#; check_errors!(src); } + +#[named] +#[test] +fn types_and_values_namespace() { + let src = " + struct foo {} + + fn foo(x: foo) -> foo { + x + } + + fn main() { + let x: foo = foo {}; + let _ = foo(x); + } + "; + assert_no_errors!(src); +} diff --git a/test_programs/compile_success_no_bug/noirc_frontend_tests_types_and_values_namespace/Nargo.toml b/test_programs/compile_success_no_bug/noirc_frontend_tests_types_and_values_namespace/Nargo.toml new file mode 100644 index 00000000000..39776a48f15 --- /dev/null +++ b/test_programs/compile_success_no_bug/noirc_frontend_tests_types_and_values_namespace/Nargo.toml @@ -0,0 +1,7 @@ + + [package] + name = "noirc_frontend_tests_types_and_values_namespace" + type = "bin" + authors = [""] + + [dependencies] \ No newline at end of file diff --git a/test_programs/compile_success_no_bug/noirc_frontend_tests_types_and_values_namespace/src/main.nr b/test_programs/compile_success_no_bug/noirc_frontend_tests_types_and_values_namespace/src/main.nr new file mode 100644 index 00000000000..9429943e48e --- /dev/null +++ b/test_programs/compile_success_no_bug/noirc_frontend_tests_types_and_values_namespace/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_types_and_values_namespace/src_hash.txt b/test_programs/compile_success_no_bug/noirc_frontend_tests_types_and_values_namespace/src_hash.txt new file mode 100644 index 00000000000..cf3bdad5a13 --- /dev/null +++ b/test_programs/compile_success_no_bug/noirc_frontend_tests_types_and_values_namespace/src_hash.txt @@ -0,0 +1 @@ +17640324439011329444 \ No newline at end of file From 620bddf36e0bed7bd48d3097689cddb446d606c8 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 30 Apr 2025 04:40:34 -0300 Subject: [PATCH 02/16] Rename a test and add a failing one --- compiler/noirc_frontend/src/tests.rs | 21 ++++++++++++++++++- .../Nargo.toml | 7 +++++++ .../src/main.nr | 11 ++++++++++ .../src_hash.txt | 1 + .../Nargo.toml | 2 +- .../src/main.nr | 0 .../src_hash.txt | 0 7 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 test_programs/compile_failure/noirc_frontend_tests_only_one_private_error_when_name_in_types_and_values_namespace_collides/Nargo.toml create mode 100644 test_programs/compile_failure/noirc_frontend_tests_only_one_private_error_when_name_in_types_and_values_namespace_collides/src/main.nr create mode 100644 test_programs/compile_failure/noirc_frontend_tests_only_one_private_error_when_name_in_types_and_values_namespace_collides/src_hash.txt rename test_programs/compile_success_no_bug/{noirc_frontend_tests_types_and_values_namespace => noirc_frontend_tests_same_name_in_types_and_values_namespace_works}/Nargo.toml (56%) rename test_programs/compile_success_no_bug/{noirc_frontend_tests_types_and_values_namespace => noirc_frontend_tests_same_name_in_types_and_values_namespace_works}/src/main.nr (100%) rename test_programs/compile_success_no_bug/{noirc_frontend_tests_types_and_values_namespace => noirc_frontend_tests_same_name_in_types_and_values_namespace_works}/src_hash.txt (100%) diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 177a6858c9e..96ba7ad00cb 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -4676,7 +4676,7 @@ fn attempt_to_divide_by_zero_at_comptime() { #[named] #[test] -fn types_and_values_namespace() { +fn same_name_in_types_and_values_namespace_works() { let src = " struct foo {} @@ -4691,3 +4691,22 @@ fn types_and_values_namespace() { "; 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 currently visibile from the current module + ~~~ foo is private + } + "; + 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..d6f913a2205 --- /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,11 @@ + + mod moo { + struct foo {} + + fn foo() {} + } + + fn main() { + let _ = moo::foo {}; + } + \ 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..baa6d54567e --- /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 @@ +9540133685024384324 \ No newline at end of file diff --git a/test_programs/compile_success_no_bug/noirc_frontend_tests_types_and_values_namespace/Nargo.toml b/test_programs/compile_success_no_bug/noirc_frontend_tests_same_name_in_types_and_values_namespace_works/Nargo.toml similarity index 56% rename from test_programs/compile_success_no_bug/noirc_frontend_tests_types_and_values_namespace/Nargo.toml rename to test_programs/compile_success_no_bug/noirc_frontend_tests_same_name_in_types_and_values_namespace_works/Nargo.toml index 39776a48f15..0c9c6b1f904 100644 --- a/test_programs/compile_success_no_bug/noirc_frontend_tests_types_and_values_namespace/Nargo.toml +++ b/test_programs/compile_success_no_bug/noirc_frontend_tests_same_name_in_types_and_values_namespace_works/Nargo.toml @@ -1,6 +1,6 @@ [package] - name = "noirc_frontend_tests_types_and_values_namespace" + name = "noirc_frontend_tests_same_name_in_types_and_values_namespace_works" type = "bin" authors = [""] diff --git a/test_programs/compile_success_no_bug/noirc_frontend_tests_types_and_values_namespace/src/main.nr b/test_programs/compile_success_no_bug/noirc_frontend_tests_same_name_in_types_and_values_namespace_works/src/main.nr similarity index 100% rename from test_programs/compile_success_no_bug/noirc_frontend_tests_types_and_values_namespace/src/main.nr rename to test_programs/compile_success_no_bug/noirc_frontend_tests_same_name_in_types_and_values_namespace_works/src/main.nr diff --git a/test_programs/compile_success_no_bug/noirc_frontend_tests_types_and_values_namespace/src_hash.txt b/test_programs/compile_success_no_bug/noirc_frontend_tests_same_name_in_types_and_values_namespace_works/src_hash.txt similarity index 100% rename from test_programs/compile_success_no_bug/noirc_frontend_tests_types_and_values_namespace/src_hash.txt rename to test_programs/compile_success_no_bug/noirc_frontend_tests_same_name_in_types_and_values_namespace_works/src_hash.txt From 5f71938087742b7ca9f23732ec984c1277a75d14 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 30 Apr 2025 04:56:32 -0300 Subject: [PATCH 03/16] Introduce `PathResolutionTarget` and make the test pass --- .../noirc_frontend/src/elaborator/enums.rs | 9 ++- compiler/noirc_frontend/src/elaborator/mod.rs | 10 +-- .../src/elaborator/path_resolution.rs | 67 ++++++++++++++----- .../noirc_frontend/src/elaborator/scope.rs | 12 ++-- .../noirc_frontend/src/elaborator/types.rs | 8 +-- compiler/noirc_frontend/src/tests.rs | 2 +- 6 files changed, 74 insertions(+), 34 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/enums.rs b/compiler/noirc_frontend/src/elaborator/enums.rs index 2dbe5a071f3..32eac11e339 100644 --- a/compiler/noirc_frontend/src/elaborator/enums.rs +++ b/compiler/noirc_frontend/src/elaborator/enums.rs @@ -27,7 +27,10 @@ use crate::{ token::Attributes, }; -use super::{Elaborator, path_resolution::PathResolutionValueItem}; +use super::{ + Elaborator, + path_resolution::{PathResolutionTarget, PathResolutionValueItem}, +}; const WILDCARD_PATTERN: &str = "_"; @@ -419,7 +422,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 +591,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 e4b880d2b75..a361645a4dc 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -70,7 +70,7 @@ use noirc_errors::{Located, Location}; pub(crate) use options::ElaboratorOptions; pub use options::{FrontendOptions, UnstableFeature}; pub use path_resolution::Turbofish; -use path_resolution::{PathResolutionMode, PathResolutionTypeItem}; +use path_resolution::{PathResolutionMode, PathResolutionTarget, PathResolutionTypeItem}; use types::bind_ordered_generics; use self::traits::check_trait_impl_method_matches_declaration; @@ -788,7 +788,7 @@ impl<'context> Elaborator<'context> { } pub fn resolve_module_by_path(&mut self, path: Path) -> Option { - match self.resolve_path(path.clone()) { + match self.resolve_path(path.clone(), PathResolutionTarget::Type) { Ok(resolution) if resolution.errors.is_empty() => { if let Some(PathResolutionTypeItem::Module(module_id)) = resolution.item.as_type() { Some(*module_id) @@ -801,7 +801,7 @@ impl<'context> Elaborator<'context> { } fn resolve_trait_by_path(&mut self, path: Path) -> Option { - let error = match self.resolve_path(path.clone()) { + let error = match self.resolve_path(path.clone(), PathResolutionTarget::Type) { Ok(resolution) => { if let Some(PathResolutionTypeItem::Trait(trait_id)) = resolution.item.as_type() { for error in resolution.errors { @@ -870,7 +870,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 { + let Ok(item) = + 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 ca7ece21072..56a3a6a4f51 100644 --- a/compiler/noirc_frontend/src/elaborator/path_resolution.rs +++ b/compiler/noirc_frontend/src/elaborator/path_resolution.rs @@ -191,27 +191,40 @@ 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. +#[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); @@ -220,12 +233,20 @@ 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( + &mut self, + path: Path, + target: PathResolutionTarget, + ) -> PathResolutionResult { + self.resolve_path_inner(path, target, PathResolutionMode::MarkAsReferenced) } - pub(super) fn use_path(&mut self, path: Path) -> PathResolutionResult { - self.resolve_path_inner(path, PathResolutionMode::MarkAsUsed) + pub(super) fn use_path( + &mut self, + path: Path, + target: PathResolutionTarget, + ) -> PathResolutionResult { + self.resolve_path_inner(path, target, PathResolutionMode::MarkAsUsed) } /// Resolves a path in the current module. @@ -235,6 +256,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(); @@ -256,7 +278,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`. @@ -266,6 +288,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() { @@ -275,7 +298,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`. @@ -286,6 +316,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. @@ -461,6 +492,7 @@ impl Elaborator<'_> { &mut errors, module_def_id, visibility, + target == PathResolutionTarget::Type, ) }); @@ -473,6 +505,7 @@ impl Elaborator<'_> { &mut errors, module_def_id, visibility, + target == PathResolutionTarget::Value, ) }); @@ -510,6 +543,7 @@ impl Elaborator<'_> { errors: &mut Vec, module_def_id: ModuleDefId, visibility: crate::ast::ItemVisibility, + error_on_private: bool, ) -> PathResolutionItem { let name = path.last_ident(); let is_self_type = name.is_self_type_name(); @@ -521,13 +555,14 @@ impl Elaborator<'_> { module_def_id, ); - if !(self.self_type_module_id() == Some(current_module_id) - || item_in_module_is_visible( - self.def_maps, - importing_module, - current_module_id, - visibility, - )) + if error_on_private + && (!(self.self_type_module_id() == Some(current_module_id) + || item_in_module_is_visible( + self.def_maps, + importing_module, + current_module_id, + visibility, + ))) { errors.push(PathResolutionError::Private(name.clone())); } diff --git a/compiler/noirc_frontend/src/elaborator/scope.rs b/compiler/noirc_frontend/src/elaborator/scope.rs index ed6bd5ddc25..340111d265a 100644 --- a/compiler/noirc_frontend/src/elaborator/scope.rs +++ b/compiler/noirc_frontend/src/elaborator/scope.rs @@ -17,7 +17,7 @@ use super::path_resolution::{ PathResolutionItem, PathResolutionMode, PathResolutionTypeItem, PathResolutionValueItem, }; use super::types::SELF_TYPE_NAME; -use super::{Elaborator, ResolverMeta}; +use super::{Elaborator, PathResolutionTarget, ResolverMeta}; type Scope = GenericScope; type ScopeTree = GenericScopeTree; @@ -84,7 +84,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)); @@ -139,7 +139,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 Some(PathResolutionTypeItem::Trait(trait_id)) = item.as_type() { Some(self.get_trait_mut(*trait_id)) @@ -166,7 +166,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 Some(PathResolutionTypeItem::Type(struct_id)) = item.as_type() { Some(self.get_type(*struct_id)) @@ -197,7 +197,7 @@ impl Elaborator<'_> { } let location = path.location; - match self.use_path_or_error(path) { + match self.use_path_or_error(path, PathResolutionTarget::Type) { Ok(item) => match item.as_type() { Some(PathResolutionTypeItem::Type(struct_id)) => { let struct_type = self.get_type(*struct_id); @@ -230,7 +230,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(item) => { if let Some(PathResolutionTypeItem::TypeAlias(type_alias_id)) = item.into_type() { 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 9deb14682eb..b96ebbdc67f 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, PathResolutionTypeItem, PathResolutionValueItem, }, @@ -509,7 +509,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) + .resolve_path_or_error_inner(path.clone(), PathResolutionTarget::Value, mode) .ok() .and_then(|item| item.into_value()) { @@ -724,7 +724,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(path.clone(), PathResolutionTarget::Type).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?); @@ -781,7 +781,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(path, PathResolutionTarget::Type).ok()?; let Some(PathResolutionTypeItem::Type(type_id)) = path_resolution.item.as_type() else { return None; }; diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 96ba7ad00cb..77a29e945be 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -4704,7 +4704,7 @@ fn only_one_private_error_when_name_in_types_and_values_namespace_collides() { fn main() { let _ = moo::foo {}; - ^^^ foo is private and not currently visibile from the current module + ^^^ foo is private and not visible from the current module ~~~ foo is private } "; From d069240f7763eb3fde39a3c149ae5a5711966c3b Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 30 Apr 2025 04:59:24 -0300 Subject: [PATCH 04/16] Simplify a bit --- compiler/noirc_frontend/src/elaborator/mod.rs | 4 ++-- .../src/elaborator/path_resolution.rs | 20 ++++++++----------- .../noirc_frontend/src/elaborator/types.rs | 4 ++-- 3 files changed, 12 insertions(+), 16 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index a361645a4dc..5e076dbce72 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -788,7 +788,7 @@ impl<'context> Elaborator<'context> { } pub fn resolve_module_by_path(&mut self, path: Path) -> Option { - match self.resolve_path(path.clone(), PathResolutionTarget::Type) { + match self.resolve_path_as_type(path.clone()) { Ok(resolution) if resolution.errors.is_empty() => { if let Some(PathResolutionTypeItem::Module(module_id)) = resolution.item.as_type() { Some(*module_id) @@ -801,7 +801,7 @@ impl<'context> Elaborator<'context> { } fn resolve_trait_by_path(&mut self, path: Path) -> Option { - let error = match self.resolve_path(path.clone(), PathResolutionTarget::Type) { + let error = match self.resolve_path_as_type(path.clone()) { Ok(resolution) => { if let Some(PathResolutionTypeItem::Trait(trait_id)) = resolution.item.as_type() { for error in resolution.errors { diff --git a/compiler/noirc_frontend/src/elaborator/path_resolution.rs b/compiler/noirc_frontend/src/elaborator/path_resolution.rs index 56a3a6a4f51..635a172aa7f 100644 --- a/compiler/noirc_frontend/src/elaborator/path_resolution.rs +++ b/compiler/noirc_frontend/src/elaborator/path_resolution.rs @@ -233,20 +233,16 @@ impl Elaborator<'_> { Ok(path_resolution.item) } - pub(super) fn resolve_path( - &mut self, - path: Path, - target: PathResolutionTarget, - ) -> PathResolutionResult { - self.resolve_path_inner(path, target, 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, - target: PathResolutionTarget, - ) -> PathResolutionResult { - self.resolve_path_inner(path, target, 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. diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index b96ebbdc67f..06823ad595d 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -724,7 +724,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(), PathResolutionTarget::Type).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?); @@ -781,7 +781,7 @@ impl Elaborator<'_> { let last_segment = path.pop(); let before_last_segment = path.last_segment(); - let path_resolution = self.use_path(path, PathResolutionTarget::Type).ok()?; + let path_resolution = self.use_path_as_type(path).ok()?; let Some(PathResolutionTypeItem::Type(type_id)) = path_resolution.item.as_type() else { return None; }; From 5b5bec1c6f753f9175c84f706e372098af37e614 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 30 Apr 2025 05:32:31 -0300 Subject: [PATCH 05/16] Go back to the roots. Much simpler. --- .../noirc_frontend/src/elaborator/enums.rs | 24 +- compiler/noirc_frontend/src/elaborator/mod.rs | 31 ++- .../src/elaborator/path_resolution.rs | 219 ++++++------------ .../noirc_frontend/src/elaborator/patterns.rs | 29 +-- .../noirc_frontend/src/elaborator/scope.rs | 49 ++-- .../noirc_frontend/src/elaborator/types.rs | 20 +- 6 files changed, 133 insertions(+), 239 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/enums.rs b/compiler/noirc_frontend/src/elaborator/enums.rs index 32eac11e339..6a585036697 100644 --- a/compiler/noirc_frontend/src/elaborator/enums.rs +++ b/compiler/noirc_frontend/src/elaborator/enums.rs @@ -27,10 +27,7 @@ use crate::{ token::Attributes, }; -use super::{ - Elaborator, - path_resolution::{PathResolutionTarget, PathResolutionValueItem}, -}; +use super::{Elaborator, path_resolution::PathResolutionTarget}; const WILDCARD_PATTERN: &str = "_"; @@ -647,8 +644,8 @@ impl Elaborator<'_> { location: Location, variables_defined: &mut Vec, ) -> Pattern { - let (actual_type, expected_arg_types, variant_index) = match resolution.as_value() { - Some(PathResolutionValueItem::Global(id)) => { + let (actual_type, expected_arg_types, variant_index) = match &resolution { + PathResolutionItem::Global(id) => { // variant constant self.elaborate_global_if_unresolved(id); let global = self.interner.get_global(*id); @@ -672,8 +669,7 @@ impl Elaborator<'_> { let actual_type = global_type.instantiate(self.interner).0; (actual_type, Vec::new(), variant_index) } - Some(PathResolutionValueItem::Method(_, _, func_id)) - | Some(PathResolutionValueItem::SelfMethod(func_id)) => { + PathResolutionItem::Method(_, _, func_id) | PathResolutionItem::SelfMethod(func_id) => { // TODO(#7430): Take type_turbofish into account when instantiating the function's type let meta = self.interner.function_meta(func_id); let Some(variant_index) = meta.enum_variant_index else { @@ -690,10 +686,14 @@ impl Elaborator<'_> { (actual_type, expected_arg_types, variant_index) } - Some(PathResolutionValueItem::ModuleFunction(_)) - | Some(PathResolutionValueItem::TypeAliasFunction(_, _, _)) - | Some(PathResolutionValueItem::TraitFunction(_, _, _)) - | None => { + PathResolutionItem::ModuleFunction(_) + | PathResolutionItem::TypeAliasFunction(_, _, _) + | PathResolutionItem::TraitFunction(_, _, _) + | PathResolutionItem::Module(_) + | PathResolutionItem::Type(_) + | PathResolutionItem::TypeAlias(_) + | PathResolutionItem::Trait(_) + | PathResolutionItem::UnexpectedTarget(_) => { // This variable refers to an existing item if let Some(name) = name { // If name is set, shadow the existing item diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 5e076dbce72..58a7b1abb28 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::{PathResolutionMode, PathResolutionTarget, PathResolutionTypeItem}; +use path_resolution::{ + PathResolution, PathResolutionItem, PathResolutionMode, PathResolutionTarget, +}; use types::bind_ordered_generics; use self::traits::check_trait_impl_method_matches_declaration; @@ -789,12 +791,10 @@ impl<'context> Elaborator<'context> { pub fn resolve_module_by_path(&mut self, path: Path) -> Option { match self.resolve_path_as_type(path.clone()) { - Ok(resolution) if resolution.errors.is_empty() => { - if let Some(PathResolutionTypeItem::Module(module_id)) = resolution.item.as_type() { - Some(*module_id) - } else { - None - } + Ok(PathResolution { item: PathResolutionItem::Module(module_id), errors }) + if errors.is_empty() => + { + Some(module_id) } _ => None, } @@ -802,16 +802,13 @@ impl<'context> Elaborator<'context> { fn resolve_trait_by_path(&mut self, path: Path) -> Option { let error = match self.resolve_path_as_type(path.clone()) { - Ok(resolution) => { - if let Some(PathResolutionTypeItem::Trait(trait_id)) = resolution.item.as_type() { - for error in resolution.errors { - self.push_err(error); - } - return Some(*trait_id); - } else { - DefCollectorErrorKind::NotATrait { not_a_trait_name: path } + Ok(PathResolution { item: PathResolutionItem::Trait(trait_id), errors }) => { + for error in errors { + self.push_err(error); } + return Some(trait_id); } + Ok(_) => DefCollectorErrorKind::NotATrait { not_a_trait_name: path }, Err(_) => DefCollectorErrorKind::TraitNotFound { trait_path: path }, }; self.push_err(error); @@ -876,11 +873,11 @@ impl<'context> Elaborator<'context> { return Vec::new(); }; - let Some(PathResolutionTypeItem::Trait(trait_id)) = item.as_type() else { + let PathResolutionItem::Trait(trait_id) = item else { return Vec::new(); }; - let the_trait = self.get_trait_mut(*trait_id); + let the_trait = self.get_trait_mut(trait_id); if the_trait.associated_types.len() > bound.trait_generics.named_args.len() { let trait_name = the_trait.name.to_string(); diff --git a/compiler/noirc_frontend/src/elaborator/path_resolution.rs b/compiler/noirc_frontend/src/elaborator/path_resolution.rs index 635a172aa7f..368ae969895 100644 --- a/compiler/noirc_frontend/src/elaborator/path_resolution.rs +++ b/compiler/noirc_frontend/src/elaborator/path_resolution.rs @@ -23,86 +23,26 @@ pub(crate) struct PathResolution { #[derive(Debug)] pub(crate) enum PathResolutionItem { - Type(PathResolutionTypeItem), - Value(PathResolutionValueItem), - TypeAndValue(PathResolutionTypeItem, PathResolutionValueItem), -} - -impl PathResolutionItem { - pub(crate) fn into_type(self) -> Option { - match self { - Self::Type(typ) | Self::TypeAndValue(typ, _) => Some(typ), - _ => None, - } - } - - pub(crate) fn as_type(&self) -> Option<&PathResolutionTypeItem> { - match self { - Self::Type(typ) | Self::TypeAndValue(typ, _) => Some(typ), - _ => None, - } - } - - pub(crate) fn into_value(self) -> Option { - match self { - Self::Value(value) | Self::TypeAndValue(_, value) => Some(value), - _ => None, - } - } - - pub(crate) fn as_value(&self) -> Option<&PathResolutionValueItem> { - match self { - Self::Value(value) | Self::TypeAndValue(_, value) => Some(value), - _ => None, - } - } - - pub(crate) fn function_id(&self) -> Option { - self.as_value().and_then(|item| item.function_id()) - } - - pub(crate) fn description(&self) -> &'static str { - match self { - Self::Type(item) => item.description(), - Self::Value(item) => item.description(), - Self::TypeAndValue(item, _) => { - // Any of the two items can be used to describe the resolution. - item.description() - } - } - } -} - -#[derive(Debug, Clone)] -pub(crate) enum PathResolutionTypeItem { + // These are types Module(ModuleId), Type(TypeId), TypeAlias(TypeAliasId), Trait(TraitId), -} - -impl PathResolutionTypeItem { - pub(crate) fn description(&self) -> &'static str { - match self { - Self::Module(..) => "module", - Self::Type(..) => "type", - Self::TypeAlias(..) => "type alias", - Self::Trait(..) => "trait", - } - } -} -#[derive(Debug, Clone)] -pub(crate) enum PathResolutionValueItem { + // These are values Global(GlobalId), ModuleFunction(FuncId), Method(TypeId, Option, FuncId), SelfMethod(FuncId), TypeAliasFunction(TypeAliasId, Option, FuncId), TraitFunction(TraitId, Option, FuncId), + + /// This is returned when the path points to something but it's + /// not in the namespace we were looking for. + UnexpectedTarget(&'static str), } -impl PathResolutionValueItem { +impl PathResolutionItem { pub(crate) fn function_id(&self) -> Option { match self { Self::ModuleFunction(func_id) @@ -110,18 +50,28 @@ impl PathResolutionValueItem { | Self::SelfMethod(func_id) | Self::TypeAliasFunction(_, _, func_id) | Self::TraitFunction(_, _, func_id) => Some(*func_id), - Self::Global(..) => None, + Self::Module(_) + | Self::Type(_) + | Self::TypeAlias(_) + | Self::Trait(_) + | Self::Global(..) + | Self::UnexpectedTarget(_) => None, } } pub(crate) fn description(&self) -> &'static str { match self { + Self::Module(..) => "module", + Self::Type(..) => "type", + Self::TypeAlias(..) => "type alias", + Self::Trait(..) => "trait", Self::Global(..) => "global", Self::ModuleFunction(..) | Self::Method(..) | Self::SelfMethod(..) | Self::TypeAliasFunction(..) | Self::TraitFunction(..) => "function", + Self::UnexpectedTarget(description) => description, } } } @@ -195,6 +145,10 @@ pub(super) enum PathResolutionMode { /// 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, if a target wasn't find but instead +/// something with that name was found on the other namespace, [`PathResolutionItem::UnexpectedTarget`] +/// will be returned. #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub(super) enum PathResolutionTarget { Type, @@ -263,7 +217,7 @@ impl Elaborator<'_> { let datatype = datatype.borrow(); if path.segments.len() == 1 { return Ok(PathResolution { - item: PathResolutionItem::Type(PathResolutionTypeItem::Type(datatype.id)), + item: PathResolutionItem::Type(datatype.id), errors: Vec::new(), }); } @@ -318,7 +272,7 @@ impl Elaborator<'_> { // There is a possibility that the import path is empty. In that case, early return. if path.segments.is_empty() { return Ok(PathResolution { - item: PathResolutionItem::Type(PathResolutionTypeItem::Module(starting_module)), + item: PathResolutionItem::Module(starting_module), errors: Vec::new(), }); } @@ -479,53 +433,29 @@ impl Elaborator<'_> { current_ns = found_ns; } - let type_item = current_ns.types.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, - target == PathResolutionTarget::Type, - ) - }); - - let value_item = current_ns.values.map(|(module_def_id, visibility, ..)| { - self.per_ns_item_to_path_resolution_item( - path, - importing_module, - intermediate_item, - current_module_id, - &mut errors, - module_def_id, - visibility, - target == PathResolutionTarget::Value, - ) - }); - - let type_item = type_item.map(|type_item| match type_item { - PathResolutionItem::Type(item) | PathResolutionItem::TypeAndValue(item, _) => item, - PathResolutionItem::Value(_) => { - unreachable!("A type should have been produced from the type namespace") - } - }); - let value_item = value_item.map(|value_item| match value_item { - PathResolutionItem::Value(item) | PathResolutionItem::TypeAndValue(_, item) => item, - PathResolutionItem::Type(_) => { - unreachable!("A value should have been produced from the value namespace") - } - }); - let item = match (type_item, value_item) { - (Some(type_item), None) => PathResolutionItem::Type(type_item), - (None, Some(value_item)) => PathResolutionItem::Value(value_item), - (Some(type_item), Some(value_item)) => { - PathResolutionItem::TypeAndValue(type_item, value_item) - } - (None, None) => unreachable!("Found an 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(|| { + PathResolutionItem::UnexpectedTarget( + fallback_ns.expect("A namespace should never be empty").0.as_str(), + ) + }); + Ok(PathResolution { item, errors }) } @@ -539,7 +469,6 @@ impl Elaborator<'_> { errors: &mut Vec, module_def_id: ModuleDefId, visibility: crate::ast::ItemVisibility, - error_on_private: bool, ) -> PathResolutionItem { let name = path.last_ident(); let is_self_type = name.is_self_type_name(); @@ -551,17 +480,17 @@ impl Elaborator<'_> { module_def_id, ); - if error_on_private - && (!(self.self_type_module_id() == Some(current_module_id) - || item_in_module_is_visible( - self.def_maps, - importing_module, - current_module_id, - visibility, - ))) + if !(self.self_type_module_id() == Some(current_module_id) + || item_in_module_is_visible( + self.def_maps, + importing_module, + current_module_id, + visibility, + )) { errors.push(PathResolutionError::Private(name.clone())); } + item } @@ -635,39 +564,23 @@ fn merge_intermediate_path_resolution_item_with_module_def_id( module_def_id: ModuleDefId, ) -> PathResolutionItem { match module_def_id { - ModuleDefId::ModuleId(module_id) => { - PathResolutionItem::Type(PathResolutionTypeItem::Module(module_id)) - } - ModuleDefId::TypeId(type_id) => { - PathResolutionItem::Type(PathResolutionTypeItem::Type(type_id)) - } - ModuleDefId::TypeAliasId(type_alias_id) => { - PathResolutionItem::Type(PathResolutionTypeItem::TypeAlias(type_alias_id)) - } - ModuleDefId::TraitId(trait_id) => { - PathResolutionItem::Type(PathResolutionTypeItem::Trait(trait_id)) - } - ModuleDefId::GlobalId(global_id) => { - PathResolutionItem::Value(PathResolutionValueItem::Global(global_id)) - } + ModuleDefId::ModuleId(module_id) => PathResolutionItem::Module(module_id), + ModuleDefId::TypeId(type_id) => PathResolutionItem::Type(type_id), + ModuleDefId::TypeAliasId(type_alias_id) => PathResolutionItem::TypeAlias(type_alias_id), + ModuleDefId::TraitId(trait_id) => PathResolutionItem::Trait(trait_id), + ModuleDefId::GlobalId(global_id) => PathResolutionItem::Global(global_id), ModuleDefId::FunctionId(func_id) => match intermediate_item { - IntermediatePathResolutionItem::SelfType => { - PathResolutionItem::Value(PathResolutionValueItem::SelfMethod(func_id)) - } - IntermediatePathResolutionItem::Module => { - PathResolutionItem::Value(PathResolutionValueItem::ModuleFunction(func_id)) + IntermediatePathResolutionItem::SelfType => PathResolutionItem::SelfMethod(func_id), + IntermediatePathResolutionItem::Module => PathResolutionItem::ModuleFunction(func_id), + IntermediatePathResolutionItem::Type(type_id, generics) => { + PathResolutionItem::Method(type_id, generics, func_id) } - IntermediatePathResolutionItem::Type(type_id, generics) => PathResolutionItem::Value( - PathResolutionValueItem::Method(type_id, generics, func_id), - ), IntermediatePathResolutionItem::TypeAlias(alias_id, generics) => { - PathResolutionItem::Value(PathResolutionValueItem::TypeAliasFunction( - alias_id, generics, func_id, - )) + PathResolutionItem::TypeAliasFunction(alias_id, generics, func_id) + } + IntermediatePathResolutionItem::Trait(trait_id, generics) => { + PathResolutionItem::TraitFunction(trait_id, generics, func_id) } - IntermediatePathResolutionItem::Trait(trait_id, generics) => PathResolutionItem::Value( - PathResolutionValueItem::TraitFunction(trait_id, generics, func_id), - ), }, } } diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 9663d550cc3..40589eefd6b 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -22,10 +22,7 @@ use crate::{ }, }; -use super::{ - Elaborator, ResolverMeta, - path_resolution::{PathResolutionItem, PathResolutionValueItem}, -}; +use super::{Elaborator, ResolverMeta, path_resolution::PathResolutionItem}; impl Elaborator<'_> { pub(super) fn elaborate_pattern( @@ -623,8 +620,8 @@ impl Elaborator<'_> { /// ``` /// Solve `` above fn resolve_item_turbofish(&mut self, item: PathResolutionItem) -> Vec { - match item.into_value() { - Some(PathResolutionValueItem::Method(struct_id, Some(generics), _func_id)) => { + match item { + PathResolutionItem::Method(struct_id, Some(generics), _func_id) => { let struct_type = self.interner.get_type(struct_id); let struct_type = struct_type.borrow(); let struct_generics = struct_type.instantiate(self.interner); @@ -635,14 +632,14 @@ impl Elaborator<'_> { generics.location, ) } - Some(PathResolutionValueItem::SelfMethod(_)) => { + PathResolutionItem::SelfMethod(_) => { if let Some(Type::DataType(_, generics)) = &self.self_type { generics.clone() } else { Vec::new() } } - Some(PathResolutionValueItem::TypeAliasFunction(type_alias_id, generics, _func_id)) => { + PathResolutionItem::TypeAliasFunction(type_alias_id, generics, _func_id) => { let type_alias = self.interner.get_type_alias(type_alias_id); let type_alias = type_alias.borrow(); let alias_generics = vecmap(&type_alias.generics, |generic| { @@ -667,7 +664,7 @@ impl Elaborator<'_> { // type Alias = Struct; get_type_alias_generics(&type_alias, &generics) } - Some(PathResolutionValueItem::TraitFunction(trait_id, Some(generics), _func_id)) => { + PathResolutionItem::TraitFunction(trait_id, Some(generics), _func_id) => { let trait_ = self.interner.get_trait(trait_id); let kinds = vecmap(&trait_.generics, |generic| generic.kind()); let trait_generics = @@ -681,11 +678,15 @@ impl Elaborator<'_> { generics.location, ) } - Some(PathResolutionValueItem::Method(_, None, _)) - | Some(PathResolutionValueItem::TraitFunction(_, None, _)) - | Some(PathResolutionValueItem::ModuleFunction(..)) - | Some(PathResolutionValueItem::Global(..)) - | None => Vec::new(), + PathResolutionItem::Method(_, None, _) + | PathResolutionItem::TraitFunction(_, None, _) + | PathResolutionItem::ModuleFunction(..) + | PathResolutionItem::Global(..) + | PathResolutionItem::Module(_) + | PathResolutionItem::Type(_) + | PathResolutionItem::TypeAlias(_) + | PathResolutionItem::Trait(_) + | PathResolutionItem::UnexpectedTarget(_) => Vec::new(), } } diff --git a/compiler/noirc_frontend/src/elaborator/scope.rs b/compiler/noirc_frontend/src/elaborator/scope.rs index 340111d265a..50d27a7afdd 100644 --- a/compiler/noirc_frontend/src/elaborator/scope.rs +++ b/compiler/noirc_frontend/src/elaborator/scope.rs @@ -13,9 +13,7 @@ use crate::{ }; use crate::{Type, TypeAlias}; -use super::path_resolution::{ - PathResolutionItem, PathResolutionMode, PathResolutionTypeItem, PathResolutionValueItem, -}; +use super::path_resolution::{PathResolutionItem, PathResolutionMode}; use super::types::SELF_TYPE_NAME; use super::{Elaborator, PathResolutionTarget, ResolverMeta}; @@ -90,8 +88,8 @@ impl Elaborator<'_> { return Ok((self.interner.function_definition_id(function), item)); } - if let Some(PathResolutionValueItem::Global(global)) = item.as_value() { - let global = self.interner.get_global(*global); + if let PathResolutionItem::Global(global) = item { + let global = self.interner.get_global(global); return Ok((global.definition_id, item)); } @@ -141,8 +139,8 @@ impl Elaborator<'_> { let location = path.location; match self.resolve_path_or_error(path, PathResolutionTarget::Type) { Ok(item) => { - if let Some(PathResolutionTypeItem::Trait(trait_id)) = item.as_type() { - Some(self.get_trait_mut(*trait_id)) + if let PathResolutionItem::Trait(trait_id) = item { + Some(self.get_trait_mut(trait_id)) } else { self.push_err(ResolverError::Expected { expected: "trait", @@ -167,17 +165,14 @@ impl Elaborator<'_> { ) -> Option> { let location = path.location; match self.resolve_path_or_error_inner(path, PathResolutionTarget::Type, mode) { - Ok(item) => { - if let Some(PathResolutionTypeItem::Type(struct_id)) = item.as_type() { - Some(self.get_type(*struct_id)) - } else { - self.push_err(ResolverError::Expected { - expected: "type", - got: item.description(), - location, - }); - None - } + Ok(PathResolutionItem::Type(struct_id)) => Some(self.get_type(struct_id)), + Ok(other) => { + self.push_err(ResolverError::Expected { + expected: "type", + got: other.description(), + location, + }); + None } Err(err) => { self.push_err(err); @@ -198,14 +193,14 @@ impl Elaborator<'_> { let location = path.location; match self.use_path_or_error(path, PathResolutionTarget::Type) { - Ok(item) => match item.as_type() { - Some(PathResolutionTypeItem::Type(struct_id)) => { - let struct_type = self.get_type(*struct_id); + Ok(item) => match item { + PathResolutionItem::Type(struct_id) => { + let struct_type = self.get_type(struct_id); let generics = struct_type.borrow().instantiate(self.interner); Some(Type::DataType(struct_type, generics)) } - Some(PathResolutionTypeItem::TypeAlias(alias_id)) => { - let alias = self.interner.get_type_alias(*alias_id); + PathResolutionItem::TypeAlias(alias_id) => { + let alias = self.interner.get_type_alias(alias_id); let alias = alias.borrow(); Some(alias.instantiate(self.interner)) } @@ -231,12 +226,8 @@ impl Elaborator<'_> { mode: PathResolutionMode, ) -> Option> { match self.resolve_path_or_error_inner(path, PathResolutionTarget::Type, mode) { - Ok(item) => { - if let Some(PathResolutionTypeItem::TypeAlias(type_alias_id)) = item.into_type() { - Some(self.interner.get_type_alias(type_alias_id)) - } else { - None - } + Ok(PathResolutionItem::TypeAlias(type_alias_id)) => { + Some(self.interner.get_type_alias(type_alias_id)) } _ => None, } diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 06823ad595d..315785c56b5 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -43,9 +43,7 @@ use crate::{ use super::{ Elaborator, FunctionContext, PathResolutionTarget, UnsafeBlockStatus, lints, - path_resolution::{ - PathResolutionItem, PathResolutionMode, PathResolutionTypeItem, PathResolutionValueItem, - }, + path_resolution::{PathResolutionItem, PathResolutionMode}, }; pub const SELF_TYPE_NAME: &str = "Self"; @@ -508,12 +506,8 @@ 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(), PathResolutionTarget::Value, mode) - .ok() - .and_then(|item| item.into_value()) - { - Some(PathResolutionValueItem::Global(id)) => { + 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); } @@ -782,11 +776,11 @@ impl Elaborator<'_> { let before_last_segment = path.last_segment(); let path_resolution = self.use_path_as_type(path).ok()?; - let Some(PathResolutionTypeItem::Type(type_id)) = path_resolution.item.as_type() else { + let PathResolutionItem::Type(type_id) = path_resolution.item else { return None; }; - let datatype = self.get_type(*type_id); + let datatype = self.get_type(type_id); let generics = datatype.borrow().instantiate(self.interner); let typ = Type::DataType(datatype, generics); let method_name = last_segment.ident.as_str(); @@ -816,9 +810,7 @@ impl Elaborator<'_> { let method = TraitMethod { method_id: trait_method_id, constraint, assumed: false }; let turbofish = before_last_segment.turbofish(); - let item = PathResolutionItem::Value(PathResolutionValueItem::TraitFunction( - trait_id, turbofish, func_id, - )); + let item = PathResolutionItem::TraitFunction(trait_id, turbofish, func_id); let mut errors = path_resolution.errors; if let Some(error) = error { errors.push(error); From 2751f373828054848d57d5154f116789322c6130 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 30 Apr 2025 05:34:55 -0300 Subject: [PATCH 06/16] Shorten diff --- compiler/noirc_frontend/src/elaborator/enums.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/enums.rs b/compiler/noirc_frontend/src/elaborator/enums.rs index 6a585036697..13f0d930f40 100644 --- a/compiler/noirc_frontend/src/elaborator/enums.rs +++ b/compiler/noirc_frontend/src/elaborator/enums.rs @@ -686,13 +686,13 @@ impl Elaborator<'_> { (actual_type, expected_arg_types, variant_index) } - PathResolutionItem::ModuleFunction(_) - | PathResolutionItem::TypeAliasFunction(_, _, _) - | PathResolutionItem::TraitFunction(_, _, _) - | PathResolutionItem::Module(_) + PathResolutionItem::Module(_) | PathResolutionItem::Type(_) | PathResolutionItem::TypeAlias(_) | PathResolutionItem::Trait(_) + | PathResolutionItem::ModuleFunction(_) + | PathResolutionItem::TypeAliasFunction(_, _, _) + | PathResolutionItem::TraitFunction(_, _, _) | PathResolutionItem::UnexpectedTarget(_) => { // This variable refers to an existing item if let Some(name) = name { From d4b19aea4eb088d5a9bd59a23587e6d8d6d497b8 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 30 Apr 2025 05:36:21 -0300 Subject: [PATCH 07/16] Put back comment --- compiler/noirc_frontend/src/elaborator/path_resolution.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compiler/noirc_frontend/src/elaborator/path_resolution.rs b/compiler/noirc_frontend/src/elaborator/path_resolution.rs index 368ae969895..3e6a683b57b 100644 --- a/compiler/noirc_frontend/src/elaborator/path_resolution.rs +++ b/compiler/noirc_frontend/src/elaborator/path_resolution.rs @@ -21,6 +21,9 @@ pub(crate) struct PathResolution { pub(crate) errors: Vec, } +/// 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)] pub(crate) enum PathResolutionItem { // These are types From 681af3d3ea84fed1ac16355b6acf1d9e9cd50974 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 30 Apr 2025 05:37:08 -0300 Subject: [PATCH 08/16] No need for clone on IntermediatePathResolutionItem --- compiler/noirc_frontend/src/elaborator/path_resolution.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/path_resolution.rs b/compiler/noirc_frontend/src/elaborator/path_resolution.rs index 3e6a683b57b..6c68268d88e 100644 --- a/compiler/noirc_frontend/src/elaborator/path_resolution.rs +++ b/compiler/noirc_frontend/src/elaborator/path_resolution.rs @@ -86,7 +86,7 @@ pub struct Turbofish { } /// Any item that can appear before the last segment in a path. -#[derive(Debug, Clone)] +#[derive(Debug)] enum IntermediatePathResolutionItem { SelfType, Module, @@ -446,7 +446,7 @@ impl Elaborator<'_> { self.per_ns_item_to_path_resolution_item( path.clone(), importing_module, - intermediate_item.clone(), + intermediate_item, current_module_id, &mut errors, module_def_id, From 570b5d05f576d694068164e52461fbbe7f9fbea2 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 30 Apr 2025 05:39:39 -0300 Subject: [PATCH 09/16] Shorten diff --- .../noirc_frontend/src/elaborator/patterns.rs | 4 ++-- .../noirc_frontend/src/elaborator/scope.rs | 19 +++++++++++-------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 40589eefd6b..9682014941e 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -680,12 +680,12 @@ impl Elaborator<'_> { } PathResolutionItem::Method(_, None, _) | PathResolutionItem::TraitFunction(_, None, _) - | PathResolutionItem::ModuleFunction(..) - | PathResolutionItem::Global(..) | PathResolutionItem::Module(_) | PathResolutionItem::Type(_) | PathResolutionItem::TypeAlias(_) | PathResolutionItem::Trait(_) + | PathResolutionItem::Global(..) + | PathResolutionItem::ModuleFunction(..) | PathResolutionItem::UnexpectedTarget(_) => Vec::new(), } } diff --git a/compiler/noirc_frontend/src/elaborator/scope.rs b/compiler/noirc_frontend/src/elaborator/scope.rs index 50d27a7afdd..08fce14348b 100644 --- a/compiler/noirc_frontend/src/elaborator/scope.rs +++ b/compiler/noirc_frontend/src/elaborator/scope.rs @@ -165,14 +165,17 @@ impl Elaborator<'_> { ) -> Option> { let location = path.location; match self.resolve_path_or_error_inner(path, PathResolutionTarget::Type, mode) { - Ok(PathResolutionItem::Type(struct_id)) => Some(self.get_type(struct_id)), - Ok(other) => { - self.push_err(ResolverError::Expected { - expected: "type", - got: other.description(), - location, - }); - None + Ok(item) => { + if let PathResolutionItem::Type(struct_id) = item { + Some(self.get_type(struct_id)) + } else { + self.push_err(ResolverError::Expected { + expected: "type", + got: item.description(), + location, + }); + None + } } Err(err) => { self.push_err(err); From ec3022e87b4e1e93afd7bf83b2cbc03455891dea Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 30 Apr 2025 05:40:39 -0300 Subject: [PATCH 10/16] Shorten diff --- .../noirc_frontend/src/elaborator/scope.rs | 38 +++++++++---------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/scope.rs b/compiler/noirc_frontend/src/elaborator/scope.rs index 08fce14348b..49aa170426c 100644 --- a/compiler/noirc_frontend/src/elaborator/scope.rs +++ b/compiler/noirc_frontend/src/elaborator/scope.rs @@ -196,26 +196,24 @@ impl Elaborator<'_> { let location = path.location; match self.use_path_or_error(path, PathResolutionTarget::Type) { - Ok(item) => match item { - PathResolutionItem::Type(struct_id) => { - let struct_type = self.get_type(struct_id); - let generics = struct_type.borrow().instantiate(self.interner); - Some(Type::DataType(struct_type, generics)) - } - PathResolutionItem::TypeAlias(alias_id) => { - let alias = self.interner.get_type_alias(alias_id); - let alias = alias.borrow(); - Some(alias.instantiate(self.interner)) - } - _ => { - self.push_err(ResolverError::Expected { - expected: "type", - got: item.description(), - location, - }); - None - } - }, + Ok(PathResolutionItem::Type(struct_id)) => { + let struct_type = self.get_type(struct_id); + let generics = struct_type.borrow().instantiate(self.interner); + Some(Type::DataType(struct_type, generics)) + } + Ok(PathResolutionItem::TypeAlias(alias_id)) => { + let alias = self.interner.get_type_alias(alias_id); + let alias = alias.borrow(); + Some(alias.instantiate(self.interner)) + } + Ok(other) => { + self.push_err(ResolverError::Expected { + expected: "type", + got: other.description(), + location, + }); + None + } Err(error) => { self.push_err(error); None From b99232366d7d2c3dd37f280f01e22a0db0da5df0 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 30 Apr 2025 08:55:37 -0300 Subject: [PATCH 11/16] No need to have UnexpectedTarget --- .../noirc_frontend/src/elaborator/enums.rs | 3 +- .../src/elaborator/path_resolution.rs | 47 ++++++++++--------- .../noirc_frontend/src/elaborator/patterns.rs | 3 +- 3 files changed, 28 insertions(+), 25 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/enums.rs b/compiler/noirc_frontend/src/elaborator/enums.rs index 13f0d930f40..7cfb7c9b1a0 100644 --- a/compiler/noirc_frontend/src/elaborator/enums.rs +++ b/compiler/noirc_frontend/src/elaborator/enums.rs @@ -692,8 +692,7 @@ impl Elaborator<'_> { | PathResolutionItem::Trait(_) | PathResolutionItem::ModuleFunction(_) | PathResolutionItem::TypeAliasFunction(_, _, _) - | PathResolutionItem::TraitFunction(_, _, _) - | PathResolutionItem::UnexpectedTarget(_) => { + | PathResolutionItem::TraitFunction(_, _, _) => { // This variable refers to an existing item if let Some(name) = name { // If name is set, shadow the existing item diff --git a/compiler/noirc_frontend/src/elaborator/path_resolution.rs b/compiler/noirc_frontend/src/elaborator/path_resolution.rs index 6c68268d88e..0b0f2733f2a 100644 --- a/compiler/noirc_frontend/src/elaborator/path_resolution.rs +++ b/compiler/noirc_frontend/src/elaborator/path_resolution.rs @@ -39,10 +39,6 @@ pub(crate) enum PathResolutionItem { SelfMethod(FuncId), TypeAliasFunction(TypeAliasId, Option, FuncId), TraitFunction(TraitId, Option, FuncId), - - /// This is returned when the path points to something but it's - /// not in the namespace we were looking for. - UnexpectedTarget(&'static str), } impl PathResolutionItem { @@ -57,8 +53,7 @@ impl PathResolutionItem { | Self::Type(_) | Self::TypeAlias(_) | Self::Trait(_) - | Self::Global(..) - | Self::UnexpectedTarget(_) => None, + | Self::Global(..) => None, } } @@ -74,7 +69,6 @@ impl PathResolutionItem { | Self::SelfMethod(..) | Self::TypeAliasFunction(..) | Self::TraitFunction(..) => "function", - Self::UnexpectedTarget(description) => description, } } } @@ -86,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, @@ -149,9 +143,8 @@ pub(super) enum PathResolutionMode { /// 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, if a target wasn't find but instead -/// something with that name was found on the other namespace, [`PathResolutionItem::UnexpectedTarget`] -/// will be returned. +/// 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, @@ -446,16 +439,26 @@ impl Elaborator<'_> { self.per_ns_item_to_path_resolution_item( path.clone(), importing_module, - intermediate_item, + intermediate_item.clone(), current_module_id, &mut errors, module_def_id, visibility, + true, ) }) .unwrap_or_else(|| { - PathResolutionItem::UnexpectedTarget( - fallback_ns.expect("A namespace should never be empty").0.as_str(), + 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, + false, ) }); @@ -472,6 +475,7 @@ impl Elaborator<'_> { errors: &mut Vec, module_def_id: ModuleDefId, visibility: crate::ast::ItemVisibility, + error_on_private: bool, ) -> PathResolutionItem { let name = path.last_ident(); let is_self_type = name.is_self_type_name(); @@ -483,13 +487,14 @@ impl Elaborator<'_> { module_def_id, ); - if !(self.self_type_module_id() == Some(current_module_id) - || item_in_module_is_visible( - self.def_maps, - importing_module, - current_module_id, - visibility, - )) + if error_on_private + && (!(self.self_type_module_id() == Some(current_module_id) + || item_in_module_is_visible( + self.def_maps, + importing_module, + current_module_id, + visibility, + ))) { errors.push(PathResolutionError::Private(name.clone())); } diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 9682014941e..bf88eb83c72 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -685,8 +685,7 @@ impl Elaborator<'_> { | PathResolutionItem::TypeAlias(_) | PathResolutionItem::Trait(_) | PathResolutionItem::Global(..) - | PathResolutionItem::ModuleFunction(..) - | PathResolutionItem::UnexpectedTarget(_) => Vec::new(), + | PathResolutionItem::ModuleFunction(..) => Vec::new(), } } From fce5f75d2011677129ebaa83759de65d225ee10e Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 30 Apr 2025 08:59:27 -0300 Subject: [PATCH 12/16] Simplify something --- compiler/noirc_frontend/src/elaborator/mod.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 58a7b1abb28..d71523e5f75 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -867,16 +867,12 @@ impl<'context> Elaborator<'context> { ) -> Vec { let mut added_generics = Vec::new(); - let Ok(item) = + let Ok(PathResolutionItem::Trait(trait_id)) = self.resolve_path_or_error(bound.trait_path.clone(), PathResolutionTarget::Type) else { return Vec::new(); }; - let PathResolutionItem::Trait(trait_id) = item else { - return Vec::new(); - }; - let the_trait = self.get_trait_mut(trait_id); if the_trait.associated_types.len() > bound.trait_generics.named_args.len() { From 2d11eef363bd18ea727d57a854122f696e4fa3e0 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 30 Apr 2025 09:08:12 -0300 Subject: [PATCH 13/16] Shorten diff a bit more --- .../src/elaborator/path_resolution.rs | 40 +++++++++---------- .../noirc_frontend/src/elaborator/patterns.rs | 8 ++-- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/path_resolution.rs b/compiler/noirc_frontend/src/elaborator/path_resolution.rs index 0b0f2733f2a..c056c894100 100644 --- a/compiler/noirc_frontend/src/elaborator/path_resolution.rs +++ b/compiler/noirc_frontend/src/elaborator/path_resolution.rs @@ -44,31 +44,31 @@ pub(crate) enum PathResolutionItem { impl PathResolutionItem { pub(crate) fn function_id(&self) -> Option { match self { - Self::ModuleFunction(func_id) - | Self::Method(_, _, func_id) - | Self::SelfMethod(func_id) - | Self::TypeAliasFunction(_, _, func_id) - | Self::TraitFunction(_, _, func_id) => Some(*func_id), - Self::Module(_) - | Self::Type(_) - | Self::TypeAlias(_) - | Self::Trait(_) - | Self::Global(..) => None, + PathResolutionItem::ModuleFunction(func_id) + | PathResolutionItem::Method(_, _, func_id) + | PathResolutionItem::SelfMethod(func_id) + | PathResolutionItem::TypeAliasFunction(_, _, func_id) + | PathResolutionItem::TraitFunction(_, _, func_id) => Some(*func_id), + PathResolutionItem::Module(..) + | PathResolutionItem::Type(..) + | PathResolutionItem::TypeAlias(..) + | PathResolutionItem::Trait(..) + | PathResolutionItem::Global(..) => None, } } pub(crate) fn description(&self) -> &'static str { match self { - Self::Module(..) => "module", - Self::Type(..) => "type", - Self::TypeAlias(..) => "type alias", - Self::Trait(..) => "trait", - Self::Global(..) => "global", - Self::ModuleFunction(..) - | Self::Method(..) - | Self::SelfMethod(..) - | Self::TypeAliasFunction(..) - | Self::TraitFunction(..) => "function", + PathResolutionItem::Module(..) => "module", + PathResolutionItem::Type(..) => "type", + PathResolutionItem::TypeAlias(..) => "type alias", + PathResolutionItem::Trait(..) => "trait", + PathResolutionItem::Global(..) => "global", + PathResolutionItem::ModuleFunction(..) + | PathResolutionItem::Method(..) + | PathResolutionItem::SelfMethod(..) + | PathResolutionItem::TypeAliasFunction(..) + | PathResolutionItem::TraitFunction(..) => "function", } } } diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index bf88eb83c72..181f787ce08 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -680,10 +680,10 @@ impl Elaborator<'_> { } PathResolutionItem::Method(_, None, _) | PathResolutionItem::TraitFunction(_, None, _) - | PathResolutionItem::Module(_) - | PathResolutionItem::Type(_) - | PathResolutionItem::TypeAlias(_) - | PathResolutionItem::Trait(_) + | PathResolutionItem::Module(..) + | PathResolutionItem::Type(..) + | PathResolutionItem::TypeAlias(..) + | PathResolutionItem::Trait(..) | PathResolutionItem::Global(..) | PathResolutionItem::ModuleFunction(..) => Vec::new(), } From 12810038b6e56ab55fd1f5df3345233cdaa5c3d6 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 30 Apr 2025 09:39:13 -0300 Subject: [PATCH 14/16] Fix test --- compiler/noirc_frontend/src/tests.rs | 3 +++ .../src/main.nr | 1 + .../src_hash.txt | 2 +- .../stderr.txt | 15 +++++++++++++++ 4 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 test_programs/compile_failure/noirc_frontend_tests_only_one_private_error_when_name_in_types_and_values_namespace_collides/stderr.txt diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 77a29e945be..b0359735545 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -4706,6 +4706,9 @@ fn only_one_private_error_when_name_in_types_and_values_namespace_collides() { 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/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 index d6f913a2205..9385de2ffac 100644 --- 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 @@ -7,5 +7,6 @@ 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 index baa6d54567e..3fa26b26993 100644 --- 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 @@ -1 +1 @@ -9540133685024384324 \ No newline at end of file +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 From bb0754fb463307e1d9dc866a65f388f80e76c364 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 30 Apr 2025 09:39:18 -0300 Subject: [PATCH 15/16] Always error if private, even if it's not in the desired target --- .../src/elaborator/path_resolution.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/path_resolution.rs b/compiler/noirc_frontend/src/elaborator/path_resolution.rs index c056c894100..25f7f63279e 100644 --- a/compiler/noirc_frontend/src/elaborator/path_resolution.rs +++ b/compiler/noirc_frontend/src/elaborator/path_resolution.rs @@ -444,7 +444,6 @@ impl Elaborator<'_> { &mut errors, module_def_id, visibility, - true, ) }) .unwrap_or_else(|| { @@ -458,7 +457,6 @@ impl Elaborator<'_> { &mut errors, module_def_id, visibility, - false, ) }); @@ -475,7 +473,6 @@ impl Elaborator<'_> { errors: &mut Vec, module_def_id: ModuleDefId, visibility: crate::ast::ItemVisibility, - error_on_private: bool, ) -> PathResolutionItem { let name = path.last_ident(); let is_self_type = name.is_self_type_name(); @@ -487,14 +484,13 @@ impl Elaborator<'_> { module_def_id, ); - if error_on_private - && (!(self.self_type_module_id() == Some(current_module_id) - || item_in_module_is_visible( - self.def_maps, - importing_module, - current_module_id, - visibility, - ))) + if !(self.self_type_module_id() == Some(current_module_id) + || item_in_module_is_visible( + self.def_maps, + importing_module, + current_module_id, + visibility, + )) { errors.push(PathResolutionError::Private(name.clone())); } From 107fa059caff6a9b16d794db75c030a01eb47937 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 7 May 2025 14:33:44 -0300 Subject: [PATCH 16/16] Snapshot --- .../execute__tests__expanded.snap | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 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 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); +}