don't calculate dtors if the self ty has impossible bounds

[sjwang05]

If we have multiple drop impls and a self ty that all have the same impossible bounds, we end up with duplicate drops with different DefIds, which causes us to ICE with a delayed bug in calculate_dtor. Coherence accepts these impls since they have impossible bounds, and nothing else in check_drop_impl checks this, leading to us accepting the code and subsequently ICEing from the delayed bug.

We now skip considering the current drop impl as a dtor candidate if the self ty and impl(s) have the same impossible bounds, since if the self ty is unnameable, it's impossible to construct an instance in the first place. Checking only the self ty is sufficient here, as check_drop_impl checks the case where the impossible bounds exist only on the drop impl, and we're not allowed in general to have a struct whose bounds are more restrictive than its impl's, so the only case we're currently missing is the case where the self ty and the Drop impls have the exact same impossible bounds.

My reasoning for accepting such code instead of emitting an error is that, in general, we allow users to write code with impossible or trivial bounds, so long as they never try to actually name or run that code. This PR makes Drop behave the same way.

This results in overlapping impossible drop impls behaving like other traits: for instance, the following compiles on nightly today:

pub trait Foo {
    fn foo(&self) {}
}

pub struct Thing<T>(T)
where
    [T]: Sized;

impl<T> Foo for Thing<T> where [T]: Sized {}
impl<T> Foo for Thing<T> where [T]: Sized {} // removing this impl is ok as well

Although this ICE only seems to be reachable for sync drop, not making the same change for async drop makes the following program, which currently compiles on nightly, stop compiling:

#![feature(async_drop)]
use core::future::AsyncDrop;
use core::pin::Pin;

pub struct Thing<T>(T) where [T]: Sized;

impl<T> Drop for Thing<T> where [T]: Sized {
    fn drop(&mut self) {}
}
impl<T> AsyncDrop for Thing<T> where [T]: Sized {
    async fn drop(self: Pin<&mut Self>) {}
}

which feels like something we don't want.

cc #159118 (comment)

fixes #153947

[rustbot]

r? @oli-obk

rustbot has assigned @oli-obk.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler, types
• compiler, types expanded to 76 candidates
• Random selection from 18 candidates

[sjwang05]

I basically lifted this logic wholesale from the is_impossible_associated_item query, though I couldn't really find a nice way to make them into a single shared thing, since is_impossible_associated_item filters the obligations to those that only mention the parent item's generics before registering them with the ocx.

View changes since the review

[rustbot]

This PR changes a file inside tests/crashes. If a crash was fixed, please move into the corresponding ui subdir and add 'Fixes #' to the PR description to autoclose the issue upon merge.

[oli-obk]

#150387 may keep existing. Can it be reproed with just specialization and no impossible bounds?

[sjwang05]

#150387 may keep existing. Can it be reproed with just specialization and no impossible bounds?

I don't think one exists if you only use specialization and nothing else, but this program does ICE with const drop, so I've removed that issue from the PR description:

#![feature(min_specialization, const_trait_impl, const_destruct)]
struct Thing;
impl Drop for Thing { default fn drop(&mut self) {} }
const impl Drop for Thing { fn drop(&mut self) {} }

since the const impl specializes the non-const one, and none of them break dropck's rules, so we get a delayed bug :D

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[oli-obk]

since the const impl specializes the non-const one, and none of them break dropck's rules, so we get a delayed bug :D

😨 Aaaaa

Nice find

[oli-obk]

@bors r+

[rust-bors]

📌 Commit 11c2b3d has been approved by oli-obk

It is now in the queue for this repository.