[wip] degenerate object safety check for crater

src/librustc/traits/error_reporting/mod.rs

@@ -704,11 +704,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                             if self.predicate_may_hold(&unit_obligation) {
                                 err.note(
                                     "the trait is implemented for `()`. \
-                                         Possibly this error has been caused by changes to \
-                                         Rust's type-inference algorithm \
-                                         (see: https://github.com/rust-lang/rust/issues/48950 \
-                                         for more info). Consider whether you meant to use the \
-                                         type `()` here instead.",
+                                     Possibly this error has been caused by changes to \
+                                     Rust's type-inference algorithm \
+                                     (see: https://github.com/rust-lang/rust/issues/48950 \
+                                     for more info). Consider whether you meant to use the \
+                                     type `()` here instead.",
                                 );
                             }
                         }
@@ -792,7 +792,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                                         *span,
                                         format!(
                                             "closure is `FnOnce` because it moves the \
-                                         variable `{}` out of its environment",
+                                             variable `{}` out of its environment",
                                             name
                                         ),
                                     );
@@ -802,7 +802,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                                         *span,
                                         format!(
                                             "closure is `FnMut` because it mutates the \
-                                         variable `{}` here",
+                                             variable `{}` here",
                                             name
                                         ),
                                     );
@@ -1012,7 +1012,7 @@ pub fn recursive_type_with_infinite_size_error(
     err.span_label(span, "recursive type has infinite size");
     err.help(&format!(
         "insert indirection (e.g., a `Box`, `Rc`, or `&`) \
-                           at some point to make `{}` representable",
+         at some point to make `{}` representable",
         tcx.def_path_str(type_def_id)
     ));
     err
@@ -1038,10 +1038,7 @@ pub fn report_object_safety_error(
     let mut reported_violations = FxHashSet::default();
     for violation in violations {
         if reported_violations.insert(violation.clone()) {
-            match violation.span() {
-                Some(span) => err.span_label(span, violation.error_msg()),
-                None => err.note(&violation.error_msg()),
-            };
+            violation.annotate_diagnostic(tcx, &mut err);
         }
     }
 

src/librustc/traits/object_safety.rs

@@ -12,15 +12,16 @@ use super::elaborate_predicates;
 
 use crate::traits::{self, Obligation, ObligationCause};
 use crate::ty::subst::{InternalSubsts, Subst};
-use crate::ty::{self, Predicate, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness};
+use crate::ty::WithConstness;
+use crate::ty::{self, ParamTy, Predicate, ToPredicate, Ty, TyCtxt, TypeFoldable};
+use rustc_errors::DiagnosticBuilder;
 use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
 use rustc_session::lint::builtin::WHERE_CLAUSES_OBJECT_SAFETY;
 use rustc_span::symbol::Symbol;
 use rustc_span::{Span, DUMMY_SP};
 use syntax::ast;
 
-use std::borrow::Cow;
 use std::iter::{self};
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
@@ -37,55 +38,91 @@ pub enum ObjectSafetyViolation {
 
     /// Associated const.
     AssocConst(ast::Name, Span),
+
+    /// Dyn-overlapping impl: a trait is not dyn-safe if it contains
+    /// associated types and an impl with an unsized `Self` type
+    /// (which may potentially overlap `dyn`). See #57893.
+    DynOverlappingImpl(DefId),
 }
 
 impl ObjectSafetyViolation {
-    pub fn error_msg(&self) -> Cow<'static, str> {
-        match *self {
+    pub fn annotate_diagnostic(&self, tcx: TyCtxt<'_>, diag: &mut DiagnosticBuilder<'_>) {
+        let span_label = |diag: &mut DiagnosticBuilder<'_>, span: Span, msg: &String| {
+            if span != DUMMY_SP {
+                diag.span_label(span, msg);
+            } else {
+                diag.note(msg);
+            }
+        };
+
+        match self {
             ObjectSafetyViolation::SizedSelf => {
-                "the trait cannot require that `Self : Sized`".into()
+                diag.note("the trait cannot require that `Self : Sized`");
             }
             ObjectSafetyViolation::SupertraitSelf => {
-                "the trait cannot use `Self` as a type parameter \
-                 in the supertraits or where-clauses"
-                    .into()
+                diag.note(
+                    "the trait cannot use `Self` as a type parameter \
+                     in the supertraits or where-clauses",
+                );
             }
-            ObjectSafetyViolation::Method(name, MethodViolationCode::StaticMethod, _) => {
-                format!("associated function `{}` has no `self` parameter", name).into()
+            ObjectSafetyViolation::Method(name, MethodViolationCode::StaticMethod, span) => {
+                span_label(
+                    diag,
+                    *span,
+                    &format!("associated function `{}` has no `self` parameter", name),
+                );
+            }
+            ObjectSafetyViolation::Method(name, MethodViolationCode::ReferencesSelf, span) => {
+                span_label(
+                    diag,
+                    *span,
+                    &format!(
+                        "method `{}` references the `Self` type in its parameters or return type",
+                        name,
+                    ),
+                );
             }
-            ObjectSafetyViolation::Method(name, MethodViolationCode::ReferencesSelf, _) => format!(
-                "method `{}` references the `Self` type in its parameters or return type",
-                name,
-            )
-            .into(),
             ObjectSafetyViolation::Method(
                 name,
                 MethodViolationCode::WhereClauseReferencesSelf,
-                _,
-            ) => format!("method `{}` references the `Self` type in where clauses", name).into(),
-            ObjectSafetyViolation::Method(name, MethodViolationCode::Generic, _) => {
-                format!("method `{}` has generic type parameters", name).into()
+                span,
+            ) => {
+                span_label(
+                    diag,
+                    *span,
+                    &format!("method `{}` references the `Self` type in where clauses", name),
+                );
             }
-            ObjectSafetyViolation::Method(name, MethodViolationCode::UndispatchableReceiver, _) => {
-                format!("method `{}`'s `self` parameter cannot be dispatched on", name).into()
+            ObjectSafetyViolation::Method(name, MethodViolationCode::Generic, span) => {
+                span_label(diag, *span, &format!("method `{}` has generic type parameters", name));
+            }
+            ObjectSafetyViolation::Method(
+                name,
+                MethodViolationCode::UndispatchableReceiver,
+                span,
+            ) => {
+                span_label(
+                    diag,
+                    *span,
+                    &format!("method `{}`'s `self` parameter cannot be dispatched on", name),
+                );
             }
-            ObjectSafetyViolation::AssocConst(name, _) => {
-                format!("the trait cannot contain associated consts like `{}`", name).into()
+            ObjectSafetyViolation::AssocConst(name, span) => {
+                span_label(
+                    diag,
+                    *span,
+                    &format!("the trait cannot contain associated consts like `{}`", name),
+                );
             }
-        }
-    }
-
-    pub fn span(&self) -> Option<Span> {
-        // When `span` comes from a separate crate, it'll be `DUMMY_SP`. Treat it as `None` so
-        // diagnostics use a `note` instead of a `span_label`.
-        match *self {
-            ObjectSafetyViolation::AssocConst(_, span)
-            | ObjectSafetyViolation::Method(_, _, span)
-                if span != DUMMY_SP =>
-            {
-                Some(span)
+            ObjectSafetyViolation::DynOverlappingImpl(impl_def_id) => {
+                span_label(
+                    diag,
+                    tcx.def_span(*impl_def_id),
+                    &format!(
+                        "traits with non-method items cannot have an impl with an unsized `Self` type"
+                    ),
+                );
             }
-            _ => None,
         }
     }
 }
@@ -179,17 +216,17 @@ fn object_safety_violations_for_trait(
             {
                 // Using `CRATE_NODE_ID` is wrong, but it's hard to get a more precise id.
                 // It's also hard to get a use site span, so we use the method definition span.
-                tcx.struct_span_lint_hir(
+                let mut lint = tcx.struct_span_lint_hir(
                     WHERE_CLAUSES_OBJECT_SAFETY,
                     hir::CRATE_HIR_ID,
                     *span,
                     &format!(
                         "the trait `{}` cannot be made into an object",
                         tcx.def_path_str(trait_def_id)
                     ),
-                )
-                .note(&violation.error_msg())
-                .emit();
+                );
+                violation.annotate_diagnostic(tcx, &mut lint);
+                lint.emit();
                 false
             } else {
                 true
@@ -211,6 +248,26 @@ fn object_safety_violations_for_trait(
             .map(|item| ObjectSafetyViolation::AssocConst(item.ident.name, item.ident.span)),
     );
 
+    // Issue #57893. A trait cannot be considered dyn-safe if:
+    //
+    // (a) it has associated items that are not functions and
+    // (b) it has a potentially dyn-overlapping impl.
+    //
+    // Why don't functions matter? Because we never resolve
+    // them to their normalizd type until code generation
+    // time, in short.
+    let has_associated_non_fn =
+        tcx.associated_items(trait_def_id).any(|assoc_item| match assoc_item.kind {
+            ty::AssocKind::Method => false,
+            ty::AssocKind::Type | ty::AssocKind::OpaqueTy | ty::AssocKind::Const => true,
+        });
+
+    if has_associated_non_fn {
+        if let Some(impl_def_id) = impl_potentially_overlapping_dyn_trait(tcx, trait_def_id) {
+            violations.push(ObjectSafetyViolation::DynOverlappingImpl(impl_def_id));
+        }
+    }
+
     debug!(
         "object_safety_violations_for_trait(trait_def_id={:?}) = {:?}",
         trait_def_id, violations
@@ -269,6 +326,59 @@ fn predicates_reference_self(tcx: TyCtxt<'_>, trait_def_id: DefId, supertraits_o
         })
 }
 
+fn generics_require_sized_param(tcx: TyCtxt<'_>, def_id: DefId, param_ty: ParamTy) -> bool {
+    debug!("generics_require_sized_param(def_id={:?}, param_ty={:?})", def_id, param_ty);
+
+    let sized_def_id = match tcx.lang_items().sized_trait() {
+        Some(def_id) => def_id,
+        None => {
+            return false; /* No Sized trait, can't require it! */
+        }
+    };
+
+    // Search for a predicate like `Self : Sized` amongst the trait bounds.
+    let predicates = tcx.predicates_of(def_id);
+    let predicates = predicates.instantiate_identity(tcx).predicates;
+    predicates.iter().any(|predicate| match predicate {
+        ty::Predicate::Trait(ref trait_pred, _) => {
+            debug!("generics_require_sized_param: trait_pred = {:?}", trait_pred);
+
+            trait_pred.def_id() == sized_def_id
+                && trait_pred.skip_binder().self_ty().is_param(param_ty.index)
+        }
+        ty::Predicate::Projection(..)
+        | ty::Predicate::Subtype(..)
+        | ty::Predicate::RegionOutlives(..)
+        | ty::Predicate::WellFormed(..)
+        | ty::Predicate::ObjectSafe(..)
+        | ty::Predicate::ClosureKind(..)
+        | ty::Predicate::TypeOutlives(..)
+        | ty::Predicate::ConstEvaluatable(..) => false,
+    })
+}
+
+/// Searches for an impl where the `Self` type potentially overlaps
+/// `dyn Trait` (where `Trait` is the trait with def-id
+/// `trait_def_id`).
+fn impl_potentially_overlapping_dyn_trait(tcx: TyCtxt<'_>, trait_def_id: DefId) -> Option<DefId> {
+    debug!("impl_potentially_overlapping_dyn_trait({:?})", trait_def_id);
+    let mut found_match = None;
+    tcx.for_each_impl(trait_def_id, |impl_def_id| {
+        let impl_self_ty = tcx.type_of(impl_def_id);
+        match impl_self_ty.kind {
+            ty::Param(param_ty) => {
+                if !generics_require_sized_param(tcx, impl_def_id, param_ty) {
+                    found_match = Some(impl_def_id);
+                    debug!("Match found = {:?}; for param_ty {}", found_match, param_ty.name);
+                }
+            }
+            _ => (),
+        }
+    });
+
+    found_match
+}
+
 fn trait_has_sized_self(tcx: TyCtxt<'_>, trait_def_id: DefId) -> bool {
     generics_require_sized_self(tcx, trait_def_id)
 }
@@ -417,7 +527,7 @@ fn virtual_call_violation_for_method<'tcx>(
                         tcx.def_span(method.def_id),
                         &format!(
                             "receiver when `Self = {}` should have a ScalarPair ABI; \
-                                 found {:?}",
+                             found {:?}",
                             trait_object_ty, abi
                         ),
                     );
@@ -718,7 +828,7 @@ fn contains_illegal_self_type_reference<'tcx>(
                 }
             }
 
-            _ => true, // walk contained types, if any
+            _ => true,
         }
     });
 

src/librustc_data_structures/lib.rs

@@ -23,6 +23,7 @@
 #![feature(integer_atomics)]
 #![feature(test)]
 #![feature(associated_type_bounds)]
+#![feature(rustc_attrs)]
 #![cfg_attr(unix, feature(libc))]
 #![allow(rustc::default_hash_types)]
 

src/librustc_data_structures/sync.rs

@@ -29,6 +29,7 @@ pub use std::sync::atomic::Ordering::SeqCst;
 cfg_if! {
     if #[cfg(not(parallel_compiler))] {
         pub auto trait Send {}
+
         pub auto trait Sync {}
 
         impl<T: ?Sized> Send for T {}

src/librustc_feature/builtin_attrs.rs

@@ -143,7 +143,7 @@ macro_rules! rustc_attr {
                 "the `#[",
                 stringify!($attr),
                 "]` attribute is just used for rustc unit tests \
-                and will never be stable",
+                 and will never be stable",
             ),
         )
     };

src/test/ui/issue-59387.rs

@@ -0,0 +1,19 @@
+trait Object<U> {
+    type Output;
+}
+
+impl<T: ?Sized, U> Object<U> for T {
+    // ^-- Here is the blanket impl; relies on `?Sized`.
+    type Output = U;
+}
+
+fn foo<T: ?Sized, U>(x: <T as Object<U>>::Output) -> U {
+    x
+}
+
+fn transmute<T, U>(x: T) -> U {
+    foo::<dyn Object<U, Output = T>, U>(x)
+    //~^ ERROR the trait `Object` cannot be made into an object
+}
+
+fn main() {}

src/test/ui/issue-59387.stderr

@@ -0,0 +1,15 @@
+error[E0038]: the trait `Object` cannot be made into an object
+  --> $DIR/issue-59387.rs:15:11
+   |
+LL | / impl<T: ?Sized, U> Object<U> for T {
+LL | |     // ^-- Here is the blanket impl; relies on `?Sized`.
+LL | |     type Output = U;
+LL | | }
+   | |_- traits with non-method items cannot have an impl with an unsized `Self` type
+...
+LL |       foo::<dyn Object<U, Output = T>, U>(x)
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Object` cannot be made into an object
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0038`.