Implement AsMut<T> and AsRef<T> for !.

[kpreid]

View all comments

This will allow e.g. &[!] to satisfy &[T] where T: AsRef<str>. It follows the recommendation from the never documentation:

When writing your own traits, ! should have an impl whenever there is an obvious impl which doesn’t panic!.

— https://doc.rust-lang.org/1.97.1/std/primitive.never.html#-and-traits

A class of use case for these impls is:

• some code is generic over T: AsRef<SomeOtherType>,
• and is still useful in the case where there are no Ts, because it takes Option<T> or &[T] or any other input that can be empty or never-called, yet still do things with other inputs,

in which case providing T = ! is slightly better than the alternative of providing an inhabited placeholder type, because it ensures that the generic code is compiled with knowledge of the fact that that there will never be any Ts, even if the specific code that cares about whether there are any Ts doesn’t get inlined into the call site that doesn’t provide any Ts.

---

The test tests/ui/impl-trait/generic-with-implicit-hrtb-without-dyn.rs had to be updated because it depended on this impl not existing. I’ve confirmed that the modified test still functions as a regression test by compiling it in nightly-2022-08-28 and seeing it ICE.

Tracking issue for never: #35121
(This change has no ACP or tracking issue of its own; I assume it is simple enough that T-libs-api can just accept or reject this PR, and the implementation will be stable when ! is stable.)

@rustbot label -T-libs +T-libs-api +F-never_type

[QuineDot]

As Infallible will become !, this conflicts with implementations that can exist today.

use std::convert::Infallible;
struct S;
impl AsRef<S> for Infallible {
    fn as_ref(&self) -> &S {
        match *self {}
    }
}

[kpreid]

@QuineDot Thanks for pointing that out. That means this PR must either be rejected or get a crater run. I think the possible benefit is significant, so I won’t just close it immediately.

…unrelatedly, I just noticed that rustbot hasn’t assigned a reviewer at all.

r? libs-api

[theemathas]

Not sure how to run crater on this one. Do we wait until after the never type stabilization PR is merged?

[JonathanBrouwer]

You could run a crater with a AsRef<Infallible> and AsMut<Infallible> implementation, that should be representative.
That said, the stabilization PR looks likely to merge in the next few days so you could also just have some patience

[clarfonthey]

@kpreid FYI, libs-api isn't a thing any more, so, please r? libs in the future

ditto for the T-libs-api tag

[steffahn]

You could run a crater with a AsRef<Infallible> and AsMut<Infallible> implementation, that should be representative.

That would be AsRef<T> for Infallible / AsMut<T> for Infallible (clarifying just to avoid confusion).

[kpreid]

I’ll rebase this PR after the stabilization PR merges.

[JonathanBrouwer]

(oops, thanks)

[kpreid]

Rebased, and the implementations are now marked #[stable].

[theemathas]

Preparing for crater run
@bors try

[rust-bors]

☀️ Try build successful (CI)
Build commit: 65c7d39 (65c7d39f9c2b9d5c316725cb48fa3d360c64c663)
Base parent: e776960 (e7769602aca3770e8d8ea55716becb22e839a579)

[JohnTitor]

r? libs

[theemathas]

@craterbot cancel

See #162233

[craterbot]

🗑️ Experiment pr-161253 deleted!

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[theemathas]

@craterbot check crates=https://crater-reports.s3.amazonaws.com/pr-162233-crater-rollup/retry-regressed-list.txt p=1

[craterbot]

👌 Experiment pr-161253 created and queued.
🤖 Automatically detected try build 65c7d39
⚠️ Try build based on commit e7ce9a3, but latest commit is e028a86. Did you forget to make a new try build?
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🚧 Experiment pr-161253 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🎉 Experiment pr-161253 is completed!
📊 0 regressed and 0 fixed (7794 total)
📊 831 spurious results on the retry-regressed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Footnotes

1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-161253/retry-regressed-list.txt ↩

[WaffleLapkin]

nit: the match is unnecessary

View changes since the review

[kpreid]

Done. Thanks for the catch — I’m used to working with empty enums but not true !.

[kpreid]

Clippy had a false positive on this code, which I have filed: rust-lang/rust-clippy#17713

[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.

[clarfonthey]

I assume we should FCP this even if there are no regressions, since it's instantly stable?

And never stability passed, so, this is an extra ask.

[WaffleLapkin]

When writing your own traits, ! should have an impl whenever there is an obvious impl which doesn’t panic!.

For the record, this is a somewhat weak recommendation. As in, you should generally have these imps if they are useful, but they are not always useful.

I don't have anything against these impls, but I'm also not sure how useful they are.

[kpreid]

@WaffleLapkin I don’t have a concrete use case, but as I see it, because these impls can conflict with user-written impls (that must do the same thing), we should add them before that conflict arises, even if we can’t think of a use case yet.

And I do think that these will come up eventually. The abstract use case is:

• some code is generic over T: AsRef<SomeOtherType>,
• and is still useful in the case where there are no Ts, because it takes Option<T> or &[T] or any other input that can be empty or never-called, yet still do things with other inputs,

in which case providing T = ! is slightly better than the alternative of providing an inhabited placeholder type, because it ensures that the generic code is compiled with knowledge of the fact that that there will never be any Ts, even if the specific code that cares about whether there are any Ts doesn’t get inlined into the call site that doesn’t provide any Ts.

[kpreid]

I’ve expanded the PR description to include the abstract use case from my previous commit.