Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking issue for rustc_reservation_impl attribute #64631

Open
2 tasks
nikomatsakis opened this issue Sep 20, 2019 · 2 comments
Open
2 tasks

Tracking issue for rustc_reservation_impl attribute #64631

nikomatsakis opened this issue Sep 20, 2019 · 2 comments
Labels
A-trait-system Area: Trait system C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-rustc_attrs Internal rustc attributes gated on the `#[rustc_attrs]` feature gate. requires-nightly This issue requires a nightly compiler in some way. S-tracking-perma-unstable Status: The feature will stay unstable indefinitely. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Sep 20, 2019

Background

The #[rustc_reservation_impl] attribute was added as part of the effort to stabilize !. Its goal is to make it possible to add a impl<T> From<!> for T impl in the future by disallowing downstream crates to do negative reasoning that might conflict with that.

Warning: the effect of this attribute is quite subtle, and it should be used with caution! In particular, adding a "reservation impl" alone does not guarantee that one can add the impl in the future, as described below.

History

Current blockers before this attribute can be used to affect end-users

Usage

You can use the #[rustc_reservation_impl] attribute as follows:

#[rustc_reservation_impl]
impl<T> From<!> for T { .. }

For the most part, rustc will act as though this impl does not exist. For example, users can add overlapping impls if they prefer:

struct LocalType1<T>(T);
impl<T> From<!> for LocalType1<T> { }

struct LocalType2<T>(T);
impl<T> From<T> for LocalType2<T> { }

Note that this implies that the #[rustc_reservation_impl] attribute alone does not guarantee that you can add the reservation impl in a future compatible way. Adding the reserved impl in the future may still cause coherence overlap (e.g., the impl for LocalType<T> for the impl for all T would overlap here). This will typically result in errors.

In order to add the impl, you must be able to ensure that coherence will allow the overlapping impls. This can be done in two ways, both of which are presently unstable:

  • Specialization: but be careful! e.g., the current specialization rules do not suffice in the LocalType2 example above, since neither impl is a subset of one another.
  • Marker traits: we have a notion of marker traits that are allowed to overlap. Marker traits currently must have no items, which excludes e.g. From, but we could grow this definition.

What does the attribute do?

The attribute prevents negative reasoning. In particular, it forbids impls like this:

trait LocalTrait;
struct LocalType3;
impl<T> LocalTrait for T where T: From<!> { }
impl<T> LocalTrait for LocalType3 { }

Without the reservation impl, this would be legal, because the crate may assume that LocalType: From<!> does not hold (since LocalType is local to the crate). With the reservation impl, however, code like this will get an error.

@nikomatsakis nikomatsakis added A-trait-system Area: Trait system C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-rustc_attrs Internal rustc attributes gated on the `#[rustc_attrs]` feature gate. labels Sep 20, 2019
@Centril Centril added T-lang Relevant to the language team, which will review and decide on the PR/issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. requires-nightly This issue requires a nightly compiler in some way. labels Sep 20, 2019
Undin added a commit to intellij-rust/intellij-rust that referenced this issue Nov 25, 2019
…erence

Some time ago `rustc_reservation_impl` attribute was introduced to reserve some implementation and don't allow them in libraries (rust-lang/rust#64631).
Such implementations don't take part in type inference but may produce duplicate item errors.
Most notable (and maybe only one) impl with such attribute is `impl<T> From<!> for T`. This impl breaks type inference in some cases.
These changes just make type inference ignore such `impl` items
Undin added a commit to intellij-rust/intellij-rust that referenced this issue Nov 25, 2019
…erence

Some time ago `rustc_reservation_impl` attribute was introduced to reserve some implementation (rust-lang/rust#64631).
Such implementations mostly don't take part in code analysis.
Most notable (and maybe only one) impl with such attribute is `impl<T> From<!> for T`. This impl breaks type inference in some cases.
These changes just make type inference ignore such `impl` items
bors bot added a commit to intellij-rust/intellij-rust that referenced this issue Nov 25, 2019
4688: TY: skip impls with `rustc_reservation_impl` attribute while type inference r=vlad20012 a=Undin

Some time ago `rustc_reservation_impl` attribute was introduced to reserve some implementation (rust-lang/rust#64631).
Such implementations mostly don't take part in code analysis.
Most notable (and maybe only one) impl with such attribute is `impl<T> From<!> for T`. This impl breaks type inference in some cases.
These changes just make type inference ignore such `impl` items


Co-authored-by: Arseniy Pendryak <a.pendryak@yandex.ru>
@joshtriplett joshtriplett added the S-tracking-perma-unstable Status: The feature will stay unstable indefinitely. label Jun 22, 2022
@nikomatsakis
Copy link
Contributor Author

nikomatsakis commented Jun 22, 2022

We visited this during lang team backlog bonanza. Conclusions:

  • This was never meant to be an end-user feature, it is a temporary crutch and at most perma-unstable.
  • It's quite special purpose to ! -- there could be a useful end-user feature of "reserving" space to add an impl later, but this feature permits people to add things that overlap the intended impl, because we wanted people to be able to add impl From<!> for MyType even though we couldn't yet add the blanket impl (can't remember why).
  • We didn't think it was a good idea to remove it immediately, because it might be useful if we move to stabilize ! again.

bors added a commit to rust-lang/rust-analyzer that referenced this issue May 6, 2023
fix: ignore impls with `#[rustc_reservation_impl]`

Fixes #12247
Fixes #14279

Currently core has two blanket impls for `From`: `impl<T> From<T> for T` and `impl<T> From<!> for T`. These are conflicting and thus chalk cannot uniquely solve `S: From<?0>` for any type `S`.

The latter impl is actually a reservation impl and should not be considered during trait selection. More generally, impls attributed with perma-unstable `#[rustc_reservation_impl]` attribute should be disregarded except for coherence checks. See rust-lang/rust#64631 and rust-lang/rust#64715 for details.

I chose to entirely ignore them in hir-ty because we don't do coherence checks.
@Pr0methean
Copy link

Pr0methean commented Jun 2, 2023

I say we should fix whatever's preventing the blanket impl. Implementations of a method that the compiler can prove is unreachable shouldn't be able to conflict with each other -- otherwise it'd be like making Clone and Debug conflict just because they both don't implement kwyjibo(&self) -> (u32,u8,u8,u16,Result<Vec<u8>,Vec<()>>).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-trait-system Area: Trait system C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-rustc_attrs Internal rustc attributes gated on the `#[rustc_attrs]` feature gate. requires-nightly This issue requires a nightly compiler in some way. S-tracking-perma-unstable Status: The feature will stay unstable indefinitely. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants