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 #123169 (comment)

r? `@compiler-errors` or compiler

fixes #123134
fixes #124182
fixes #126939
fixes #127737
  • Loading branch information
bors committed Sep 17, 2024
2 parents bde6bf2 + 20d2414 commit e2dc1a1
Show file tree
Hide file tree
Showing 29 changed files with 1,341 additions and 1,227 deletions.
1,872 changes: 952 additions & 920 deletions compiler/rustc_abi/src/layout.rs

Large diffs are not rendered by default.

10 changes: 9 additions & 1 deletion compiler/rustc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ mod layout;
#[cfg(test)]
mod tests;

pub use layout::LayoutCalculator;
pub use layout::{LayoutCalculator, LayoutCalculatorError};

/// Requirements for a `StableHashingContext` to be used in this crate.
/// This is a hack to allow using the `HashStable_Generic` derive macro
Expand Down Expand Up @@ -393,6 +393,14 @@ impl HasDataLayout for TargetDataLayout {
}
}

// used by rust-analyzer
impl HasDataLayout for &TargetDataLayout {
#[inline]
fn data_layout(&self) -> &TargetDataLayout {
(**self).data_layout()
}
}

/// Endianness of the target, which must match cfg(target-endian).
#[derive(Copy, Clone, PartialEq, Eq)]
pub enum Endian {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/const_eval/valtrees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ pub fn valtree_to_const_value<'tcx>(
let branches = valtree.unwrap_branch();
// Find the non-ZST field. (There can be aligned ZST!)
for (i, &inner_valtree) in branches.iter().enumerate() {
let field = layout.field(&LayoutCx { tcx, param_env }, i);
let field = layout.field(&LayoutCx::new(tcx, param_env), i);
if !field.is_zst() {
return valtree_to_const_value(tcx, param_env.and(field.ty), inner_valtree);
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use rustc_middle::mir::interpret::{
UnsupportedOpInfo, ValidationErrorInfo,
};
use rustc_middle::ty::layout::{LayoutCx, LayoutOf, TyAndLayout};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_middle::ty::{self, Ty};
use rustc_span::symbol::{sym, Symbol};
use rustc_target::abi::{
Abi, FieldIdx, FieldsShape, Scalar as ScalarAbi, Size, VariantIdx, Variants, WrappingRange,
Expand Down Expand Up @@ -940,7 +940,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
) -> Cow<'e, RangeSet> {
assert!(layout.ty.is_union());
assert!(layout.abi.is_sized(), "there are no unsized unions");
let layout_cx = LayoutCx { tcx: *ecx.tcx, param_env: ecx.param_env };
let layout_cx = LayoutCx::new(*ecx.tcx, ecx.param_env);
return M::cached_union_data_range(ecx, layout.ty, || {
let mut out = RangeSet(Vec::new());
union_data_range_uncached(&layout_cx, layout, Size::ZERO, &mut out);
Expand All @@ -949,7 +949,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {

/// Helper for recursive traversal: add data ranges of the given type to `out`.
fn union_data_range_uncached<'tcx>(
cx: &LayoutCx<'tcx, TyCtxt<'tcx>>,
cx: &LayoutCx<'tcx>,
layout: TyAndLayout<'tcx>,
base_offset: Size,
out: &mut RangeSet,
Expand Down
12 changes: 7 additions & 5 deletions compiler/rustc_const_eval/src/util/check_validity_requirement.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use rustc_middle::bug;
use rustc_middle::ty::layout::{LayoutCx, LayoutError, LayoutOf, TyAndLayout, ValidityRequirement};
use rustc_middle::ty::layout::{
HasTyCtxt, LayoutCx, LayoutError, LayoutOf, TyAndLayout, ValidityRequirement,
};
use rustc_middle::ty::{ParamEnvAnd, Ty, TyCtxt};
use rustc_target::abi::{Abi, FieldsShape, Scalar, Variants};

Expand Down Expand Up @@ -30,7 +32,7 @@ pub fn check_validity_requirement<'tcx>(
return Ok(!layout.abi.is_uninhabited());
}

let layout_cx = LayoutCx { tcx, param_env: param_env_and_ty.param_env };
let layout_cx = LayoutCx::new(tcx, param_env_and_ty.param_env);
if kind == ValidityRequirement::Uninit || tcx.sess.opts.unstable_opts.strict_init_checks {
check_validity_requirement_strict(layout, &layout_cx, kind)
} else {
Expand All @@ -42,12 +44,12 @@ pub fn check_validity_requirement<'tcx>(
/// for details.
fn check_validity_requirement_strict<'tcx>(
ty: TyAndLayout<'tcx>,
cx: &LayoutCx<'tcx, TyCtxt<'tcx>>,
cx: &LayoutCx<'tcx>,
kind: ValidityRequirement,
) -> Result<bool, &'tcx LayoutError<'tcx>> {
let machine = CompileTimeMachine::new(CanAccessMutGlobal::No, CheckAlignment::Error);

let mut cx = InterpCx::new(cx.tcx, rustc_span::DUMMY_SP, cx.param_env, machine);
let mut cx = InterpCx::new(cx.tcx(), rustc_span::DUMMY_SP, cx.param_env, machine);

let allocated = cx
.allocate(ty, MemoryKind::Machine(crate::const_eval::MemoryKind::Heap))
Expand Down Expand Up @@ -80,7 +82,7 @@ fn check_validity_requirement_strict<'tcx>(
/// function for details.
fn check_validity_requirement_lax<'tcx>(
this: TyAndLayout<'tcx>,
cx: &LayoutCx<'tcx, TyCtxt<'tcx>>,
cx: &LayoutCx<'tcx>,
init_kind: ValidityRequirement,
) -> Result<bool, &'tcx LayoutError<'tcx>> {
let scalar_allows_raw_init = move |s: Scalar| -> bool {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_index/src/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub struct IndexSlice<I: Idx, T> {

impl<I: Idx, T> IndexSlice<I, T> {
#[inline]
pub const fn empty() -> &'static Self {
pub const fn empty<'a>() -> &'a Self {
Self::from_raw(&[])
}

Expand Down
60 changes: 17 additions & 43 deletions compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::borrow::Cow;
use std::num::NonZero;
use std::ops::Bound;
use std::{cmp, fmt};
Expand Down Expand Up @@ -286,20 +285,14 @@ impl<'tcx> IntoDiagArg for LayoutError<'tcx> {
}

#[derive(Clone, Copy)]
pub struct LayoutCx<'tcx, C> {
pub tcx: C,
pub struct LayoutCx<'tcx> {
pub calc: LayoutCalculator<TyCtxt<'tcx>>,
pub param_env: ty::ParamEnv<'tcx>,
}

impl<'tcx> LayoutCalculator for LayoutCx<'tcx, TyCtxt<'tcx>> {
type TargetDataLayoutRef = &'tcx TargetDataLayout;

fn delayed_bug(&self, txt: impl Into<Cow<'static, str>>) {
self.tcx.dcx().delayed_bug(txt);
}

fn current_data_layout(&self) -> Self::TargetDataLayoutRef {
&self.tcx.data_layout
impl<'tcx> LayoutCx<'tcx> {
pub fn new(tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> Self {
Self { calc: LayoutCalculator::new(tcx), param_env }
}
}

Expand Down Expand Up @@ -568,33 +561,33 @@ impl<'tcx> HasTyCtxt<'tcx> for TyCtxtAt<'tcx> {
}
}

impl<'tcx, C> HasParamEnv<'tcx> for LayoutCx<'tcx, C> {
impl<'tcx> HasParamEnv<'tcx> for LayoutCx<'tcx> {
fn param_env(&self) -> ty::ParamEnv<'tcx> {
self.param_env
}
}

impl<'tcx, T: HasDataLayout> HasDataLayout for LayoutCx<'tcx, T> {
impl<'tcx> HasDataLayout for LayoutCx<'tcx> {
fn data_layout(&self) -> &TargetDataLayout {
self.tcx.data_layout()
self.calc.cx.data_layout()
}
}

impl<'tcx, T: HasTargetSpec> HasTargetSpec for LayoutCx<'tcx, T> {
impl<'tcx> HasTargetSpec for LayoutCx<'tcx> {
fn target_spec(&self) -> &Target {
self.tcx.target_spec()
self.calc.cx.target_spec()
}
}

impl<'tcx, T: HasWasmCAbiOpt> HasWasmCAbiOpt for LayoutCx<'tcx, T> {
impl<'tcx> HasWasmCAbiOpt for LayoutCx<'tcx> {
fn wasm_c_abi_opt(&self) -> WasmCAbi {
self.tcx.wasm_c_abi_opt()
self.calc.cx.wasm_c_abi_opt()
}
}

impl<'tcx, T: HasTyCtxt<'tcx>> HasTyCtxt<'tcx> for LayoutCx<'tcx, T> {
impl<'tcx> HasTyCtxt<'tcx> for LayoutCx<'tcx> {
fn tcx(&self) -> TyCtxt<'tcx> {
self.tcx.tcx()
self.calc.cx
}
}

Expand Down Expand Up @@ -685,7 +678,7 @@ pub trait LayoutOf<'tcx>: LayoutOfHelpers<'tcx> {

impl<'tcx, C: LayoutOfHelpers<'tcx>> LayoutOf<'tcx> for C {}

impl<'tcx> LayoutOfHelpers<'tcx> for LayoutCx<'tcx, TyCtxt<'tcx>> {
impl<'tcx> LayoutOfHelpers<'tcx> for LayoutCx<'tcx> {
type LayoutOfResult = Result<TyAndLayout<'tcx>, &'tcx LayoutError<'tcx>>;

#[inline]
Expand All @@ -695,26 +688,7 @@ impl<'tcx> LayoutOfHelpers<'tcx> for LayoutCx<'tcx, TyCtxt<'tcx>> {
_: Span,
_: Ty<'tcx>,
) -> &'tcx LayoutError<'tcx> {
self.tcx.arena.alloc(err)
}
}

impl<'tcx> LayoutOfHelpers<'tcx> for LayoutCx<'tcx, TyCtxtAt<'tcx>> {
type LayoutOfResult = Result<TyAndLayout<'tcx>, &'tcx LayoutError<'tcx>>;

#[inline]
fn layout_tcx_at_span(&self) -> Span {
self.tcx.span
}

#[inline]
fn handle_layout_err(
&self,
err: LayoutError<'tcx>,
_: Span,
_: Ty<'tcx>,
) -> &'tcx LayoutError<'tcx> {
self.tcx.arena.alloc(err)
self.tcx().arena.alloc(err)
}
}

Expand Down Expand Up @@ -1342,7 +1316,7 @@ impl<'tcx> TyCtxt<'tcx> {
where
I: Iterator<Item = (VariantIdx, FieldIdx)>,
{
let cx = LayoutCx { tcx: self, param_env };
let cx = LayoutCx::new(self, param_env);
let mut offset = Size::ZERO;

for (variant, field) in indices {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_passes/src/layout_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ fn dump_layout_of(tcx: TyCtxt<'_>, item_def_id: LocalDefId, attr: &Attribute) {
}

Err(layout_error) => {
tcx.dcx().emit_fatal(Spanned { node: layout_error.into_diagnostic(), span });
tcx.dcx().emit_err(Spanned { node: layout_error.into_diagnostic(), span });
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_transmute/src/layout/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ pub mod rustc {
use std::fmt::{self, Write};

use rustc_middle::mir::Mutability;
use rustc_middle::ty::layout::{LayoutCx, LayoutError};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_middle::ty::layout::{HasTyCtxt, LayoutCx, LayoutError};
use rustc_middle::ty::{self, Ty};
use rustc_target::abi::Layout;

/// A reference in the layout.
Expand Down Expand Up @@ -124,11 +124,11 @@ pub mod rustc {
}

pub(crate) fn layout_of<'tcx>(
cx: LayoutCx<'tcx, TyCtxt<'tcx>>,
cx: LayoutCx<'tcx>,
ty: Ty<'tcx>,
) -> Result<Layout<'tcx>, &'tcx LayoutError<'tcx>> {
use rustc_middle::ty::layout::LayoutOf;
let ty = cx.tcx.erase_regions(ty);
let ty = cx.tcx().erase_regions(ty);
cx.layout_of(ty).map(|tl| tl.layout)
}
}
28 changes: 14 additions & 14 deletions compiler/rustc_transmute/src/layout/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,15 +204,15 @@ pub(crate) mod rustc {
}

impl<'tcx> Tree<Def<'tcx>, Ref<'tcx>> {
pub(crate) fn from_ty(ty: Ty<'tcx>, cx: LayoutCx<'tcx, TyCtxt<'tcx>>) -> Result<Self, Err> {
pub(crate) fn from_ty(ty: Ty<'tcx>, cx: LayoutCx<'tcx>) -> Result<Self, Err> {
use rustc_target::abi::HasDataLayout;
let layout = layout_of(cx, ty)?;

if let Err(e) = ty.error_reported() {
return Err(Err::TypeError(e));
}

let target = cx.tcx.data_layout();
let target = cx.data_layout();
let pointer_size = target.pointer_size;

match ty.kind() {
Expand Down Expand Up @@ -274,7 +274,7 @@ pub(crate) mod rustc {
fn from_tuple(
(ty, layout): (Ty<'tcx>, Layout<'tcx>),
members: &'tcx List<Ty<'tcx>>,
cx: LayoutCx<'tcx, TyCtxt<'tcx>>,
cx: LayoutCx<'tcx>,
) -> Result<Self, Err> {
match &layout.fields {
FieldsShape::Primitive => {
Expand All @@ -299,7 +299,7 @@ pub(crate) mod rustc {
fn from_struct(
(ty, layout): (Ty<'tcx>, Layout<'tcx>),
def: AdtDef<'tcx>,
cx: LayoutCx<'tcx, TyCtxt<'tcx>>,
cx: LayoutCx<'tcx>,
) -> Result<Self, Err> {
assert!(def.is_struct());
let def = Def::Adt(def);
Expand All @@ -314,13 +314,13 @@ pub(crate) mod rustc {
fn from_enum(
(ty, layout): (Ty<'tcx>, Layout<'tcx>),
def: AdtDef<'tcx>,
cx: LayoutCx<'tcx, TyCtxt<'tcx>>,
cx: LayoutCx<'tcx>,
) -> Result<Self, Err> {
assert!(def.is_enum());

// Computes the variant of a given index.
let layout_of_variant = |index, encoding: Option<TagEncoding<VariantIdx>>| {
let tag = cx.tcx.tag_for_variant((cx.tcx.erase_regions(ty), index));
let tag = cx.tcx().tag_for_variant((cx.tcx().erase_regions(ty), index));
let variant_def = Def::Variant(def.variant(index));
let variant_layout = ty_variant(cx, (ty, layout), index);
Self::from_variant(
Expand Down Expand Up @@ -389,7 +389,7 @@ pub(crate) mod rustc {
tag: Option<(ScalarInt, VariantIdx, TagEncoding<VariantIdx>)>,
(ty, layout): (Ty<'tcx>, Layout<'tcx>),
total_size: Size,
cx: LayoutCx<'tcx, TyCtxt<'tcx>>,
cx: LayoutCx<'tcx>,
) -> Result<Self, Err> {
// This constructor does not support non-`FieldsShape::Arbitrary`
// layouts.
Expand Down Expand Up @@ -417,7 +417,7 @@ pub(crate) mod rustc {
}
}
}
struct_tree = struct_tree.then(Self::from_tag(*tag, cx.tcx));
struct_tree = struct_tree.then(Self::from_tag(*tag, cx.tcx()));
}

// Append the fields, in memory order, to the layout.
Expand Down Expand Up @@ -470,7 +470,7 @@ pub(crate) mod rustc {
fn from_union(
(ty, layout): (Ty<'tcx>, Layout<'tcx>),
def: AdtDef<'tcx>,
cx: LayoutCx<'tcx, TyCtxt<'tcx>>,
cx: LayoutCx<'tcx>,
) -> Result<Self, Err> {
assert!(def.is_union());

Expand Down Expand Up @@ -500,7 +500,7 @@ pub(crate) mod rustc {
}

fn ty_field<'tcx>(
cx: LayoutCx<'tcx, TyCtxt<'tcx>>,
cx: LayoutCx<'tcx>,
(ty, layout): (Ty<'tcx>, Layout<'tcx>),
i: FieldIdx,
) -> Ty<'tcx> {
Expand All @@ -509,12 +509,12 @@ pub(crate) mod rustc {
match layout.variants {
Variants::Single { index } => {
let field = &def.variant(index).fields[i];
field.ty(cx.tcx, args)
field.ty(cx.tcx(), args)
}
// Discriminant field for enums (where applicable).
Variants::Multiple { tag, .. } => {
assert_eq!(i.as_usize(), 0);
ty::layout::PrimitiveExt::to_ty(&tag.primitive(), cx.tcx)
ty::layout::PrimitiveExt::to_ty(&tag.primitive(), cx.tcx())
}
}
}
Expand All @@ -527,11 +527,11 @@ pub(crate) mod rustc {
}

fn ty_variant<'tcx>(
cx: LayoutCx<'tcx, TyCtxt<'tcx>>,
cx: LayoutCx<'tcx>,
(ty, layout): (Ty<'tcx>, Layout<'tcx>),
i: VariantIdx,
) -> Layout<'tcx> {
let ty = cx.tcx.erase_regions(ty);
let ty = cx.tcx().erase_regions(ty);
TyAndLayout { ty, layout }.for_variant(&cx, i).layout
}
}
2 changes: 1 addition & 1 deletion compiler/rustc_transmute/src/maybe_transmutable/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ mod rustc {
pub fn answer(self) -> Answer<<TyCtxt<'tcx> as QueryContext>::Ref> {
let Self { src, dst, assume, context } = self;

let layout_cx = LayoutCx { tcx: context, param_env: ParamEnv::reveal_all() };
let layout_cx = LayoutCx::new(context, ParamEnv::reveal_all());

// Convert `src` and `dst` from their rustc representations, to `Tree`-based
// representations.
Expand Down
Loading

0 comments on commit e2dc1a1

Please sign in to comment.