Skip to content

Commit

Permalink
Auto merge of #129970 - lukas-code:LayoutCalculator, r=compiler-errors
Browse files Browse the repository at this point in the history
layout computation: gracefully handle unsized types in unexpected locations

This PR reworks the layout computation to eagerly return an error when encountering an unsized field where a sized field was expected, rather than delaying a bug and attempting to recover a layout. This is required, because with trivially false where clauses like `[T]: Sized`, any field can possible be an unsized type, without causing a compile error.

Since this PR removes the `delayed_bug` method from the `LayoutCalculator` trait, it essentially becomes the same as the `HasDataLayout` trait, so I've also refactored the `LayoutCalculator` to be a simple wrapper struct around a type that implements `HasDataLayout`.

The majority of the diff is whitespace changes, so viewing with whitespace ignored is advised.

implements rust-lang/rust#123169 (comment)

r? `@compiler-errors` or compiler

fixes rust-lang/rust#123134
fixes rust-lang/rust#124182
fixes rust-lang/rust#126939
fixes rust-lang/rust#127737
  • Loading branch information
bors committed Sep 17, 2024
2 parents 4e34f33 + cd442a0 commit 705d602
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 38 deletions.
65 changes: 34 additions & 31 deletions crates/hir-ty/src/layout.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,20 @@
//! Compute the binary representation of a type

use std::{borrow::Cow, fmt};
use std::fmt;

use base_db::salsa::Cycle;
use chalk_ir::{AdtId, FloatTy, IntTy, TyKind, UintTy};
use hir_def::{
layout::{
Abi, FieldsShape, Float, Integer, LayoutCalculator, LayoutS, Primitive, ReprOptions,
Scalar, Size, StructKind, TargetDataLayout, WrappingRange,
Abi, FieldsShape, Float, Integer, LayoutCalculator, LayoutCalculatorError, LayoutS,
Primitive, ReprOptions, Scalar, Size, StructKind, TargetDataLayout, WrappingRange,
},
LocalFieldId, StructId,
};
use la_arena::{Idx, RawIdx};
use rustc_abi::AddressSpace;
use rustc_index::{IndexSlice, IndexVec};

use stdx::never;
use triomphe::Arc;

use crate::{
Expand Down Expand Up @@ -107,19 +106,24 @@ impl fmt::Display for LayoutError {
}
}

struct LayoutCx<'a> {
target: &'a TargetDataLayout,
impl<F> From<LayoutCalculatorError<F>> for LayoutError {
fn from(err: LayoutCalculatorError<F>) -> Self {
match err {
LayoutCalculatorError::UnexpectedUnsized(_) | LayoutCalculatorError::EmptyUnion => {
LayoutError::Unknown
}
LayoutCalculatorError::SizeOverflow => LayoutError::SizeOverflow,
}
}
}

impl<'a> LayoutCalculator for LayoutCx<'a> {
type TargetDataLayoutRef = &'a TargetDataLayout;

fn delayed_bug(&self, txt: impl Into<Cow<'static, str>>) {
never!("{}", txt.into());
}
struct LayoutCx<'a> {
calc: LayoutCalculator<&'a TargetDataLayout>,
}

fn current_data_layout(&self) -> &'a TargetDataLayout {
self.target
impl<'a> LayoutCx<'a> {
fn new(target: &'a TargetDataLayout) -> Self {
Self { calc: LayoutCalculator::new(target) }
}
}

