Auto merge of #125380 - compiler-errors:wc-obj-safety, r=oli-obk

Make `WHERE_CLAUSES_OBJECT_SAFETY` a regular object safety violation

#### The issue

In #50781, we have known about unsound `where` clauses in function arguments:

```rust
trait Impossible {}

trait Foo {
    fn impossible(&self)
    where
        Self: Impossible;
}

impl Foo for &() {
    fn impossible(&self)
    where
        Self: Impossible,
    {}
}

// `where` clause satisfied for the object, meaning that the function now *looks* callable.
impl Impossible for dyn Foo {}

fn main() {
    let x: &dyn Foo = &&();
    x.impossible();
}
```

... which currently segfaults at runtime because we try to call a method in the vtable that doesn't exist. :(

#### What did u change

This PR removes the `WHERE_CLAUSES_OBJECT_SAFETY` lint and instead makes it a regular object safety violation. I choose to make this into a hard error immediately rather than a `deny` because of the time that has passed since this lint was authored, and the single (1) regression (see below).

That means that it's OK to mention `where Self: Trait` where clauses in your trait, but making such a trait into a `dyn Trait` object will report an object safety violation just like `where Self: Sized`, etc.

```rust
trait Impossible {}

trait Foo {
    fn impossible(&self)
    where
        Self: Impossible; // <~ This definition is valid, just not object-safe.
}

impl Foo for &() {
    fn impossible(&self)
    where
        Self: Impossible,
    {}
}

fn main() {
    let x: &dyn Foo = &&(); // <~ THIS is where we emit an error.
}
```

#### Regressions

From a recent crater run, there's only one crate that relies on this behavior: https://github.com/rust-lang/rust/pull/124305#issuecomment-2122381740. The crate looks unmaintained and there seems to be no dependents.

#### Further

We may later choose to relax this (e.g. when the where clause is implied by the supertraits of the trait or something), but this is not something I propose to do in this FCP.

For example, given:

```
trait Tr {
  fn f(&self) where Self: Blanket;
}

impl<T: ?Sized> Blanket for T {}
```

Proving that some placeholder `S` implements `S: Blanket` would be sufficient to prove that the same (blanket) impl applies for both `Concerete: Blanket` and `dyn Trait: Blanket`.

Repeating here that I don't think we need to implement this behavior right now.

----

r? lcnr

8 files changed, 12 insertions, 87 deletions

diff --git a/compiler/rustc_trait_selection/src/solve/assembly/mod.rs b/compiler/rustc_trait_selection/src/solve/assembly/mod.rs
index aae6fa9f635..b51efd339c4 100644
--- a/compiler/rustc_trait_selection/src/solve/assembly/mod.rs
+++ b/compiler/rustc_trait_selection/src/solve/assembly/mod.rs
@@ -714,7 +714,7 @@ impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> {
         };
 
         // Do not consider built-in object impls for non-object-safe types.
