Auto merge of #14802 - HKalbasi:layout, r=HKalbasi

Fix layout for `hir_ty::Ty` and friends
rust-lang · May 18, 2023 · 8080008 · 8080008
2 parents 2f8cd66 + 1e2dcfb
commit 8080008
Show file tree

Hide file tree

Showing 8 changed files with 87 additions and 28 deletions.
diff --git a/crates/hir-ty/src/consteval/tests.rs b/crates/hir-ty/src/consteval/tests.rs
@@ -1958,6 +1958,36 @@ fn const_generic_subst_fn() {
     );
 }
 
+#[test]
+fn layout_of_type_with_associated_type_field_defined_inside_body() {
+    check_number(
+        r#"
+trait Tr {
+    type Ty;
+}
+
+struct St<T: Tr>(T::Ty);
+
+const GOAL: i64 = {
+    // if we move `St2` out of body, the test will fail, as we don't see the impl anymore. That
+    // case will probably be rejected by rustc in some later edition, but we should support this
+    // case.
+    struct St2;
+
+    impl Tr for St2 {
+        type Ty = i64;
+    }
+
+    struct Goal(St<St2>);
+
+    let x = Goal(St(5));
+    x.0.0
+};
+"#,
+        5,
+    );
+}
+
 #[test]
 fn const_generic_subst_assoc_const_impl() {
     check_number(

diff --git a/crates/hir-ty/src/db.rs b/crates/hir-ty/src/db.rs
@@ -74,7 +74,12 @@ pub trait HirDatabase: DefDatabase + Upcast<dyn DefDatabase> {
 
     #[salsa::invoke(crate::layout::layout_of_adt_query)]
     #[salsa::cycle(crate::layout::layout_of_adt_recover)]
-    fn layout_of_adt(&self, def: AdtId, subst: Substitution) -> Result<Layout, LayoutError>;
+    fn layout_of_adt(
+        &self,
+        def: AdtId,
+        subst: Substitution,
+        krate: CrateId,
+    ) -> Result<Layout, LayoutError>;
 
     #[salsa::invoke(crate::layout::target_data_layout_query)]
     fn target_data_layout(&self, krate: CrateId) -> Option<Arc<TargetDataLayout>>;

diff --git a/crates/hir-ty/src/display.rs b/crates/hir-ty/src/display.rs
@@ -463,6 +463,9 @@ fn render_const_scalar(
     memory_map: &MemoryMap,
     ty: &Ty,
 ) -> Result<(), HirDisplayError> {
+    // FIXME: We need to get krate from the final callers of the hir display
+    // infrastructure and have it here as a field on `f`.
+    let krate = *f.db.crate_graph().crates_in_topological_order().last().unwrap();
     match ty.kind(Interner) {
         chalk_ir::TyKind::Scalar(s) => match s {
             Scalar::Bool => write!(f, "{}", if b[0] == 0 { false } else { true }),
@@ -502,11 +505,6 @@ fn render_const_scalar(
             _ => f.write_str("<ref-not-supported>"),
         },
         chalk_ir::TyKind::Tuple(_, subst) => {
-            // FIXME: Remove this line. If the target data layout is independent
-            // of the krate, the `db.target_data_layout` and its callers like `layout_of_ty` don't need
-            // to get krate. Otherwise, we need to get krate from the final callers of the hir display
-            // infrastructure and have it here as a field on `f`.
-            let krate = *f.db.crate_graph().crates_in_topological_order().last().unwrap();
             let Ok(layout) = layout_of_ty(f.db, ty, krate) else {
                 return f.write_str("<layout-error>");
             };
@@ -532,7 +530,7 @@ fn render_const_scalar(
         chalk_ir::TyKind::Adt(adt, subst) => match adt.0 {
             hir_def::AdtId::StructId(s) => {
                 let data = f.db.struct_data(s);
-                let Ok(layout) = f.db.layout_of_adt(adt.0, subst.clone()) else {
+                let Ok(layout) = f.db.layout_of_adt(adt.0, subst.clone(), krate) else {
                     return f.write_str("<layout-error>");
                 };
                 match data.variant_data.as_ref() {

diff --git a/crates/hir-ty/src/layout.rs b/crates/hir-ty/src/layout.rs
@@ -84,7 +84,7 @@ pub fn layout_of_ty(db: &dyn HirDatabase, ty: &Ty, krate: CrateId) -> Result<Lay
     let trait_env = Arc::new(TraitEnvironment::empty(krate));
     let ty = normalize(db, trait_env, ty.clone());
     Ok(match ty.kind(Interner) {
-        TyKind::Adt(AdtId(def), subst) => db.layout_of_adt(*def, subst.clone())?,
+        TyKind::Adt(AdtId(def), subst) => db.layout_of_adt(*def, subst.clone(), krate)?,
         TyKind::Scalar(s) => match s {
             chalk_ir::Scalar::Bool => Layout::scalar(
                 dl,

diff --git a/crates/hir-ty/src/layout/adt.rs b/crates/hir-ty/src/layout/adt.rs
@@ -2,10 +2,11 @@
 
 use std::{cmp, ops::Bound};
 
+use base_db::CrateId;
 use hir_def::{
     data::adt::VariantData,
     layout::{Integer, LayoutCalculator, ReprOptions, TargetDataLayout},
-    AdtId, EnumVariantId, HasModule, LocalEnumVariantId, VariantId,
+    AdtId, EnumVariantId, LocalEnumVariantId, VariantId,
 };
 use la_arena::RawIdx;
 use smallvec::SmallVec;
@@ -27,8 +28,8 @@ pub fn layout_of_adt_query(
     db: &dyn HirDatabase,
     def: AdtId,
     subst: Substitution,
+    krate: CrateId,
 ) -> Result<Layout, LayoutError> {
-    let krate = def.module(db.upcast()).krate();
     let Some(target) = db.target_data_layout(krate) else { return Err(LayoutError::TargetLayoutNotAvailable) };
     let cx = LayoutCx { krate, target: &target };
     let dl = cx.current_data_layout();
@@ -127,6 +128,7 @@ pub fn layout_of_adt_recover(
     _: &[String],
     _: &AdtId,
     _: &Substitution,
+    _: &CrateId,
 ) -> Result<Layout, LayoutError> {
     user_error!("infinite sized recursive type");
 }

diff --git a/crates/hir-ty/src/layout/tests.rs b/crates/hir-ty/src/layout/tests.rs
@@ -25,22 +25,25 @@ fn eval_goal(ra_fixture: &str, minicore: &str) -> Result<Layout, LayoutError> {
         "{minicore}//- /main.rs crate:test target_data_layout:{target_data_layout}\n{ra_fixture}",
     );
 
-    let (db, file_id) = TestDB::with_single_file(&ra_fixture);
-    let module_id = db.module_for_file(file_id);
-    let def_map = module_id.def_map(&db);
-    let scope = &def_map[module_id.local_id].scope;
-    let adt_id = scope
-        .declarations()
-        .find_map(|x| match x {
-            hir_def::ModuleDefId::AdtId(x) => {
-                let name = match x {
-                    hir_def::AdtId::StructId(x) => db.struct_data(x).name.to_smol_str(),
-                    hir_def::AdtId::UnionId(x) => db.union_data(x).name.to_smol_str(),
-                    hir_def::AdtId::EnumId(x) => db.enum_data(x).name.to_smol_str(),
-                };
-                (name == "Goal").then_some(x)
-            }
-            _ => None,
+    let (db, file_ids) = TestDB::with_many_files(&ra_fixture);
+    let (adt_id, module_id) = file_ids
+        .into_iter()
+        .find_map(|file_id| {
+            let module_id = db.module_for_file(file_id);
+            let def_map = module_id.def_map(&db);
+            let scope = &def_map[module_id.local_id].scope;
+            let adt_id = scope.declarations().find_map(|x| match x {
+                hir_def::ModuleDefId::AdtId(x) => {
+                    let name = match x {
+                        hir_def::AdtId::StructId(x) => db.struct_data(x).name.to_smol_str(),
+                        hir_def::AdtId::UnionId(x) => db.union_data(x).name.to_smol_str(),
+                        hir_def::AdtId::EnumId(x) => db.enum_data(x).name.to_smol_str(),
+                    };
+                    (name == "Goal").then_some(x)
+                }
+                _ => None,
+            })?;
+            Some((adt_id, module_id))
         })
         .unwrap();
     let goal_ty = TyKind::Adt(AdtId(adt_id), Substitution::empty(Interner)).intern(Interner);
@@ -232,6 +235,27 @@ fn associated_types() {
         struct Foo<A: Tr>(<A as Tr>::Ty);
         struct Goal(Foo<i32>);
     }
+    check_size_and_align(
+        r#"
+//- /b/mod.rs crate:b
+pub trait Tr {
+    type Ty;
+}
+pub struct Foo<A: Tr>(<A as Tr>::Ty);
+
+//- /a/mod.rs crate:a deps:b
+use b::{Tr, Foo};
+
+struct S;
+impl Tr for S {
+    type Ty = i64;
+}
+struct Goal(Foo<S>);
+        "#,
+        "",
+        8,
+        8,
+    );
 }
 
 #[test]

diff --git a/crates/hir-ty/src/mir/eval.rs b/crates/hir-ty/src/mir/eval.rs
@@ -574,7 +574,7 @@ impl Evaluator<'_> {
     }
 
     fn layout_adt(&self, adt: AdtId, subst: Substitution) -> Result<Layout> {
-        self.db.layout_of_adt(adt, subst.clone()).map_err(|e| {
+        self.db.layout_of_adt(adt, subst.clone(), self.crate_id).map_err(|e| {
             MirEvalError::LayoutError(e, TyKind::Adt(chalk_ir::AdtId(adt), subst).intern(Interner))
         })
     }

diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
@@ -1202,7 +1202,7 @@ impl Adt {
         if db.generic_params(self.into()).iter().count() != 0 {
             return Err(LayoutError::HasPlaceholder);
         }
-        db.layout_of_adt(self.into(), Substitution::empty(Interner))
+        db.layout_of_adt(self.into(), Substitution::empty(Interner), self.krate(db).id)
     }
 
     /// Turns this ADT into a type. Any type parameters of the ADT will be