about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2025-02-04 18:49:41 +0100
committerGitHub <noreply@github.com>2025-02-04 18:49:41 +0100
commit5da750001d5209be4a10595f260cd2a93a76c299 (patch)
tree0c342ff40a4a01339b7257bbf8af388d94c17a6b
parent29e1dddd83cfff48281654aa5bf348e943d52b9c (diff)
parent0b26dc0b51c87a90ce61f50dd7141ff451513b23 (diff)
downloadrust-5da750001d5209be4a10595f260cd2a93a76c299.tar.gz
rust-5da750001d5209be4a10595f260cd2a93a76c299.zip
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, 6f2a161b1bbe6234188d6cfb3cabddef1e6ef20f.

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.
-rw-r--r--compiler/rustc_trait_selection/src/traits/dyn_compatibility.rs107
-rw-r--r--tests/crashes/125810.rs10
-rw-r--r--tests/crashes/90110.rs57
-rw-r--r--tests/ui/self/dispatch-from-dyn-layout-2.rs16
-rw-r--r--tests/ui/self/dispatch-from-dyn-layout-3.rs19
-rw-r--r--tests/ui/self/dispatch-from-dyn-layout.rs (renamed from tests/crashes/57276.rs)6
6 files changed, 42 insertions, 173 deletions
diff --git a/compiler/rustc_trait_selection/src/traits/dyn_compatibility.rs b/compiler/rustc_trait_selection/src/traits/dyn_compatibility.rs
index baa94ead9d4..617bc87a9d2 100644
--- a/compiler/rustc_trait_selection/src/traits/dyn_compatibility.rs
+++ b/compiler/rustc_trait_selection/src/traits/dyn_compatibility.rs
@@ -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;
@@ -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
 }
 
@@ -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>(
@@ -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:
diff --git a/tests/crashes/125810.rs b/tests/crashes/125810.rs
deleted file mode 100644
index 4a152da8ddf..00000000000
--- a/tests/crashes/125810.rs
+++ /dev/null
@@ -1,10 +0,0 @@
-//@ known-bug: rust-lang/rust#125810
-#![feature(arbitrary_self_types, dispatch_from_dyn)]
-
-use std::ops::{Deref, DispatchFromDyn};
-
-trait Trait<T: Deref<Target = Self> + DispatchFromDyn<T>> {
-    fn MONO_BUF(self: T) -> dyn Trait<T>;
-}
-
-fn main() {}
diff --git a/tests/crashes/90110.rs b/tests/crashes/90110.rs
deleted file mode 100644
index a27a1f42b7a..00000000000
--- a/tests/crashes/90110.rs
+++ /dev/null
@@ -1,57 +0,0 @@
-//@ known-bug: #90110
-
-use std::fs::File;
-use std::io::{BufReader, BufRead};
-use std::str::Split;
-use std::path::Path;
-
-pub trait Parser<D>
-where dyn Parser<D>: Sized
-{
-    fn new(split_header: Split<&str>) -> Self where Self: Sized;
-    fn parse_line(&self, split_line: &Split<&str>) -> D;
-}
-
-
-pub struct CsvReader<D> {
-    parser: Box<dyn Parser<D>>,
-
-    reader: BufReader<File>,
-    buf: String,    // Buffer we will read into. Avoids re-allocation on each line.
-    path: String,   // Record this so we can return more informative error messages.
-    line: usize,    // Same motivation for this.
-}
-
-impl<D> CsvReader<D>
-where dyn Parser<D>: Sized
-{
-    fn new<F>(path: &str, make_parser: F) -> CsvReader<D>
-    where F: Fn(Split<char>) -> dyn Parser<D> {
-        let file = match File::open(Path::new(path)) {
-            Err(err) => panic!("Couldn't read {}: {}", path, err),
-            Ok(file) => file,
-        };
-
-        let mut reader = BufReader::new(file);
-
-        let mut buf = String::new();
-
-        let parser = Box::new(match reader.read_line(&mut buf) {
-            Err(err) => panic!("Failed to read the header line from {}: {}", path, err),
-            Ok(_) => {
-                let split_header = buf.split(',');
-                make_parser(split_header)
-            },
-        });
-
-        CsvReader {
-            parser: parser,
-            reader,
-            buf,
-            path: path.to_string(),
-            line: 2,
-        }
-    }
-}
-
-pub fn main() {}
diff --git a/tests/ui/self/dispatch-from-dyn-layout-2.rs b/tests/ui/self/dispatch-from-dyn-layout-2.rs
new file mode 100644
index 00000000000..cd52f060dc8
--- /dev/null
+++ b/tests/ui/self/dispatch-from-dyn-layout-2.rs
@@ -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() {}
diff --git a/tests/ui/self/dispatch-from-dyn-layout-3.rs b/tests/ui/self/dispatch-from-dyn-layout-3.rs
new file mode 100644
index 00000000000..6878a4f4ac2
--- /dev/null
+++ b/tests/ui/self/dispatch-from-dyn-layout-3.rs
@@ -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() {}
diff --git a/tests/crashes/57276.rs b/tests/ui/self/dispatch-from-dyn-layout.rs
index f70be4fba6d..468dc89a73e 100644
--- a/tests/crashes/57276.rs
+++ b/tests/ui/self/dispatch-from-dyn-layout.rs
@@ -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)]