-        if bounds.principal_def_id().is_some_and(|def_id| !tcx.check_is_object_safe(def_id)) {
+        if bounds.principal_def_id().is_some_and(|def_id| !tcx.is_object_safe(def_id)) {
             return;
         }
 
diff --git a/compiler/rustc_trait_selection/src/solve/mod.rs b/compiler/rustc_trait_selection/src/solve/mod.rs
index ffeafd0e89f..68a4831c335 100644
--- a/compiler/rustc_trait_selection/src/solve/mod.rs
+++ b/compiler/rustc_trait_selection/src/solve/mod.rs
@@ -133,7 +133,7 @@ impl<'a, 'tcx> EvalCtxt<'a, InferCtxt<'tcx>> {
     }
 
     fn compute_object_safe_goal(&mut self, trait_def_id: DefId) -> QueryResult<'tcx> {
-        if self.interner().check_is_object_safe(trait_def_id) {
+        if self.interner().is_object_safe(trait_def_id) {
             self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
         } else {
             Err(NoSolution)
diff --git a/compiler/rustc_trait_selection/src/solve/trait_goals.rs b/compiler/rustc_trait_selection/src/solve/trait_goals.rs
index e93d5c96b88..a741f488901 100644
--- a/compiler/rustc_trait_selection/src/solve/trait_goals.rs
+++ b/compiler/rustc_trait_selection/src/solve/trait_goals.rs
@@ -789,7 +789,7 @@ impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> {
         let Goal { predicate: (a_ty, _), .. } = goal;
 
         // Can only unsize to an object-safe trait.
-        if b_data.principal_def_id().is_some_and(|def_id| !tcx.check_is_object_safe(def_id)) {
+        if b_data.principal_def_id().is_some_and(|def_id| !tcx.is_object_safe(def_id)) {
             return Err(NoSolution);
         }
 
diff --git a/compiler/rustc_trait_selection/src/traits/fulfill.rs b/compiler/rustc_trait_selection/src/traits/fulfill.rs
index 8ab9d5754c0..a26288efc96 100644
--- a/compiler/rustc_trait_selection/src/traits/fulfill.rs
+++ b/compiler/rustc_trait_selection/src/traits/fulfill.rs
@@ -421,7 +421,7 @@ impl<'a, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'tcx> {
                 }
 
                 ty::PredicateKind::ObjectSafe(trait_def_id) => {
-                    if !self.selcx.tcx().check_is_object_safe(trait_def_id) {
+                    if !self.selcx.tcx().is_object_safe(trait_def_id) {
                         ProcessResult::Error(FulfillmentErrorCode::Select(Unimplemented))
                     } else {
                         ProcessResult::Changed(vec![])
diff --git a/compiler/rustc_trait_selection/src/traits/object_safety.rs b/compiler/rustc_trait_selection/src/traits/object_safety.rs
index 0f32b6ef9e3..08355ef55c4 100644
--- a/compiler/rustc_trait_selection/src/traits/object_safety.rs
+++ b/compiler/rustc_trait_selection/src/traits/object_safety.rs
@@ -13,7 +13,7 @@ use super::elaborate;
 use crate::infer::TyCtxtInferExt;
 use crate::traits::query::evaluate_obligation::InferCtxtExt;
 use crate::traits::{self, Obligation, ObligationCause};
-use rustc_errors::{FatalError, MultiSpan};
+use rustc_errors::FatalError;
 use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
 use rustc_middle::query::Providers;
@@ -23,7 +23,6 @@ use rustc_middle::ty::{
 };
 use rustc_middle::ty::{GenericArg, GenericArgs};
 use rustc_middle::ty::{TypeVisitableExt, Upcast};
-use rustc_session::lint::builtin::WHERE_CLAUSES_OBJECT_SAFETY;
 use rustc_span::symbol::Symbol;
 use rustc_span::Span;
 use rustc_target::abi::Abi;
@@ -65,45 +64,14 @@ fn object_safety_violations(tcx: TyCtxt<'_>, trait_def_id: DefId) -> &'_ [Object
     )
 }
 
-fn check_is_object_safe(tcx: TyCtxt<'_>, trait_def_id: DefId) -> bool {
-    let violations = tcx.object_safety_violations(trait_def_id);
-
-    if violations.is_empty() {
-        return true;
-    }
-
-    // If the trait contains any other violations, then let the error reporting path
-    // report it instead of emitting a warning here.
-    if violations.iter().all(|violation| {
-        matches!(
-            violation,
-            ObjectSafetyViolation::Method(_, MethodViolationCode::WhereClauseReferencesSelf, _)
-        )
-    }) {
-        for violation in violations {
-            if let ObjectSafetyViolation::Method(
-                _,
-                MethodViolationCode::WhereClauseReferencesSelf,
-                span,
-            ) = violation
-            {
-                lint_object_unsafe_trait(tcx, *span, trait_def_id, violation);
-            }
-        }
-        return true;
-    }
-
-    false
+fn is_object_safe(tcx: TyCtxt<'_>, trait_def_id: DefId) -> bool {
+    tcx.object_safety_violations(trait_def_id).is_empty()
 }
 
 /// We say a method is *vtable safe* if it can be invoked on a trait
 /// object. Note that object-safe traits can have some
 /// non-vtable-safe methods, so long as they require `Self: Sized` or
 /// otherwise ensure that they cannot be used when `Self = Trait`.
-///
-/// [`MethodViolationCode::WhereClauseReferencesSelf`] is considered object safe due to backwards
-/// compatibility, see <https://github.com/rust-lang/rust/issues/51443> and
-/// [`WHERE_CLAUSES_OBJECT_SAFETY`].
 pub fn is_vtable_safe_method(tcx: TyCtxt<'_>, trait_def_id: DefId, method: ty::AssocItem) -> bool {
     debug_assert!(tcx.generics_of(trait_def_id).has_self);
     debug!("is_vtable_safe_method({:?}, {:?})", trait_def_id, method);
@@ -112,9 +80,7 @@ pub fn is_vtable_safe_method(tcx: TyCtxt<'_>, trait_def_id: DefId, method: ty::A
         return false;
     }
 
-    virtual_call_violations_for_method(tcx, trait_def_id, method)
-        .iter()
-        .all(|v| matches!(v, MethodViolationCode::WhereClauseReferencesSelf))
+    virtual_call_violations_for_method(tcx, trait_def_id, method).is_empty()
 }
 
 fn object_safety_violations_for_trait(
@@ -163,47 +129,6 @@ fn object_safety_violations_for_trait(
     violations
 }
 
-/// Lint object-unsafe trait.
-fn lint_object_unsafe_trait(
-    tcx: TyCtxt<'_>,
-    span: Span,
-    trait_def_id: DefId,
-    violation: &ObjectSafetyViolation,
-) {
-    // 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.node_span_lint(WHERE_CLAUSES_OBJECT_SAFETY, hir::CRATE_HIR_ID, span, |err| {
-        err.primary_message(format!(
-            "the trait `{}` cannot be made into an object",
-            tcx.def_path_str(trait_def_id)
-        ));
-        let node = tcx.hir().get_if_local(trait_def_id);
-        let mut spans = MultiSpan::from_span(span);
-        if let Some(hir::Node::Item(item)) = node {
-            spans.push_span_label(item.ident.span, "this trait cannot be made into an object...");
-            spans.push_span_label(span, format!("...because {}", violation.error_msg()));
-        } else {
-            spans.push_span_label(
-                span,
-                format!(
-                    "the trait cannot be made into an object because {}",
-                    violation.error_msg()
-                ),
-            );
-        };
-        err.span_note(
-            spans,
-            "for a trait to be \"object safe\" it needs to allow building a vtable to allow the \
-                call to be resolvable dynamically; for more information visit \
-                <https://doc.rust-lang.org/reference/items/traits.html#object-safety>",
-        );
-        if node.is_some() {
-            // Only provide the help if its a local trait, otherwise it's not
-            violation.solution().add_to(err);
-        }
-    });
-}
-
 fn sized_trait_bound_spans<'tcx>(
     tcx: TyCtxt<'tcx>,
     bounds: hir::GenericBounds<'tcx>,
@@ -929,7 +854,7 @@ pub fn contains_illegal_impl_trait_in_trait<'tcx>(
 pub fn provide(providers: &mut Providers) {
     *providers = Providers {
         object_safety_violations,
-        check_is_object_safe,
+        is_object_safe,
         generics_require_sized_self,
         ..*providers
     };
diff --git a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs
index d513a5d2996..bb44be7799b 100644
--- a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs
+++ b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs
@@ -870,7 +870,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                         if let Some(principal) = data.principal() {
                             if !self.infcx.tcx.features().object_safe_for_dispatch {
                                 principal.with_self_ty(self.tcx(), self_ty)
-                            } else if self.tcx().check_is_object_safe(principal.def_id()) {
+                            } else if self.tcx().is_object_safe(principal.def_id()) {
                                 principal.with_self_ty(self.tcx(), self_ty)
                             } else {
                                 return;
diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs
index c684f087d32..ef0d8735d35 100644
--- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs
+++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs
@@ -1222,7 +1222,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             // `T` -> `Trait`
             (_, &ty::Dynamic(data, r, ty::Dyn)) => {
                 let mut object_dids = data.auto_traits().chain(data.principal_def_id());
-                if let Some(did) = object_dids.find(|did| !tcx.check_is_object_safe(*did)) {
+                if let Some(did) = object_dids.find(|did| !tcx.is_object_safe(*did)) {
                     return Err(TraitNotObjectSafe(did));
                 }
 
diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs
index dcb0875639f..2489b8916d1 100644
--- a/compiler/rustc_trait_selection/src/traits/select/mod.rs
+++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs
@@ -798,7 +798,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 }
 
                 ty::PredicateKind::ObjectSafe(trait_def_id) => {
-                    if self.tcx().check_is_object_safe(trait_def_id) {
+                    if self.tcx().is_object_safe(trait_def_id) {
                         Ok(EvaluatedToOk)
                     } else {
                         Ok(EvaluatedToErr)