Skip to content

Commit

Permalink
Unrolled build for #136520
Browse files Browse the repository at this point in the history
Rollup merge of #136520 - compiler-errors:redundant-layout-assert, r=lcnr

Remove unnecessary layout assertions for object-safe receivers

The soundness of `DispatchFromDyn` relies on the fact that, like all other built-in marker-like layout traits (e.g. `Sized`, `CoerceUnsized`), the guarantees that they enforce in *generic* code via traits will result in assumptions that we can rely on in codegen.

Specifically, `DispatchFromDyn` ensures that we end up with a receiver that is a valid pointer type, and its implementation validity recursively ensures that the ABI of that pointer type upholds the `Scalar` or `ScalarPair` representation for sized and unsized pointees, respectively.

The check that this layout guarantee holds for arbitrary, possibly generic receiver types that also may exist in possibly impossible-to-instantiate where clauses is overkill IMO, and leads to several ICEs due to the fact that computing layouts before monomorphization is going to be fallible at best.

This PR removes the check altogether, since it just exists as a sanity check from very long ago, 6f2a161.

Fixes #125810
Fixes #90110

This PR is an alternative to #136195. cc `@adetaylor.` I didn't realize in that PR that the layout checks that were being modified were simply *sanity checks*, rather than being actually necessary for soundness.
  • Loading branch information
rust-timer authored Feb 4, 2025
2 parents 3f33b30 + 0b26dc0 commit 2c194e7
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 173 deletions.
107 changes: 2 additions & 105 deletions compiler/rustc_trait_selection/src/traits/dyn_compatibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,16 @@
//!
//! [^1]: Formerly known as "object safety".
use std::iter;
use std::ops::ControlFlow;

use rustc_abi::BackendRepr;
use rustc_errors::FatalError;
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_middle::bug;
use rustc_middle::query::Providers;
use rustc_middle::ty::{
self, EarlyBinder, ExistentialPredicateStableCmpExt as _, GenericArgs, Ty, TyCtxt,
TypeFoldable, TypeFolder, TypeSuperFoldable, TypeSuperVisitable, TypeVisitable,
TypeVisitableExt, TypeVisitor, TypingMode, Upcast,
self, EarlyBinder, GenericArgs, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable,
TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor, TypingMode, Upcast,
};
use rustc_span::Span;
use rustc_type_ir::elaborate;
Expand Down Expand Up @@ -109,14 +106,6 @@ fn dyn_compatibility_violations_for_trait(
violations.push(DynCompatibilityViolation::SupertraitNonLifetimeBinder(spans));
}

if violations.is_empty() {
for item in tcx.associated_items(trait_def_id).in_definition_order() {
if let ty::AssocKind::Fn = item.kind {
check_receiver_correct(tcx, trait_def_id, *item);
}
}
}

violations
}

Expand Down Expand Up @@ -499,55 +488,6 @@ fn virtual_call_violations_for_method<'tcx>(
errors
}

/// This code checks that `receiver_is_dispatchable` is correctly implemented.
///
/// This check is outlined from the dyn-compatibility check to avoid cycles with
/// layout computation, which relies on knowing whether methods are dyn-compatible.
fn check_receiver_correct<'tcx>(tcx: TyCtxt<'tcx>, trait_def_id: DefId, method: ty::AssocItem) {
if !is_vtable_safe_method(tcx, trait_def_id, method) {
return;
}

let method_def_id = method.def_id;
let sig = tcx.fn_sig(method_def_id).instantiate_identity();
let typing_env = ty::TypingEnv::non_body_analysis(tcx, method_def_id);
let receiver_ty = tcx.liberate_late_bound_regions(method_def_id, sig.input(0));

if receiver_ty == tcx.types.self_param {
// Assumed OK, may change later if unsized_locals permits `self: Self` as dispatchable.
return;
}

// e.g., `Rc<()>`
let unit_receiver_ty = receiver_for_self_ty(tcx, receiver_ty, tcx.types.unit, method_def_id);
match tcx.layout_of(typing_env.as_query_input(unit_receiver_ty)).map(|l| l.backend_repr) {
Ok(BackendRepr::Scalar(..)) => (),
abi => {
tcx.dcx().span_delayed_bug(
tcx.def_span(method_def_id),
format!("receiver {unit_receiver_ty:?} when `Self = ()` should have a Scalar ABI; found {abi:?}"),
);
}
}

let trait_object_ty = object_ty_for_trait(tcx, trait_def_id, tcx.lifetimes.re_static);

// e.g., `Rc<dyn Trait>`
let trait_object_receiver =
receiver_for_self_ty(tcx, receiver_ty, trait_object_ty, method_def_id);
match tcx.layout_of(typing_env.as_query_input(trait_object_receiver)).map(|l| l.backend_repr) {
Ok(BackendRepr::ScalarPair(..)) => (),
abi => {
tcx.dcx().span_delayed_bug(
tcx.def_span(method_def_id),
format!(
"receiver {trait_object_receiver:?} when `Self = {trait_object_ty}` should have a ScalarPair ABI; found {abi:?}"
),
);
}
}
}

