From 91eb3eb25c233d75bae5c25a794ec7c47fd72a58 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 21 Oct 2024 17:39:23 -0300 Subject: [PATCH 01/14] Try to progress a bit on generic types in the middle of a path --- .../src/elaborator/expressions.rs | 3 - compiler/noirc_frontend/src/elaborator/mod.rs | 20 ++- .../noirc_frontend/src/elaborator/patterns.rs | 61 ++++--- .../noirc_frontend/src/elaborator/scope.rs | 52 +++--- .../src/elaborator/statements.rs | 4 +- .../noirc_frontend/src/elaborator/types.rs | 23 +-- .../src/hir/def_collector/dc_crate.rs | 25 +-- .../src/hir/resolution/import.rs | 166 ++++++++++++------ .../src/hir/resolution/path_resolver.rs | 6 +- .../src/hir/type_check/errors.rs | 6 - .../noirc_frontend/src/tests/turbofish.rs | 97 +++++++--- 11 files changed, 288 insertions(+), 175 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index 31a518ca97f..6128fc6d57c 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -528,9 +528,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 aef0771c486..337b26fabd5 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -653,11 +653,15 @@ 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 { + module_def_id: ModuleDefId::ModuleId(module_id), + generic_type_in_path: _, + errors, + }) => { + if errors.is_empty() { Some(module_id) + } else { + None } } _ => None, @@ -666,8 +670,12 @@ 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 { + module_def_id: ModuleDefId::TraitId(trait_id), + generic_type_in_path: _, + 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..02608db2956 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::GenericTypeInPath}, 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,15 +465,20 @@ 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, generic_type_in_path) = self.resolve_variable(variable); let definition_id = expr.id; + // TODO: generic_type_in_path might be Some. + // In that case, the variable looks like this: + // + // foo::Bar::::baz + // + // That is, the type in the path before the function has some generics on it. + // How to handle this? + let definition_kind = self.interner.try_definition(definition_id).map(|definition| definition.kind.clone()); @@ -498,24 +500,30 @@ impl<'context> Elaborator<'context> { (id, typ) } - fn resolve_variable(&mut self, path: Path) -> HirIdent { + 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), - } + // TODO: GenericTypeInPath + let generic_type_in_path = None; + ( + 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), + }, + generic_type_in_path, + ) } 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), generic_type_in_path) = + self.get_ident_from_path(path); if hir_ident.id != DefinitionId::dummy_id() { match self.interner.definition(hir_ident.id).kind { @@ -557,7 +565,7 @@ impl<'context> Elaborator<'context> { } } - hir_ident + (hir_ident, generic_type_in_path) } } @@ -668,24 +676,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, generic_type_in_path)) => { + return ((HirIdent::non_trait_method(id, location), 0), generic_type_in_path) + } Err(_) => error, }, None => match self.lookup_global(path) { - Ok(id) => return (HirIdent::non_trait_method(id, location), 0), + Ok((id, generic_type_in_path)) => { + return ((HirIdent::non_trait_method(id, location), 0), generic_type_in_path) + } 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 b83a1494ab9..a0c2185905c 100644 --- a/compiler/noirc_frontend/src/elaborator/scope.rs +++ b/compiler/noirc_frontend/src/elaborator/scope.rs @@ -2,7 +2,7 @@ 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::{GenericTypeInPath, PathResolution, PathResolutionResult}; use crate::hir::resolution::path_resolver::{PathResolver, StandardPathResolver}; use crate::hir::scope::{Scope as GenericScope, ScopeTree as GenericScopeTree}; use crate::{ @@ -26,13 +26,18 @@ type Scope = GenericScope; type ScopeTree = GenericScopeTree; impl<'context> Elaborator<'context> { - pub(super) fn lookup(&mut self, path: Path) -> Result { + pub(super) fn lookup( + &mut self, + path: Path, + ) -> Result<(T, Option), ResolverError> { 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, + let (id, generic_type_in_path) = self.resolve_path_or_error(path)?; + T::try_from(id).map(|id| (id, generic_type_in_path)).ok_or_else(|| { + ResolverError::Expected { + expected: T::description(), + got: id.as_str().to_owned(), + span, + } }) } @@ -53,14 +58,14 @@ impl<'context> Elaborator<'context> { pub(super) fn resolve_path_or_error( &mut self, path: Path, - ) -> Result { + ) -> Result<(ModuleDefId, Option), ResolverError> { 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.module_def_id, path_resolution.generic_type_in_path)) } pub(super) fn resolve_path(&mut self, path: Path) -> PathResolutionResult { @@ -71,9 +76,11 @@ impl<'context> Elaborator<'context> { if let Some(Type::Struct(struct_type, _)) = &self.self_type { let struct_type = struct_type.borrow(); if path.segments.len() == 1 { + // TODO: handle generics return Ok(PathResolution { module_def_id: ModuleDefId::TypeId(struct_type.id), - error: None, + generic_type_in_path: None, + errors: Vec::new(), }); } @@ -183,17 +190,20 @@ 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, Option), ResolverError> { let span = path.span(); - let id = self.resolve_path_or_error(path)?; + let (id, generic_type_in_path) = self.resolve_path_or_error(path)?; if let Some(function) = TryFromModuleDefId::try_from(id) { - return Ok(self.interner.function_definition_id(function)); + return Ok((self.interner.function_definition_id(function), generic_type_in_path)); } if let Some(global) = TryFromModuleDefId::try_from(id) { let global = self.interner.get_global(global); - return Ok(global.definition_id); + return Ok((global.definition_id, generic_type_in_path)); } let expected = "global variable".into(); @@ -240,7 +250,7 @@ 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)), + Ok((trait_id, _generic_type_in_path)) => Some(self.get_trait_mut(trait_id)), Err(error) => { self.push_err(error); None @@ -251,7 +261,7 @@ 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)), + Ok((struct_id, _generic_type_in_path)) => Some(self.get_struct(struct_id)), Err(error) => { self.push_err(error); None @@ -270,10 +280,10 @@ impl<'context> Elaborator<'context> { } match self.lookup(path) { - Ok(struct_id) => { + Ok((struct_id, _generic_type_in_path)) => { let struct_type = self.get_struct(struct_id); - let generics = struct_type.borrow().instantiate(self.interner); - Some(Type::Struct(struct_type, generics)) + let struct_generics = struct_type.borrow().instantiate(self.interner); + Some(Type::Struct(struct_type, struct_generics)) } Err(error) => { self.push_err(error); @@ -283,6 +293,6 @@ 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)) + self.lookup(path).ok().map(|(id, _generic_type_in_path)| self.interner.get_type_alias(id)) } } diff --git a/compiler/noirc_frontend/src/elaborator/statements.rs b/compiler/noirc_frontend/src/elaborator/statements.rs index 238160e5aa4..1432f57a165 100644 --- a/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/compiler/noirc_frontend/src/elaborator/statements.rs @@ -293,9 +293,11 @@ 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), generic_type_in_path) = self.get_ident_from_path(path); self.resolve_local_variable(ident.clone(), scope_index); + // TODO: generic_type_in_path + let typ = if ident.id == DefinitionId::dummy_id() { Type::Error } else { diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 8ffbd15bdab..ad76228d931 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -46,7 +46,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 +404,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((ModuleDefId::GlobalId(id), _generic_type_in_path)) => { if let Some(current_item) = self.current_item { self.interner.add_global_dependency(current_item, id); } @@ -551,7 +551,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(), }); } } @@ -572,7 +572,7 @@ impl<'context> Elaborator<'context> { 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 +600,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 +1826,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 16fd43ba2a2..13496768b64 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -380,9 +380,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, @@ -552,15 +551,17 @@ fn inject_prelude( span: Span::default(), }; - if let Ok(PathResolution { module_def_id, error }) = path_resolver::resolve_path( - &context.def_maps, - ModuleId { krate: crate_id, local_id: crate_root }, - None, - path, - &mut context.def_interner.usage_tracker, - &mut None, - ) { - assert!(error.is_none(), "Tried to add private item to prelude"); + if let Ok(PathResolution { module_def_id, generic_type_in_path: _, errors }) = + path_resolver::resolve_path( + &context.def_maps, + ModuleId { krate: crate_id, local_id: crate_root }, + None, + path, + &mut context.def_interner.usage_tracker, + &mut None, + ) + { + assert!(errors.is_empty(), "Tried to add private item to prelude"); let module_id = module_def_id.as_module().expect("std::prelude should be a module"); let prelude = context.module(module_id).scope().names(); diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index 93039b1ea7f..269bee71ae3 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -3,17 +3,38 @@ use thiserror::Error; use crate::graph::CrateId; use crate::hir::def_collector::dc_crate::CompilationError; -use crate::node_interner::ReferenceId; + +use crate::node_interner::{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; use super::visibility::can_reference_module_id; +/// Represents a generic type in the before-last segment of a path, for example: +/// +/// foo::Bar::::baz +/// ^^^^^^^^^^ +/// +/// In the above example, if `Bar` is a struct, `GenericTypeInPath` would be a +/// `GenericTypeInPathKind::StructId(..)` with the generics being `[i32]`. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct GenericTypeInPath { + pub kind: GenericTypeInPathKind, + pub generics: Vec, +} + +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +pub enum GenericTypeInPathKind { + StructId(StructId), + TypeAliasId(TypeAliasId), + TraitId(TraitId), +} + #[derive(Debug, Clone)] pub struct ImportDirective { pub visibility: ItemVisibility, @@ -26,16 +47,17 @@ pub struct ImportDirective { struct NamespaceResolution { module_id: ModuleId, + generic_type_in_path: Option, namespace: PerNs, - error: Option, + errors: Vec, } type NamespaceResolutionResult = Result; pub struct PathResolution { pub module_def_id: ModuleDefId, - - pub error: Option, + pub generic_type_in_path: Option, + pub errors: Vec, } pub(crate) type PathResolutionResult = Result; @@ -48,6 +70,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 +80,11 @@ pub struct ResolvedImport { pub name: Ident, // The symbol which we have resolved to pub resolved_namespace: PerNs, + pub generic_type_in_path: Option, // 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 +108,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 +123,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, + generic_type_in_path, + 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 +145,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, + generic_type_in_path, module_scope, is_prelude: import_directive.is_prelude, - error, + errors, }) } @@ -275,13 +304,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 generic_type_in_path = None; + // 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, + generic_type_in_path, namespace: PerNs::types(current_mod_id.into()), - error: None, + errors: Vec::new(), }); } @@ -293,15 +325,16 @@ 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), }; @@ -311,6 +344,12 @@ fn resolve_name_in_module( if let Some(path_references) = path_references { path_references.push(ReferenceId::Module(id)); } + if last_segment_generics.is_some() { + errors.push(PathResolutionError::TurbofishNotAllowedOnItem { + item: format!("module `{last_ident}`"), + span: last_segment.turbofish_span(), + }); + } id } ModuleDefId::FunctionId(_) => panic!("functions cannot be in the type namespace"), @@ -319,6 +358,12 @@ fn resolve_name_in_module( if let Some(path_references) = path_references { path_references.push(ReferenceId::Struct(id)); } + if let Some(last_segment_generics) = last_segment_generics { + generic_type_in_path = Some(GenericTypeInPath { + kind: GenericTypeInPathKind::StructId(id), + generics: last_segment_generics.clone(), + }); + } id.module_id() } ModuleDefId::TypeAliasId(_) => panic!("type aliases cannot be used in type namespace"), @@ -326,44 +371,51 @@ fn resolve_name_in_module( if let Some(path_references) = path_references { path_references.push(ReferenceId::Trait(id)); } + if let Some(last_segment_generics) = last_segment_generics { + generic_type_in_path = Some(GenericTypeInPath { + kind: GenericTypeInPathKind::TraitId(id), + generics: last_segment_generics.clone(), + }); + } id.0 } 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 }) + Ok(NamespaceResolution { + module_id: current_mod_id, + generic_type_in_path, + namespace: current_ns, + errors, + }) } fn resolve_path_name(import_directive: &ImportDirective) -> Ident { diff --git a/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs b/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs index 562366fae77..9692871eea6 100644 --- a/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs @@ -92,5 +92,9 @@ pub fn resolve_path( 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 { + module_def_id: id, + generic_type_in_path: resolved_import.generic_type_in_path, + errors: resolved_import.errors, + }) } diff --git a/compiler/noirc_frontend/src/hir/type_check/errors.rs b/compiler/noirc_frontend/src/hir/type_check/errors.rs index 99de6bca434..459b374372a 100644 --- a/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -174,8 +174,6 @@ pub enum TypeCheckError { StringIndexAssign { span: Span }, #[error("Macro calls may only return `Quoted` values")] MacroReturningNonExpr { typ: Type, span: Span }, - #[error("turbofish (`::<_>`) usage at this position isn't supported yet")] - UnsupportedTurbofishUsage { span: Span }, #[error("`{name}` has already been specified")] DuplicateNamedTypeArg { name: Ident, prev_span: Span }, #[error("`{item}` has no associated type named `{name}`")] @@ -439,10 +437,6 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic { "Macro calls must return quoted values, otherwise there is no code to insert".into(), *span, ), - TypeCheckError::UnsupportedTurbofishUsage { span } => { - let msg = "turbofish (`::<_>`) usage at this position isn't supported yet"; - Diagnostic::simple_error(msg.to_string(), "".to_string(), *span) - }, TypeCheckError::DuplicateNamedTypeArg { name, prev_span } => { let msg = format!("`{name}` has already been specified"); let mut error = Diagnostic::simple_error(msg.to_string(), "".to_string(), name.span()); diff --git a/compiler/noirc_frontend/src/tests/turbofish.rs b/compiler/noirc_frontend/src/tests/turbofish.rs index 71e63e878e8..64e5c4e8d1d 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,68 @@ 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() { + 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); +} From d267b4ed985fce74d591365f7485e8184381ef36 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 22 Oct 2024 10:33:47 -0300 Subject: [PATCH 02/14] WIP --- .../noirc_frontend/src/elaborator/patterns.rs | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 02608db2956..73997223037 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -9,7 +9,10 @@ use crate::{ }, hir::{ def_collector::dc_crate::CompilationError, - resolution::{errors::ResolverError, import::GenericTypeInPath}, + resolution::{ + errors::ResolverError, + import::{GenericTypeInPath, GenericTypeInPathKind}, + }, type_check::{Source, TypeCheckError}, }, hir_def::{ @@ -471,6 +474,25 @@ impl<'context> Elaborator<'context> { let (expr, generic_type_in_path) = self.resolve_variable(variable); let definition_id = expr.id; + if let Some(generic_type_in_path) = generic_type_in_path { + match generic_type_in_path.kind { + GenericTypeInPathKind::StructId(struct_id) => { + let struct_type = self.interner.get_struct(struct_id); + let struct_type = struct_type.borrow(); + let struct_generics = struct_type.instantiate(self.interner); + let struct_generics = self.resolve_struct_turbofish_generics( + &struct_type, + struct_generics, + Some(generic_type_in_path.generics), + span, + ); + dbg!(struct_generics); + } + GenericTypeInPathKind::TypeAliasId(type_alias_id) => todo!(), + GenericTypeInPathKind::TraitId(trait_id) => todo!(), + } + } + // TODO: generic_type_in_path might be Some. // In that case, the variable looks like this: // From 1c21640d8ffd0e6155c3902731797fed96e20f24 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 22 Oct 2024 11:53:48 -0300 Subject: [PATCH 03/14] Got turbofish in the middle of a path working for structs --- .../noirc_frontend/src/elaborator/patterns.rs | 53 ++++++++++++++----- .../noirc_frontend/src/tests/turbofish.rs | 14 ++++- 2 files changed, 54 insertions(+), 13 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 73997223037..f2a7f943ae2 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -474,24 +474,30 @@ impl<'context> Elaborator<'context> { let (expr, generic_type_in_path) = self.resolve_variable(variable); let definition_id = expr.id; - if let Some(generic_type_in_path) = generic_type_in_path { + // Solve any generics that are part of the path before the function, for example: + // + // foo::Bar::::baz + // ^^^^^ + // solve these + let type_generics = if let Some(generic_type_in_path) = generic_type_in_path { match generic_type_in_path.kind { GenericTypeInPathKind::StructId(struct_id) => { let struct_type = self.interner.get_struct(struct_id); let struct_type = struct_type.borrow(); let struct_generics = struct_type.instantiate(self.interner); - let struct_generics = self.resolve_struct_turbofish_generics( + self.resolve_struct_turbofish_generics( &struct_type, struct_generics, Some(generic_type_in_path.generics), span, - ); - dbg!(struct_generics); + ) } GenericTypeInPathKind::TypeAliasId(type_alias_id) => todo!(), GenericTypeInPathKind::TraitId(trait_id) => todo!(), } - } + } else { + Vec::new() + }; // TODO: generic_type_in_path might be Some. // In that case, the variable looks like this: @@ -504,19 +510,33 @@ impl<'context> Elaborator<'context> { 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) @@ -597,8 +617,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 diff --git a/compiler/noirc_frontend/src/tests/turbofish.rs b/compiler/noirc_frontend/src/tests/turbofish.rs index 64e5c4e8d1d..3ded243a280 100644 --- a/compiler/noirc_frontend/src/tests/turbofish.rs +++ b/compiler/noirc_frontend/src/tests/turbofish.rs @@ -218,7 +218,7 @@ fn errors_if_turbofish_after_module() { } #[test] -fn turbofish_in_type_before_call() { +fn turbofish_in_type_before_call_does_not_error() { let src = r#" struct Foo { x: T @@ -256,4 +256,16 @@ fn turbofish_in_type_before_call_errors() { "#; 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"); } From 38134b4bee62acdbd0f8079057fc5ceafdb58f2a Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 22 Oct 2024 12:04:40 -0300 Subject: [PATCH 04/14] Leave traits and aliases for later --- compiler/noirc_frontend/src/elaborator/patterns.rs | 12 ++++++++++-- compiler/noirc_frontend/src/elaborator/statements.rs | 4 ++-- compiler/noirc_frontend/src/hir/type_check/errors.rs | 6 ++++++ 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index f2a7f943ae2..9e8a8bbd916 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -492,8 +492,16 @@ impl<'context> Elaborator<'context> { span, ) } - GenericTypeInPathKind::TypeAliasId(type_alias_id) => todo!(), - GenericTypeInPathKind::TraitId(trait_id) => todo!(), + GenericTypeInPathKind::TypeAliasId(_) => { + // TODO: https://github.com/noir-lang/noir/issues/6311 + self.push_err(TypeCheckError::UnsupportedTurbofishUsage { span }); + Vec::new() + } + GenericTypeInPathKind::TraitId(_trait_id) => { + // TODO: https://github.com/noir-lang/noir/issues/6310 + self.push_err(TypeCheckError::UnsupportedTurbofishUsage { span }); + Vec::new() + } } } else { Vec::new() diff --git a/compiler/noirc_frontend/src/elaborator/statements.rs b/compiler/noirc_frontend/src/elaborator/statements.rs index 1432f57a165..1bbf7b16f3b 100644 --- a/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/compiler/noirc_frontend/src/elaborator/statements.rs @@ -294,9 +294,9 @@ impl<'context> Elaborator<'context> { let span = ident.span(); let path = Path::from_single(ident.0.contents, span); let ((ident, scope_index), generic_type_in_path) = self.get_ident_from_path(path); - self.resolve_local_variable(ident.clone(), scope_index); + assert!(generic_type_in_path.is_none()); // There shouldn't be generics for a single-segment path - // TODO: generic_type_in_path + self.resolve_local_variable(ident.clone(), scope_index); let typ = if ident.id == DefinitionId::dummy_id() { Type::Error diff --git a/compiler/noirc_frontend/src/hir/type_check/errors.rs b/compiler/noirc_frontend/src/hir/type_check/errors.rs index 459b374372a..99de6bca434 100644 --- a/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -174,6 +174,8 @@ pub enum TypeCheckError { StringIndexAssign { span: Span }, #[error("Macro calls may only return `Quoted` values")] MacroReturningNonExpr { typ: Type, span: Span }, + #[error("turbofish (`::<_>`) usage at this position isn't supported yet")] + UnsupportedTurbofishUsage { span: Span }, #[error("`{name}` has already been specified")] DuplicateNamedTypeArg { name: Ident, prev_span: Span }, #[error("`{item}` has no associated type named `{name}`")] @@ -437,6 +439,10 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic { "Macro calls must return quoted values, otherwise there is no code to insert".into(), *span, ), + TypeCheckError::UnsupportedTurbofishUsage { span } => { + let msg = "turbofish (`::<_>`) usage at this position isn't supported yet"; + Diagnostic::simple_error(msg.to_string(), "".to_string(), *span) + }, TypeCheckError::DuplicateNamedTypeArg { name, prev_span } => { let msg = format!("`{name}` has already been specified"); let mut error = Diagnostic::simple_error(msg.to_string(), "".to_string(), name.span()); From c52a08c125906995104a36211699856ce7fd82fc Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 22 Oct 2024 12:19:06 -0300 Subject: [PATCH 05/14] Extract `resolve_generic_type_in_path` --- .../noirc_frontend/src/elaborator/patterns.rs | 80 +++++++++---------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 9e8a8bbd916..bb960f60687 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -474,46 +474,11 @@ impl<'context> Elaborator<'context> { let (expr, generic_type_in_path) = self.resolve_variable(variable); let definition_id = expr.id; - // Solve any generics that are part of the path before the function, for example: - // - // foo::Bar::::baz - // ^^^^^ - // solve these - let type_generics = if let Some(generic_type_in_path) = generic_type_in_path { - match generic_type_in_path.kind { - GenericTypeInPathKind::StructId(struct_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(generic_type_in_path.generics), - span, - ) - } - GenericTypeInPathKind::TypeAliasId(_) => { - // TODO: https://github.com/noir-lang/noir/issues/6311 - self.push_err(TypeCheckError::UnsupportedTurbofishUsage { span }); - Vec::new() - } - GenericTypeInPathKind::TraitId(_trait_id) => { - // TODO: https://github.com/noir-lang/noir/issues/6310 - self.push_err(TypeCheckError::UnsupportedTurbofishUsage { span }); - Vec::new() - } - } - } else { - Vec::new() - }; - - // TODO: generic_type_in_path might be Some. - // In that case, the variable looks like this: - // - // foo::Bar::::baz - // - // That is, the type in the path before the function has some generics on it. - // How to handle this? + let type_generics = generic_type_in_path + .map(|generic_type_in_path| { + self.resolve_generic_type_in_path(generic_type_in_path, span) + }) + .unwrap_or_default(); let definition_kind = self.interner.try_definition(definition_id).map(|definition| definition.kind.clone()); @@ -550,6 +515,41 @@ impl<'context> Elaborator<'context> { (id, typ) } + /// Solve any generics that are part of the path before the function, for example: + /// + /// foo::Bar::::baz + /// ^^^^^ + /// solve these + fn resolve_generic_type_in_path( + &mut self, + generic_type_in_path: GenericTypeInPath, + span: Span, + ) -> Vec { + match generic_type_in_path.kind { + GenericTypeInPathKind::StructId(struct_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(generic_type_in_path.generics), + span, + ) + } + GenericTypeInPathKind::TypeAliasId(_) => { + // TODO: https://github.com/noir-lang/noir/issues/6311 + self.push_err(TypeCheckError::UnsupportedTurbofishUsage { span }); + Vec::new() + } + GenericTypeInPathKind::TraitId(_trait_id) => { + // TODO: https://github.com/noir-lang/noir/issues/6310 + self.push_err(TypeCheckError::UnsupportedTurbofishUsage { span }); + Vec::new() + } + } + } + fn resolve_variable(&mut self, path: Path) -> (HirIdent, Option) { if let Some(trait_path_resolution) = self.resolve_trait_generic_path(&path) { for error in trait_path_resolution.errors { From 769d9094e1e60686af5b49b90403f047960c7b4b Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 22 Oct 2024 12:20:07 -0300 Subject: [PATCH 06/14] Use correct span for errors --- compiler/noirc_frontend/src/elaborator/patterns.rs | 7 +++---- compiler/noirc_frontend/src/hir/resolution/import.rs | 3 +++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index bb960f60687..f3f8c2a4f57 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -475,9 +475,7 @@ impl<'context> Elaborator<'context> { let definition_id = expr.id; let type_generics = generic_type_in_path - .map(|generic_type_in_path| { - self.resolve_generic_type_in_path(generic_type_in_path, span) - }) + .map(|generic_type_in_path| self.resolve_generic_type_in_path(generic_type_in_path)) .unwrap_or_default(); let definition_kind = @@ -523,8 +521,9 @@ impl<'context> Elaborator<'context> { fn resolve_generic_type_in_path( &mut self, generic_type_in_path: GenericTypeInPath, - span: Span, ) -> Vec { + let span = generic_type_in_path.span; + match generic_type_in_path.kind { GenericTypeInPathKind::StructId(struct_id) => { let struct_type = self.interner.get_struct(struct_id); diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index 269bee71ae3..1473586f900 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -26,6 +26,7 @@ use super::visibility::can_reference_module_id; pub struct GenericTypeInPath { pub kind: GenericTypeInPathKind, pub generics: Vec, + pub span: Span, } #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] @@ -362,6 +363,7 @@ fn resolve_name_in_module( generic_type_in_path = Some(GenericTypeInPath { kind: GenericTypeInPathKind::StructId(id), generics: last_segment_generics.clone(), + span: last_segment.turbofish_span(), }); } id.module_id() @@ -375,6 +377,7 @@ fn resolve_name_in_module( generic_type_in_path = Some(GenericTypeInPath { kind: GenericTypeInPathKind::TraitId(id), generics: last_segment_generics.clone(), + span: last_segment.turbofish_span(), }); } id.0 From 1ac596bab8820d6869b61f382fde62a88dbfe9fb Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 25 Oct 2024 12:37:35 -0300 Subject: [PATCH 07/14] Introduce PathResolutionKind --- compiler/noirc_frontend/src/elaborator/mod.rs | 17 +-- .../noirc_frontend/src/elaborator/patterns.rs | 59 ++++++---- .../noirc_frontend/src/elaborator/scope.rs | 111 +++++++++--------- .../noirc_frontend/src/elaborator/types.rs | 11 +- .../src/hir/def_collector/dc_crate.rs | 20 ++-- .../src/hir/def_map/module_def.rs | 76 ------------ .../src/hir/resolution/errors.rs | 2 +- .../src/hir/resolution/import.rs | 52 +++++++- .../src/hir/resolution/path_resolver.rs | 69 +++++++++-- compiler/noirc_frontend/src/locations.rs | 36 +++++- compiler/noirc_frontend/src/tests.rs | 4 +- tooling/lsp/src/attribute_reference_finder.rs | 23 +++- 12 files changed, 280 insertions(+), 200 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index bb215c34c71..18535c8c35b 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::PathResolutionKind, + 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,7 @@ 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), - generic_type_in_path: _, - errors, - }) => { + Ok(PathResolution { kind: PathResolutionKind::Module(module_id), errors }) => { if errors.is_empty() { Some(module_id) } else { @@ -684,11 +681,7 @@ 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), - generic_type_in_path: _, - errors, - }) => { + Ok(PathResolution { kind: PathResolutionKind::Trait(trait_id, _generics), errors }) => { for error in errors { self.push_err(error); } diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index f3f8c2a4f57..5ce430df943 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -9,10 +9,7 @@ use crate::{ }, hir::{ def_collector::dc_crate::CompilationError, - resolution::{ - errors::ResolverError, - import::{GenericTypeInPath, GenericTypeInPathKind}, - }, + resolution::{errors::ResolverError, import::PathResolutionKind}, type_check::{Source, TypeCheckError}, }, hir_def::{ @@ -471,11 +468,13 @@ impl<'context> Elaborator<'context> { let unresolved_turbofish = variable.segments.last().unwrap().generics.clone(); let span = variable.span; - let (expr, generic_type_in_path) = self.resolve_variable(variable); + let (expr, path_resolution_kind) = self.resolve_variable(variable); let definition_id = expr.id; - let type_generics = generic_type_in_path - .map(|generic_type_in_path| self.resolve_generic_type_in_path(generic_type_in_path)) + let type_generics = path_resolution_kind + .map(|path_resolution_kind| { + self.resolve_path_resolution_kind_generics(path_resolution_kind) + }) .unwrap_or_default(); let definition_kind = @@ -518,38 +517,46 @@ impl<'context> Elaborator<'context> { /// foo::Bar::::baz /// ^^^^^ /// solve these - fn resolve_generic_type_in_path( + fn resolve_path_resolution_kind_generics( &mut self, - generic_type_in_path: GenericTypeInPath, + path_resolution_kind: PathResolutionKind, ) -> Vec { - let span = generic_type_in_path.span; + // let span = path_resolution_kind.span; + let span = Span::default(); // TODO: span - match generic_type_in_path.kind { - GenericTypeInPathKind::StructId(struct_id) => { + match path_resolution_kind { + PathResolutionKind::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(generic_type_in_path.generics), + Some(generics), span, ) } - GenericTypeInPathKind::TypeAliasId(_) => { + PathResolutionKind::TypeAliasFunction(..) => { // TODO: https://github.com/noir-lang/noir/issues/6311 self.push_err(TypeCheckError::UnsupportedTurbofishUsage { span }); Vec::new() } - GenericTypeInPathKind::TraitId(_trait_id) => { + PathResolutionKind::TraitFunction(..) => { // TODO: https://github.com/noir-lang/noir/issues/6310 self.push_err(TypeCheckError::UnsupportedTurbofishUsage { span }); Vec::new() } + PathResolutionKind::StructFunction(_, None, _) + | PathResolutionKind::Module(..) + | PathResolutionKind::Struct(..) + | PathResolutionKind::TypeAlias(..) + | PathResolutionKind::Trait(..) + | PathResolutionKind::Global(..) + | PathResolutionKind::ModuleFunction(..) => Vec::new(), } } - fn resolve_variable(&mut self, path: Path) -> (HirIdent, Option) { + fn resolve_variable(&mut self, path: Path) -> (HirIdent, Option) { if let Some(trait_path_resolution) = self.resolve_trait_generic_path(&path) { for error in trait_path_resolution.errors { self.push_err(error); @@ -571,7 +578,7 @@ impl<'context> Elaborator<'context> { // 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), generic_type_in_path) = + let ((hir_ident, var_scope_index), path_resolution_kind) = self.get_ident_from_path(path); if hir_ident.id != DefinitionId::dummy_id() { @@ -614,7 +621,7 @@ impl<'context> Elaborator<'context> { } } - (hir_ident, generic_type_in_path) + (hir_ident, path_resolution_kind) } } @@ -737,21 +744,27 @@ impl<'context> Elaborator<'context> { pub fn get_ident_from_path( &mut self, path: Path, - ) -> ((HirIdent, usize), Option) { + ) -> ((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, 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, generic_type_in_path)) => { - return ((HirIdent::non_trait_method(id, location), 0), generic_type_in_path) + Ok((id, path_resolution_kind)) => { + return ( + (HirIdent::non_trait_method(id, location), 0), + Some(path_resolution_kind), + ) } Err(_) => error, }, None => match self.lookup_global(path) { - Ok((id, generic_type_in_path)) => { - return ((HirIdent::non_trait_method(id, location), 0), generic_type_in_path) + Ok((id, path_resolution_kind)) => { + return ( + (HirIdent::non_trait_method(id, location), 0), + Some(path_resolution_kind), + ) } Err(error) => error, }, diff --git a/compiler/noirc_frontend/src/elaborator/scope.rs b/compiler/noirc_frontend/src/elaborator/scope.rs index a0c2185905c..cde2100dd37 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::{GenericTypeInPath, PathResolution, PathResolutionResult}; +use crate::hir::resolution::import::{PathResolution, PathResolutionKind, 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,21 +23,6 @@ type Scope = GenericScope; type ScopeTree = GenericScopeTree; impl<'context> Elaborator<'context> { - pub(super) fn lookup( - &mut self, - path: Path, - ) -> Result<(T, Option), ResolverError> { - let span = path.span(); - let (id, generic_type_in_path) = self.resolve_path_or_error(path)?; - T::try_from(id).map(|id| (id, generic_type_in_path)).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 } @@ -58,14 +40,14 @@ impl<'context> Elaborator<'context> { pub(super) fn resolve_path_or_error( &mut self, path: Path, - ) -> Result<(ModuleDefId, Option), ResolverError> { + ) -> Result { let path_resolution = self.resolve_path(path)?; for error in path_resolution.errors { self.push_err(error); } - Ok((path_resolution.module_def_id, path_resolution.generic_type_in_path)) + Ok(path_resolution.kind) } pub(super) fn resolve_path(&mut self, path: Path) -> PathResolutionResult { @@ -78,8 +60,7 @@ impl<'context> Elaborator<'context> { if path.segments.len() == 1 { // TODO: handle generics return Ok(PathResolution { - module_def_id: ModuleDefId::TypeId(struct_type.id), - generic_type_in_path: None, + kind: PathResolutionKind::Struct(struct_type.id, None), errors: Vec::new(), }); } @@ -139,8 +120,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.kind.clone(), location, is_self_type_name, ); @@ -193,21 +174,21 @@ impl<'context> Elaborator<'context> { pub(super) fn lookup_global( &mut self, path: Path, - ) -> Result<(DefinitionId, Option), ResolverError> { + ) -> Result<(DefinitionId, PathResolutionKind), ResolverError> { let span = path.span(); - let (id, generic_type_in_path) = self.resolve_path_or_error(path)?; + let path_resolution_kind = self.resolve_path_or_error(path)?; - if let Some(function) = TryFromModuleDefId::try_from(id) { - return Ok((self.interner.function_definition_id(function), generic_type_in_path)); + if let Some(function) = path_resolution_kind.function_id() { + return Ok((self.interner.function_definition_id(function), path_resolution_kind)); } - if let Some(global) = TryFromModuleDefId::try_from(id) { + if let PathResolutionKind::Global(global) = path_resolution_kind { let global = self.interner.get_global(global); - return Ok((global.definition_id, generic_type_in_path)); + return Ok((global.definition_id, path_resolution_kind)); } - let expected = "global variable".into(); - let got = "local variable".into(); + let expected = "global variable"; + let got = "local variable"; Err(ResolverError::Expected { span, expected, got }) } @@ -249,10 +230,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, _generic_type_in_path)) => 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(path_resolution_kind) => { + if let PathResolutionKind::Trait(trait_id, _) = path_resolution_kind { + Some(self.get_trait_mut(trait_id)) + } else { + self.push_err(ResolverError::Expected { + expected: "trait", + got: path_resolution_kind.description(), + span, + }); + None + } + } + Err(err) => { + self.push_err(err); None } } @@ -260,10 +253,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, _generic_type_in_path)) => Some(self.get_struct(struct_id)), - Err(error) => { - self.push_err(error); + let span = path.span(); + match self.resolve_path_or_error(path) { + Ok(path_resolution_kind) => { + if let PathResolutionKind::Struct(struct_id, _) = path_resolution_kind { + Some(self.get_struct(struct_id)) + } else { + self.push_err(ResolverError::Expected { + expected: "type", + got: path_resolution_kind.description(), + span, + }); + None + } + } + Err(err) => { + self.push_err(err); None } } @@ -279,20 +284,18 @@ impl<'context> Elaborator<'context> { } } - match self.lookup(path) { - Ok((struct_id, _generic_type_in_path)) => { - let struct_type = self.get_struct(struct_id); - let struct_generics = struct_type.borrow().instantiate(self.interner); - Some(Type::Struct(struct_type, struct_generics)) - } - Err(error) => { - self.push_err(error); - None - } - } + self.lookup_struct_or_error(path).map(|struct_type| { + let struct_generics = struct_type.borrow().instantiate(self.interner); + Type::Struct(struct_type, struct_generics) + }) } pub fn lookup_type_alias(&mut self, path: Path) -> Option> { - self.lookup(path).ok().map(|(id, _generic_type_in_path)| self.interner.get_type_alias(id)) + match self.resolve_path_or_error(path) { + Ok(PathResolutionKind::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 ad76228d931..6a2bc97f8a5 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, PathResolutionKind}, + }, type_check::{ generics::{Generic, TraitGenerics}, NoMatchingImplFoundError, Source, TypeCheckError, @@ -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), _generic_type_in_path)) => { + Ok(PathResolutionKind::Global(id)) => { if let Some(current_item) = self.current_item { self.interner.add_global_dependency(current_item, id); } @@ -564,8 +566,7 @@ 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.kind.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())?; 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 13496768b64..3dc6ebb44ec 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -551,18 +551,16 @@ fn inject_prelude( span: Span::default(), }; - if let Ok(PathResolution { module_def_id, generic_type_in_path: _, errors }) = - path_resolver::resolve_path( - &context.def_maps, - ModuleId { krate: crate_id, local_id: crate_root }, - None, - path, - &mut context.def_interner.usage_tracker, - &mut None, - ) - { + if let Ok(PathResolution { kind, errors }) = path_resolver::resolve_path( + &context.def_maps, + ModuleId { krate: crate_id, local_id: crate_root }, + None, + path, + &mut context.def_interner.usage_tracker, + &mut None, + ) { assert!(errors.is_empty(), "Tried to add private item to prelude"); - let module_id = module_def_id.as_module().expect("std::prelude should be a module"); + let module_id = kind.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/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 1473586f900..f54f5e49055 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -4,7 +4,7 @@ use thiserror::Error; use crate::graph::CrateId; use crate::hir::def_collector::dc_crate::CompilationError; -use crate::node_interner::{ReferenceId, StructId, TraitId, TypeAliasId}; +use crate::node_interner::{FuncId, GlobalId, ReferenceId, StructId, TraitId, TypeAliasId}; use crate::usage_tracker::UsageTracker; use std::collections::BTreeMap; @@ -55,12 +55,58 @@ struct NamespaceResolution { type NamespaceResolutionResult = Result; +#[derive(Debug)] pub struct PathResolution { - pub module_def_id: ModuleDefId, - pub generic_type_in_path: Option, + pub kind: PathResolutionKind, pub errors: Vec, } +#[derive(Debug, Clone)] +pub enum PathResolutionKind { + Module(ModuleId), + Struct(StructId, Option>), // The generics, if any (using Option to distinguish the `::<>` case) + TypeAlias(TypeAliasId, Option>), + Trait(TraitId, Option>), + Global(GlobalId), + ModuleFunction(FuncId), + StructFunction(StructId, Option>, FuncId), + TypeAliasFunction(TypeAliasId, Option>, FuncId), + TraitFunction(TraitId, Option>, FuncId), +} + +impl PathResolutionKind { + pub fn function_id(&self) -> Option { + match self { + PathResolutionKind::ModuleFunction(func_id) + | PathResolutionKind::StructFunction(_, _, func_id) + | PathResolutionKind::TypeAliasFunction(_, _, func_id) + | PathResolutionKind::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 { + PathResolutionKind::Module(..) => "module", + PathResolutionKind::Struct(..) => "type", + PathResolutionKind::TypeAlias(..) => "type alias", + PathResolutionKind::Trait(..) => "trait", + PathResolutionKind::Global(..) => "global", + PathResolutionKind::ModuleFunction(..) + | PathResolutionKind::StructFunction(..) + | PathResolutionKind::TypeAliasFunction(..) + | PathResolutionKind::TraitFunction(..) => "function", + } + } +} + pub(crate) type PathResolutionResult = Result; #[derive(Debug, Clone, PartialEq, Eq, Error)] diff --git a/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs b/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs index 9692871eea6..4047b6e8271 100644 --- a/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs @@ -1,4 +1,7 @@ -use super::import::{resolve_import, ImportDirective, PathResolution, PathResolutionResult}; +use super::import::{ + resolve_import, GenericTypeInPath, GenericTypeInPathKind, ImportDirective, PathResolution, + PathResolutionKind, PathResolutionResult, +}; use crate::ast::{ItemVisibility, Path}; use crate::node_interner::ReferenceId; use crate::usage_tracker::UsageTracker; @@ -6,7 +9,7 @@ use crate::usage_tracker::UsageTracker; use std::collections::BTreeMap; use crate::graph::CrateId; -use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}; +use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleDefId, ModuleId}; pub trait PathResolver { /// Resolve the given path returning the resolved ModuleDefId. @@ -89,12 +92,62 @@ pub fn resolve_path( resolve_import(module_id.krate, &import, def_maps, usage_tracker, path_references)?; let namespace = resolved_import.resolved_namespace; - let id = + let module_def_id = namespace.values.or(namespace.types).map(|(id, _, _)| id).expect("Found empty namespace"); - Ok(PathResolution { - module_def_id: id, - generic_type_in_path: resolved_import.generic_type_in_path, - errors: resolved_import.errors, - }) + let kind = path_resolution_kind_from_module_def_id_and_generic_type_in_path( + module_def_id, + resolved_import.generic_type_in_path, + ); + + Ok(PathResolution { kind, errors: resolved_import.errors }) +} + +fn path_resolution_kind_from_module_def_id_and_generic_type_in_path( + module_def_id: ModuleDefId, + generic_type_in_path: Option, +) -> PathResolutionKind { + if let Some(generic_type_in_path) = generic_type_in_path { + match generic_type_in_path.kind { + GenericTypeInPathKind::StructId(struct_id) => match module_def_id { + ModuleDefId::FunctionId(func_id) => PathResolutionKind::StructFunction( + struct_id, + Some(generic_type_in_path.generics), + func_id, + ), + _ => path_resolution_kind_from_module_def_if(module_def_id), + }, + GenericTypeInPathKind::TypeAliasId(type_alias_id) => match module_def_id { + ModuleDefId::FunctionId(func_id) => PathResolutionKind::TypeAliasFunction( + type_alias_id, + Some(generic_type_in_path.generics), + func_id, + ), + _ => path_resolution_kind_from_module_def_if(module_def_id), + }, + GenericTypeInPathKind::TraitId(trait_id) => match module_def_id { + ModuleDefId::FunctionId(func_id) => PathResolutionKind::TraitFunction( + trait_id, + Some(generic_type_in_path.generics), + func_id, + ), + _ => path_resolution_kind_from_module_def_if(module_def_id), + }, + } + } else { + path_resolution_kind_from_module_def_if(module_def_id) + } +} + +fn path_resolution_kind_from_module_def_if(module_def_id: ModuleDefId) -> PathResolutionKind { + match module_def_id { + ModuleDefId::ModuleId(module_id) => PathResolutionKind::Module(module_id), + ModuleDefId::FunctionId(func_id) => PathResolutionKind::ModuleFunction(func_id), + ModuleDefId::TypeId(struct_id) => PathResolutionKind::Struct(struct_id, None), + ModuleDefId::TypeAliasId(type_alias_id) => { + PathResolutionKind::TypeAlias(type_alias_id, None) + } + ModuleDefId::TraitId(trait_id) => PathResolutionKind::Trait(trait_id, None), + ModuleDefId::GlobalId(global_id) => PathResolutionKind::Global(global_id), + } } diff --git a/compiler/noirc_frontend/src/locations.rs b/compiler/noirc_frontend/src/locations.rs index 48660142d0a..49ddf7b15e2 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::PathResolutionKind, + }, 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: PathResolutionKind, + location: Location, + is_self_type: bool, + ) { + match kind { + PathResolutionKind::Module(module_id) => { + self.add_module_reference(module_id, location); + } + PathResolutionKind::Struct(struct_id, _generics) => { + self.add_struct_reference(struct_id, location, is_self_type); + } + PathResolutionKind::TypeAlias(type_alias_id, _generics) => { + self.add_alias_reference(type_alias_id, location); + } + PathResolutionKind::Trait(trait_id, _generics) => { + self.add_trait_reference(trait_id, location, is_self_type); + } + PathResolutionKind::Global(global_id) => { + self.add_global_reference(global_id, location); + } + PathResolutionKind::ModuleFunction(func_id) + | PathResolutionKind::StructFunction(_, _, func_id) + | PathResolutionKind::TypeAliasFunction(_, _, func_id) + | PathResolutionKind::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 e06881127fd..d5635c9d391 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/tooling/lsp/src/attribute_reference_finder.rs b/tooling/lsp/src/attribute_reference_finder.rs index 39e1385a6e8..f9a45590186 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::PathResolutionKind, + 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_kind_to_reference_id(result.kind)); + } +} + +fn path_resolution_kind_to_reference_id(path_resolution_kind: PathResolutionKind) -> ReferenceId { + match path_resolution_kind { + PathResolutionKind::Module(module_id) => ReferenceId::Module(module_id), + PathResolutionKind::Struct(struct_id, _) => ReferenceId::Struct(struct_id), + PathResolutionKind::TypeAlias(type_alias_id, _) => ReferenceId::Alias(type_alias_id), + PathResolutionKind::Trait(trait_id, _) => ReferenceId::Trait(trait_id), + PathResolutionKind::Global(global_id) => ReferenceId::Global(global_id), + PathResolutionKind::ModuleFunction(func_id) + | PathResolutionKind::StructFunction(_, _, func_id) + | PathResolutionKind::TypeAliasFunction(_, _, func_id) + | PathResolutionKind::TraitFunction(_, _, func_id) => ReferenceId::Function(func_id), } } From 9dff8644f34ad8b3b80150577f13531131268096 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 25 Oct 2024 12:40:33 -0300 Subject: [PATCH 08/14] Use correct span for error --- .../noirc_frontend/src/elaborator/patterns.rs | 21 +++++++------------ .../src/hir/resolution/import.rs | 6 +++--- .../src/hir/resolution/path_resolver.rs | 6 +++--- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 5ce430df943..8c455f63c16 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -521,11 +521,8 @@ impl<'context> Elaborator<'context> { &mut self, path_resolution_kind: PathResolutionKind, ) -> Vec { - // let span = path_resolution_kind.span; - let span = Span::default(); // TODO: span - match path_resolution_kind { - PathResolutionKind::StructFunction(struct_id, Some(generics), _func_id) => { + PathResolutionKind::StructFunction(struct_id, Some((generics, span)), _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); @@ -536,23 +533,21 @@ impl<'context> Elaborator<'context> { span, ) } - PathResolutionKind::TypeAliasFunction(..) => { + PathResolutionKind::TypeAliasFunction( + _type_alias_id, + Some((_generics, span)), + _func_id, + ) => { // TODO: https://github.com/noir-lang/noir/issues/6311 self.push_err(TypeCheckError::UnsupportedTurbofishUsage { span }); Vec::new() } - PathResolutionKind::TraitFunction(..) => { + PathResolutionKind::TraitFunction(_trait_id, Some((_generics, span)), _func_id) => { // TODO: https://github.com/noir-lang/noir/issues/6310 self.push_err(TypeCheckError::UnsupportedTurbofishUsage { span }); Vec::new() } - PathResolutionKind::StructFunction(_, None, _) - | PathResolutionKind::Module(..) - | PathResolutionKind::Struct(..) - | PathResolutionKind::TypeAlias(..) - | PathResolutionKind::Trait(..) - | PathResolutionKind::Global(..) - | PathResolutionKind::ModuleFunction(..) => Vec::new(), + _ => Vec::new(), } } diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index f54f5e49055..b9f7c483162 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -69,9 +69,9 @@ pub enum PathResolutionKind { Trait(TraitId, Option>), Global(GlobalId), ModuleFunction(FuncId), - StructFunction(StructId, Option>, FuncId), - TypeAliasFunction(TypeAliasId, Option>, FuncId), - TraitFunction(TraitId, Option>, FuncId), + StructFunction(StructId, Option<(Vec, Span)>, FuncId), + TypeAliasFunction(TypeAliasId, Option<(Vec, Span)>, FuncId), + TraitFunction(TraitId, Option<(Vec, Span)>, FuncId), } impl PathResolutionKind { diff --git a/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs b/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs index 4047b6e8271..b351e85c5b9 100644 --- a/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs @@ -112,7 +112,7 @@ fn path_resolution_kind_from_module_def_id_and_generic_type_in_path( GenericTypeInPathKind::StructId(struct_id) => match module_def_id { ModuleDefId::FunctionId(func_id) => PathResolutionKind::StructFunction( struct_id, - Some(generic_type_in_path.generics), + Some((generic_type_in_path.generics, generic_type_in_path.span)), func_id, ), _ => path_resolution_kind_from_module_def_if(module_def_id), @@ -120,7 +120,7 @@ fn path_resolution_kind_from_module_def_id_and_generic_type_in_path( GenericTypeInPathKind::TypeAliasId(type_alias_id) => match module_def_id { ModuleDefId::FunctionId(func_id) => PathResolutionKind::TypeAliasFunction( type_alias_id, - Some(generic_type_in_path.generics), + Some((generic_type_in_path.generics, generic_type_in_path.span)), func_id, ), _ => path_resolution_kind_from_module_def_if(module_def_id), @@ -128,7 +128,7 @@ fn path_resolution_kind_from_module_def_id_and_generic_type_in_path( GenericTypeInPathKind::TraitId(trait_id) => match module_def_id { ModuleDefId::FunctionId(func_id) => PathResolutionKind::TraitFunction( trait_id, - Some(generic_type_in_path.generics), + Some((generic_type_in_path.generics, generic_type_in_path.span)), func_id, ), _ => path_resolution_kind_from_module_def_if(module_def_id), From dc26a0bcdd498f366889602ff0e90456649ab5cd Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 25 Oct 2024 13:51:42 -0300 Subject: [PATCH 09/14] Remove two unused methods --- compiler/noirc_frontend/src/hir/def_map/namespace.rs | 8 -------- 1 file changed, 8 deletions(-) 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)) } From 32832c6b51397f0a4a22d66cf8c371945005bd70 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 25 Oct 2024 13:52:02 -0300 Subject: [PATCH 10/14] Remove GenericTypeInPath, make path resolution return PathResolutionKind --- .../noirc_frontend/src/elaborator/patterns.rs | 22 ++- .../src/elaborator/statements.rs | 3 +- .../src/hir/resolution/import.rs | 136 +++++++++++------- .../src/hir/resolution/path_resolver.rs | 70 +-------- 4 files changed, 99 insertions(+), 132 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 8c455f63c16..5a53ddb4530 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -522,29 +522,25 @@ impl<'context> Elaborator<'context> { path_resolution_kind: PathResolutionKind, ) -> Vec { match path_resolution_kind { - PathResolutionKind::StructFunction(struct_id, Some((generics, span)), _func_id) => { + PathResolutionKind::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), - span, + Some(generics.generics), + generics.span, ) } - PathResolutionKind::TypeAliasFunction( - _type_alias_id, - Some((_generics, span)), - _func_id, - ) => { + PathResolutionKind::TypeAliasFunction(_type_alias_id, Some(generics), _func_id) => { // TODO: https://github.com/noir-lang/noir/issues/6311 - self.push_err(TypeCheckError::UnsupportedTurbofishUsage { span }); + self.push_err(TypeCheckError::UnsupportedTurbofishUsage { span: generics.span }); Vec::new() } - PathResolutionKind::TraitFunction(_trait_id, Some((_generics, span)), _func_id) => { + PathResolutionKind::TraitFunction(_trait_id, Some(generics), _func_id) => { // TODO: https://github.com/noir-lang/noir/issues/6310 - self.push_err(TypeCheckError::UnsupportedTurbofishUsage { span }); + self.push_err(TypeCheckError::UnsupportedTurbofishUsage { span: generics.span }); Vec::new() } _ => Vec::new(), @@ -557,15 +553,13 @@ impl<'context> Elaborator<'context> { self.push_err(error); } - // TODO: GenericTypeInPath - let generic_type_in_path = None; ( 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), }, - generic_type_in_path, + None, ) } else { // If the Path is being used as an Expression, then it is referring to a global from a separate module diff --git a/compiler/noirc_frontend/src/elaborator/statements.rs b/compiler/noirc_frontend/src/elaborator/statements.rs index 1bbf7b16f3b..757def16a93 100644 --- a/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/compiler/noirc_frontend/src/elaborator/statements.rs @@ -293,8 +293,7 @@ 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), generic_type_in_path) = self.get_ident_from_path(path); - assert!(generic_type_in_path.is_none()); // There shouldn't be generics for a single-segment path + let ((ident, scope_index), _) = self.get_ident_from_path(path); self.resolve_local_variable(ident.clone(), scope_index); diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index b9f7c483162..610c0925126 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -15,27 +15,6 @@ use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleDefId, ModuleId, Per use super::errors::ResolverError; use super::visibility::can_reference_module_id; -/// Represents a generic type in the before-last segment of a path, for example: -/// -/// foo::Bar::::baz -/// ^^^^^^^^^^ -/// -/// In the above example, if `Bar` is a struct, `GenericTypeInPath` would be a -/// `GenericTypeInPathKind::StructId(..)` with the generics being `[i32]`. -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub struct GenericTypeInPath { - pub kind: GenericTypeInPathKind, - pub generics: Vec, - pub span: Span, -} - -#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] -pub enum GenericTypeInPathKind { - StructId(StructId), - TypeAliasId(TypeAliasId), - TraitId(TraitId), -} - #[derive(Debug, Clone)] pub struct ImportDirective { pub visibility: ItemVisibility, @@ -48,7 +27,7 @@ pub struct ImportDirective { struct NamespaceResolution { module_id: ModuleId, - generic_type_in_path: Option, + path_resolution_kind: PathResolutionKind, namespace: PerNs, errors: Vec, } @@ -64,14 +43,14 @@ pub struct PathResolution { #[derive(Debug, Clone)] pub enum PathResolutionKind { Module(ModuleId), - Struct(StructId, Option>), // The generics, if any (using Option to distinguish the `::<>` case) - TypeAlias(TypeAliasId, Option>), - Trait(TraitId, Option>), + Struct(StructId, Option), + TypeAlias(TypeAliasId, Option), + Trait(TraitId, Option), Global(GlobalId), ModuleFunction(FuncId), - StructFunction(StructId, Option<(Vec, Span)>, FuncId), - TypeAliasFunction(TypeAliasId, Option<(Vec, Span)>, FuncId), - TraitFunction(TraitId, Option<(Vec, Span)>, FuncId), + StructFunction(StructId, Option, FuncId), + TypeAliasFunction(TypeAliasId, Option, FuncId), + TraitFunction(TraitId, Option, FuncId), } impl PathResolutionKind { @@ -107,6 +86,19 @@ impl PathResolutionKind { } } +#[derive(Debug, Clone)] +pub struct PathResolutionGenerics { + pub generics: Vec, + pub span: Span, +} + +#[derive(Debug)] +enum IntermediatePathResolutionKind { + Module(ModuleId), + Struct(StructId, Option), + Trait(TraitId, Option), +} + pub(crate) type PathResolutionResult = Result; #[derive(Debug, Clone, PartialEq, Eq, Error)] @@ -127,7 +119,7 @@ pub struct ResolvedImport { pub name: Ident, // The symbol which we have resolved to pub resolved_namespace: PerNs, - pub generic_type_in_path: Option, + pub path_resolution_kind: PathResolutionKind, // The module which we must add the resolved namespace to pub module_scope: LocalModuleId, pub is_prelude: bool, @@ -172,7 +164,7 @@ pub fn resolve_import( let module_scope = import_directive.module_id; let NamespaceResolution { module_id: resolved_module, - generic_type_in_path, + path_resolution_kind, namespace: resolved_namespace, mut errors, } = resolve_path_to_ns( @@ -207,7 +199,7 @@ pub fn resolve_import( Ok(ResolvedImport { name, resolved_namespace, - generic_type_in_path, + path_resolution_kind, module_scope, is_prelude: import_directive.is_prelude, errors, @@ -351,14 +343,14 @@ 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 generic_type_in_path = None; + let mut path_resolution_kind = IntermediatePathResolutionKind::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, - generic_type_in_path, + path_resolution_kind: PathResolutionKind::Module(current_mod_id), namespace: PerNs::types(current_mod_id.into()), errors: Vec::new(), }); @@ -386,18 +378,20 @@ fn resolve_name_in_module( }; // In the type namespace, only Mod can be used in a path. - current_mod_id = match typ { + (current_mod_id, path_resolution_kind) = match typ { ModuleDefId::ModuleId(id) => { if let Some(path_references) = path_references { path_references.push(ReferenceId::Module(id)); } + if last_segment_generics.is_some() { errors.push(PathResolutionError::TurbofishNotAllowedOnItem { item: format!("module `{last_ident}`"), span: last_segment.turbofish_span(), }); } - id + + (id, IntermediatePathResolutionKind::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 @@ -405,28 +399,34 @@ fn resolve_name_in_module( if let Some(path_references) = path_references { path_references.push(ReferenceId::Struct(id)); } - if let Some(last_segment_generics) = last_segment_generics { - generic_type_in_path = Some(GenericTypeInPath { - kind: GenericTypeInPathKind::StructId(id), - generics: last_segment_generics.clone(), - span: last_segment.turbofish_span(), - }); - } - id.module_id() + + ( + id.module_id(), + IntermediatePathResolutionKind::Struct( + id, + last_segment_generics.as_ref().map(|generics| PathResolutionGenerics { + 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)); } - if let Some(last_segment_generics) = last_segment_generics { - generic_type_in_path = Some(GenericTypeInPath { - kind: GenericTypeInPathKind::TraitId(id), - generics: last_segment_generics.clone(), - span: last_segment.turbofish_span(), - }); - } - id.0 + + ( + id.0, + IntermediatePathResolutionKind::Trait( + id, + last_segment_generics.as_ref().map(|generics| PathResolutionGenerics { + generics: generics.clone(), + span: last_segment.turbofish_span(), + }), + ), + ) } ModuleDefId::GlobalId(_) => panic!("globals cannot be in the type namespace"), }; @@ -459,9 +459,15 @@ fn resolve_name_in_module( current_ns = found_ns; } + let module_def_id = + current_ns.values.or(current_ns.types).map(|(id, _, _)| id).expect("Found empty namespace"); + + let path_resolution_kind = + merge_path_resolution_kind_with_module_def_id(path_resolution_kind, module_def_id); + Ok(NamespaceResolution { module_id: current_mod_id, - generic_type_in_path, + path_resolution_kind, namespace: current_ns, errors, }) @@ -526,3 +532,29 @@ fn resolve_external_dep( path_references, ) } + +fn merge_path_resolution_kind_with_module_def_id( + path_resolution_kind: IntermediatePathResolutionKind, + module_def_id: ModuleDefId, +) -> PathResolutionKind { + match module_def_id { + ModuleDefId::ModuleId(module_id) => PathResolutionKind::Module(module_id), + ModuleDefId::TypeId(struct_id) => PathResolutionKind::Struct(struct_id, None), + ModuleDefId::TypeAliasId(type_alias_id) => { + PathResolutionKind::TypeAlias(type_alias_id, None) + } + ModuleDefId::TraitId(trait_id) => PathResolutionKind::Trait(trait_id, None), + ModuleDefId::GlobalId(global_id) => PathResolutionKind::Global(global_id), + ModuleDefId::FunctionId(func_id) => match path_resolution_kind { + IntermediatePathResolutionKind::Module(_) => { + PathResolutionKind::ModuleFunction(func_id) + } + IntermediatePathResolutionKind::Struct(struct_id, generics) => { + PathResolutionKind::StructFunction(struct_id, generics, func_id) + } + IntermediatePathResolutionKind::Trait(trait_id, generics) => { + PathResolutionKind::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 b351e85c5b9..8dffe66ce15 100644 --- a/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs @@ -1,7 +1,4 @@ -use super::import::{ - resolve_import, GenericTypeInPath, GenericTypeInPathKind, ImportDirective, PathResolution, - PathResolutionKind, PathResolutionResult, -}; +use super::import::{resolve_import, ImportDirective, PathResolution, PathResolutionResult}; use crate::ast::{ItemVisibility, Path}; use crate::node_interner::ReferenceId; use crate::usage_tracker::UsageTracker; @@ -9,7 +6,7 @@ use crate::usage_tracker::UsageTracker; use std::collections::BTreeMap; use crate::graph::CrateId; -use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleDefId, ModuleId}; +use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}; pub trait PathResolver { /// Resolve the given path returning the resolved ModuleDefId. @@ -91,63 +88,8 @@ 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 module_def_id = - namespace.values.or(namespace.types).map(|(id, _, _)| id).expect("Found empty namespace"); - - let kind = path_resolution_kind_from_module_def_id_and_generic_type_in_path( - module_def_id, - resolved_import.generic_type_in_path, - ); - - Ok(PathResolution { kind, errors: resolved_import.errors }) -} - -fn path_resolution_kind_from_module_def_id_and_generic_type_in_path( - module_def_id: ModuleDefId, - generic_type_in_path: Option, -) -> PathResolutionKind { - if let Some(generic_type_in_path) = generic_type_in_path { - match generic_type_in_path.kind { - GenericTypeInPathKind::StructId(struct_id) => match module_def_id { - ModuleDefId::FunctionId(func_id) => PathResolutionKind::StructFunction( - struct_id, - Some((generic_type_in_path.generics, generic_type_in_path.span)), - func_id, - ), - _ => path_resolution_kind_from_module_def_if(module_def_id), - }, - GenericTypeInPathKind::TypeAliasId(type_alias_id) => match module_def_id { - ModuleDefId::FunctionId(func_id) => PathResolutionKind::TypeAliasFunction( - type_alias_id, - Some((generic_type_in_path.generics, generic_type_in_path.span)), - func_id, - ), - _ => path_resolution_kind_from_module_def_if(module_def_id), - }, - GenericTypeInPathKind::TraitId(trait_id) => match module_def_id { - ModuleDefId::FunctionId(func_id) => PathResolutionKind::TraitFunction( - trait_id, - Some((generic_type_in_path.generics, generic_type_in_path.span)), - func_id, - ), - _ => path_resolution_kind_from_module_def_if(module_def_id), - }, - } - } else { - path_resolution_kind_from_module_def_if(module_def_id) - } -} - -fn path_resolution_kind_from_module_def_if(module_def_id: ModuleDefId) -> PathResolutionKind { - match module_def_id { - ModuleDefId::ModuleId(module_id) => PathResolutionKind::Module(module_id), - ModuleDefId::FunctionId(func_id) => PathResolutionKind::ModuleFunction(func_id), - ModuleDefId::TypeId(struct_id) => PathResolutionKind::Struct(struct_id, None), - ModuleDefId::TypeAliasId(type_alias_id) => { - PathResolutionKind::TypeAlias(type_alias_id, None) - } - ModuleDefId::TraitId(trait_id) => PathResolutionKind::Trait(trait_id, None), - ModuleDefId::GlobalId(global_id) => PathResolutionKind::Global(global_id), - } + Ok(PathResolution { + kind: resolved_import.path_resolution_kind, + errors: resolved_import.errors, + }) } From c3a3713ea302df1985c68df3b6dd23e777145429 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 25 Oct 2024 13:54:42 -0300 Subject: [PATCH 11/14] Some renames and simplifications --- compiler/noirc_frontend/src/elaborator/mod.rs | 6 +- .../noirc_frontend/src/elaborator/patterns.rs | 14 +-- .../noirc_frontend/src/elaborator/scope.rs | 21 ++-- .../noirc_frontend/src/elaborator/types.rs | 6 +- .../src/hir/def_collector/dc_crate.rs | 2 +- .../src/hir/resolution/import.rs | 106 +++++++++--------- .../src/hir/resolution/path_resolver.rs | 2 +- compiler/noirc_frontend/src/locations.rs | 22 ++-- tooling/lsp/src/attribute_reference_finder.rs | 24 ++-- 9 files changed, 101 insertions(+), 102 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 18535c8c35b..2a723286d8b 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -4,7 +4,7 @@ use std::{ }; use crate::{ - ast::ItemVisibility, hir::resolution::import::PathResolutionKind, + ast::ItemVisibility, hir::resolution::import::PathResolutionItem, hir_def::traits::ResolvedTraitBound, StructField, StructType, TypeBindings, }; use crate::{ @@ -668,7 +668,7 @@ impl<'context> Elaborator<'context> { pub fn resolve_module_by_path(&mut self, path: Path) -> Option { match self.resolve_path(path.clone()) { - Ok(PathResolution { kind: PathResolutionKind::Module(module_id), errors }) => { + Ok(PathResolution { item: PathResolutionItem::Module(module_id), errors }) => { if errors.is_empty() { Some(module_id) } else { @@ -681,7 +681,7 @@ impl<'context> Elaborator<'context> { fn resolve_trait_by_path(&mut self, path: Path) -> Option { let error = match self.resolve_path(path.clone()) { - Ok(PathResolution { kind: PathResolutionKind::Trait(trait_id, _generics), errors }) => { + Ok(PathResolution { item: PathResolutionItem::Trait(trait_id), errors }) => { for error in errors { self.push_err(error); } diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 5a53ddb4530..7ecf807fd4f 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, import::PathResolutionKind}, + resolution::{errors::ResolverError, import::PathResolutionItem}, type_check::{Source, TypeCheckError}, }, hir_def::{ @@ -519,10 +519,10 @@ impl<'context> Elaborator<'context> { /// solve these fn resolve_path_resolution_kind_generics( &mut self, - path_resolution_kind: PathResolutionKind, + path_resolution_kind: PathResolutionItem, ) -> Vec { match path_resolution_kind { - PathResolutionKind::StructFunction(struct_id, Some(generics), _func_id) => { + 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); @@ -533,12 +533,12 @@ impl<'context> Elaborator<'context> { generics.span, ) } - PathResolutionKind::TypeAliasFunction(_type_alias_id, Some(generics), _func_id) => { + 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() } - PathResolutionKind::TraitFunction(_trait_id, Some(generics), _func_id) => { + 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() @@ -547,7 +547,7 @@ impl<'context> Elaborator<'context> { } } - fn resolve_variable(&mut self, path: Path) -> (HirIdent, Option) { + fn resolve_variable(&mut self, path: Path) -> (HirIdent, Option) { if let Some(trait_path_resolution) = self.resolve_trait_generic_path(&path) { for error in trait_path_resolution.errors { self.push_err(error); @@ -733,7 +733,7 @@ impl<'context> Elaborator<'context> { pub fn get_ident_from_path( &mut self, path: Path, - ) -> ((HirIdent, usize), Option) { + ) -> ((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)) { diff --git a/compiler/noirc_frontend/src/elaborator/scope.rs b/compiler/noirc_frontend/src/elaborator/scope.rs index cde2100dd37..4ce21089589 100644 --- a/compiler/noirc_frontend/src/elaborator/scope.rs +++ b/compiler/noirc_frontend/src/elaborator/scope.rs @@ -2,7 +2,7 @@ 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, PathResolutionKind, 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::{ @@ -40,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)?; for error in path_resolution.errors { self.push_err(error); } - Ok(path_resolution.kind) + Ok(path_resolution.item) } pub(super) fn resolve_path(&mut self, path: Path) -> PathResolutionResult { @@ -58,9 +58,8 @@ impl<'context> Elaborator<'context> { if let Some(Type::Struct(struct_type, _)) = &self.self_type { let struct_type = struct_type.borrow(); if path.segments.len() == 1 { - // TODO: handle generics return Ok(PathResolution { - kind: PathResolutionKind::Struct(struct_type.id, None), + item: PathResolutionItem::Struct(struct_type.id), errors: Vec::new(), }); } @@ -121,7 +120,7 @@ impl<'context> Elaborator<'context> { }; self.interner.add_path_resolution_kind_reference( - path_resolution.kind.clone(), + path_resolution.item.clone(), location, is_self_type_name, ); @@ -174,7 +173,7 @@ impl<'context> Elaborator<'context> { pub(super) fn lookup_global( &mut self, path: Path, - ) -> Result<(DefinitionId, PathResolutionKind), ResolverError> { + ) -> Result<(DefinitionId, PathResolutionItem), ResolverError> { let span = path.span(); let path_resolution_kind = self.resolve_path_or_error(path)?; @@ -182,7 +181,7 @@ impl<'context> Elaborator<'context> { return Ok((self.interner.function_definition_id(function), path_resolution_kind)); } - if let PathResolutionKind::Global(global) = path_resolution_kind { + if let PathResolutionItem::Global(global) = path_resolution_kind { let global = self.interner.get_global(global); return Ok((global.definition_id, path_resolution_kind)); } @@ -233,7 +232,7 @@ impl<'context> Elaborator<'context> { let span = path.span(); match self.resolve_path_or_error(path) { Ok(path_resolution_kind) => { - if let PathResolutionKind::Trait(trait_id, _) = path_resolution_kind { + if let PathResolutionItem::Trait(trait_id) = path_resolution_kind { Some(self.get_trait_mut(trait_id)) } else { self.push_err(ResolverError::Expected { @@ -256,7 +255,7 @@ impl<'context> Elaborator<'context> { let span = path.span(); match self.resolve_path_or_error(path) { Ok(path_resolution_kind) => { - if let PathResolutionKind::Struct(struct_id, _) = path_resolution_kind { + if let PathResolutionItem::Struct(struct_id) = path_resolution_kind { Some(self.get_struct(struct_id)) } else { self.push_err(ResolverError::Expected { @@ -292,7 +291,7 @@ impl<'context> Elaborator<'context> { pub fn lookup_type_alias(&mut self, path: Path) -> Option> { match self.resolve_path_or_error(path) { - Ok(PathResolutionKind::TypeAlias(type_alias_id, _)) => { + 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 6a2bc97f8a5..2879204d3ee 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -16,7 +16,7 @@ use crate::{ def_collector::dc_crate::CompilationError, resolution::{ errors::ResolverError, - import::{PathResolutionError, PathResolutionKind}, + import::{PathResolutionError, PathResolutionItem}, }, type_check::{ generics::{Generic, TraitGenerics}, @@ -406,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(PathResolutionKind::Global(id)) => { + Ok(PathResolutionItem::Global(id)) => { if let Some(current_item) = self.current_item { self.interner.add_global_dependency(current_item, id); } @@ -566,7 +566,7 @@ 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 func_id = path_resolution.kind.function_id()?; + 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())?; 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 3dc6ebb44ec..a2fd8da376e 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -551,7 +551,7 @@ fn inject_prelude( span: Span::default(), }; - if let Ok(PathResolution { kind, errors }) = path_resolver::resolve_path( + if let Ok(PathResolution { item: kind, errors }) = path_resolver::resolve_path( &context.def_maps, ModuleId { krate: crate_id, local_id: crate_root }, None, diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index 610c0925126..7d4dba9486d 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -27,7 +27,7 @@ pub struct ImportDirective { struct NamespaceResolution { module_id: ModuleId, - path_resolution_kind: PathResolutionKind, + path_resolution_kind: PathResolutionItem, namespace: PerNs, errors: Vec, } @@ -36,30 +36,30 @@ type NamespaceResolutionResult = Result, } #[derive(Debug, Clone)] -pub enum PathResolutionKind { +pub enum PathResolutionItem { Module(ModuleId), - Struct(StructId, Option), - TypeAlias(TypeAliasId, Option), - Trait(TraitId, Option), + Struct(StructId), + TypeAlias(TypeAliasId), + Trait(TraitId), Global(GlobalId), ModuleFunction(FuncId), - StructFunction(StructId, Option, FuncId), - TypeAliasFunction(TypeAliasId, Option, FuncId), - TraitFunction(TraitId, Option, FuncId), + StructFunction(StructId, Option, FuncId), + TypeAliasFunction(TypeAliasId, Option, FuncId), + TraitFunction(TraitId, Option, FuncId), } -impl PathResolutionKind { +impl PathResolutionItem { pub fn function_id(&self) -> Option { match self { - PathResolutionKind::ModuleFunction(func_id) - | PathResolutionKind::StructFunction(_, _, func_id) - | PathResolutionKind::TypeAliasFunction(_, _, func_id) - | PathResolutionKind::TraitFunction(_, _, func_id) => Some(*func_id), + PathResolutionItem::ModuleFunction(func_id) + | PathResolutionItem::StructFunction(_, _, func_id) + | PathResolutionItem::TypeAliasFunction(_, _, func_id) + | PathResolutionItem::TraitFunction(_, _, func_id) => Some(*func_id), _ => None, } } @@ -73,30 +73,30 @@ impl PathResolutionKind { pub fn description(&self) -> &'static str { match self { - PathResolutionKind::Module(..) => "module", - PathResolutionKind::Struct(..) => "type", - PathResolutionKind::TypeAlias(..) => "type alias", - PathResolutionKind::Trait(..) => "trait", - PathResolutionKind::Global(..) => "global", - PathResolutionKind::ModuleFunction(..) - | PathResolutionKind::StructFunction(..) - | PathResolutionKind::TypeAliasFunction(..) - | PathResolutionKind::TraitFunction(..) => "function", + 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 PathResolutionGenerics { +pub struct Turbofish { pub generics: Vec, pub span: Span, } #[derive(Debug)] -enum IntermediatePathResolutionKind { +enum IntermediatePathResolutionItem { Module(ModuleId), - Struct(StructId, Option), - Trait(TraitId, Option), + Struct(StructId, Option), + Trait(TraitId, Option), } pub(crate) type PathResolutionResult = Result; @@ -119,7 +119,7 @@ pub struct ResolvedImport { pub name: Ident, // The symbol which we have resolved to pub resolved_namespace: PerNs, - pub path_resolution_kind: PathResolutionKind, + pub path_resolution_kind: PathResolutionItem, // The module which we must add the resolved namespace to pub module_scope: LocalModuleId, pub is_prelude: bool, @@ -343,14 +343,14 @@ 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 path_resolution_kind = IntermediatePathResolutionKind::Module(current_mod_id); + let mut path_resolution_kind = 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, - path_resolution_kind: PathResolutionKind::Module(current_mod_id), + path_resolution_kind: PathResolutionItem::Module(current_mod_id), namespace: PerNs::types(current_mod_id.into()), errors: Vec::new(), }); @@ -391,7 +391,7 @@ fn resolve_name_in_module( }); } - (id, IntermediatePathResolutionKind::Module(id)) + (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 @@ -402,9 +402,9 @@ fn resolve_name_in_module( ( id.module_id(), - IntermediatePathResolutionKind::Struct( + IntermediatePathResolutionItem::Struct( id, - last_segment_generics.as_ref().map(|generics| PathResolutionGenerics { + last_segment_generics.as_ref().map(|generics| Turbofish { generics: generics.clone(), span: last_segment.turbofish_span(), }), @@ -419,9 +419,9 @@ fn resolve_name_in_module( ( id.0, - IntermediatePathResolutionKind::Trait( + IntermediatePathResolutionItem::Trait( id, - last_segment_generics.as_ref().map(|generics| PathResolutionGenerics { + last_segment_generics.as_ref().map(|generics| Turbofish { generics: generics.clone(), span: last_segment.turbofish_span(), }), @@ -462,8 +462,10 @@ fn resolve_name_in_module( let module_def_id = current_ns.values.or(current_ns.types).map(|(id, _, _)| id).expect("Found empty namespace"); - let path_resolution_kind = - merge_path_resolution_kind_with_module_def_id(path_resolution_kind, module_def_id); + let path_resolution_kind = merge_intermediate_path_resolution_item_with_module_def_id( + path_resolution_kind, + module_def_id, + ); Ok(NamespaceResolution { module_id: current_mod_id, @@ -533,27 +535,25 @@ fn resolve_external_dep( ) } -fn merge_path_resolution_kind_with_module_def_id( - path_resolution_kind: IntermediatePathResolutionKind, +fn merge_intermediate_path_resolution_item_with_module_def_id( + path_resolution_kind: IntermediatePathResolutionItem, module_def_id: ModuleDefId, -) -> PathResolutionKind { +) -> PathResolutionItem { match module_def_id { - ModuleDefId::ModuleId(module_id) => PathResolutionKind::Module(module_id), - ModuleDefId::TypeId(struct_id) => PathResolutionKind::Struct(struct_id, None), - ModuleDefId::TypeAliasId(type_alias_id) => { - PathResolutionKind::TypeAlias(type_alias_id, None) - } - ModuleDefId::TraitId(trait_id) => PathResolutionKind::Trait(trait_id, None), - ModuleDefId::GlobalId(global_id) => PathResolutionKind::Global(global_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 path_resolution_kind { - IntermediatePathResolutionKind::Module(_) => { - PathResolutionKind::ModuleFunction(func_id) + IntermediatePathResolutionItem::Module(_) => { + PathResolutionItem::ModuleFunction(func_id) } - IntermediatePathResolutionKind::Struct(struct_id, generics) => { - PathResolutionKind::StructFunction(struct_id, generics, func_id) + IntermediatePathResolutionItem::Struct(struct_id, generics) => { + PathResolutionItem::StructFunction(struct_id, generics, func_id) } - IntermediatePathResolutionKind::Trait(trait_id, generics) => { - PathResolutionKind::TraitFunction(trait_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 8dffe66ce15..8ca15cf4189 100644 --- a/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs @@ -89,7 +89,7 @@ pub fn resolve_path( resolve_import(module_id.krate, &import, def_maps, usage_tracker, path_references)?; Ok(PathResolution { - kind: resolved_import.path_resolution_kind, + item: resolved_import.path_resolution_kind, errors: resolved_import.errors, }) } diff --git a/compiler/noirc_frontend/src/locations.rs b/compiler/noirc_frontend/src/locations.rs index 49ddf7b15e2..4dd699251a6 100644 --- a/compiler/noirc_frontend/src/locations.rs +++ b/compiler/noirc_frontend/src/locations.rs @@ -7,7 +7,7 @@ use crate::{ ast::{FunctionDefinition, ItemVisibility}, hir::{ def_map::{ModuleDefId, ModuleId}, - resolution::import::PathResolutionKind, + resolution::import::PathResolutionItem, }, node_interner::{ DefinitionId, FuncId, GlobalId, NodeInterner, ReferenceId, StructId, TraitId, TypeAliasId, @@ -104,30 +104,30 @@ impl NodeInterner { pub(crate) fn add_path_resolution_kind_reference( &mut self, - kind: PathResolutionKind, + kind: PathResolutionItem, location: Location, is_self_type: bool, ) { match kind { - PathResolutionKind::Module(module_id) => { + PathResolutionItem::Module(module_id) => { self.add_module_reference(module_id, location); } - PathResolutionKind::Struct(struct_id, _generics) => { + PathResolutionItem::Struct(struct_id) => { self.add_struct_reference(struct_id, location, is_self_type); } - PathResolutionKind::TypeAlias(type_alias_id, _generics) => { + PathResolutionItem::TypeAlias(type_alias_id) => { self.add_alias_reference(type_alias_id, location); } - PathResolutionKind::Trait(trait_id, _generics) => { + PathResolutionItem::Trait(trait_id) => { self.add_trait_reference(trait_id, location, is_self_type); } - PathResolutionKind::Global(global_id) => { + PathResolutionItem::Global(global_id) => { self.add_global_reference(global_id, location); } - PathResolutionKind::ModuleFunction(func_id) - | PathResolutionKind::StructFunction(_, _, func_id) - | PathResolutionKind::TypeAliasFunction(_, _, func_id) - | PathResolutionKind::TraitFunction(_, _, func_id) => { + PathResolutionItem::ModuleFunction(func_id) + | PathResolutionItem::StructFunction(_, _, func_id) + | PathResolutionItem::TypeAliasFunction(_, _, func_id) + | PathResolutionItem::TraitFunction(_, _, func_id) => { self.add_function_reference(func_id, location); } } diff --git a/tooling/lsp/src/attribute_reference_finder.rs b/tooling/lsp/src/attribute_reference_finder.rs index f9a45590186..0b988c3bc48 100644 --- a/tooling/lsp/src/attribute_reference_finder.rs +++ b/tooling/lsp/src/attribute_reference_finder.rs @@ -14,7 +14,7 @@ use noirc_frontend::{ hir::{ def_map::{CrateDefMap, LocalModuleId, ModuleId}, resolution::{ - import::PathResolutionKind, + import::PathResolutionItem, path_resolver::{PathResolver, StandardPathResolver}, }, }, @@ -107,20 +107,20 @@ impl<'a> Visitor for AttributeReferenceFinder<'a> { return; }; - self.reference_id = Some(path_resolution_kind_to_reference_id(result.kind)); + self.reference_id = Some(path_resolution_kind_to_reference_id(result.item)); } } -fn path_resolution_kind_to_reference_id(path_resolution_kind: PathResolutionKind) -> ReferenceId { +fn path_resolution_kind_to_reference_id(path_resolution_kind: PathResolutionItem) -> ReferenceId { match path_resolution_kind { - PathResolutionKind::Module(module_id) => ReferenceId::Module(module_id), - PathResolutionKind::Struct(struct_id, _) => ReferenceId::Struct(struct_id), - PathResolutionKind::TypeAlias(type_alias_id, _) => ReferenceId::Alias(type_alias_id), - PathResolutionKind::Trait(trait_id, _) => ReferenceId::Trait(trait_id), - PathResolutionKind::Global(global_id) => ReferenceId::Global(global_id), - PathResolutionKind::ModuleFunction(func_id) - | PathResolutionKind::StructFunction(_, _, func_id) - | PathResolutionKind::TypeAliasFunction(_, _, func_id) - | PathResolutionKind::TraitFunction(_, _, func_id) => ReferenceId::Function(func_id), + 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), } } From 5f790af87c912c8ffd57230005acaa5a8807809e Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 25 Oct 2024 13:59:49 -0300 Subject: [PATCH 12/14] More renames and some docs --- .../noirc_frontend/src/elaborator/patterns.rs | 34 ++++++------------- .../noirc_frontend/src/elaborator/scope.rs | 22 ++++++------ .../src/hir/resolution/import.rs | 34 +++++++++---------- .../src/hir/resolution/path_resolver.rs | 5 +-- tooling/lsp/src/attribute_reference_finder.rs | 6 ++-- 5 files changed, 42 insertions(+), 59 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 7ecf807fd4f..b45f455633b 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -468,14 +468,10 @@ impl<'context> Elaborator<'context> { let unresolved_turbofish = variable.segments.last().unwrap().generics.clone(); let span = variable.span; - let (expr, path_resolution_kind) = self.resolve_variable(variable); + let (expr, item) = self.resolve_variable(variable); let definition_id = expr.id; - let type_generics = path_resolution_kind - .map(|path_resolution_kind| { - self.resolve_path_resolution_kind_generics(path_resolution_kind) - }) - .unwrap_or_default(); + 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()); @@ -517,11 +513,8 @@ impl<'context> Elaborator<'context> { /// foo::Bar::::baz /// ^^^^^ /// solve these - fn resolve_path_resolution_kind_generics( - &mut self, - path_resolution_kind: PathResolutionItem, - ) -> Vec { - match path_resolution_kind { + 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(); @@ -567,8 +560,7 @@ impl<'context> Elaborator<'context> { // 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), path_resolution_kind) = - 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 { @@ -610,7 +602,7 @@ impl<'context> Elaborator<'context> { } } - (hir_ident, path_resolution_kind) + (hir_ident, item) } } @@ -740,20 +732,14 @@ impl<'context> Elaborator<'context> { 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, path_resolution_kind)) => { - return ( - (HirIdent::non_trait_method(id, location), 0), - Some(path_resolution_kind), - ) + Ok((id, item)) => { + return ((HirIdent::non_trait_method(id, location), 0), Some(item)) } Err(_) => error, }, None => match self.lookup_global(path) { - Ok((id, path_resolution_kind)) => { - return ( - (HirIdent::non_trait_method(id, location), 0), - Some(path_resolution_kind), - ) + Ok((id, item)) => { + return ((HirIdent::non_trait_method(id, location), 0), Some(item)) } Err(error) => error, }, diff --git a/compiler/noirc_frontend/src/elaborator/scope.rs b/compiler/noirc_frontend/src/elaborator/scope.rs index 4ce21089589..f5083ca25a6 100644 --- a/compiler/noirc_frontend/src/elaborator/scope.rs +++ b/compiler/noirc_frontend/src/elaborator/scope.rs @@ -175,15 +175,15 @@ impl<'context> Elaborator<'context> { path: Path, ) -> Result<(DefinitionId, PathResolutionItem), ResolverError> { let span = path.span(); - let path_resolution_kind = self.resolve_path_or_error(path)?; + let item = self.resolve_path_or_error(path)?; - if let Some(function) = path_resolution_kind.function_id() { - return Ok((self.interner.function_definition_id(function), path_resolution_kind)); + if let Some(function) = item.function_id() { + return Ok((self.interner.function_definition_id(function), item)); } - if let PathResolutionItem::Global(global) = path_resolution_kind { + if let PathResolutionItem::Global(global) = item { let global = self.interner.get_global(global); - return Ok((global.definition_id, path_resolution_kind)); + return Ok((global.definition_id, item)); } let expected = "global variable"; @@ -231,13 +231,13 @@ impl<'context> Elaborator<'context> { pub fn lookup_trait_or_error(&mut self, path: Path) -> Option<&mut Trait> { let span = path.span(); match self.resolve_path_or_error(path) { - Ok(path_resolution_kind) => { - if let PathResolutionItem::Trait(trait_id) = path_resolution_kind { + 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: path_resolution_kind.description(), + got: item.description(), span, }); None @@ -254,13 +254,13 @@ impl<'context> Elaborator<'context> { pub fn lookup_struct_or_error(&mut self, path: Path) -> Option> { let span = path.span(); match self.resolve_path_or_error(path) { - Ok(path_resolution_kind) => { - if let PathResolutionItem::Struct(struct_id) = path_resolution_kind { + Ok(item) => { + if let PathResolutionItem::Struct(struct_id) = item { Some(self.get_struct(struct_id)) } else { self.push_err(ResolverError::Expected { expected: "type", - got: path_resolution_kind.description(), + got: item.description(), span, }); None diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index 7d4dba9486d..58a3a841801 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -27,7 +27,7 @@ pub struct ImportDirective { struct NamespaceResolution { module_id: ModuleId, - path_resolution_kind: PathResolutionItem, + item: PathResolutionItem, namespace: PerNs, errors: Vec, } @@ -40,6 +40,9 @@ pub struct PathResolution { pub 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, Clone)] pub enum PathResolutionItem { Module(ModuleId), @@ -92,6 +95,7 @@ pub struct Turbofish { pub span: Span, } +/// Any item that can appear before the last segment in a path. #[derive(Debug)] enum IntermediatePathResolutionItem { Module(ModuleId), @@ -119,7 +123,8 @@ pub struct ResolvedImport { pub name: Ident, // The symbol which we have resolved to pub resolved_namespace: PerNs, - pub path_resolution_kind: PathResolutionItem, + // 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, @@ -164,7 +169,7 @@ pub fn resolve_import( let module_scope = import_directive.module_id; let NamespaceResolution { module_id: resolved_module, - path_resolution_kind, + item, namespace: resolved_namespace, mut errors, } = resolve_path_to_ns( @@ -199,7 +204,7 @@ pub fn resolve_import( Ok(ResolvedImport { name, resolved_namespace, - path_resolution_kind, + item, module_scope, is_prelude: import_directive.is_prelude, errors, @@ -343,14 +348,14 @@ 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 path_resolution_kind = IntermediatePathResolutionItem::Module(current_mod_id); + 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, - path_resolution_kind: PathResolutionItem::Module(current_mod_id), + item: PathResolutionItem::Module(current_mod_id), namespace: PerNs::types(current_mod_id.into()), errors: Vec::new(), }); @@ -378,7 +383,7 @@ fn resolve_name_in_module( }; // In the type namespace, only Mod can be used in a path. - (current_mod_id, path_resolution_kind) = 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)); @@ -462,17 +467,12 @@ fn resolve_name_in_module( let module_def_id = current_ns.values.or(current_ns.types).map(|(id, _, _)| id).expect("Found empty namespace"); - let path_resolution_kind = merge_intermediate_path_resolution_item_with_module_def_id( - path_resolution_kind, + let item = merge_intermediate_path_resolution_item_with_module_def_id( + intermediate_item, module_def_id, ); - Ok(NamespaceResolution { - module_id: current_mod_id, - path_resolution_kind, - namespace: current_ns, - errors, - }) + Ok(NamespaceResolution { module_id: current_mod_id, item, namespace: current_ns, errors }) } fn resolve_path_name(import_directive: &ImportDirective) -> Ident { @@ -536,7 +536,7 @@ fn resolve_external_dep( } fn merge_intermediate_path_resolution_item_with_module_def_id( - path_resolution_kind: IntermediatePathResolutionItem, + intermediate_item: IntermediatePathResolutionItem, module_def_id: ModuleDefId, ) -> PathResolutionItem { match module_def_id { @@ -545,7 +545,7 @@ fn merge_intermediate_path_resolution_item_with_module_def_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 path_resolution_kind { + ModuleDefId::FunctionId(func_id) => match intermediate_item { IntermediatePathResolutionItem::Module(_) => { PathResolutionItem::ModuleFunction(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 8ca15cf4189..705820e9101 100644 --- a/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs @@ -88,8 +88,5 @@ pub fn resolve_path( let resolved_import = resolve_import(module_id.krate, &import, def_maps, usage_tracker, path_references)?; - Ok(PathResolution { - item: resolved_import.path_resolution_kind, - errors: resolved_import.errors, - }) + Ok(PathResolution { item: resolved_import.item, errors: resolved_import.errors }) } diff --git a/tooling/lsp/src/attribute_reference_finder.rs b/tooling/lsp/src/attribute_reference_finder.rs index 0b988c3bc48..22afa086303 100644 --- a/tooling/lsp/src/attribute_reference_finder.rs +++ b/tooling/lsp/src/attribute_reference_finder.rs @@ -107,12 +107,12 @@ impl<'a> Visitor for AttributeReferenceFinder<'a> { return; }; - self.reference_id = Some(path_resolution_kind_to_reference_id(result.item)); + self.reference_id = Some(path_resolution_item_to_reference_id(result.item)); } } -fn path_resolution_kind_to_reference_id(path_resolution_kind: PathResolutionItem) -> ReferenceId { - match path_resolution_kind { +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), From ab72fc86c8d9d138d921b408e6f7694771201194 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 25 Oct 2024 14:11:15 -0300 Subject: [PATCH 13/14] One more rename --- compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 a2fd8da376e..ba6758cf49d 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -551,7 +551,7 @@ fn inject_prelude( span: Span::default(), }; - if let Ok(PathResolution { item: kind, errors }) = 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, @@ -560,7 +560,7 @@ fn inject_prelude( &mut None, ) { assert!(errors.is_empty(), "Tried to add private item to prelude"); - let module_id = kind.module_id().expect("std::prelude should be a module"); + 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 { From d2da46ea330ada914e78b82ffc0dd3ecbe99d833 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 29 Oct 2024 13:37:26 -0300 Subject: [PATCH 14/14] Fix after merge --- compiler/noirc_frontend/src/elaborator/expressions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index 91b73cb683f..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))