From 1e2dcfb243c4adeebc7c978bb444628fc28725c9 Mon Sep 17 00:00:00 2001 From: hkalbasi Date: Sat, 13 May 2023 23:44:36 +0330 Subject: [PATCH] Fix layout for `hir_ty::Ty` and friends --- crates/hir-ty/src/consteval/tests.rs | 30 +++++++++++++++ crates/hir-ty/src/db.rs | 7 +++- crates/hir-ty/src/display.rs | 10 ++--- crates/hir-ty/src/layout.rs | 2 +- crates/hir-ty/src/layout/adt.rs | 6 ++- crates/hir-ty/src/layout/tests.rs | 56 ++++++++++++++++++++-------- crates/hir-ty/src/mir/eval.rs | 2 +- crates/hir/src/lib.rs | 2 +- 8 files changed, 87 insertions(+), 28 deletions(-) diff --git a/crates/hir-ty/src/consteval/tests.rs b/crates/hir-ty/src/consteval/tests.rs index 12b15065ddf9..b2b511fc034a 100644 --- 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::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); + + 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 index b37d90c75895..0196d7b9e0c7 100644 --- a/crates/hir-ty/src/db.rs +++ b/crates/hir-ty/src/db.rs @@ -74,7 +74,12 @@ pub trait HirDatabase: DefDatabase + Upcast { #[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; + fn layout_of_adt( + &self, + def: AdtId, + subst: Substitution, + krate: CrateId, + ) -> Result; #[salsa::invoke(crate::layout::target_data_layout_query)] fn target_data_layout(&self, krate: CrateId) -> Option>; diff --git a/crates/hir-ty/src/display.rs b/crates/hir-ty/src/display.rs index 3cfe78141d43..1d7f6cc0c15b 100644 --- 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(""), }, 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(""); }; @@ -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(""); }; match data.variant_data.as_ref() { diff --git a/crates/hir-ty/src/layout.rs b/crates/hir-ty/src/layout.rs index f74a2253a424..f67b49907cec 100644 --- 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 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 index 9dbf9b2419c1..21c52f2c6a87 100644 --- 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 { - 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 { user_error!("infinite sized recursive type"); } diff --git a/crates/hir-ty/src/layout/tests.rs b/crates/hir-ty/src/layout/tests.rs index e1038c0affe9..0b5d9df0c1d1 100644 --- 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 { "{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(::Ty); struct Goal(Foo); } + check_size_and_align( + r#" +//- /b/mod.rs crate:b +pub trait Tr { + type Ty; +} +pub struct Foo(::Ty); + +//- /a/mod.rs crate:a deps:b +use b::{Tr, Foo}; + +struct S; +impl Tr for S { + type Ty = i64; +} +struct Goal(Foo); + "#, + "", + 8, + 8, + ); } #[test] diff --git a/crates/hir-ty/src/mir/eval.rs b/crates/hir-ty/src/mir/eval.rs index 8ce16df82958..d543159b0e00 100644 --- 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 { - 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 index 1fac95ae5e37..eb9c26b82894 100644 --- 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