Skip to content

Commit

Permalink
Auto merge of #88337 - eddyb:field-failure-is-not-an-option, r=nagisa
Browse files Browse the repository at this point in the history
rustc_target: `TyAndLayout::field` should never error.

This refactor (making `TyAndLayout::field` return `TyAndLayout` without any `Result` around it) is based on a simple observation, regarding `TyAndLayout::field`:

If `cx.layout_of(ty)` succeeds (for some `cx` and `ty`), then `.field(cx, i)` on the resulting `TyAndLayout` should *always* succeed in computing `cx.layout_of(field_ty)` (where `field_ty` is the type of the `i`th field of `ty`).

The reason for this is that no matter which field is chosen, `cx.layout_of(field_ty)` *will have already been computed*, as part of computing `cx.layout_of(ty)`, as we cannot determine the layout of *any* type without considering the layouts of *all* of its fields.

And so it should be fine to turn any errors into ICEs, since they likely indicate a `cx` mismatch, or some other edge case that is due to a compiler bug (as opposed to ever being an user-facing error).

<hr/>

Each commit should probably be reviewed separately, though note that there's some `where` clauses (in `rustc_target::abi::call::*`) that change in most commits.

cc `@nagisa` `@oli-obk`
  • Loading branch information
bors committed Aug 29, 2021
2 parents 2f662b1 + 78778fc commit 9556d7a
Show file tree
Hide file tree
Showing 32 changed files with 308 additions and 271 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_cranelift/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ pub(crate) struct FunctionCx<'m, 'clif, 'tcx: 'm> {
pub(crate) inline_asm_index: u32,
}

impl<'tcx> LayoutOf for FunctionCx<'_, '_, 'tcx> {
impl<'tcx> LayoutOf<'tcx> for FunctionCx<'_, '_, 'tcx> {
type Ty = Ty<'tcx>;
type TyAndLayout = TyAndLayout<'tcx>;

Expand Down Expand Up @@ -364,7 +364,7 @@ impl<'tcx> FunctionCx<'_, '_, 'tcx> {

pub(crate) struct RevealAllLayoutCx<'tcx>(pub(crate) TyCtxt<'tcx>);

impl<'tcx> LayoutOf for RevealAllLayoutCx<'tcx> {
impl<'tcx> LayoutOf<'tcx> for RevealAllLayoutCx<'tcx> {
type Ty = Ty<'tcx>;
type TyAndLayout = TyAndLayout<'tcx>;

Expand Down
10 changes: 4 additions & 6 deletions compiler/rustc_codegen_cranelift/src/debuginfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,12 +160,10 @@ impl<'tcx> DebugContext<'tcx> {

for (field_idx, field_def) in variant.fields.iter().enumerate() {
let field_offset = layout.fields.offset(field_idx);
let field_layout = layout
.field(
&layout::LayoutCx { tcx: self.tcx, param_env: ParamEnv::reveal_all() },
field_idx,
)
.unwrap();
let field_layout = layout.field(
&layout::LayoutCx { tcx: self.tcx, param_env: ParamEnv::reveal_all() },
field_idx,
);

let field_type = self.dwarf_ty(field_layout.ty);

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ pub(crate) fn codegen_intrinsic_call<'tcx>(
return;
}

if intrinsic == sym::assert_zero_valid && !layout.might_permit_raw_init(fx, /*zero:*/ true).unwrap() {
if intrinsic == sym::assert_zero_valid && !layout.might_permit_raw_init(fx, /*zero:*/ true) {
with_no_trimmed_paths(|| crate::base::codegen_panic(
fx,
&format!("attempted to zero-initialize type `{}`, which is invalid", T),
Expand All @@ -798,7 +798,7 @@ pub(crate) fn codegen_intrinsic_call<'tcx>(
return;
}

if intrinsic == sym::assert_uninit_valid && !layout.might_permit_raw_init(fx, /*zero:*/ false).unwrap() {
if intrinsic == sym::assert_uninit_valid && !layout.might_permit_raw_init(fx, /*zero:*/ false) {
with_no_trimmed_paths(|| crate::base::codegen_panic(
fx,
&format!("attempted to leave type `{}` uninitialized, which is invalid", T),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ impl HasTargetSpec for Builder<'_, '_, 'tcx> {
}
}

impl abi::LayoutOf for Builder<'_, '_, 'tcx> {
impl abi::LayoutOf<'tcx> for Builder<'_, '_, 'tcx> {
type Ty = Ty<'tcx>;
type TyAndLayout = TyAndLayout<'tcx>;

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,7 @@ impl ty::layout::HasTyCtxt<'tcx> for CodegenCx<'ll, 'tcx> {
}
}

impl LayoutOf for CodegenCx<'ll, 'tcx> {
impl LayoutOf<'tcx> for CodegenCx<'ll, 'tcx> {
type Ty = Ty<'tcx>;
type TyAndLayout = TyAndLayout<'tcx>;

Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_codegen_llvm/src/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::{self, Ty, TypeFoldable};
use rustc_target::abi::{Abi, AddressSpace, Align, FieldsShape};
use rustc_target::abi::{Int, Pointer, F32, F64};
use rustc_target::abi::{LayoutOf, PointeeInfo, Scalar, Size, TyAndLayoutMethods, Variants};
use rustc_target::abi::{LayoutOf, PointeeInfo, Scalar, Size, TyAbiInterface, Variants};
use smallvec::{smallvec, SmallVec};
use tracing::debug;

Expand Down Expand Up @@ -393,12 +393,15 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> {
}
}

// FIXME(eddyb) this having the same name as `TyAndLayout::pointee_info_at`
// (the inherent method, which is lacking this caching logic) can result in
// the uncached version being called - not wrong, but potentially inefficient.
fn pointee_info_at<'a>(&self, cx: &CodegenCx<'a, 'tcx>, offset: Size) -> Option<PointeeInfo> {
if let Some(&pointee) = cx.pointee_infos.borrow().get(&(self.ty, offset)) {
return pointee;
}

let result = Ty::pointee_info_at(*self, cx, offset);
let result = Ty::ty_and_layout_pointee_info_at(*self, cx, offset);

cx.pointee_infos.borrow_mut().insert((self.ty, offset), result);
result
Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,10 +472,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let layout = bx.layout_of(ty);
let do_panic = match intrinsic {
Inhabited => layout.abi.is_uninhabited(),
// We unwrap as the error type is `!`.
ZeroValid => !layout.might_permit_raw_init(bx, /*zero:*/ true).unwrap(),
// We unwrap as the error type is `!`.
UninitValid => !layout.might_permit_raw_init(bx, /*zero:*/ false).unwrap(),
ZeroValid => !layout.might_permit_raw_init(bx, /*zero:*/ true),
UninitValid => !layout.might_permit_raw_init(bx, /*zero:*/ false),
};
if do_panic {
let msg_str = with_no_trimmed_paths(|| {
Expand Down
9 changes: 7 additions & 2 deletions compiler/rustc_codegen_ssa/src/traits/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,17 @@ pub trait BackendTypes {
}

pub trait Backend<'tcx>:
Sized + BackendTypes + HasTyCtxt<'tcx> + LayoutOf<Ty = Ty<'tcx>, TyAndLayout = TyAndLayout<'tcx>>
Sized
+ BackendTypes
+ HasTyCtxt<'tcx>
+ LayoutOf<'tcx, Ty = Ty<'tcx>, TyAndLayout = TyAndLayout<'tcx>>
{
}

impl<'tcx, T> Backend<'tcx> for T where
Self: BackendTypes + HasTyCtxt<'tcx> + LayoutOf<Ty = Ty<'tcx>, TyAndLayout = TyAndLayout<'tcx>>
Self: BackendTypes
+ HasTyCtxt<'tcx>
+ LayoutOf<'tcx, Ty = Ty<'tcx>, TyAndLayout = TyAndLayout<'tcx>>
{
}

Expand Down
25 changes: 23 additions & 2 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ use rustc_session::Session;
use rustc_session::SessionLintStore;
use rustc_span::lev_distance::find_best_match_for_name;
use rustc_span::{symbol::Symbol, MultiSpan, Span, DUMMY_SP};
use rustc_target::abi::LayoutOf;
use rustc_target::abi::{self, LayoutOf};
use tracing::debug;

use std::cell::Cell;
Expand Down Expand Up @@ -1059,7 +1059,28 @@ impl<'tcx> LateContext<'tcx> {
}
}

impl<'tcx> LayoutOf for LateContext<'tcx> {
impl<'tcx> abi::HasDataLayout for LateContext<'tcx> {
#[inline]
fn data_layout(&self) -> &abi::TargetDataLayout {
&self.tcx.data_layout
}
}

impl<'tcx> ty::layout::HasTyCtxt<'tcx> for LateContext<'tcx> {
#[inline]
fn tcx(&self) -> TyCtxt<'tcx> {
self.tcx
}
}

impl<'tcx> ty::layout::HasParamEnv<'tcx> for LateContext<'tcx> {
#[inline]
fn param_env(&self) -> ty::ParamEnv<'tcx> {
self.param_env
}
}

impl<'tcx> LayoutOf<'tcx> for LateContext<'tcx> {
type Ty = Ty<'tcx>;
type TyAndLayout = Result<TyAndLayout<'tcx>, LayoutError<'tcx>>;

Expand Down
Loading

0 comments on commit 9556d7a

Please sign in to comment.