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
This commit is contained in:
bors 2024-06-04 02:34:20 +00:00
commit 90d6255d82
28 changed files with 93 additions and 210 deletions

View file

@ -881,7 +881,7 @@ fn check_object_unsafe_self_trait_by_name(tcx: TyCtxt<'_>, item: &hir::TraitItem
_ => {}
}
if !trait_should_be_self.is_empty() {
if tcx.check_is_object_safe(trait_def_id) {
if tcx.is_object_safe(trait_def_id) {
return;
}
let sugg = trait_should_be_self.iter().map(|span| (*span, "Self".to_string())).collect();

View file

@ -191,7 +191,7 @@ fn check_object_overlap<'tcx>(
});
for component_def_id in component_def_ids {
if !tcx.check_is_object_safe(component_def_id) {
if !tcx.is_object_safe(component_def_id) {
// Without the 'object_safe_for_dispatch' feature this is an error
// which will be reported by wfcheck. Ignore it here.
// This is tested by `coherence-impl-trait-for-trait-object-safe.rs`.

View file

@ -182,7 +182,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
// For recursive traits, don't downgrade the error. (#119652)
is_downgradable = false;
}
tcx.check_is_object_safe(id)
tcx.is_object_safe(id)
}
_ => false,
})