Merge #7399

7399: Make `ModuleId`'s `krate` field private and audit uses r=jonas-schievink a=jonas-schievink

bors r+

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>

crates/hir/src/code_model.rs

@@ -270,13 +270,13 @@ impl ModuleDef {
             None => return,
         };
 
-        hir_ty::diagnostics::validate_module_item(db, module.id.krate, id, sink)
+        hir_ty::diagnostics::validate_module_item(db, module.id.krate(), id, sink)
     }
 }
 
 impl Module {
     pub(crate) fn new(krate: Crate, crate_module_id: LocalModuleId) -> Module {
-        Module { id: ModuleId { krate: krate.id, local_id: crate_module_id } }
+        Module { id: ModuleId::top_level(krate.id, crate_module_id) }
     }
 
     /// Name of this module.
@@ -294,14 +294,14 @@ impl Module {
 
     /// Returns the crate this module is part of.
     pub fn krate(self) -> Crate {
-        Crate { id: self.id.krate }
+        Crate { id: self.id.krate() }
     }
 
     /// Topmost parent of this module. Every module has a `crate_root`, but some
     /// might be missing `krate`. This can happen if a module's file is not included
     /// in the module tree of any target in `Cargo.toml`.
     pub fn crate_root(self, db: &dyn HirDatabase) -> Module {
-        let def_map = db.crate_def_map(self.id.krate);
+        let def_map = db.crate_def_map(self.id.krate());
         self.with_module_id(def_map.root())
     }
 
@@ -318,6 +318,7 @@ impl Module {
 
     /// Finds a parent module.
     pub fn parent(self, db: &dyn HirDatabase) -> Option<Module> {
+        // FIXME: handle block expressions as modules (their parent is in a different DefMap)
         let def_map = self.id.def_map(db.upcast());
         let parent_id = def_map[self.id.local_id].parent?;
         Some(self.with_module_id(parent_id))
@@ -457,7 +458,7 @@ impl Field {
         };
         let substs = Substs::type_params(db, generic_def_id);
         let ty = db.field_types(var_id)[self.id].clone().subst(&substs);
-        Type::new(db, self.parent.module(db).id.krate, var_id, ty)
+        Type::new(db, self.parent.module(db).id.krate(), var_id, ty)
     }
 
     pub fn parent_def(&self, _db: &dyn HirDatabase) -> VariantDef {
@@ -502,7 +503,11 @@ impl Struct {
     }
 
     pub fn ty(self, db: &dyn HirDatabase) -> Type {
-        Type::from_def(db, self.id.lookup(db.upcast()).container.module(db.upcast()).krate, self.id)
+        Type::from_def(
+            db,
+            self.id.lookup(db.upcast()).container.module(db.upcast()).krate(),
+            self.id,
+        )
     }
 
     pub fn repr(self, db: &dyn HirDatabase) -> Option<ReprKind> {
@@ -533,7 +538,11 @@ impl Union {
     }
 
     pub fn ty(self, db: &dyn HirDatabase) -> Type {
-        Type::from_def(db, self.id.lookup(db.upcast()).container.module(db.upcast()).krate, self.id)
+        Type::from_def(
+            db,
+            self.id.lookup(db.upcast()).container.module(db.upcast()).krate(),
+            self.id,
+        )
     }
 
     pub fn fields(self, db: &dyn HirDatabase) -> Vec<Field> {
@@ -573,7 +582,11 @@ impl Enum {
     }
 
     pub fn ty(self, db: &dyn HirDatabase) -> Type {
-        Type::from_def(db, self.id.lookup(db.upcast()).container.module(db.upcast()).krate, self.id)
+        Type::from_def(
+            db,
+            self.id.lookup(db.upcast()).container.module(db.upcast()).krate(),
+            self.id,
+        )
     }
 }
 
@@ -632,7 +645,7 @@ impl Adt {
     /// general set of completions, but will not look very nice when printed.
     pub fn ty(self, db: &dyn HirDatabase) -> Type {
         let id = AdtId::from(self);
-        Type::from_def(db, id.module(db.upcast()).krate, id)
+        Type::from_def(db, id.module(db.upcast()).krate(), id)
     }
 
     pub fn module(self, db: &dyn HirDatabase) -> Module {
@@ -750,7 +763,7 @@ impl Function {
         let ctx = hir_ty::TyLoweringContext::new(db, &resolver);
         let environment = TraitEnvironment::lower(db, &resolver);
         Type {
-            krate: self.id.lookup(db.upcast()).container.module(db.upcast()).krate,
+            krate: self.id.lookup(db.upcast()).container.module(db.upcast()).krate(),
             ty: InEnvironment { value: Ty::from_hir_ext(&ctx, ret_type).0, environment },
         }
     }
@@ -771,7 +784,7 @@ impl Function {
             .iter()
             .map(|type_ref| {
                 let ty = Type {
-                    krate: self.id.lookup(db.upcast()).container.module(db.upcast()).krate,
+                    krate: self.id.lookup(db.upcast()).container.module(db.upcast()).krate(),
                     ty: InEnvironment {
                         value: Ty::from_hir_ext(&ctx, type_ref).0,
                         environment: environment.clone(),
@@ -795,7 +808,7 @@ impl Function {
     }
 
     pub fn diagnostics(self, db: &dyn HirDatabase, sink: &mut DiagnosticSink) {
-        let krate = self.module(db).id.krate;
+        let krate = self.module(db).id.krate();
         hir_def::diagnostics::validate_body(db.upcast(), self.id.into(), sink);
         hir_ty::diagnostics::validate_module_item(db, krate, self.id.into(), sink);
         hir_ty::diagnostics::validate_body(db, self.id.into(), sink);
@@ -973,7 +986,7 @@ impl TypeAlias {
     }
 
     pub fn ty(self, db: &dyn HirDatabase) -> Type {
-        Type::from_def(db, self.id.lookup(db.upcast()).module(db.upcast()).krate, self.id)
+        Type::from_def(db, self.id.lookup(db.upcast()).module(db.upcast()).krate(), self.id)
     }
 
     pub fn name(self, db: &dyn HirDatabase) -> Name {
@@ -1230,7 +1243,7 @@ impl Local {
         let def = DefWithBodyId::from(self.parent);
         let infer = db.infer(def);
         let ty = infer[self.pat_id].clone();
-        let krate = def.module(db.upcast()).krate;
+        let krate = def.module(db.upcast()).krate();
         Type::new(db, krate, def, ty)
     }
 
@@ -1318,7 +1331,7 @@ impl TypeParam {
         let environment = TraitEnvironment::lower(db, &resolver);
         let ty = Ty::Placeholder(self.id);
         Type {
-            krate: self.id.parent.module(db.upcast()).krate,
+            krate: self.id.parent.module(db.upcast()).krate(),
             ty: InEnvironment { value: ty, environment },
         }
     }
@@ -1344,7 +1357,7 @@ impl TypeParam {
         let subst = Substs::type_params(db, self.id.parent);
         let ty = ty.subst(&subst.prefix(local_idx));
         Some(Type {
-            krate: self.id.parent.module(db.upcast()).krate,
+            krate: self.id.parent.module(db.upcast()).krate(),
             ty: InEnvironment { value: ty, environment },
         })
     }
@@ -1405,7 +1418,7 @@ impl ConstParam {
 
     pub fn ty(self, db: &dyn HirDatabase) -> Type {
         let def = self.id.parent;
-        let krate = def.module(db.upcast()).krate;
+        let krate = def.module(db.upcast()).krate();
         Type::new(db, krate, def, db.const_param_ty(self.id))
     }
 }
@@ -1440,7 +1453,7 @@ impl Impl {
         let environment = TraitEnvironment::lower(db, &resolver);
         let ty = Ty::from_hir(&ctx, &impl_data.target_type);
         Type {
-            krate: self.id.lookup(db.upcast()).container.module(db.upcast()).krate,
+            krate: self.id.lookup(db.upcast()).container.module(db.upcast()).krate(),
             ty: InEnvironment { value: ty, environment },
         }
     }
@@ -1458,7 +1471,7 @@ impl Impl {
     }
 
     pub fn krate(self, db: &dyn HirDatabase) -> Crate {
-        Crate { id: self.module(db).id.krate }
+        Crate { id: self.module(db).id.krate() }
     }
 
     pub fn is_builtin_derive(self, db: &dyn HirDatabase) -> Option<InFile<ast::Attr>> {

crates/hir/src/semantics/source_to_def.rs

@@ -36,7 +36,7 @@ impl SourceToDefCtx<'_, '_> {
             let local_id = crate_def_map.modules_for_file(file).next()?;
             Some((crate_id, local_id))
         })?;
-        Some(ModuleId { krate, local_id })
+        Some(ModuleId::top_level(krate, local_id))
     }
 
     pub(super) fn module_to_def(&mut self, src: InFile<ast::Module>) -> Option<ModuleId> {
@@ -63,7 +63,8 @@ impl SourceToDefCtx<'_, '_> {
         let child_name = src.value.name()?.as_name();
         let def_map = parent_module.def_map(self.db.upcast());
         let child_id = *def_map[parent_module.local_id].children.get(&child_name)?;
-        Some(ModuleId { krate: parent_module.krate, local_id: child_id })
+        // FIXME: handle block expression modules
+        Some(ModuleId::top_level(parent_module.krate(), child_id))
     }
 
     pub(super) fn trait_to_def(&mut self, src: InFile<ast::Trait>) -> Option<TraitId> {
@@ -186,7 +187,7 @@ impl SourceToDefCtx<'_, '_> {
     ) -> Option<MacroDefId> {
         let kind = MacroDefKind::Declarative;
         let file_id = src.file_id.original_file(self.db.upcast());
-        let krate = self.file_to_def(file_id)?.krate;
+        let krate = self.file_to_def(file_id)?.krate();
         let file_ast_id = self.db.ast_id_map(src.file_id).ast_id(&src.value);
         let ast_id = Some(AstId::new(src.file_id, file_ast_id.upcast()));
         Some(MacroDefId { krate, ast_id, kind, local_inner: false })

crates/hir_def/src/lib.rs

@@ -73,14 +73,22 @@ use stdx::impl_from;
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct ModuleId {
-    pub krate: CrateId,
+    krate: CrateId,
     pub local_id: LocalModuleId,
 }
 
 impl ModuleId {
+    pub fn top_level(krate: CrateId, local_id: LocalModuleId) -> Self {
+        Self { krate, local_id }
+    }
+
     pub fn def_map(&self, db: &dyn db::DefDatabase) -> Arc<DefMap> {
         db.crate_def_map(self.krate)
     }
+
+    pub fn krate(&self) -> CrateId {
+        self.krate
+    }
 }
 
 /// An ID of a module, **local** to a specific crate

crates/hir_ty/src/display.rs

@@ -617,7 +617,7 @@ impl HirDisplay for FnSig {
 }
 
 fn fn_traits(db: &dyn DefDatabase, trait_: TraitId) -> impl Iterator<Item = TraitId> {
-    let krate = trait_.lookup(db).container.module(db).krate;
+    let krate = trait_.lookup(db).container.module(db).krate();
     let fn_traits = [
         db.lang_item(krate, "fn".into()),
         db.lang_item(krate, "fn_mut".into()),

crates/hir_ty/src/lib.rs

@@ -211,19 +211,21 @@ impl TypeCtor {
             | TypeCtor::Tuple { .. } => None,
             // Closure's krate is irrelevant for coherence I would think?
             TypeCtor::Closure { .. } => None,
-            TypeCtor::Adt(adt) => Some(adt.module(db.upcast()).krate),
+            TypeCtor::Adt(adt) => Some(adt.module(db.upcast()).krate()),
             TypeCtor::FnDef(callable) => Some(callable.krate(db)),
             TypeCtor::AssociatedType(type_alias) => {
-                Some(type_alias.lookup(db.upcast()).module(db.upcast()).krate)
+                Some(type_alias.lookup(db.upcast()).module(db.upcast()).krate())
             }
             TypeCtor::ForeignType(type_alias) => {
-                Some(type_alias.lookup(db.upcast()).module(db.upcast()).krate)
+                Some(type_alias.lookup(db.upcast()).module(db.upcast()).krate())
             }
             TypeCtor::OpaqueType(opaque_ty_id) => match opaque_ty_id {
                 OpaqueTyId::ReturnTypeImplTrait(func, _) => {
-                    Some(func.lookup(db.upcast()).module(db.upcast()).krate)
+                    Some(func.lookup(db.upcast()).module(db.upcast()).krate())
+                }
+                OpaqueTyId::AsyncBlockTypeImplTrait(def, _) => {
+                    Some(def.module(db.upcast()).krate())
                 }
-                OpaqueTyId::AsyncBlockTypeImplTrait(def, _) => Some(def.module(db.upcast()).krate),
             },
         }
     }
@@ -870,7 +872,7 @@ impl Ty {
             Ty::Apply(ApplicationTy { ctor: TypeCtor::OpaqueType(opaque_ty_id), .. }) => {
                 match opaque_ty_id {
                     OpaqueTyId::AsyncBlockTypeImplTrait(def, _expr) => {
-                        let krate = def.module(db.upcast()).krate;
+                        let krate = def.module(db.upcast()).krate();
                         if let Some(future_trait) = db
                             .lang_item(krate, "future_trait".into())
                             .and_then(|item| item.as_trait())

crates/hir_ty/src/lower.rs

@@ -1147,7 +1147,7 @@ impl CallableDefId {
             CallableDefId::StructId(s) => s.lookup(db).container.module(db),
             CallableDefId::EnumVariantId(e) => e.parent.lookup(db).container.module(db),
         }
-        .krate
+        .krate()
     }
 }
 

crates/hir_ty/src/method_resolution.rs

@@ -248,12 +248,12 @@ impl Ty {
         let lang_item_targets = match self {
             Ty::Apply(a_ty) => match a_ty.ctor {
                 TypeCtor::Adt(def_id) => {
-                    return Some(std::iter::once(def_id.module(db.upcast()).krate).collect())
+                    return Some(std::iter::once(def_id.module(db.upcast()).krate()).collect())
                 }
                 TypeCtor::ForeignType(type_alias_id) => {
                     return Some(
                         std::iter::once(
-                            type_alias_id.lookup(db.upcast()).module(db.upcast()).krate,
+                            type_alias_id.lookup(db.upcast()).module(db.upcast()).krate(),
                         )
                         .collect(),
                     )
@@ -280,7 +280,7 @@ impl Ty {
                 LangItemTarget::ImplDefId(it) => Some(it),
                 _ => None,
             })
-            .map(|it| it.lookup(db.upcast()).container.module(db.upcast()).krate)
+            .map(|it| it.lookup(db.upcast()).container.module(db.upcast()).krate())
             .collect();
         Some(res)
     }

crates/hir_ty/src/test_db.rs

@@ -83,7 +83,7 @@ impl TestDB {
             let crate_def_map = self.crate_def_map(krate);
             for (local_id, data) in crate_def_map.modules() {
                 if data.origin.file_id() == Some(file_id) {
-                    return ModuleId { krate, local_id };
+                    return ModuleId::top_level(krate, local_id);
                 }
             }
         }

crates/hir_ty/src/tests.rs

@@ -343,7 +343,7 @@ fn typing_whitespace_inside_a_function_should_not_invalidate_types() {
     {
         let events = db.log_executed(|| {
             let module = db.module_for_file(pos.file_id);
-            let crate_def_map = db.crate_def_map(module.krate);
+            let crate_def_map = module.def_map(&db);
             visit_module(&db, &crate_def_map, module.local_id, &mut |def| {
                 db.infer(def);
             });

crates/hir_ty/src/traits/chalk.rs

@@ -426,7 +426,7 @@ pub(crate) fn trait_datum_query(
     let bound_vars = Substs::bound_vars(&generic_params, DebruijnIndex::INNERMOST);
     let flags = rust_ir::TraitFlags {
         auto: trait_data.auto,
-        upstream: trait_.lookup(db.upcast()).container.module(db.upcast()).krate != krate,
+        upstream: trait_.lookup(db.upcast()).container.module(db.upcast()).krate() != krate,
         non_enumerable: true,
         coinductive: false, // only relevant for Chalk testing
         // FIXME: set these flags correctly
@@ -549,7 +549,7 @@ fn impl_def_datum(
     let generic_params = generics(db.upcast(), impl_id.into());
     let bound_vars = Substs::bound_vars(&generic_params, DebruijnIndex::INNERMOST);
     let trait_ = trait_ref.trait_;
-    let impl_type = if impl_id.lookup(db.upcast()).container.module(db.upcast()).krate == krate {
+    let impl_type = if impl_id.lookup(db.upcast()).container.module(db.upcast()).krate() == krate {
         rust_ir::ImplType::Local
     } else {
         rust_ir::ImplType::External