From a8606e536347d21fa13768354f17048c54d7d89d Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 5 Mar 2023 14:37:44 +0100 Subject: [PATCH 1/4] Re-use the resolver in InferenceContext instead of rebuilding it on every expression change --- crates/hir-def/src/body/scope.rs | 1 + crates/hir-def/src/resolver.rs | 102 +++++++++++++++++++++++++++++-- crates/hir-ty/src/infer.rs | 6 +- crates/hir-ty/src/infer/expr.rs | 14 ++--- crates/hir-ty/src/infer/pat.rs | 5 +- crates/hir-ty/src/infer/path.rs | 22 ++----- crates/hir-ty/src/tests.rs | 4 +- 7 files changed, 116 insertions(+), 38 deletions(-) diff --git a/crates/hir-def/src/body/scope.rs b/crates/hir-def/src/body/scope.rs index e7078b7953b..cab657b807e 100644 --- a/crates/hir-def/src/body/scope.rs +++ b/crates/hir-def/src/body/scope.rs @@ -66,6 +66,7 @@ pub fn label(&self, scope: ScopeId) -> Option<(LabelId, Name)> { self.scopes[scope].label.clone() } + /// Returns the scopes in ascending order. pub fn scope_chain(&self, scope: Option) -> impl Iterator + '_ { std::iter::successors(scope, move |&scope| self.scopes[scope].parent) } diff --git a/crates/hir-def/src/resolver.rs b/crates/hir-def/src/resolver.rs index 664db292a7f..620e9202aad 100644 --- a/crates/hir-def/src/resolver.rs +++ b/crates/hir-def/src/resolver.rs @@ -1,5 +1,5 @@ //! Name resolution façade. -use std::{hash::BuildHasherDefault, sync::Arc}; +use std::{fmt, hash::BuildHasherDefault, sync::Arc}; use base_db::CrateId; use hir_expand::name::{name, Name}; @@ -36,19 +36,34 @@ pub struct Resolver { module_scope: ModuleItemMap, } -#[derive(Debug, Clone)] +#[derive(Clone)] struct ModuleItemMap { def_map: Arc, module_id: LocalModuleId, } -#[derive(Debug, Clone)] +impl fmt::Debug for ModuleItemMap { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("ModuleItemMap").field("module_id", &self.module_id).finish() + } +} + +#[derive(Clone)] struct ExprScope { owner: DefWithBodyId, expr_scopes: Arc, scope_id: ScopeId, } +impl fmt::Debug for ExprScope { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("ExprScope") + .field("owner", &self.owner) + .field("scope_id", &self.scope_id) + .finish() + } +} + #[derive(Debug, Clone)] enum Scope { /// All the items and imported names of a module @@ -478,8 +493,72 @@ pub fn body_owner(&self) -> Option { _ => None, }) } + /// `expr_id` is required to be an expression id that comes after the top level expression scope in the given resolver + #[must_use] + pub fn update_to_inner_scope( + &mut self, + db: &dyn DefDatabase, + owner: DefWithBodyId, + expr_id: ExprId, + ) -> UpdateGuard { + #[inline(always)] + fn append_expr_scope( + db: &dyn DefDatabase, + resolver: &mut Resolver, + owner: DefWithBodyId, + expr_scopes: &Arc, + scope_id: ScopeId, + ) { + resolver.scopes.push(Scope::ExprScope(ExprScope { + owner, + expr_scopes: expr_scopes.clone(), + scope_id, + })); + if let Some(block) = expr_scopes.block(scope_id) { + if let Some(def_map) = db.block_def_map(block) { + let root = def_map.root(); + resolver + .scopes + .push(Scope::BlockScope(ModuleItemMap { def_map, module_id: root })); + // FIXME: This adds as many module scopes as there are blocks, but resolving in each + // already traverses all parents, so this is O(n²). I think we could only store the + // innermost module scope instead? + } + } + } + + let start = self.scopes.len(); + let innermost_scope = self.scopes().next(); + match innermost_scope { + Some(&Scope::ExprScope(ExprScope { scope_id, ref expr_scopes, owner })) => { + let expr_scopes = expr_scopes.clone(); + let scope_chain = expr_scopes + .scope_chain(expr_scopes.scope_for(expr_id)) + .take_while(|&it| it != scope_id); + for scope_id in scope_chain { + append_expr_scope(db, self, owner, &expr_scopes, scope_id); + } + } + _ => { + let expr_scopes = db.expr_scopes(owner); + let scope_chain = expr_scopes.scope_chain(expr_scopes.scope_for(expr_id)); + + for scope_id in scope_chain { + append_expr_scope(db, self, owner, &expr_scopes, scope_id); + } + } + } + self.scopes[start..].reverse(); + UpdateGuard(start) + } + + pub fn reset_to_guard(&mut self, UpdateGuard(start): UpdateGuard) { + self.scopes.truncate(start); + } } +pub struct UpdateGuard(usize); + impl Resolver { fn scopes(&self) -> impl Iterator { self.scopes.iter().rev() @@ -576,10 +655,11 @@ fn process_names(&self, acc: &mut ScopeNames, db: &dyn DefDatabase) { } } -// needs arbitrary_self_types to be a method... or maybe move to the def? pub fn resolver_for_expr(db: &dyn DefDatabase, owner: DefWithBodyId, expr_id: ExprId) -> Resolver { + let r = owner.resolver(db); let scopes = db.expr_scopes(owner); - resolver_for_scope(db, owner, scopes.scope_for(expr_id)) + let scope_id = scopes.scope_for(expr_id); + resolver_for_scope_(db, scopes, scope_id, r, owner) } pub fn resolver_for_scope( @@ -587,8 +667,18 @@ pub fn resolver_for_scope( owner: DefWithBodyId, scope_id: Option, ) -> Resolver { - let mut r = owner.resolver(db); + let r = owner.resolver(db); let scopes = db.expr_scopes(owner); + resolver_for_scope_(db, scopes, scope_id, r, owner) +} + +fn resolver_for_scope_( + db: &dyn DefDatabase, + scopes: Arc, + scope_id: Option, + mut r: Resolver, + owner: DefWithBodyId, +) -> Resolver { let scope_chain = scopes.scope_chain(scope_id).collect::>(); r.scopes.reserve(scope_chain.len()); diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index 22dcea8fcd4..56ae786193e 100644 --- a/crates/hir-ty/src/infer.rs +++ b/crates/hir-ty/src/infer.rs @@ -706,7 +706,6 @@ fn push_diagnostic(&mut self, diagnostic: InferenceDiagnostic) { } fn make_ty(&mut self, type_ref: &TypeRef) -> Ty { - // FIXME use right resolver for block let ctx = crate::lower::TyLoweringContext::new(self.db, &self.resolver); let ty = ctx.lower_ty(type_ref); let ty = self.insert_type_vars(ty); @@ -822,12 +821,11 @@ fn resolve_variant(&mut self, path: Option<&Path>, value_ns: bool) -> (Ty, Optio Some(path) => path, None => return (self.err_ty(), None), }; - let resolver = &self.resolver; let ctx = crate::lower::TyLoweringContext::new(self.db, &self.resolver); // FIXME: this should resolve assoc items as well, see this example: // https://play.rust-lang.org/?gist=087992e9e22495446c01c0d4e2d69521 let (resolution, unresolved) = if value_ns { - match resolver.resolve_path_in_value_ns(self.db.upcast(), path.mod_path()) { + match self.resolver.resolve_path_in_value_ns(self.db.upcast(), path.mod_path()) { Some(ResolveValueResult::ValueNs(value)) => match value { ValueNs::EnumVariantId(var) => { let substs = ctx.substs_from_path(path, var.into(), true); @@ -848,7 +846,7 @@ fn resolve_variant(&mut self, path: Option<&Path>, value_ns: bool) -> (Ty, Optio None => return (self.err_ty(), None), } } else { - match resolver.resolve_path_in_type_ns(self.db.upcast(), path.mod_path()) { + match self.resolver.resolve_path_in_type_ns(self.db.upcast(), path.mod_path()) { Some(it) => it, None => return (self.err_ty(), None), } diff --git a/crates/hir-ty/src/infer/expr.rs b/crates/hir-ty/src/infer/expr.rs index 8895dc095f9..13ca0534722 100644 --- a/crates/hir-ty/src/infer/expr.rs +++ b/crates/hir-ty/src/infer/expr.rs @@ -15,7 +15,6 @@ generics::TypeOrConstParamData, lang_item::LangItem, path::{GenericArg, GenericArgs}, - resolver::resolver_for_expr, ConstParamId, FieldId, ItemContainerId, Lookup, }; use hir_expand::name::{name, Name}; @@ -457,9 +456,10 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { } } Expr::Path(p) => { - // FIXME this could be more efficient... - let resolver = resolver_for_expr(self.db.upcast(), self.owner, tgt_expr); - self.infer_path(&resolver, p, tgt_expr.into()).unwrap_or_else(|| self.err_ty()) + let g = self.resolver.update_to_inner_scope(self.db.upcast(), self.owner, tgt_expr); + let ty = self.infer_path(p, tgt_expr.into()).unwrap_or_else(|| self.err_ty()); + self.resolver.reset_to_guard(g); + ty } Expr::Continue { label } => { if let None = find_continuable(&mut self.breakables, label.as_ref()) { @@ -1168,8 +1168,8 @@ fn infer_block( expected: &Expectation, ) -> Ty { let coerce_ty = expected.coercion_target_type(&mut self.table); - let old_resolver = - mem::replace(&mut self.resolver, resolver_for_expr(self.db.upcast(), self.owner, expr)); + let g = self.resolver.update_to_inner_scope(self.db.upcast(), self.owner, expr); + let (break_ty, ty) = self.with_breakable_ctx(BreakableKind::Block, Some(coerce_ty.clone()), label, |this| { for stmt in statements { @@ -1256,7 +1256,7 @@ fn infer_block( } } }); - self.resolver = old_resolver; + self.resolver.reset_to_guard(g); break_ty.unwrap_or(ty) } diff --git a/crates/hir-ty/src/infer/pat.rs b/crates/hir-ty/src/infer/pat.rs index 3d03c2a527c..a7bd009e34b 100644 --- a/crates/hir-ty/src/infer/pat.rs +++ b/crates/hir-ty/src/infer/pat.rs @@ -245,9 +245,8 @@ fn infer_pat(&mut self, pat: PatId, expected: &Ty, mut default_bm: BindingMode) self.infer_record_pat_like(p.as_deref(), &expected, default_bm, pat, subs) } Pat::Path(path) => { - // FIXME use correct resolver for the surrounding expression - let resolver = self.resolver.clone(); - self.infer_path(&resolver, path, pat.into()).unwrap_or_else(|| self.err_ty()) + // FIXME update resolver for the surrounding expression + self.infer_path(path, pat.into()).unwrap_or_else(|| self.err_ty()) } Pat::Bind { mode, name: _, subpat } => { return self.infer_bind_pat(pat, *mode, default_bm, *subpat, &expected); diff --git a/crates/hir-ty/src/infer/path.rs b/crates/hir-ty/src/infer/path.rs index b3867623f37..93dbd8b3633 100644 --- a/crates/hir-ty/src/infer/path.rs +++ b/crates/hir-ty/src/infer/path.rs @@ -3,7 +3,7 @@ use chalk_ir::cast::Cast; use hir_def::{ path::{Path, PathSegment}, - resolver::{ResolveValueResult, Resolver, TypeNs, ValueNs}, + resolver::{ResolveValueResult, TypeNs, ValueNs}, AdtId, AssocItemId, EnumVariantId, ItemContainerId, Lookup, }; use hir_expand::name::Name; @@ -21,35 +21,25 @@ use super::{ExprOrPatId, InferenceContext, TraitRef}; impl<'a> InferenceContext<'a> { - pub(super) fn infer_path( - &mut self, - resolver: &Resolver, - path: &Path, - id: ExprOrPatId, - ) -> Option { - let ty = self.resolve_value_path(resolver, path, id)?; + pub(super) fn infer_path(&mut self, path: &Path, id: ExprOrPatId) -> Option { + let ty = self.resolve_value_path(path, id)?; let ty = self.insert_type_vars(ty); let ty = self.normalize_associated_types_in(ty); Some(ty) } - fn resolve_value_path( - &mut self, - resolver: &Resolver, - path: &Path, - id: ExprOrPatId, - ) -> Option { + fn resolve_value_path(&mut self, path: &Path, id: ExprOrPatId) -> Option { let (value, self_subst) = if let Some(type_ref) = path.type_anchor() { let Some(last) = path.segments().last() else { return None }; let ty = self.make_ty(type_ref); let remaining_segments_for_ty = path.segments().take(path.segments().len() - 1); - let ctx = crate::lower::TyLoweringContext::new(self.db, resolver); + let ctx = crate::lower::TyLoweringContext::new(self.db, &self.resolver); let (ty, _) = ctx.lower_ty_relative_path(ty, None, remaining_segments_for_ty); self.resolve_ty_assoc_item(ty, last.name, id)? } else { // FIXME: report error, unresolved first path segment let value_or_partial = - resolver.resolve_path_in_value_ns(self.db.upcast(), path.mod_path())?; + self.resolver.resolve_path_in_value_ns(self.db.upcast(), path.mod_path())?; match value_or_partial { ResolveValueResult::ValueNs(it) => (it, None), diff --git a/crates/hir-ty/src/tests.rs b/crates/hir-ty/src/tests.rs index 759878b10bb..bcd63d9472a 100644 --- a/crates/hir-ty/src/tests.rs +++ b/crates/hir-ty/src/tests.rs @@ -163,7 +163,7 @@ fn check_impl(ra_fixture: &str, allow_none: bool, only_types: bool, display_sour } else { ty.display_test(&db).to_string() }; - assert_eq!(actual, expected); + assert_eq!(actual, expected, "type annotation differs at {:#?}", range.range); } } @@ -179,7 +179,7 @@ fn check_impl(ra_fixture: &str, allow_none: bool, only_types: bool, display_sour } else { ty.display_test(&db).to_string() }; - assert_eq!(actual, expected); + assert_eq!(actual, expected, "type annotation differs at {:#?}", range.range); } if let Some(expected) = adjustments.remove(&range) { let adjustments = inference_result From a51267c5e0f00f050378892d671317d912ff8257 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 5 Mar 2023 15:04:46 +0100 Subject: [PATCH 2/4] Allocate traits in scope upfront when type checking instead of recollecting them everytime --- crates/hir-def/src/resolver.rs | 9 ++++ crates/hir-ty/src/infer.rs | 14 +++++- crates/hir-ty/src/infer/expr.rs | 10 ++-- crates/hir-ty/src/infer/path.rs | 83 ++++++++++++++++----------------- 4 files changed, 66 insertions(+), 50 deletions(-) diff --git a/crates/hir-def/src/resolver.rs b/crates/hir-def/src/resolver.rs index 620e9202aad..57af92172a7 100644 --- a/crates/hir-def/src/resolver.rs +++ b/crates/hir-def/src/resolver.rs @@ -449,6 +449,15 @@ pub fn traits_in_scope(&self, db: &dyn DefDatabase) -> FxHashSet { traits } + pub fn traits_in_scope_from_block_scopes(&self) -> impl Iterator + '_ { + self.scopes() + .filter_map(|scope| match scope { + Scope::BlockScope(m) => Some(m.def_map[m.module_id].scope.traits()), + _ => None, + }) + .flatten() + } + pub fn module(&self) -> ModuleId { let (def_map, local_id) = self.item_scope(); def_map.module_id(local_id) diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index 56ae786193e..869b39ab37d 100644 --- a/crates/hir-ty/src/infer.rs +++ b/crates/hir-ty/src/infer.rs @@ -33,7 +33,7 @@ }; use hir_expand::name::{name, Name}; use la_arena::ArenaMap; -use rustc_hash::FxHashMap; +use rustc_hash::{FxHashMap, FxHashSet}; use stdx::always; use crate::{ @@ -423,6 +423,8 @@ pub(crate) struct InferenceContext<'a> { pub(crate) resolver: Resolver, table: unify::InferenceTable<'a>, trait_env: Arc, + /// The traits in scope, disregarding block modules. This is used for caching purposes. + traits_in_scope: FxHashSet, pub(crate) result: InferenceResult, /// The return type of the function being inferred, the closure or async block if we're /// currently within one. @@ -505,6 +507,7 @@ fn new( db, owner, body, + traits_in_scope: resolver.traits_in_scope(db.upcast()), resolver, diverges: Diverges::Maybe, breakables: Vec::new(), @@ -1056,6 +1059,15 @@ fn resolve_va_list(&self) -> Option { let struct_ = self.resolve_lang_item(LangItem::VaList)?.as_struct()?; Some(struct_.into()) } + + fn get_traits_in_scope(&self) -> Either, &FxHashSet> { + let mut b_traits = self.resolver.traits_in_scope_from_block_scopes().peekable(); + if b_traits.peek().is_some() { + Either::Left(self.traits_in_scope.iter().copied().chain(b_traits).collect()) + } else { + Either::Right(&self.traits_in_scope) + } + } } /// When inferring an expression, we propagate downward whatever type hint we diff --git a/crates/hir-ty/src/infer/expr.rs b/crates/hir-ty/src/infer/expr.rs index 13ca0534722..cca84488c94 100644 --- a/crates/hir-ty/src/infer/expr.rs +++ b/crates/hir-ty/src/infer/expr.rs @@ -1349,14 +1349,14 @@ fn infer_field_access(&mut self, tgt_expr: ExprId, receiver: ExprId, name: &Name None => { // no field found, let method_with_same_name_exists = { - let canonicalized_receiver = self.canonicalize(receiver_ty.clone()); - let traits_in_scope = self.resolver.traits_in_scope(self.db.upcast()); + self.get_traits_in_scope(); + let canonicalized_receiver = self.canonicalize(receiver_ty.clone()); method_resolution::lookup_method( self.db, &canonicalized_receiver.value, self.trait_env.clone(), - &traits_in_scope, + self.get_traits_in_scope().as_ref().left_or_else(|&it| it), VisibleFromModule::Filter(self.resolver.module()), name, ) @@ -1385,13 +1385,11 @@ fn infer_method_call( let receiver_ty = self.infer_expr(receiver, &Expectation::none()); let canonicalized_receiver = self.canonicalize(receiver_ty.clone()); - let traits_in_scope = self.resolver.traits_in_scope(self.db.upcast()); - let resolved = method_resolution::lookup_method( self.db, &canonicalized_receiver.value, self.trait_env.clone(), - &traits_in_scope, + self.get_traits_in_scope().as_ref().left_or_else(|&it| it), VisibleFromModule::Filter(self.resolver.module()), method_name, ); diff --git a/crates/hir-ty/src/infer/path.rs b/crates/hir-ty/src/infer/path.rs index 93dbd8b3633..d4d0d081968 100644 --- a/crates/hir-ty/src/infer/path.rs +++ b/crates/hir-ty/src/infer/path.rs @@ -205,6 +205,7 @@ fn resolve_trait_assoc_item( Some((def, Some(trait_ref.substitution))) } + // FIXME: Change sig to -> Option<(ValueNs, Substitution)>, subs aren't optional from here anymore fn resolve_ty_assoc_item( &mut self, ty: Ty, @@ -220,70 +221,66 @@ fn resolve_ty_assoc_item( } let canonical_ty = self.canonicalize(ty.clone()); - let traits_in_scope = self.resolver.traits_in_scope(self.db.upcast()); let mut not_visible = None; let res = method_resolution::iterate_method_candidates( &canonical_ty.value, self.db, self.table.trait_env.clone(), - &traits_in_scope, + self.get_traits_in_scope().as_ref().left_or_else(|&it| it), VisibleFromModule::Filter(self.resolver.module()), Some(name), method_resolution::LookupMode::Path, |_ty, item, visible| { - let (def, container) = match item { - AssocItemId::FunctionId(f) => { - (ValueNs::FunctionId(f), f.lookup(self.db.upcast()).container) - } - AssocItemId::ConstId(c) => { - (ValueNs::ConstId(c), c.lookup(self.db.upcast()).container) - } - AssocItemId::TypeAliasId(_) => unreachable!(), - }; - let substs = match container { - ItemContainerId::ImplId(impl_id) => { - let impl_substs = TyBuilder::subst_for_def(self.db, impl_id, None) - .fill_with_inference_vars(&mut self.table) - .build(); - let impl_self_ty = - self.db.impl_self_ty(impl_id).substitute(Interner, &impl_substs); - self.unify(&impl_self_ty, &ty); - impl_substs - } - ItemContainerId::TraitId(trait_) => { - // we're picking this method - let trait_ref = TyBuilder::trait_ref(self.db, trait_) - .push(ty.clone()) - .fill_with_inference_vars(&mut self.table) - .build(); - self.push_obligation(trait_ref.clone().cast(Interner)); - trait_ref.substitution - } - ItemContainerId::ModuleId(_) | ItemContainerId::ExternBlockId(_) => { - never!("assoc item contained in module/extern block"); - return None; - } - }; - if visible { - Some((def, item, Some(substs), true)) + Some((item, true)) } else { if not_visible.is_none() { - not_visible = Some((def, item, Some(substs), false)); + not_visible = Some((item, false)); } None } }, ); let res = res.or(not_visible); - if let Some((_, item, Some(ref substs), visible)) = res { - self.write_assoc_resolution(id, item, substs.clone()); - if !visible { - self.push_diagnostic(InferenceDiagnostic::PrivateAssocItem { id, item }) + let (item, visible) = res?; + + let (def, container) = match item { + AssocItemId::FunctionId(f) => { + (ValueNs::FunctionId(f), f.lookup(self.db.upcast()).container) } + AssocItemId::ConstId(c) => (ValueNs::ConstId(c), c.lookup(self.db.upcast()).container), + AssocItemId::TypeAliasId(_) => unreachable!(), + }; + let substs = match container { + ItemContainerId::ImplId(impl_id) => { + let impl_substs = TyBuilder::subst_for_def(self.db, impl_id, None) + .fill_with_inference_vars(&mut self.table) + .build(); + let impl_self_ty = self.db.impl_self_ty(impl_id).substitute(Interner, &impl_substs); + self.unify(&impl_self_ty, &ty); + impl_substs + } + ItemContainerId::TraitId(trait_) => { + // we're picking this method + let trait_ref = TyBuilder::trait_ref(self.db, trait_) + .push(ty.clone()) + .fill_with_inference_vars(&mut self.table) + .build(); + self.push_obligation(trait_ref.clone().cast(Interner)); + trait_ref.substitution + } + ItemContainerId::ModuleId(_) | ItemContainerId::ExternBlockId(_) => { + never!("assoc item contained in module/extern block"); + return None; + } + }; + + self.write_assoc_resolution(id, item, substs.clone()); + if !visible { + self.push_diagnostic(InferenceDiagnostic::PrivateAssocItem { id, item }); } - res.map(|(def, _, substs, _)| (def, substs)) + Some((def, Some(substs))) } fn resolve_enum_variant_on_ty( From 27fad2ad751bc813a98db18d8c31ea3cb178d228 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 5 Mar 2023 15:42:40 +0100 Subject: [PATCH 3/4] Lift segment check out of the loop in resolve_path_in_value_ns --- crates/hir-def/src/resolver.rs | 102 ++++++++++++++++++--------------- 1 file changed, 57 insertions(+), 45 deletions(-) diff --git a/crates/hir-def/src/resolver.rs b/crates/hir-def/src/resolver.rs index 57af92172a7..eea837ddd23 100644 --- a/crates/hir-def/src/resolver.rs +++ b/crates/hir-def/src/resolver.rs @@ -255,55 +255,67 @@ pub fn resolve_path_in_value_ns( return self.module_scope.resolve_path_in_value_ns(db, path); } - for scope in self.scopes() { - match scope { - Scope::ExprScope(_) if n_segments > 1 => continue, - Scope::ExprScope(scope) => { - let entry = scope - .expr_scopes - .entries(scope.scope_id) - .iter() - .find(|entry| entry.name() == first_name); + if n_segments <= 1 { + for scope in self.scopes() { + match scope { + Scope::ExprScope(scope) => { + let entry = scope + .expr_scopes + .entries(scope.scope_id) + .iter() + .find(|entry| entry.name() == first_name); - if let Some(e) = entry { - return Some(ResolveValueResult::ValueNs(ValueNs::LocalBinding(e.pat()))); + if let Some(e) = entry { + return Some(ResolveValueResult::ValueNs(ValueNs::LocalBinding( + e.pat(), + ))); + } + } + Scope::GenericParams { params, def } => { + if let Some(id) = params.find_const_by_name(first_name, *def) { + let val = ValueNs::GenericParam(id); + return Some(ResolveValueResult::ValueNs(val)); + } + } + &Scope::ImplDefScope(impl_) => { + if first_name == &name![Self] { + return Some(ResolveValueResult::ValueNs(ValueNs::ImplSelf(impl_))); + } + } + // bare `Self` doesn't work in the value namespace in a struct/enum definition + Scope::AdtScope(_) => continue, + Scope::BlockScope(m) => { + if let Some(def) = m.resolve_path_in_value_ns(db, path) { + return Some(def); + } } } - Scope::GenericParams { params, def } if n_segments > 1 => { - if let Some(id) = params.find_type_by_name(first_name, *def) { - let ty = TypeNs::GenericParam(id); - return Some(ResolveValueResult::Partial(ty, 1)); + } + } else { + for scope in self.scopes() { + match scope { + Scope::ExprScope(_) => continue, + Scope::GenericParams { params, def } => { + if let Some(id) = params.find_type_by_name(first_name, *def) { + let ty = TypeNs::GenericParam(id); + return Some(ResolveValueResult::Partial(ty, 1)); + } } - } - Scope::GenericParams { .. } if n_segments != 1 => continue, - Scope::GenericParams { params, def } => { - if let Some(id) = params.find_const_by_name(first_name, *def) { - let val = ValueNs::GenericParam(id); - return Some(ResolveValueResult::ValueNs(val)); + &Scope::ImplDefScope(impl_) => { + if first_name == &name![Self] { + return Some(ResolveValueResult::Partial(TypeNs::SelfType(impl_), 1)); + } } - } - - &Scope::ImplDefScope(impl_) => { - if first_name == &name![Self] { - return Some(if n_segments > 1 { - ResolveValueResult::Partial(TypeNs::SelfType(impl_), 1) - } else { - ResolveValueResult::ValueNs(ValueNs::ImplSelf(impl_)) - }); + Scope::AdtScope(adt) => { + if first_name == &name![Self] { + let ty = TypeNs::AdtSelfType(*adt); + return Some(ResolveValueResult::Partial(ty, 1)); + } } - } - // bare `Self` doesn't work in the value namespace in a struct/enum definition - Scope::AdtScope(_) if n_segments == 1 => continue, - Scope::AdtScope(adt) => { - if first_name == &name![Self] { - let ty = TypeNs::AdtSelfType(*adt); - return Some(ResolveValueResult::Partial(ty, 1)); - } - } - - Scope::BlockScope(m) => { - if let Some(def) = m.resolve_path_in_value_ns(db, path) { - return Some(def); + Scope::BlockScope(m) => { + if let Some(def) = m.resolve_path_in_value_ns(db, path) { + return Some(def); + } } } } @@ -316,8 +328,8 @@ pub fn resolve_path_in_value_ns( // If a path of the shape `u16::from_le_bytes` failed to resolve at all, then we fall back // to resolving to the primitive type, to allow this to still work in the presence of // `use core::u16;`. - if path.kind == PathKind::Plain && path.segments().len() > 1 { - if let Some(builtin) = BuiltinType::by_name(&path.segments()[0]) { + if path.kind == PathKind::Plain && n_segments > 1 { + if let Some(builtin) = BuiltinType::by_name(first_name) { return Some(ResolveValueResult::Partial(TypeNs::BuiltinType(builtin), 1)); } } From d8b1ec6a25d800691da50fa7834f8db2a2f124b6 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 5 Mar 2023 15:43:02 +0100 Subject: [PATCH 4/4] Remove unnecessary option wrapping --- crates/hir-ty/src/infer/path.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/crates/hir-ty/src/infer/path.rs b/crates/hir-ty/src/infer/path.rs index d4d0d081968..891e1fab2ed 100644 --- a/crates/hir-ty/src/infer/path.rs +++ b/crates/hir-ty/src/infer/path.rs @@ -35,7 +35,7 @@ fn resolve_value_path(&mut self, path: &Path, id: ExprOrPatId) -> Option { let remaining_segments_for_ty = path.segments().take(path.segments().len() - 1); let ctx = crate::lower::TyLoweringContext::new(self.db, &self.resolver); let (ty, _) = ctx.lower_ty_relative_path(ty, None, remaining_segments_for_ty); - self.resolve_ty_assoc_item(ty, last.name, id)? + self.resolve_ty_assoc_item(ty, last.name, id).map(|(it, substs)| (it, Some(substs)))? } else { // FIXME: report error, unresolved first path segment let value_or_partial = @@ -43,9 +43,9 @@ fn resolve_value_path(&mut self, path: &Path, id: ExprOrPatId) -> Option { match value_or_partial { ResolveValueResult::ValueNs(it) => (it, None), - ResolveValueResult::Partial(def, remaining_index) => { - self.resolve_assoc_item(def, path, remaining_index, id)? - } + ResolveValueResult::Partial(def, remaining_index) => self + .resolve_assoc_item(def, path, remaining_index, id) + .map(|(it, substs)| (it, Some(substs)))?, } }; @@ -113,7 +113,7 @@ fn resolve_assoc_item( path: &Path, remaining_index: usize, id: ExprOrPatId, - ) -> Option<(ValueNs, Option)> { + ) -> Option<(ValueNs, Substitution)> { assert!(remaining_index < path.segments().len()); // there may be more intermediate segments between the resolved one and // the end. Only the last segment needs to be resolved to a value; from @@ -166,7 +166,7 @@ fn resolve_trait_assoc_item( trait_ref: TraitRef, segment: PathSegment<'_>, id: ExprOrPatId, - ) -> Option<(ValueNs, Option)> { + ) -> Option<(ValueNs, Substitution)> { let trait_ = trait_ref.hir_trait_id(); let item = self.db.trait_data(trait_).items.iter().map(|(_name, id)| (*id)).find_map(|item| { @@ -202,16 +202,15 @@ fn resolve_trait_assoc_item( }; self.write_assoc_resolution(id, item, trait_ref.substitution.clone()); - Some((def, Some(trait_ref.substitution))) + Some((def, trait_ref.substitution)) } - // FIXME: Change sig to -> Option<(ValueNs, Substitution)>, subs aren't optional from here anymore fn resolve_ty_assoc_item( &mut self, ty: Ty, name: &Name, id: ExprOrPatId, - ) -> Option<(ValueNs, Option)> { + ) -> Option<(ValueNs, Substitution)> { if let TyKind::Error = ty.kind(Interner) { return None; } @@ -280,7 +279,7 @@ fn resolve_ty_assoc_item( if !visible { self.push_diagnostic(InferenceDiagnostic::PrivateAssocItem { id, item }); } - Some((def, Some(substs))) + Some((def, substs)) } fn resolve_enum_variant_on_ty( @@ -288,7 +287,7 @@ fn resolve_enum_variant_on_ty( ty: &Ty, name: &Name, id: ExprOrPatId, - ) -> Option<(ValueNs, Option)> { + ) -> Option<(ValueNs, Substitution)> { let ty = self.resolve_ty_shallow(ty); let (enum_id, subst) = match ty.as_adt() { Some((AdtId::EnumId(e), subst)) => (e, subst), @@ -298,6 +297,6 @@ fn resolve_enum_variant_on_ty( let local_id = enum_data.variant(name)?; let variant = EnumVariantId { parent: enum_id, local_id }; self.write_variant_resolution(id, variant.into()); - Some((ValueNs::EnumVariantId(variant), Some(subst.clone()))) + Some((ValueNs::EnumVariantId(variant), subst.clone())) } }