From 4ed5fe1554ad0133b7eaec0c93c72a69b2aab271 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Thu, 3 Feb 2022 12:43:15 +0100 Subject: [PATCH] Fix assoc type shorthand from method bounds In code like this: ```rust impl Option { fn as_deref(&self) -> T::Target where T: Deref {} } ``` when trying to resolve the associated type `T::Target`, we were only looking at the bounds on the impl (where the type parameter is defined), but the method can add additional bounds that can also be used to refer to associated types. Hence, when resolving such an associated type, it's not enough to just know the type parameter T, we also need to know exactly where we are currently. This fixes #11364 (beta apparently switched some bounds around). --- crates/hir/src/display.rs | 2 +- crates/hir/src/lib.rs | 5 +- crates/hir/src/semantics.rs | 32 +++--- crates/hir/src/source_analyzer.rs | 12 ++- crates/hir_ty/src/db.rs | 1 + crates/hir_ty/src/lower.rs | 99 ++++++++++--------- crates/hir_ty/src/tests/traits.rs | 19 ++++ crates/hir_ty/src/utils.rs | 2 +- .../src/completions/qualified_path.rs | 2 +- 9 files changed, 107 insertions(+), 67 deletions(-) diff --git a/crates/hir/src/display.rs b/crates/hir/src/display.rs index 46815cff864..d94f55e3262 100644 --- a/crates/hir/src/display.rs +++ b/crates/hir/src/display.rs @@ -239,7 +239,7 @@ impl HirDisplay for TypeParam { return Ok(()); } - let bounds = f.db.generic_predicates_for_param(self.id, None); + let bounds = f.db.generic_predicates_for_param(self.id.parent, self.id, None); let substs = TyBuilder::type_params_subst(f.db, self.id.parent); let predicates: Vec<_> = bounds.iter().cloned().map(|b| b.substitute(Interner, &substs)).collect(); diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index e091af4bf8c..918cadc8696 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -2235,8 +2235,11 @@ impl TypeParam { Type::new_with_resolver_inner(db, krate, &resolver, ty) } + /// FIXME: this only lists trait bounds from the item defining the type + /// parameter, not additional bounds that might be added e.g. by a method if + /// the parameter comes from an impl! pub fn trait_bounds(self, db: &dyn HirDatabase) -> Vec { - db.generic_predicates_for_param(self.id, None) + db.generic_predicates_for_param(self.id.parent, self.id, None) .iter() .filter_map(|pred| match &pred.skip_binders().skip_binders() { hir_ty::WhereClause::Implemented(trait_ref) => { diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs index e3ff05b49af..9ac88e260c4 100644 --- a/crates/hir/src/semantics.rs +++ b/crates/hir/src/semantics.rs @@ -12,7 +12,7 @@ use hir_def::{ AsMacroCall, FunctionId, TraitId, VariantId, }; use hir_expand::{name::AsName, ExpansionInfo, MacroCallId}; -use hir_ty::{associated_type_shorthand_candidates, Interner}; +use hir_ty::Interner; use itertools::Itertools; use rustc_hash::{FxHashMap, FxHashSet}; use smallvec::{smallvec, SmallVec}; @@ -50,7 +50,7 @@ pub enum PathResolution { } impl PathResolution { - fn in_type_ns(&self) -> Option { + pub(crate) fn in_type_ns(&self) -> Option { match self { PathResolution::Def(ModuleDef::Adt(adt)) => Some(TypeNs::AdtId((*adt).into())), PathResolution::Def(ModuleDef::BuiltinType(builtin)) => { @@ -80,18 +80,6 @@ impl PathResolution { } } } - - /// Returns an iterator over associated types that may be specified after this path (using - /// `Ty::Assoc` syntax). - pub fn assoc_type_shorthand_candidates( - &self, - db: &dyn HirDatabase, - mut cb: impl FnMut(&Name, TypeAlias) -> Option, - ) -> Option { - associated_type_shorthand_candidates(db, self.in_type_ns()?, |name, _, id| { - cb(name, id.into()) - }) - } } #[derive(Debug)] @@ -1314,4 +1302,20 @@ impl<'a> SemanticsScope<'a> { let path = Path::from_src(path.clone(), &ctx)?; resolve_hir_path(self.db, &self.resolver, &path) } + + /// Iterates over associated types that may be specified after the given path (using + /// `Ty::Assoc` syntax). + pub fn assoc_type_shorthand_candidates( + &self, + resolution: &PathResolution, + mut cb: impl FnMut(&Name, TypeAlias) -> Option, + ) -> Option { + let def = self.resolver.generic_def()?; + hir_ty::associated_type_shorthand_candidates( + self.db, + def, + resolution.in_type_ns()?, + |name, _, id| cb(name, id.into()), + ) + } } diff --git a/crates/hir/src/source_analyzer.rs b/crates/hir/src/source_analyzer.rs index 869f4a10f84..03e4420985e 100644 --- a/crates/hir/src/source_analyzer.rs +++ b/crates/hir/src/source_analyzer.rs @@ -619,9 +619,15 @@ fn resolve_hir_path_( TypeNs::TraitId(it) => PathResolution::Def(Trait::from(it).into()), }; match unresolved { - Some(unresolved) => res - .assoc_type_shorthand_candidates(db, |name, alias| { - (name == unresolved.name).then(|| alias) + Some(unresolved) => resolver + .generic_def() + .and_then(|def| { + hir_ty::associated_type_shorthand_candidates( + db, + def, + res.in_type_ns()?, + |name, _, id| (name == unresolved.name).then(|| id), + ) }) .map(TypeAlias::from) .map(Into::into) diff --git a/crates/hir_ty/src/db.rs b/crates/hir_ty/src/db.rs index 2583227ff40..c3b9255baf4 100644 --- a/crates/hir_ty/src/db.rs +++ b/crates/hir_ty/src/db.rs @@ -60,6 +60,7 @@ pub trait HirDatabase: DefDatabase + Upcast { #[salsa::cycle(crate::lower::generic_predicates_for_param_recover)] fn generic_predicates_for_param( &self, + def: GenericDefId, param_id: TypeParamId, assoc_name: Option, ) -> Arc<[Binders]>; diff --git a/crates/hir_ty/src/lower.rs b/crates/hir_ty/src/lower.rs index a450f6c75cc..55b1a67ea7b 100644 --- a/crates/hir_ty/src/lower.rs +++ b/crates/hir_ty/src/lower.rs @@ -530,49 +530,50 @@ impl<'a> TyLoweringContext<'a> { } fn select_associated_type(&self, res: Option, segment: PathSegment<'_>) -> Ty { - if let Some(res) = res { - let ty = named_associated_type_shorthand_candidates( - self.db, - res, - Some(segment.name.clone()), - move |name, t, associated_ty| { - if name == segment.name { - let substs = match self.type_param_mode { - TypeParamLoweringMode::Placeholder => { - // if we're lowering to placeholders, we have to put - // them in now - let generics = generics( - self.db.upcast(), - self.resolver.generic_def().expect( - "there should be generics if there's a generic param", - ), - ); - let s = generics.type_params_subst(self.db); - s.apply(t.substitution.clone(), Interner) - } - TypeParamLoweringMode::Variable => t.substitution.clone(), - }; - // We need to shift in the bound vars, since - // associated_type_shorthand_candidates does not do that - let substs = substs.shifted_in_from(Interner, self.in_binders); - // FIXME handle type parameters on the segment - Some( - TyKind::Alias(AliasTy::Projection(ProjectionTy { - associated_ty_id: to_assoc_type_id(associated_ty), - substitution: substs, - })) - .intern(Interner), - ) - } else { - None - } - }, - ); + let (def, res) = match (self.resolver.generic_def(), res) { + (Some(def), Some(res)) => (def, res), + _ => return TyKind::Error.intern(Interner), + }; + let ty = named_associated_type_shorthand_candidates( + self.db, + def, + res, + Some(segment.name.clone()), + move |name, t, associated_ty| { + if name == segment.name { + let substs = match self.type_param_mode { + TypeParamLoweringMode::Placeholder => { + // if we're lowering to placeholders, we have to put + // them in now + let generics = generics( + self.db.upcast(), + self.resolver + .generic_def() + .expect("there should be generics if there's a generic param"), + ); + let s = generics.type_params_subst(self.db); + s.apply(t.substitution.clone(), Interner) + } + TypeParamLoweringMode::Variable => t.substitution.clone(), + }; + // We need to shift in the bound vars, since + // associated_type_shorthand_candidates does not do that + let substs = substs.shifted_in_from(Interner, self.in_binders); + // FIXME handle type parameters on the segment + Some( + TyKind::Alias(AliasTy::Projection(ProjectionTy { + associated_ty_id: to_assoc_type_id(associated_ty), + substitution: substs, + })) + .intern(Interner), + ) + } else { + None + } + }, + ); - ty.unwrap_or_else(|| TyKind::Error.intern(Interner)) - } else { - TyKind::Error.intern(Interner) - } + ty.unwrap_or_else(|| TyKind::Error.intern(Interner)) } fn lower_path_inner( @@ -934,14 +935,18 @@ pub fn callable_item_sig(db: &dyn HirDatabase, def: CallableDefId) -> PolyFnSig pub fn associated_type_shorthand_candidates( db: &dyn HirDatabase, + def: GenericDefId, res: TypeNs, cb: impl FnMut(&Name, &TraitRef, TypeAliasId) -> Option, ) -> Option { - named_associated_type_shorthand_candidates(db, res, None, cb) + named_associated_type_shorthand_candidates(db, def, res, None, cb) } fn named_associated_type_shorthand_candidates( db: &dyn HirDatabase, + // If the type parameter is defined in an impl and we're in a method, there + // might be additional where clauses to consider + def: GenericDefId, res: TypeNs, assoc_name: Option, mut cb: impl FnMut(&Name, &TraitRef, TypeAliasId) -> Option, @@ -968,7 +973,7 @@ fn named_associated_type_shorthand_candidates( db.impl_trait(impl_id)?.into_value_and_skipped_binders().0, ), TypeNs::GenericParam(param_id) => { - let predicates = db.generic_predicates_for_param(param_id, assoc_name); + let predicates = db.generic_predicates_for_param(def, param_id, assoc_name); let res = predicates.iter().find_map(|pred| match pred.skip_binders().skip_binders() { // FIXME: how to correctly handle higher-ranked bounds here? WhereClause::Implemented(tr) => search( @@ -1030,13 +1035,14 @@ pub(crate) fn field_types_query( /// these are fine: `T: Foo, U: Foo<()>`. pub(crate) fn generic_predicates_for_param_query( db: &dyn HirDatabase, + def: GenericDefId, param_id: TypeParamId, assoc_name: Option, ) -> Arc<[Binders]> { - let resolver = param_id.parent.resolver(db.upcast()); + let resolver = def.resolver(db.upcast()); let ctx = TyLoweringContext::new(db, &resolver).with_type_param_mode(TypeParamLoweringMode::Variable); - let generics = generics(db.upcast(), param_id.parent); + let generics = generics(db.upcast(), def); let mut predicates: Vec<_> = resolver .where_predicates_in_scope() // we have to filter out all other predicates *first*, before attempting to lower them @@ -1098,6 +1104,7 @@ pub(crate) fn generic_predicates_for_param_query( pub(crate) fn generic_predicates_for_param_recover( _db: &dyn HirDatabase, _cycle: &[String], + _def: &GenericDefId, _param_id: &TypeParamId, _assoc_name: &Option, ) -> Arc<[Binders]> { diff --git a/crates/hir_ty/src/tests/traits.rs b/crates/hir_ty/src/tests/traits.rs index 9531be760e9..c2669646e22 100644 --- a/crates/hir_ty/src/tests/traits.rs +++ b/crates/hir_ty/src/tests/traits.rs @@ -392,6 +392,25 @@ fn test() { ); } +#[test] +fn associated_type_shorthand_from_method_bound() { + check_types( + r#" +trait Iterable { + type Item; +} +struct S; +impl S { + fn foo(self) -> T::Item where T: Iterable { loop {} } +} +fn test() { + let s: S; + s.foo(); + // ^^^^^^^ Iterable::Item +}"#, + ); +} + #[test] fn infer_associated_type_bound() { check_types( diff --git a/crates/hir_ty/src/utils.rs b/crates/hir_ty/src/utils.rs index 4e16cabdfcd..c5646e08e6c 100644 --- a/crates/hir_ty/src/utils.rs +++ b/crates/hir_ty/src/utils.rs @@ -83,7 +83,7 @@ fn direct_super_trait_refs(db: &dyn HirDatabase, trait_ref: &TraitRef) -> Vec TypeParamId { parent: trait_ref.hir_trait_id().into(), local_id: p }, None => return Vec::new(), }; - db.generic_predicates_for_param(trait_self, None) + db.generic_predicates_for_param(trait_self.parent, trait_self, None) .iter() .filter_map(|pred| { pred.as_ref().filter_map(|pred| match pred.skip_binders() { diff --git a/crates/ide_completion/src/completions/qualified_path.rs b/crates/ide_completion/src/completions/qualified_path.rs index cd1022b2e35..2c6899ff0cd 100644 --- a/crates/ide_completion/src/completions/qualified_path.rs +++ b/crates/ide_completion/src/completions/qualified_path.rs @@ -119,7 +119,7 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon if !matches!(kind, Some(PathKind::Pat)) { // Add associated types on type parameters and `Self`. - resolution.assoc_type_shorthand_candidates(ctx.db, |_, alias| { + ctx.scope.assoc_type_shorthand_candidates(&resolution, |_, alias| { acc.add_type_alias(ctx, alias); None::<()> });