Expand Down Expand Up @@ -205,8 +209,8 @@ pub fn layout_of_ty_query(
let Ok(target) = db.target_data_layout(krate) else {
return Err(LayoutError::TargetLayoutNotAvailable);
};
let cx = LayoutCx { target: &target };
let dl = cx.current_data_layout();
let dl = &*target;
let cx = LayoutCx::new(dl);
let ty = normalize(db, trait_env.clone(), ty);
let result = match ty.kind(Interner) {
TyKind::Adt(AdtId(def), subst) => {
Expand Down Expand Up @@ -281,7 +285,7 @@ pub fn layout_of_ty_query(
.collect::<Result<Vec<_>, _>>()?;
let fields = fields.iter().map(|it| &**it).collect::<Vec<_>>();
let fields = fields.iter().collect::<IndexVec<_, _>>();
cx.univariant(dl, &fields, &ReprOptions::default(), kind).ok_or(LayoutError::Unknown)?
cx.calc.univariant(&fields, &ReprOptions::default(), kind)?
}
TyKind::Array(element, count) => {
let count = try_const_usize(db, count).ok_or(LayoutError::HasErrorConst)? as u64;
Expand Down Expand Up @@ -367,12 +371,12 @@ pub fn layout_of_ty_query(
};

// Effectively a (ptr, meta) tuple.
cx.scalar_pair(data_ptr, metadata)
cx.calc.scalar_pair(data_ptr, metadata)
}
TyKind::FnDef(_, _) => layout_of_unit(&cx, dl)?,
TyKind::Never => cx.layout_of_never_type(),
TyKind::FnDef(_, _) => layout_of_unit(&cx)?,
TyKind::Never => cx.calc.layout_of_never_type(),
TyKind::Dyn(_) | TyKind::Foreign(_) => {
let mut unit = layout_of_unit(&cx, dl)?;
let mut unit = layout_of_unit(&cx)?;
match &mut unit.abi {
Abi::Aggregate { sized } => *sized = false,
_ => return Err(LayoutError::Unknown),
Expand Down Expand Up @@ -414,8 +418,7 @@ pub fn layout_of_ty_query(
.collect::<Result<Vec<_>, _>>()?;
let fields = fields.iter().map(|it| &**it).collect::<Vec<_>>();
let fields = fields.iter().collect::<IndexVec<_, _>>();
cx.univariant(dl, &fields, &ReprOptions::default(), StructKind::AlwaysSized)
.ok_or(LayoutError::Unknown)?
cx.calc.univariant(&fields, &ReprOptions::default(), StructKind::AlwaysSized)?
}
TyKind::Coroutine(_, _) | TyKind::CoroutineWitness(_, _) => {
return Err(LayoutError::NotImplemented)
Expand Down Expand Up @@ -447,14 +450,14 @@ pub fn layout_of_ty_recover(
Err(LayoutError::RecursiveTypeWithoutIndirection)
}

fn layout_of_unit(cx: &LayoutCx<'_>, dl: &TargetDataLayout) -> Result<Layout, LayoutError> {
cx.univariant::<RustcFieldIdx, RustcEnumVariantIdx, &&Layout>(
dl,
IndexSlice::empty(),
&ReprOptions::default(),
StructKind::AlwaysSized,
)
.ok_or(LayoutError::Unknown)
fn layout_of_unit(cx: &LayoutCx<'_>) -> Result<Layout, LayoutError> {
cx.calc
.univariant::<RustcFieldIdx, RustcEnumVariantIdx, &&Layout>(
IndexSlice::empty(),
&ReprOptions::default(),
StructKind::AlwaysSized,
)
.map_err(Into::into)
}

fn struct_tail_erasing_lifetimes(db: &dyn HirDatabase, pointee: Ty) -> Ty {
Expand Down
13 changes: 6 additions & 7 deletions crates/hir-ty/src/layout/adt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::{cmp, ops::Bound};
use base_db::salsa::Cycle;
use hir_def::{
data::adt::VariantData,
layout::{Integer, LayoutCalculator, ReprOptions, TargetDataLayout},
layout::{Integer, ReprOptions, TargetDataLayout},
AdtId, VariantId,
};
use intern::sym;
Expand Down Expand Up @@ -36,8 +36,8 @@ pub fn layout_of_adt_query(
let Ok(target) = db.target_data_layout(krate) else {
return Err(LayoutError::TargetLayoutNotAvailable);
};
let cx = LayoutCx { target: &target };
let dl = cx.current_data_layout();
let dl = &*target;
let cx = LayoutCx::new(dl);
let handle_variant = |def: VariantId, var: &VariantData| {
var.fields()
.iter()
Expand Down Expand Up @@ -73,9 +73,9 @@ pub fn layout_of_adt_query(
.collect::<SmallVec<[_; 1]>>();
let variants = variants.iter().map(|it| it.iter().collect()).collect::<IndexVec<_, _>>();
let result = if matches!(def, AdtId::UnionId(..)) {
cx.layout_of_union(&repr, &variants).ok_or(LayoutError::Unknown)?
cx.calc.layout_of_union(&repr, &variants)?
} else {
cx.layout_of_struct_or_enum(
cx.calc.layout_of_struct_or_enum(
&repr,
&variants,
matches!(def, AdtId::EnumId(..)),
Expand Down Expand Up @@ -103,8 +103,7 @@ pub fn layout_of_adt_query(
.next()
.and_then(|it| it.iter().last().map(|it| !it.is_unsized()))
.unwrap_or(true),
)
.ok_or(LayoutError::SizeOverflow)?
)?
};
Ok(Arc::new(result))
}
Expand Down

0 comments on commit 705d602

Please sign in to comment.