/// Performs a type instantiation to produce the version of `receiver_ty` when `Self = self_ty`.
/// For example, for `receiver_ty = Rc<Self>` and `self_ty = Foo`, returns `Rc<Foo>`.
fn receiver_for_self_ty<'tcx>(
Expand All @@ -569,49 +509,6 @@ fn receiver_for_self_ty<'tcx>(
result
}

/// Creates the object type for the current trait. For example,
/// if the current trait is `Deref`, then this will be
/// `dyn Deref<Target = Self::Target> + 'static`.
#[instrument(level = "trace", skip(tcx), ret)]
fn object_ty_for_trait<'tcx>(
tcx: TyCtxt<'tcx>,
trait_def_id: DefId,
lifetime: ty::Region<'tcx>,
) -> Ty<'tcx> {
let trait_ref = ty::TraitRef::identity(tcx, trait_def_id);
debug!(?trait_ref);

let trait_predicate = ty::Binder::dummy(ty::ExistentialPredicate::Trait(
ty::ExistentialTraitRef::erase_self_ty(tcx, trait_ref),
));
debug!(?trait_predicate);

let pred: ty::Predicate<'tcx> = trait_ref.upcast(tcx);
let mut elaborated_predicates: Vec<_> = elaborate(tcx, [pred])
.filter_map(|pred| {
debug!(?pred);
let pred = pred.as_projection_clause()?;
Some(pred.map_bound(|p| {
ty::ExistentialPredicate::Projection(ty::ExistentialProjection::erase_self_ty(
tcx, p,
))
}))
})
.collect();
// NOTE: Since #37965, the existential predicates list has depended on the
// list of predicates to be sorted. This is mostly to enforce that the primary
// predicate comes first.
elaborated_predicates.sort_by(|a, b| a.skip_binder().stable_cmp(tcx, &b.skip_binder()));
elaborated_predicates.dedup();

let existential_predicates = tcx.mk_poly_existential_predicates_from_iter(
iter::once(trait_predicate).chain(elaborated_predicates),
);
debug!(?existential_predicates);

Ty::new_dynamic(tcx, existential_predicates, lifetime, ty::Dyn)
}

/// Checks the method's receiver (the `self` argument) can be dispatched on when `Self` is a
/// trait object. We require that `DispatchableFromDyn` be implemented for the receiver type
/// in the following way:
Expand Down
10 changes: 0 additions & 10 deletions tests/crashes/125810.rs

This file was deleted.

57 changes: 0 additions & 57 deletions tests/crashes/90110.rs

This file was deleted.

16 changes: 16 additions & 0 deletions tests/ui/self/dispatch-from-dyn-layout-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//@ check-pass
// Regression test for #90110.

// Make sure that object safety checking doesn't freak out when
// we have impossible-to-satisfy `Sized` predicates.

trait Parser
where
for<'a> (dyn Parser + 'a): Sized,
{
fn parse_line(&self);
}

fn foo(_: &dyn Parser) {}

fn main() {}
19 changes: 19 additions & 0 deletions tests/ui/self/dispatch-from-dyn-layout-3.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//@ check-pass

// Make sure that object safety checking doesn't freak out when
// we have impossible-to-satisfy `DispatchFromDyn` predicates.

#![feature(dispatch_from_dyn)]
#![feature(arbitrary_self_types)]

use std::ops::Deref;
use std::ops::DispatchFromDyn;

trait Trait<T: Deref<Target = Self>>
where
for<'a> &'a T: DispatchFromDyn<&'a T>,
{
fn foo(self: &T) -> Box<dyn Trait<T>>;
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
//@ known-bug: #57276
//@ check-pass
// Regression test for #57276.

// Make sure that object safety checking doesn't freak out when
// we have impossible-to-satisfy `DispatchFromDyn` predicates.

#![feature(arbitrary_self_types, dispatch_from_dyn)]

Expand Down

0 comments on commit 2c194e7

Please sign in to comment.