Merge #8186

8186: Lower traits to `TraitRef` instead of `TypeRef` r=matklad a=Veykril



Co-authored-by: Lukas Wirth <lukastw97@gmail.com>

crates/hir/src/lib.rs

@@ -44,6 +44,7 @@ use hir_def::{
     per_ns::PerNs,
     resolver::{HasResolver, Resolver},
     src::HasSource as _,
+    type_ref::TraitRef,
     AdtId, AssocContainerId, AssocItemId, AssocItemLoc, AttrDefId, ConstId, ConstParamId,
     DefWithBodyId, EnumId, FunctionId, GenericDefId, HasModule, ImplId, LifetimeParamId,
     LocalEnumVariantId, LocalFieldId, Lookup, ModuleId, StaticId, StructId, TraitId, TypeAliasId,
@@ -1573,9 +1574,9 @@ impl Impl {
         };
 
         let filter = |impl_def: &Impl| {
-            let target_ty = impl_def.target_ty(db);
+            let self_ty = impl_def.self_ty(db);
-            let rref = target_ty.remove_ref();
+            let rref = self_ty.remove_ref();
-            ty.equals_ctor(rref.as_ref().map_or(&target_ty.ty, |it| &it.ty))
+            ty.equals_ctor(rref.as_ref().map_or(&self_ty.ty, |it| &it.ty))
         };
 
         let mut all = Vec::new();
@@ -1613,16 +1614,16 @@ impl Impl {
 
     // FIXME: the return type is wrong. This should be a hir version of
     // `TraitRef` (ie, resolved `TypeRef`).
-    pub fn target_trait(self, db: &dyn HirDatabase) -> Option<TypeRef> {
+    pub fn trait_(self, db: &dyn HirDatabase) -> Option<TraitRef> {
         db.impl_data(self.id).target_trait.clone()
     }
 
-    pub fn target_ty(self, db: &dyn HirDatabase) -> Type {
+    pub fn self_ty(self, db: &dyn HirDatabase) -> Type {
         let impl_data = db.impl_data(self.id);
         let resolver = self.id.resolver(db.upcast());
         let krate = self.id.lookup(db.upcast()).container.krate();
         let ctx = hir_ty::TyLoweringContext::new(db, &resolver);
-        let ty = ctx.lower_ty(&impl_data.target_type);
+        let ty = ctx.lower_ty(&impl_data.self_ty);
         Type::new_with_resolver_inner(db, krate, &resolver, ty)
     }
 

crates/hir_def/src/data.rs

@@ -10,7 +10,7 @@ use crate::{
     body::Expander,
     db::DefDatabase,
     item_tree::{AssocItem, FunctionQualifier, ItemTreeId, ModItem, Param},
-    type_ref::{TypeBound, TypeRef},
+    type_ref::{TraitRef, TypeBound, TypeRef},
     visibility::RawVisibility,
     AssocContainerId, AssocItemId, ConstId, ConstLoc, FunctionId, FunctionLoc, HasModule, ImplId,
     Intern, Lookup, ModuleId, StaticId, TraitId, TypeAliasId, TypeAliasLoc,
@@ -156,8 +156,8 @@ impl TraitData {
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct ImplData {
-    pub target_trait: Option<TypeRef>,
+    pub target_trait: Option<TraitRef>,
-    pub target_type: TypeRef,
+    pub self_ty: TypeRef,
     pub items: Vec<AssocItemId>,
     pub is_negative: bool,
 }
@@ -170,7 +170,7 @@ impl ImplData {
         let item_tree = impl_loc.id.item_tree(db);
         let impl_def = &item_tree[impl_loc.id.value];
         let target_trait = impl_def.target_trait.map(|id| item_tree[id].clone());
-        let target_type = item_tree[impl_def.target_type].clone();
+        let self_ty = item_tree[impl_def.self_ty].clone();
         let is_negative = impl_def.is_negative;
         let module_id = impl_loc.container;
         let container = AssocContainerId::ImplId(id);
@@ -187,7 +187,7 @@ impl ImplData {
         );
         let items = items.into_iter().map(|(_, item)| item).collect();
 
-        Arc::new(ImplData { target_trait, target_type, items, is_negative })
+        Arc::new(ImplData { target_trait, self_ty, items, is_negative })
     }
 }
 

crates/hir_def/src/item_tree.rs

@@ -31,7 +31,7 @@ use crate::{
     db::DefDatabase,
     generics::GenericParams,
     path::{path, AssociatedTypeBinding, GenericArgs, ImportAlias, ModPath, Path, PathKind},
-    type_ref::{Mutability, TypeBound, TypeRef},
+    type_ref::{Mutability, TraitRef, TypeBound, TypeRef},
     visibility::RawVisibility,
 };
 
@@ -147,6 +147,7 @@ impl ItemTree {
                 vis,
                 generics,
                 type_refs,
+                trait_refs,
                 inner_items,
             } = &mut **data;
 
@@ -173,6 +174,7 @@ impl ItemTree {
             generics.arena.shrink_to_fit();
             type_refs.arena.shrink_to_fit();
             type_refs.map.shrink_to_fit();
+            trait_refs.map.shrink_to_fit();
 
             inner_items.shrink_to_fit();
         }
@@ -295,6 +297,32 @@ impl TypeRefStorage {
     }
 }
 
+/// `TraitRef` interner.
+#[derive(Default, Debug, Eq, PartialEq)]
+struct TraitRefStorage {
+    arena: Arena<Arc<TraitRef>>,
+    map: FxHashMap<Arc<TraitRef>, Idx<Arc<TraitRef>>>,
+}
+
+impl TraitRefStorage {
+    // Note: We lie about the `Idx<TraitRef>` to hide the interner details.
+
+    fn intern(&mut self, ty: TraitRef) -> Idx<TraitRef> {
+        if let Some(id) = self.map.get(&ty) {
+            return Idx::from_raw(id.into_raw());
+        }
+
+        let ty = Arc::new(ty);
+        let idx = self.arena.alloc(ty.clone());
+        self.map.insert(ty, idx);
+        Idx::from_raw(idx.into_raw())
+    }
+
+    fn lookup(&self, id: Idx<TraitRef>) -> &TraitRef {
+        &self.arena[Idx::from_raw(id.into_raw())]
+    }
+}
+
 #[derive(Default, Debug, Eq, PartialEq)]
 struct ItemTreeData {
     imports: Arena<Import>,
@@ -319,6 +347,7 @@ struct ItemTreeData {
     vis: ItemVisibilities,
     generics: GenericParamsStorage,
     type_refs: TypeRefStorage,
+    trait_refs: TraitRefStorage,
 
     inner_items: FxHashMap<FileAstId<ast::BlockExpr>, SmallVec<[ModItem; 1]>>,
 }
@@ -556,6 +585,14 @@ impl Index<Idx<TypeRef>> for ItemTree {
     }
 }
 
+impl Index<Idx<TraitRef>> for ItemTree {
+    type Output = TraitRef;
+
+    fn index(&self, id: Idx<TraitRef>) -> &Self::Output {
+        self.data().trait_refs.lookup(id)
+    }
+}
+
 impl<N: ItemTreeNode> Index<FileItemTreeId<N>> for ItemTree {
     type Output = N;
     fn index(&self, id: FileItemTreeId<N>) -> &N {
@@ -692,8 +729,8 @@ pub struct Trait {
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct Impl {
     pub generic_params: GenericParamsId,
-    pub target_trait: Option<Idx<TypeRef>>,
+    pub target_trait: Option<Idx<TraitRef>>,
-    pub target_type: Idx<TypeRef>,
+    pub self_ty: Idx<TypeRef>,
     pub is_negative: bool,
     pub items: Box<[AssocItem]>,
     pub ast_id: FileAstId<ast::Impl>,

crates/hir_def/src/item_tree/lower.rs

@@ -11,7 +11,7 @@ use syntax::{
 
 use crate::{
     generics::{GenericParams, TypeParamData, TypeParamProvenance},
-    type_ref::LifetimeRef,
+    type_ref::{LifetimeRef, TraitRef},
 };
 
 use super::*;
@@ -536,8 +536,11 @@ impl Ctx {
     fn lower_impl(&mut self, impl_def: &ast::Impl) -> Option<FileItemTreeId<Impl>> {
         let generic_params =
             self.lower_generic_params_and_inner_items(GenericsOwner::Impl, impl_def);
-        let target_trait = impl_def.trait_().map(|tr| self.lower_type_ref(&tr));
+        // FIXME: If trait lowering fails, due to a non PathType for example, we treat this impl
-        let target_type = self.lower_type_ref(&impl_def.self_ty()?);
+        // as if it was an non-trait impl. Ideally we want to create a unique missing ref that only
+        // equals itself.
+        let target_trait = impl_def.trait_().and_then(|tr| self.lower_trait_ref(&tr));
+        let self_ty = self.lower_type_ref(&impl_def.self_ty()?);
         let is_negative = impl_def.excl_token().is_some();
 
         // We cannot use `assoc_items()` here as that does not include macro calls.
@@ -554,7 +557,7 @@ impl Ctx {
             })
             .collect();
         let ast_id = self.source_ast_id_map.ast_id(impl_def);
-        let res = Impl { generic_params, target_trait, target_type, is_negative, items, ast_id };
+        let res = Impl { generic_params, target_trait, self_ty, is_negative, items, ast_id };
         Some(id(self.data().impls.alloc(res)))
     }
 
@@ -740,10 +743,16 @@ impl Ctx {
         self.data().vis.alloc(vis)
     }
 
+    fn lower_trait_ref(&mut self, trait_ref: &ast::Type) -> Option<Idx<TraitRef>> {
+        let trait_ref = TraitRef::from_ast(&self.body_ctx, trait_ref.clone())?;
+        Some(self.data().trait_refs.intern(trait_ref))
+    }
+
     fn lower_type_ref(&mut self, type_ref: &ast::Type) -> Idx<TypeRef> {
         let tyref = TypeRef::from_ast(&self.body_ctx, type_ref.clone());
         self.data().type_refs.intern(tyref)
     }
+
     fn lower_type_ref_opt(&mut self, type_ref: Option<ast::Type>) -> Idx<TypeRef> {
         match type_ref.map(|ty| self.lower_type_ref(&ty)) {
             Some(it) => it,

crates/hir_def/src/type_ref.rs

@@ -51,6 +51,23 @@ impl Rawness {
     }
 }
 
+#[derive(Clone, PartialEq, Eq, Hash, Debug)]
+pub struct TraitRef {
+    pub path: Path,
+}
+
+impl TraitRef {
+    /// Converts an `ast::PathType` to a `hir::TraitRef`.
+    pub(crate) fn from_ast(ctx: &LowerCtx, node: ast::Type) -> Option<Self> {
+        // FIXME: Use `Path::from_src`
+        match node {
+            ast::Type::PathType(path) => {
+                path.path().and_then(|it| ctx.lower_path(it)).map(|path| TraitRef { path })
+            }
+            _ => None,
+        }
+    }
+}
 /// Compare ty::Ty
 #[derive(Clone, PartialEq, Eq, Hash, Debug)]
 pub enum TypeRef {

crates/hir_ty/src/lower.rs

@@ -15,7 +15,7 @@ use hir_def::{
     generics::{TypeParamProvenance, WherePredicate, WherePredicateTypeTarget},
     path::{GenericArg, Path, PathSegment, PathSegments},
     resolver::{HasResolver, Resolver, TypeNs},
-    type_ref::{TypeBound, TypeRef},
+    type_ref::{TraitRef as HirTraitRef, TypeBound, TypeRef},
     AdtId, AssocContainerId, AssocItemId, ConstId, ConstParamId, EnumId, EnumVariantId, FunctionId,
     GenericDefId, HasModule, ImplId, LocalFieldId, Lookup, StaticId, StructId, TraitId,
     TypeAliasId, TypeParamId, UnionId, VariantId,
@@ -667,14 +667,10 @@ impl<'a> TyLoweringContext<'a> {
 
     fn lower_trait_ref(
         &self,
-        type_ref: &TypeRef,
+        trait_ref: &HirTraitRef,
         explicit_self_ty: Option<Ty>,
     ) -> Option<TraitRef> {
-        let path = match type_ref {
+        self.lower_trait_ref_from_path(&trait_ref.path, explicit_self_ty)
-            TypeRef::Path(path) => path,
-            _ => return None,
-        };
-        self.lower_trait_ref_from_path(path, explicit_self_ty)
     }
 
     fn trait_ref_substs_from_path(
@@ -1253,7 +1249,7 @@ pub(crate) fn impl_self_ty_query(db: &dyn HirDatabase, impl_id: ImplId) -> Binde
     let generics = generics(db.upcast(), impl_id.into());
     let ctx =
         TyLoweringContext::new(db, &resolver).with_type_param_mode(TypeParamLoweringMode::Variable);
-    Binders::new(generics.len(), ctx.lower_ty(&impl_data.target_type))
+    Binders::new(generics.len(), ctx.lower_ty(&impl_data.self_ty))
 }
 
 pub(crate) fn const_param_ty_query(db: &dyn HirDatabase, def: ConstParamId) -> Ty {

crates/ide/src/doc_links.rs

@@ -237,7 +237,7 @@ fn get_doc_link(db: &RootDatabase, definition: Definition) -> Option<String> {
                 .and_then(|assoc| match assoc.container(db) {
                     AssocItemContainer::Trait(t) => Some(t.into()),
                     AssocItemContainer::Impl(impld) => {
-                        impld.target_ty(db).as_adt().map(|adt| adt.into())
+                        impld.self_ty(db).as_adt().map(|adt| adt.into())
                     }
                 })
                 .unwrap_or_else(|| f.clone().into()),

crates/ide/src/hover.rs

@@ -195,7 +195,7 @@ fn show_implementations_action(db: &RootDatabase, def: Definition) -> Option<Hov
     let adt = match def {
         Definition::ModuleDef(ModuleDef::Trait(it)) => return it.try_to_nav(db).map(to_action),
         Definition::ModuleDef(ModuleDef::Adt(it)) => Some(it),
-        Definition::SelfType(it) => it.target_ty(db).as_adt(),
+        Definition::SelfType(it) => it.self_ty(db).as_adt(),
         _ => None,
     }?;
     adt.try_to_nav(db).map(to_action)
@@ -318,7 +318,7 @@ fn definition_owner_name(db: &RootDatabase, def: &Definition) -> Option<String>
         Definition::ModuleDef(md) => match md {
             ModuleDef::Function(f) => match f.as_assoc_item(db)?.container(db) {
                 AssocItemContainer::Trait(t) => Some(t.name(db)),
-                AssocItemContainer::Impl(i) => i.target_ty(db).as_adt().map(|adt| adt.name(db)),
+                AssocItemContainer::Impl(i) => i.self_ty(db).as_adt().map(|adt| adt.name(db)),
             },
             ModuleDef::Variant(e) => Some(e.parent_enum(db).name(db)),
             _ => None,
@@ -376,7 +376,7 @@ fn hover_for_definition(
         },
         Definition::Local(it) => hover_for_local(it, db),
         Definition::SelfType(impl_def) => {
-            impl_def.target_ty(db).as_adt().and_then(|adt| from_hir_fmt(db, adt, mod_path))
+            impl_def.self_ty(db).as_adt().and_then(|adt| from_hir_fmt(db, adt, mod_path))
         }
         Definition::GenericParam(it) => from_hir_fmt(db, it, None),
         Definition::Label(it) => Some(Markup::fenced_block(&it.name(db))),

crates/ide/src/references/rename.rs

@@ -307,7 +307,7 @@ fn rename_to_self(sema: &Semantics<RootDatabase>, local: hir::Local) -> RenameRe
         hir::AssocItemContainer::Impl(impl_) => impl_,
     };
     let first_param_ty = first_param.ty();
-    let impl_ty = impl_.target_ty(sema.db);
+    let impl_ty = impl_.self_ty(sema.db);
     let (ty, self_param) = if impl_ty.remove_ref().is_some() {
         // if the impl is a ref to the type we can just match the `&T` with self directly
         (first_param_ty.clone(), "self")

crates/ide/src/runnables.rs

@@ -298,7 +298,7 @@ fn module_def_doctest(sema: &Semantics<RootDatabase>, def: hir::ModuleDef) -> Op
             // FIXME: this also looks very wrong
             if let Some(assoc_def) = assoc_def {
                 if let hir::AssocItemContainer::Impl(imp) = assoc_def.container(sema.db) {
-                    let ty = imp.target_ty(sema.db);
+                    let ty = imp.self_ty(sema.db);
                     if let Some(adt) = ty.as_adt() {
                         let name = adt.name(sema.db);
                         let idx = path.rfind(':').map_or(0, |idx| idx + 1);

crates/ide_assists/src/handlers/generate_default_from_new.rs

@@ -92,7 +92,7 @@ fn is_default_implemented(ctx: &AssistContext, impl_: &Impl) -> bool {
         None => return false,
     };
 
-    let ty = impl_def.target_ty(db);
+    let ty = impl_def.self_ty(db);
     let krate = impl_def.module(db).krate();
     let default = FamousDefs(&ctx.sema, Some(krate)).core_default_Default();
     let default_trait = match default {

crates/ide_assists/src/handlers/generate_is_empty_from_len.rs

@@ -91,7 +91,7 @@ fn get_impl_method(
 
     let scope = ctx.sema.scope(impl_.syntax());
     let krate = impl_def.module(db).krate();
-    let ty = impl_def.target_ty(db);
+    let ty = impl_def.self_ty(db);
     let traits_in_scope = scope.traits_in_scope();
     ty.iterate_method_candidates(db, krate, &traits_in_scope, Some(fn_name), |_, func| Some(func))
 }

crates/ide_assists/src/utils.rs

@@ -338,11 +338,11 @@ pub(crate) fn find_struct_impl(
         // (we currently use the wrong type parameter)
         // also we wouldn't want to use e.g. `impl S<u32>`
 
-        let same_ty = match blk.target_ty(db).as_adt() {
+        let same_ty = match blk.self_ty(db).as_adt() {
             Some(def) => def == struct_def,
             None => false,
         };
-        let not_trait_impl = blk.target_trait(db).is_none();
+        let not_trait_impl = blk.trait_(db).is_none();
 
         if !(same_ty && not_trait_impl) {
             None

crates/ide_completion/src/completions.rs

@@ -220,7 +220,7 @@ fn complete_enum_variants(
         };
 
         if let Some(impl_) = ctx.impl_def.as_ref().and_then(|impl_| ctx.sema.to_def(impl_)) {
-            if impl_.target_ty(ctx.db) == *ty {
+            if impl_.self_ty(ctx.db) == *ty {
                 for &variant in &variants {
                     let self_path = hir::ModPath::from_segments(
                         hir::PathKind::Plain,

crates/ide_completion/src/completions/pattern.rs

@@ -40,7 +40,7 @@ pub(crate) fn complete_pattern(acc: &mut Completions, ctx: &CompletionContext) {
                 _ => false,
             },
             hir::ScopeDef::MacroDef(_) => true,
-            hir::ScopeDef::ImplSelfType(impl_) => match impl_.target_ty(ctx.db).as_adt() {
+            hir::ScopeDef::ImplSelfType(impl_) => match impl_.self_ty(ctx.db).as_adt() {
                 Some(hir::Adt::Struct(strukt)) => {
                     acc.add_struct_pat(ctx, strukt, Some(name.clone()));
                     true

crates/ide_completion/src/completions/qualified_path.rs

@@ -117,7 +117,7 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon
             if let Some(krate) = ctx.krate {
                 let ty = match resolution {
                     PathResolution::TypeParam(param) => param.ty(ctx.db),
-                    PathResolution::SelfType(impl_def) => impl_def.target_ty(ctx.db),
+                    PathResolution::SelfType(impl_def) => impl_def.self_ty(ctx.db),
                     _ => return,
                 };
 

crates/ide_db/src/helpers/import_assets.rs

@@ -361,7 +361,7 @@ fn item_for_path_search(db: &RootDatabase, item: ItemInNs) -> Option<ItemInNs> {
             Some(assoc_item) => match assoc_item.container(db) {
                 AssocItemContainer::Trait(trait_) => ItemInNs::from(ModuleDef::from(trait_)),
                 AssocItemContainer::Impl(impl_) => {
-                    ItemInNs::from(ModuleDef::from(impl_.target_ty(db).as_adt()?))
+                    ItemInNs::from(ModuleDef::from(impl_.self_ty(db).as_adt()?))
                 }
             },
             None => item,