diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index c0a91cca4d7..6dfd295db6f 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -251,7 +251,7 @@ impl<'context> Elaborator<'context> { let hir_ident = if let Some((old_value, _)) = variable { old_value.num_times_used += 1; old_value.ident.clone() - } else if let Ok(definition_id) = + } else if let Ok((definition_id, _)) = self.lookup_global(Path::from_single(ident_name.to_string(), call_expr_span)) { HirIdent::non_trait_method(definition_id, Location::new(call_expr_span, self.file)) @@ -536,9 +536,6 @@ impl<'context> Elaborator<'context> { last_segment.generics = Some(generics.ordered_args); } - let exclude_last_segment = true; - self.check_unsupported_turbofish_usage(&path, exclude_last_segment); - let last_segment = path.last_segment(); let is_self_type = last_segment.ident.is_self_type_name(); diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 9f56b72f4e9..2a723286d8b 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -4,7 +4,8 @@ use std::{ }; use crate::{ - ast::ItemVisibility, hir_def::traits::ResolvedTraitBound, StructField, StructType, TypeBindings, + ast::ItemVisibility, hir::resolution::import::PathResolutionItem, + hir_def::traits::ResolvedTraitBound, StructField, StructType, TypeBindings, }; use crate::{ ast::{ @@ -20,7 +21,7 @@ use crate::{ }, def_collector::{dc_crate::CollectedItems, errors::DefCollectorErrorKind}, def_map::{DefMaps, ModuleData}, - def_map::{LocalModuleId, ModuleDefId, ModuleId, MAIN_FUNCTION}, + def_map::{LocalModuleId, ModuleId, MAIN_FUNCTION}, resolution::errors::ResolverError, resolution::import::PathResolution, scope::ScopeForest as GenericScopeForest, @@ -667,11 +668,11 @@ impl<'context> Elaborator<'context> { pub fn resolve_module_by_path(&mut self, path: Path) -> Option { match self.resolve_path(path.clone()) { - Ok(PathResolution { module_def_id: ModuleDefId::ModuleId(module_id), error }) => { - if error.is_some() { - None - } else { + Ok(PathResolution { item: PathResolutionItem::Module(module_id), errors }) => { + if errors.is_empty() { Some(module_id) + } else { + None } } _ => None, @@ -680,8 +681,8 @@ impl<'context> Elaborator<'context> { fn resolve_trait_by_path(&mut self, path: Path) -> Option { let error = match self.resolve_path(path.clone()) { - Ok(PathResolution { module_def_id: ModuleDefId::TraitId(trait_id), error }) => { - if let Some(error) = error { + Ok(PathResolution { item: PathResolutionItem::Trait(trait_id), errors }) => { + for error in errors { self.push_err(error); } return Some(trait_id); diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index d55011f98a1..b45f455633b 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -9,7 +9,7 @@ use crate::{ }, hir::{ def_collector::dc_crate::CompilationError, - resolution::errors::ResolverError, + resolution::{errors::ResolverError, import::PathResolutionItem}, type_check::{Source, TypeCheckError}, }, hir_def::{ @@ -178,9 +178,6 @@ impl<'context> Elaborator<'context> { mutable: Option, new_definitions: &mut Vec, ) -> HirPattern { - let exclude_last_segment = true; - self.check_unsupported_turbofish_usage(&name, exclude_last_segment); - let last_segment = name.last_segment(); let name_span = last_segment.ident.span(); let is_self_type = last_segment.ident.is_self_type_name(); @@ -195,7 +192,7 @@ impl<'context> Elaborator<'context> { }; let (struct_type, generics) = match self.lookup_type_or_error(name) { - Some(Type::Struct(struct_type, generics)) => (struct_type, generics), + Some(Type::Struct(struct_type, struct_generics)) => (struct_type, struct_generics), None => return error_identifier(self), Some(typ) => { let typ = typ.to_string(); @@ -468,54 +465,102 @@ impl<'context> Elaborator<'context> { } pub(super) fn elaborate_variable(&mut self, variable: Path) -> (ExprId, Type) { - let exclude_last_segment = true; - self.check_unsupported_turbofish_usage(&variable, exclude_last_segment); - let unresolved_turbofish = variable.segments.last().unwrap().generics.clone(); let span = variable.span; - let expr = self.resolve_variable(variable); + let (expr, item) = self.resolve_variable(variable); let definition_id = expr.id; + let type_generics = item.map(|item| self.resolve_item_turbofish(item)).unwrap_or_default(); + let definition_kind = self.interner.try_definition(definition_id).map(|definition| definition.kind.clone()); + let mut bindings = TypeBindings::new(); + // Resolve any generics if we the variable we have resolved is a function // and if the turbofish operator was used. - let generics = definition_kind.and_then(|definition_kind| match &definition_kind { - DefinitionKind::Function(function) => { - self.resolve_function_turbofish_generics(function, unresolved_turbofish, span) + let generics = if let Some(DefinitionKind::Function(func_id)) = &definition_kind { + self.resolve_function_turbofish_generics(func_id, unresolved_turbofish, span) + } else { + None + }; + + // If this is a function call on a type that has generics, we need to bind those generic types. + if !type_generics.is_empty() { + if let Some(DefinitionKind::Function(func_id)) = &definition_kind { + // `all_generics` will always have the enclosing type generics first, so we need to bind those + let func_generics = &self.interner.function_meta(func_id).all_generics; + for (type_generic, func_generic) in type_generics.into_iter().zip(func_generics) { + let type_var = &func_generic.type_var; + bindings + .insert(type_var.id(), (type_var.clone(), type_var.kind(), type_generic)); + } } - _ => None, - }); + } let id = self.interner.push_expr(HirExpression::Ident(expr.clone(), generics.clone())); self.interner.push_expr_location(id, span, self.file); - let typ = self.type_check_variable(expr, id, generics); + let typ = self.type_check_variable_with_bindings(expr, id, generics, bindings); self.interner.push_expr_type(id, typ.clone()); (id, typ) } - fn resolve_variable(&mut self, path: Path) -> HirIdent { + /// Solve any generics that are part of the path before the function, for example: + /// + /// foo::Bar::::baz + /// ^^^^^ + /// solve these + fn resolve_item_turbofish(&mut self, item: PathResolutionItem) -> Vec { + match item { + PathResolutionItem::StructFunction(struct_id, Some(generics), _func_id) => { + let struct_type = self.interner.get_struct(struct_id); + let struct_type = struct_type.borrow(); + let struct_generics = struct_type.instantiate(self.interner); + self.resolve_struct_turbofish_generics( + &struct_type, + struct_generics, + Some(generics.generics), + generics.span, + ) + } + PathResolutionItem::TypeAliasFunction(_type_alias_id, Some(generics), _func_id) => { + // TODO: https://github.com/noir-lang/noir/issues/6311 + self.push_err(TypeCheckError::UnsupportedTurbofishUsage { span: generics.span }); + Vec::new() + } + PathResolutionItem::TraitFunction(_trait_id, Some(generics), _func_id) => { + // TODO: https://github.com/noir-lang/noir/issues/6310 + self.push_err(TypeCheckError::UnsupportedTurbofishUsage { span: generics.span }); + Vec::new() + } + _ => Vec::new(), + } + } + + fn resolve_variable(&mut self, path: Path) -> (HirIdent, Option) { if let Some(trait_path_resolution) = self.resolve_trait_generic_path(&path) { - if let Some(error) = trait_path_resolution.error { + for error in trait_path_resolution.errors { self.push_err(error); } - HirIdent { - location: Location::new(path.span, self.file), - id: self.interner.trait_method_id(trait_path_resolution.method.method_id), - impl_kind: ImplKind::TraitMethod(trait_path_resolution.method), - } + ( + HirIdent { + location: Location::new(path.span, self.file), + id: self.interner.trait_method_id(trait_path_resolution.method.method_id), + impl_kind: ImplKind::TraitMethod(trait_path_resolution.method), + }, + None, + ) } else { // If the Path is being used as an Expression, then it is referring to a global from a separate module // Otherwise, then it is referring to an Identifier // This lookup allows support of such statements: let x = foo::bar::SOME_GLOBAL + 10; // If the expression is a singular indent, we search the resolver's current scope as normal. let span = path.span(); - let (hir_ident, var_scope_index) = self.get_ident_from_path(path); + let ((hir_ident, var_scope_index), item) = self.get_ident_from_path(path); if hir_ident.id != DefinitionId::dummy_id() { match self.interner.definition(hir_ident.id).kind { @@ -557,7 +602,7 @@ impl<'context> Elaborator<'context> { } } - hir_ident + (hir_ident, item) } } @@ -567,8 +612,17 @@ impl<'context> Elaborator<'context> { expr_id: ExprId, generics: Option>, ) -> Type { - let mut bindings = TypeBindings::new(); + let bindings = TypeBindings::new(); + self.type_check_variable_with_bindings(ident, expr_id, generics, bindings) + } + pub(super) fn type_check_variable_with_bindings( + &mut self, + ident: HirIdent, + expr_id: ExprId, + generics: Option>, + mut bindings: TypeBindings, + ) -> Type { // Add type bindings from any constraints that were used. // We need to do this first since otherwise instantiating the type below // will replace each trait generic with a fresh type variable, rather than @@ -668,24 +722,31 @@ impl<'context> Elaborator<'context> { } } - pub fn get_ident_from_path(&mut self, path: Path) -> (HirIdent, usize) { + pub fn get_ident_from_path( + &mut self, + path: Path, + ) -> ((HirIdent, usize), Option) { let location = Location::new(path.last_ident().span(), self.file); let error = match path.as_ident().map(|ident| self.use_variable(ident)) { - Some(Ok(found)) => return found, + Some(Ok(found)) => return (found, None), // Try to look it up as a global, but still issue the first error if we fail Some(Err(error)) => match self.lookup_global(path) { - Ok(id) => return (HirIdent::non_trait_method(id, location), 0), + Ok((id, item)) => { + return ((HirIdent::non_trait_method(id, location), 0), Some(item)) + } Err(_) => error, }, None => match self.lookup_global(path) { - Ok(id) => return (HirIdent::non_trait_method(id, location), 0), + Ok((id, item)) => { + return ((HirIdent::non_trait_method(id, location), 0), Some(item)) + } Err(error) => error, }, }; self.push_err(error); let id = DefinitionId::dummy_id(); - (HirIdent::non_trait_method(id, location), 0) + ((HirIdent::non_trait_method(id, location), 0), None) } pub(super) fn elaborate_type_path(&mut self, path: TypePath) -> (ExprId, Type) { diff --git a/compiler/noirc_frontend/src/elaborator/scope.rs b/compiler/noirc_frontend/src/elaborator/scope.rs index d38b7a50175..8e033c914be 100644 --- a/compiler/noirc_frontend/src/elaborator/scope.rs +++ b/compiler/noirc_frontend/src/elaborator/scope.rs @@ -2,14 +2,11 @@ use noirc_errors::{Location, Spanned}; use crate::ast::{Ident, Path, PathKind, ERROR_IDENT}; use crate::hir::def_map::{LocalModuleId, ModuleId}; -use crate::hir::resolution::import::{PathResolution, PathResolutionResult}; +use crate::hir::resolution::import::{PathResolution, PathResolutionItem, PathResolutionResult}; use crate::hir::resolution::path_resolver::{PathResolver, StandardPathResolver}; use crate::hir::scope::{Scope as GenericScope, ScopeTree as GenericScopeTree}; use crate::{ - hir::{ - def_map::{ModuleDefId, TryFromModuleDefId}, - resolution::errors::ResolverError, - }, + hir::resolution::errors::ResolverError, hir_def::{ expr::{HirCapturedVar, HirIdent}, traits::Trait, @@ -26,16 +23,6 @@ type Scope = GenericScope; type ScopeTree = GenericScopeTree; impl<'context> Elaborator<'context> { - pub(super) fn lookup(&mut self, path: Path) -> Result { - let span = path.span(); - let id = self.resolve_path_or_error(path)?; - T::try_from(id).ok_or_else(|| ResolverError::Expected { - expected: T::description(), - got: id.as_str().to_owned(), - span, - }) - } - pub fn module_id(&self) -> ModuleId { assert_ne!(self.local_module, LocalModuleId::dummy_id(), "local_module is unset"); ModuleId { krate: self.crate_id, local_id: self.local_module } @@ -53,14 +40,14 @@ impl<'context> Elaborator<'context> { pub(super) fn resolve_path_or_error( &mut self, path: Path, - ) -> Result { + ) -> Result { let path_resolution = self.resolve_path(path)?; - if let Some(error) = path_resolution.error { + for error in path_resolution.errors { self.push_err(error); } - Ok(path_resolution.module_def_id) + Ok(path_resolution.item) } pub(super) fn resolve_path(&mut self, path: Path) -> PathResolutionResult { @@ -72,8 +59,8 @@ impl<'context> Elaborator<'context> { let struct_type = struct_type.borrow(); if path.segments.len() == 1 { return Ok(PathResolution { - module_def_id: ModuleDefId::TypeId(struct_type.id), - error: None, + item: PathResolutionItem::Struct(struct_type.id), + errors: Vec::new(), }); } @@ -132,8 +119,8 @@ impl<'context> Elaborator<'context> { Err(err) => return Err(err), }; - self.interner.add_module_def_id_reference( - path_resolution.module_def_id, + self.interner.add_path_resolution_kind_reference( + path_resolution.item.clone(), location, is_self_type_name, ); @@ -183,21 +170,24 @@ impl<'context> Elaborator<'context> { } } - pub(super) fn lookup_global(&mut self, path: Path) -> Result { + pub(super) fn lookup_global( + &mut self, + path: Path, + ) -> Result<(DefinitionId, PathResolutionItem), ResolverError> { let span = path.span(); - let id = self.resolve_path_or_error(path)?; + let item = self.resolve_path_or_error(path)?; - if let Some(function) = TryFromModuleDefId::try_from(id) { - return Ok(self.interner.function_definition_id(function)); + if let Some(function) = item.function_id() { + return Ok((self.interner.function_definition_id(function), item)); } - if let Some(global) = TryFromModuleDefId::try_from(id) { + if let PathResolutionItem::Global(global) = item { let global = self.interner.get_global(global); - return Ok(global.definition_id); + return Ok((global.definition_id, item)); } - let expected = "global variable".into(); - let got = "local variable".into(); + let expected = "global variable"; + let got = "local variable"; Err(ResolverError::Expected { span, expected, got }) } @@ -239,10 +229,22 @@ impl<'context> Elaborator<'context> { /// Lookup a given trait by name/path. pub fn lookup_trait_or_error(&mut self, path: Path) -> Option<&mut Trait> { - match self.lookup(path) { - Ok(trait_id) => Some(self.get_trait_mut(trait_id)), - Err(error) => { - self.push_err(error); + let span = path.span(); + match self.resolve_path_or_error(path) { + Ok(item) => { + if let PathResolutionItem::Trait(trait_id) = item { + Some(self.get_trait_mut(trait_id)) + } else { + self.push_err(ResolverError::Expected { + expected: "trait", + got: item.description(), + span, + }); + None + } + } + Err(err) => { + self.push_err(err); None } } @@ -250,10 +252,22 @@ impl<'context> Elaborator<'context> { /// Lookup a given struct type by name. pub fn lookup_struct_or_error(&mut self, path: Path) -> Option> { - match self.lookup(path) { - Ok(struct_id) => Some(self.get_struct(struct_id)), - Err(error) => { - self.push_err(error); + let span = path.span(); + match self.resolve_path_or_error(path) { + Ok(item) => { + if let PathResolutionItem::Struct(struct_id) = item { + Some(self.get_struct(struct_id)) + } else { + self.push_err(ResolverError::Expected { + expected: "type", + got: item.description(), + span, + }); + None + } + } + Err(err) => { + self.push_err(err); None } } @@ -271,20 +285,20 @@ impl<'context> Elaborator<'context> { let span = path.span; match self.resolve_path_or_error(path) { - Ok(ModuleDefId::TypeId(struct_id)) => { + Ok(PathResolutionItem::Struct(struct_id)) => { let struct_type = self.get_struct(struct_id); let generics = struct_type.borrow().instantiate(self.interner); Some(Type::Struct(struct_type, generics)) } - Ok(ModuleDefId::TypeAliasId(alias_id)) => { + 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: StructId::description(), - got: other.as_str().to_owned(), + expected: "type", + got: other.description(), span, }); None @@ -297,6 +311,11 @@ impl<'context> Elaborator<'context> { } pub fn lookup_type_alias(&mut self, path: Path) -> Option> { - self.lookup(path).ok().map(|id| self.interner.get_type_alias(id)) + match self.resolve_path_or_error(path) { + Ok(PathResolutionItem::TypeAlias(type_alias_id)) => { + Some(self.interner.get_type_alias(type_alias_id)) + } + _ => None, + } } } diff --git a/compiler/noirc_frontend/src/elaborator/statements.rs b/compiler/noirc_frontend/src/elaborator/statements.rs index 238160e5aa4..757def16a93 100644 --- a/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/compiler/noirc_frontend/src/elaborator/statements.rs @@ -293,7 +293,8 @@ impl<'context> Elaborator<'context> { let mut mutable = true; let span = ident.span(); let path = Path::from_single(ident.0.contents, span); - let (ident, scope_index) = self.get_ident_from_path(path); + let ((ident, scope_index), _) = self.get_ident_from_path(path); + self.resolve_local_variable(ident.clone(), scope_index); let typ = if ident.id == DefinitionId::dummy_id() { diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 8ffbd15bdab..2879204d3ee 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -14,8 +14,10 @@ use crate::{ hir::{ comptime::{Interpreter, Value}, def_collector::dc_crate::CompilationError, - def_map::ModuleDefId, - resolution::{errors::ResolverError, import::PathResolutionError}, + resolution::{ + errors::ResolverError, + import::{PathResolutionError, PathResolutionItem}, + }, type_check::{ generics::{Generic, TraitGenerics}, NoMatchingImplFoundError, Source, TypeCheckError, @@ -46,7 +48,7 @@ pub const WILDCARD_TYPE: &str = "_"; pub(super) struct TraitPathResolution { pub(super) method: TraitMethod, - pub(super) error: Option, + pub(super) errors: Vec, } impl<'context> Elaborator<'context> { @@ -404,7 +406,7 @@ impl<'context> Elaborator<'context> { // If we cannot find a local generic of the same name, try to look up a global match self.resolve_path_or_error(path.clone()) { - Ok(ModuleDefId::GlobalId(id)) => { + Ok(PathResolutionItem::Global(id)) => { if let Some(current_item) = self.current_item { self.interner.add_global_dependency(current_item, id); } @@ -551,7 +553,7 @@ impl<'context> Elaborator<'context> { let constraint = the_trait.as_constraint(path.span); return Some(TraitPathResolution { method: TraitMethod { method_id: method, constraint, assumed: true }, - error: None, + errors: Vec::new(), }); } } @@ -564,15 +566,14 @@ impl<'context> Elaborator<'context> { // 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.resolve_path(path.clone()).ok()?; - let ModuleDefId::FunctionId(func_id) = path_resolution.module_def_id else { return None }; - + let func_id = path_resolution.item.function_id()?; let meta = self.interner.function_meta(&func_id); let the_trait = self.interner.get_trait(meta.trait_id?); let method = the_trait.find_method(path.last_name())?; let constraint = the_trait.as_constraint(path.span); Some(TraitPathResolution { method: TraitMethod { method_id: method, constraint, assumed: false }, - error: path_resolution.error, + errors: path_resolution.errors, }) } @@ -600,7 +601,7 @@ impl<'context> Elaborator<'context> { if let Some(method) = the_trait.find_method(path.last_name()) { return Some(TraitPathResolution { method: TraitMethod { method_id: method, constraint, assumed: true }, - error: None, + errors: Vec::new(), }); } } @@ -1826,19 +1827,6 @@ impl<'context> Elaborator<'context> { context.trait_constraints.push((constraint, expr_id)); } - pub fn check_unsupported_turbofish_usage(&mut self, path: &Path, exclude_last_segment: bool) { - for (index, segment) in path.segments.iter().enumerate() { - if exclude_last_segment && index == path.segments.len() - 1 { - continue; - } - - if segment.generics.is_some() { - let span = segment.turbofish_span(); - self.push_err(TypeCheckError::UnsupportedTurbofishUsage { span }); - } - } - } - pub fn bind_generics_from_trait_constraint( &self, constraint: &TraitConstraint, diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 658812be324..20c162fbd3a 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -385,9 +385,8 @@ impl DefCollector { let current_def_map = context.def_maps.get_mut(&crate_id).unwrap(); let file_id = current_def_map.file_id(module_id); - let has_path_resolution_error = resolved_import.error.is_some(); - - if let Some(error) = resolved_import.error { + let has_path_resolution_error = !resolved_import.errors.is_empty(); + for error in resolved_import.errors { errors.push(( DefCollectorErrorKind::PathResolutionError(error).into(), file_id, @@ -557,7 +556,7 @@ fn inject_prelude( span: Span::default(), }; - if let Ok(PathResolution { module_def_id, error }) = path_resolver::resolve_path( + if let Ok(PathResolution { item, errors }) = path_resolver::resolve_path( &context.def_maps, ModuleId { krate: crate_id, local_id: crate_root }, None, @@ -565,8 +564,8 @@ fn inject_prelude( &mut context.def_interner.usage_tracker, &mut None, ) { - assert!(error.is_none(), "Tried to add private item to prelude"); - let module_id = module_def_id.as_module().expect("std::prelude should be a module"); + assert!(errors.is_empty(), "Tried to add private item to prelude"); + let module_id = item.module_id().expect("std::prelude should be a module"); let prelude = context.module(module_id).scope().names(); for path in prelude { diff --git a/compiler/noirc_frontend/src/hir/def_map/module_def.rs b/compiler/noirc_frontend/src/hir/def_map/module_def.rs index a487bda81b3..a751eacd2dd 100644 --- a/compiler/noirc_frontend/src/hir/def_map/module_def.rs +++ b/compiler/noirc_frontend/src/hir/def_map/module_def.rs @@ -99,79 +99,3 @@ impl From for ModuleDefId { ModuleDefId::TraitId(trait_id) } } - -pub trait TryFromModuleDefId: Sized { - fn try_from(id: ModuleDefId) -> Option; - fn dummy_id() -> Self; - fn description() -> String; -} - -impl TryFromModuleDefId for FuncId { - fn try_from(id: ModuleDefId) -> Option { - id.as_function() - } - - fn dummy_id() -> Self { - FuncId::dummy_id() - } - - fn description() -> String { - "function".to_string() - } -} - -impl TryFromModuleDefId for StructId { - fn try_from(id: ModuleDefId) -> Option { - id.as_type() - } - - fn dummy_id() -> Self { - StructId::dummy_id() - } - - fn description() -> String { - "type".to_string() - } -} - -impl TryFromModuleDefId for TypeAliasId { - fn try_from(id: ModuleDefId) -> Option { - id.as_type_alias() - } - - fn dummy_id() -> Self { - TypeAliasId::dummy_id() - } - - fn description() -> String { - "type alias".to_string() - } -} - -impl TryFromModuleDefId for TraitId { - fn try_from(id: ModuleDefId) -> Option { - id.as_trait() - } - - fn dummy_id() -> Self { - TraitId::dummy_id() - } - - fn description() -> String { - "trait".to_string() - } -} - -impl TryFromModuleDefId for GlobalId { - fn try_from(id: ModuleDefId) -> Option { - id.as_global() - } - - fn dummy_id() -> Self { - GlobalId::dummy_id() - } - - fn description() -> String { - "global".to_string() - } -} diff --git a/compiler/noirc_frontend/src/hir/def_map/namespace.rs b/compiler/noirc_frontend/src/hir/def_map/namespace.rs index 6fac6d2b991..a600d98dd8b 100644 --- a/compiler/noirc_frontend/src/hir/def_map/namespace.rs +++ b/compiler/noirc_frontend/src/hir/def_map/namespace.rs @@ -13,14 +13,6 @@ impl PerNs { PerNs { types: Some((t, ItemVisibility::Public, false)), values: None } } - pub fn take_types(self) -> Option { - self.types.map(|it| it.0) - } - - pub fn take_values(self) -> Option { - self.values.map(|it| it.0) - } - pub fn iter_defs(self) -> impl Iterator { self.types.map(|it| it.0).into_iter().chain(self.values.map(|it| it.0)) } diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index 3c4022b58bb..e1e60daff60 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -38,7 +38,7 @@ pub enum ResolverError { #[error("could not resolve path")] PathResolutionError(#[from] PathResolutionError), #[error("Expected")] - Expected { span: Span, expected: String, got: String }, + Expected { span: Span, expected: &'static str, got: &'static str }, #[error("Duplicate field in constructor")] DuplicateField { field: Ident }, #[error("No such field in struct")] diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index 93039b1ea7f..58a3a841801 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -3,12 +3,13 @@ use thiserror::Error; use crate::graph::CrateId; use crate::hir::def_collector::dc_crate::CompilationError; -use crate::node_interner::ReferenceId; + +use crate::node_interner::{FuncId, GlobalId, ReferenceId, StructId, TraitId, TypeAliasId}; use crate::usage_tracker::UsageTracker; use std::collections::BTreeMap; -use crate::ast::{Ident, ItemVisibility, Path, PathKind, PathSegment}; +use crate::ast::{Ident, ItemVisibility, Path, PathKind, PathSegment, UnresolvedType}; use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleDefId, ModuleId, PerNs}; use super::errors::ResolverError; @@ -26,16 +27,80 @@ pub struct ImportDirective { struct NamespaceResolution { module_id: ModuleId, + item: PathResolutionItem, namespace: PerNs, - error: Option, + errors: Vec, } type NamespaceResolutionResult = Result; +#[derive(Debug)] pub struct PathResolution { - pub module_def_id: ModuleDefId, + pub item: PathResolutionItem, + pub errors: Vec, +} - pub error: Option, +/// All possible items that result from resolving a Path. +/// Note that this item doesn't include the last turbofish in a Path, +/// only intermediate ones, if any. +#[derive(Debug, Clone)] +pub enum PathResolutionItem { + Module(ModuleId), + Struct(StructId), + TypeAlias(TypeAliasId), + Trait(TraitId), + Global(GlobalId), + ModuleFunction(FuncId), + StructFunction(StructId, Option, FuncId), + TypeAliasFunction(TypeAliasId, Option, FuncId), + TraitFunction(TraitId, Option, FuncId), +} + +impl PathResolutionItem { + pub fn function_id(&self) -> Option { + match self { + PathResolutionItem::ModuleFunction(func_id) + | PathResolutionItem::StructFunction(_, _, func_id) + | PathResolutionItem::TypeAliasFunction(_, _, func_id) + | PathResolutionItem::TraitFunction(_, _, func_id) => Some(*func_id), + _ => None, + } + } + + pub fn module_id(&self) -> Option { + match self { + Self::Module(module_id) => Some(*module_id), + _ => None, + } + } + + pub fn description(&self) -> &'static str { + match self { + PathResolutionItem::Module(..) => "module", + PathResolutionItem::Struct(..) => "type", + PathResolutionItem::TypeAlias(..) => "type alias", + PathResolutionItem::Trait(..) => "trait", + PathResolutionItem::Global(..) => "global", + PathResolutionItem::ModuleFunction(..) + | PathResolutionItem::StructFunction(..) + | PathResolutionItem::TypeAliasFunction(..) + | PathResolutionItem::TraitFunction(..) => "function", + } + } +} + +#[derive(Debug, Clone)] +pub struct Turbofish { + pub generics: Vec, + pub span: Span, +} + +/// Any item that can appear before the last segment in a path. +#[derive(Debug)] +enum IntermediatePathResolutionItem { + Module(ModuleId), + Struct(StructId, Option), + Trait(TraitId, Option), } pub(crate) type PathResolutionResult = Result; @@ -48,6 +113,8 @@ pub enum PathResolutionError { Private(Ident), #[error("There is no super module")] NoSuper(Span), + #[error("turbofish (`::<_>`) not allowed on {item}")] + TurbofishNotAllowedOnItem { item: String, span: Span }, } #[derive(Debug)] @@ -56,10 +123,12 @@ pub struct ResolvedImport { pub name: Ident, // The symbol which we have resolved to pub resolved_namespace: PerNs, + // The item which we have resolved to + pub item: PathResolutionItem, // The module which we must add the resolved namespace to pub module_scope: LocalModuleId, pub is_prelude: bool, - pub error: Option, + pub errors: Vec, } impl From for CompilationError { @@ -83,6 +152,9 @@ impl<'a> From<&'a PathResolutionError> for CustomDiagnostic { PathResolutionError::NoSuper(span) => { CustomDiagnostic::simple_error(error.to_string(), String::new(), *span) } + PathResolutionError::TurbofishNotAllowedOnItem { item: _, span } => { + CustomDiagnostic::simple_error(error.to_string(), String::new(), *span) + } } } } @@ -95,15 +167,19 @@ pub fn resolve_import( path_references: &mut Option<&mut Vec>, ) -> Result { let module_scope = import_directive.module_id; - let NamespaceResolution { module_id: resolved_module, namespace: resolved_namespace, error } = - resolve_path_to_ns( - import_directive, - crate_id, - crate_id, - def_maps, - usage_tracker, - path_references, - )?; + let NamespaceResolution { + module_id: resolved_module, + item, + namespace: resolved_namespace, + mut errors, + } = resolve_path_to_ns( + import_directive, + crate_id, + crate_id, + def_maps, + usage_tracker, + path_references, + )?; let name = resolve_path_name(import_directive); @@ -113,28 +189,25 @@ pub fn resolve_import( .map(|(_, visibility, _)| visibility) .expect("Found empty namespace"); - let error = error.or_else(|| { - if import_directive.self_type_module_id == Some(resolved_module) - || can_reference_module_id( - def_maps, - crate_id, - import_directive.module_id, - resolved_module, - visibility, - ) - { - None - } else { - Some(PathResolutionError::Private(name.clone())) - } - }); + if !(import_directive.self_type_module_id == Some(resolved_module) + || can_reference_module_id( + def_maps, + crate_id, + import_directive.module_id, + resolved_module, + visibility, + )) + { + errors.push(PathResolutionError::Private(name.clone())); + } Ok(ResolvedImport { name, resolved_namespace, + item, module_scope, is_prelude: import_directive.is_prelude, - error, + errors, }) } @@ -275,13 +348,16 @@ fn resolve_name_in_module( let mut current_mod_id = ModuleId { krate, local_id: starting_mod }; let mut current_mod = &def_map.modules[current_mod_id.local_id.0]; + let mut intermediate_item = IntermediatePathResolutionItem::Module(current_mod_id); + // There is a possibility that the import path is empty // In that case, early return if import_path.is_empty() { return Ok(NamespaceResolution { module_id: current_mod_id, + item: PathResolutionItem::Module(current_mod_id), namespace: PerNs::types(current_mod_id.into()), - error: None, + errors: Vec::new(), }); } @@ -293,25 +369,34 @@ fn resolve_name_in_module( usage_tracker.mark_as_referenced(current_mod_id, first_segment); - let mut warning: Option = None; + let mut errors = Vec::new(); for (index, (last_segment, current_segment)) in import_path.iter().zip(import_path.iter().skip(1)).enumerate() { - let last_segment = &last_segment.ident; - let current_segment = ¤t_segment.ident; + let last_ident = &last_segment.ident; + let current_ident = ¤t_segment.ident; + let last_segment_generics = &last_segment.generics; let (typ, visibility) = match current_ns.types { - None => return Err(PathResolutionError::Unresolved(last_segment.clone())), + None => return Err(PathResolutionError::Unresolved(last_ident.clone())), Some((typ, visibility, _)) => (typ, visibility), }; // In the type namespace, only Mod can be used in a path. - current_mod_id = match typ { + (current_mod_id, intermediate_item) = match typ { ModuleDefId::ModuleId(id) => { if let Some(path_references) = path_references { path_references.push(ReferenceId::Module(id)); } - id + + if last_segment_generics.is_some() { + errors.push(PathResolutionError::TurbofishNotAllowedOnItem { + item: format!("module `{last_ident}`"), + span: last_segment.turbofish_span(), + }); + } + + (id, IntermediatePathResolutionItem::Module(id)) } ModuleDefId::FunctionId(_) => panic!("functions cannot be in the type namespace"), // TODO: If impls are ever implemented, types can be used in a path @@ -319,51 +404,75 @@ fn resolve_name_in_module( if let Some(path_references) = path_references { path_references.push(ReferenceId::Struct(id)); } - id.module_id() + + ( + id.module_id(), + IntermediatePathResolutionItem::Struct( + id, + last_segment_generics.as_ref().map(|generics| Turbofish { + generics: generics.clone(), + span: last_segment.turbofish_span(), + }), + ), + ) } ModuleDefId::TypeAliasId(_) => panic!("type aliases cannot be used in type namespace"), ModuleDefId::TraitId(id) => { if let Some(path_references) = path_references { path_references.push(ReferenceId::Trait(id)); } - id.0 + + ( + id.0, + IntermediatePathResolutionItem::Trait( + id, + last_segment_generics.as_ref().map(|generics| Turbofish { + generics: generics.clone(), + span: last_segment.turbofish_span(), + }), + ), + ) } ModuleDefId::GlobalId(_) => panic!("globals cannot be in the type namespace"), }; - warning = warning.or_else(|| { - // If the path is plain or crate, the first segment will always refer to - // something that's visible from the current module. - if (plain_or_crate && index == 0) - || can_reference_module_id( - def_maps, - importing_crate, - starting_mod, - current_mod_id, - visibility, - ) - { - None - } else { - Some(PathResolutionError::Private(last_segment.clone())) - } - }); + // If the path is plain or crate, the first segment will always refer to + // something that's visible from the current module. + if !((plain_or_crate && index == 0) + || can_reference_module_id( + def_maps, + importing_crate, + starting_mod, + current_mod_id, + visibility, + )) + { + errors.push(PathResolutionError::Private(last_ident.clone())); + } current_mod = &def_maps[¤t_mod_id.krate].modules[current_mod_id.local_id.0]; // Check if namespace - let found_ns = current_mod.find_name(current_segment); + let found_ns = current_mod.find_name(current_ident); if found_ns.is_none() { - return Err(PathResolutionError::Unresolved(current_segment.clone())); + return Err(PathResolutionError::Unresolved(current_ident.clone())); } - usage_tracker.mark_as_referenced(current_mod_id, current_segment); + usage_tracker.mark_as_referenced(current_mod_id, current_ident); current_ns = found_ns; } - Ok(NamespaceResolution { module_id: current_mod_id, namespace: current_ns, error: warning }) + let module_def_id = + current_ns.values.or(current_ns.types).map(|(id, _, _)| id).expect("Found empty namespace"); + + let item = merge_intermediate_path_resolution_item_with_module_def_id( + intermediate_item, + module_def_id, + ); + + Ok(NamespaceResolution { module_id: current_mod_id, item, namespace: current_ns, errors }) } fn resolve_path_name(import_directive: &ImportDirective) -> Ident { @@ -425,3 +534,27 @@ fn resolve_external_dep( path_references, ) } + +fn merge_intermediate_path_resolution_item_with_module_def_id( + intermediate_item: IntermediatePathResolutionItem, + module_def_id: ModuleDefId, +) -> PathResolutionItem { + match module_def_id { + ModuleDefId::ModuleId(module_id) => PathResolutionItem::Module(module_id), + ModuleDefId::TypeId(struct_id) => PathResolutionItem::Struct(struct_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::Module(_) => { + PathResolutionItem::ModuleFunction(func_id) + } + IntermediatePathResolutionItem::Struct(struct_id, generics) => { + PathResolutionItem::StructFunction(struct_id, generics, func_id) + } + IntermediatePathResolutionItem::Trait(trait_id, generics) => { + PathResolutionItem::TraitFunction(trait_id, generics, func_id) + } + }, + } +} diff --git a/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs b/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs index 562366fae77..705820e9101 100644 --- a/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs @@ -88,9 +88,5 @@ pub fn resolve_path( let resolved_import = resolve_import(module_id.krate, &import, def_maps, usage_tracker, path_references)?; - let namespace = resolved_import.resolved_namespace; - let id = - namespace.values.or(namespace.types).map(|(id, _, _)| id).expect("Found empty namespace"); - - Ok(PathResolution { module_def_id: id, error: resolved_import.error }) + Ok(PathResolution { item: resolved_import.item, errors: resolved_import.errors }) } diff --git a/compiler/noirc_frontend/src/locations.rs b/compiler/noirc_frontend/src/locations.rs index 48660142d0a..4dd699251a6 100644 --- a/compiler/noirc_frontend/src/locations.rs +++ b/compiler/noirc_frontend/src/locations.rs @@ -5,7 +5,10 @@ use rustc_hash::FxHashMap as HashMap; use crate::{ ast::{FunctionDefinition, ItemVisibility}, - hir::def_map::{ModuleDefId, ModuleId}, + hir::{ + def_map::{ModuleDefId, ModuleId}, + resolution::import::PathResolutionItem, + }, node_interner::{ DefinitionId, FuncId, GlobalId, NodeInterner, ReferenceId, StructId, TraitId, TypeAliasId, }, @@ -99,6 +102,37 @@ impl NodeInterner { }; } + pub(crate) fn add_path_resolution_kind_reference( + &mut self, + kind: PathResolutionItem, + location: Location, + is_self_type: bool, + ) { + match kind { + PathResolutionItem::Module(module_id) => { + self.add_module_reference(module_id, location); + } + PathResolutionItem::Struct(struct_id) => { + self.add_struct_reference(struct_id, location, is_self_type); + } + PathResolutionItem::TypeAlias(type_alias_id) => { + self.add_alias_reference(type_alias_id, location); + } + PathResolutionItem::Trait(trait_id) => { + self.add_trait_reference(trait_id, location, is_self_type); + } + PathResolutionItem::Global(global_id) => { + self.add_global_reference(global_id, location); + } + PathResolutionItem::ModuleFunction(func_id) + | PathResolutionItem::StructFunction(_, _, func_id) + | PathResolutionItem::TypeAliasFunction(_, _, func_id) + | PathResolutionItem::TraitFunction(_, _, func_id) => { + self.add_function_reference(func_id, location); + } + } + } + pub(crate) fn add_module_reference(&mut self, id: ModuleId, location: Location) { self.add_reference(ReferenceId::Module(id), location, false); } diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 56c4b5e9a12..5ce3ec6686c 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -617,8 +617,8 @@ fn check_trait_impl_for_non_type() { for (err, _file_id) in errors { match &err { CompilationError::ResolverError(ResolverError::Expected { expected, got, .. }) => { - assert_eq!(expected, "type"); - assert_eq!(got, "function"); + assert_eq!(*expected, "type"); + assert_eq!(*got, "function"); } _ => { panic!("No other errors are expected! Found = {:?}", err); diff --git a/compiler/noirc_frontend/src/tests/turbofish.rs b/compiler/noirc_frontend/src/tests/turbofish.rs index 71e63e878e8..3ded243a280 100644 --- a/compiler/noirc_frontend/src/tests/turbofish.rs +++ b/compiler/noirc_frontend/src/tests/turbofish.rs @@ -1,4 +1,8 @@ -use crate::hir::{def_collector::dc_crate::CompilationError, type_check::TypeCheckError}; +use crate::hir::{ + def_collector::dc_crate::CompilationError, + resolution::{errors::ResolverError, import::PathResolutionError}, + type_check::TypeCheckError, +}; use super::{assert_no_errors, get_program_errors}; @@ -100,32 +104,6 @@ fn turbofish_in_constructor() { assert_eq!(expr_typ, "Field"); } -#[test] -fn turbofish_in_middle_of_variable_unsupported_yet() { - let src = r#" - struct Foo { - x: T - } - - impl Foo { - pub fn new(x: T) -> Self { - Foo { x } - } - } - - fn main() { - let _ = Foo::::new(1); - } - "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - assert!(matches!( - errors[0].0, - CompilationError::TypeError(TypeCheckError::UnsupportedTurbofishUsage { .. }), - )); -} - #[test] fn turbofish_in_struct_pattern() { let src = r#" @@ -214,3 +192,80 @@ fn numeric_turbofish() { "#; assert_no_errors(src); } + +#[test] +fn errors_if_turbofish_after_module() { + let src = r#" + mod moo { + pub fn foo() {} + } + + fn main() { + moo::::foo(); + } + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::ResolverError(ResolverError::PathResolutionError( + PathResolutionError::TurbofishNotAllowedOnItem { item, .. }, + )) = &errors[0].0 + else { + panic!("Expected a turbofish not allowed on item error, got {:?}", errors[0].0); + }; + assert_eq!(item, "module `moo`"); +} + +#[test] +fn turbofish_in_type_before_call_does_not_error() { + let src = r#" + struct Foo { + x: T + } + + impl Foo { + fn new(x: T) -> Self { + Foo { x } + } + } + + fn main() { + let _ = Foo::::new(1); + } + "#; + assert_no_errors(src); +} + +#[test] +fn turbofish_in_type_before_call_errors() { + let src = r#" + struct Foo { + x: T + } + + impl Foo { + fn new(x: T) -> Self { + Foo { x } + } + } + + fn main() { + let _ = Foo::::new(true); + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::TypeError(TypeCheckError::TypeMismatch { + expected_typ, + expr_typ, + expr_span: _, + }) = &errors[0].0 + else { + panic!("Expected a type mismatch error, got {:?}", errors[0].0); + }; + + assert_eq!(expected_typ, "i32"); + assert_eq!(expr_typ, "bool"); +} diff --git a/tooling/lsp/src/attribute_reference_finder.rs b/tooling/lsp/src/attribute_reference_finder.rs index 39e1385a6e8..22afa086303 100644 --- a/tooling/lsp/src/attribute_reference_finder.rs +++ b/tooling/lsp/src/attribute_reference_finder.rs @@ -13,7 +13,10 @@ use noirc_frontend::{ graph::CrateId, hir::{ def_map::{CrateDefMap, LocalModuleId, ModuleId}, - resolution::path_resolver::{PathResolver, StandardPathResolver}, + resolution::{ + import::PathResolutionItem, + path_resolver::{PathResolver, StandardPathResolver}, + }, }, node_interner::ReferenceId, parser::{ParsedSubModule, Parser}, @@ -22,8 +25,6 @@ use noirc_frontend::{ ParsedModule, }; -use crate::modules::module_def_id_to_reference_id; - pub(crate) struct AttributeReferenceFinder<'a> { byte_index: usize, /// The module ID in scope. This might change as we traverse the AST @@ -106,6 +107,20 @@ impl<'a> Visitor for AttributeReferenceFinder<'a> { return; }; - self.reference_id = Some(module_def_id_to_reference_id(result.module_def_id)); + self.reference_id = Some(path_resolution_item_to_reference_id(result.item)); + } +} + +fn path_resolution_item_to_reference_id(item: PathResolutionItem) -> ReferenceId { + match item { + PathResolutionItem::Module(module_id) => ReferenceId::Module(module_id), + PathResolutionItem::Struct(struct_id) => ReferenceId::Struct(struct_id), + PathResolutionItem::TypeAlias(type_alias_id) => ReferenceId::Alias(type_alias_id), + PathResolutionItem::Trait(trait_id) => ReferenceId::Trait(trait_id), + PathResolutionItem::Global(global_id) => ReferenceId::Global(global_id), + PathResolutionItem::ModuleFunction(func_id) + | PathResolutionItem::StructFunction(_, _, func_id) + | PathResolutionItem::TypeAliasFunction(_, _, func_id) + | PathResolutionItem::TraitFunction(_, _, func_id) => ReferenceId::Function(func_id), } }