How to handle reservation impls in rustdoc / error messages

[nikomatsakis]

I've opened this issue for discussion on how to handle reservation impls (see #64631) in rustdoc and how to handle their error messages.

[nikomatsakis]

My take:

We should not show reservation impls in rustdoc at all. They don't exist and they may never exist. I'm not convinced users want to see them. Moreover, they are only relevant if you're trying to rely on negative reasoning about From<!> for T, which seems unusual in practice.

[nikomatsakis]

The current error message is this:

error[E0119]: conflicting implementations of trait `OtherTrait` for type `()`:
  --> $DIR/reservation-impl-coherence-conflict.rs:13:1
   |
LL | impl OtherTrait for () {}
   | ---------------------- first implementation here
LL | impl<T: MyTrait> OtherTrait for T {}
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `()`
   |
   = note: this impl is reserved

error: aborting due to previous error

I think this is pretty clearly not good enough. However, I also don't think we're going to do a lot better in the error message. I suggest opening an issue for each use of #[rustc_reservation_impl] and explaining the situation there. Then we can say something like:

   = note: permitting this impl would forbid us from adding `impl<T> From<!> for T` later; see rust-lang/rust#12345 for details

[nikomatsakis]

Nominating for @rust-lang/lang discussion -- I'd love to get feedback on how to handle these errors, so that we can land this feature and unblock the ! stabilization.

[joshtriplett]

@nikomatsakis That error text sounds good to me.

[Centril]

I think diagnostics and rustdoc is squarely a T-compiler and T-rustdoc matter. I have no opinion other than to say that @nikomatsakis's suggestion seems reasonable but this is not my decision to make.

[joshtriplett]

I think there ought to be documentation of "you may not make any impl like this" somewhere. How about this: we lint on a rustc_reservation_impl that doesn't have a doc comment, we explain in the doc comment, and rustdoc just emits the doc comment next to the list of documented impls?

[nikomatsakis]

@joshtriplett

I think there ought to be documentation of "you may not make any impl like this" somewhere.

I'm not 100% sure what you are suggesting. I think maybe you are suggesting we extend the documentation of the From trait (in this case) with a special "Note" or something like that which says:

"Note that we intend to add From<!> to T in the future, which implies some limitations on the kinds of impls you can make, see #12345 for details"?

We could do that, though it feels like a lot of work to extend rustdoc in that way. We could also of course just extend the docs for From itself readily enough to cite the issue in question -- honestly that's probably my preference, vs writing a bunch of code to make a lint and all that.

I guess I still sort of feel like it's better not to throw this at users until they need to know about it. (I could see documenting the special treatment in the rust reference or something, though.)

[nikomatsakis]

I created #64715 as a first stab at a "user-readable" tracking issue.

[joshtriplett]

@nikomatsakis I think I'm just suggesting that we have a doc comment on the reservation impl, and put the contents of that doc comment (however we choose to explain it) in the documentation. That doesn't seem like it would be a huge amount of change to rustdoc to just add that one comment.

[nikomatsakis]

First, we should definitely cc @QuietMisdreavus at minimum -- not sure what is the best alias for rustdoc folks?

In any case, @joshtriplett, that seems pretty reasonable. In fact, I see rustdoc is already including doc comments, so I'll push a quick comment on the one existing impl. The main question then is whether to "badge" the impl or something (maybe just mark it as unstable).

[Centril]

cc @rust-lang/rustdoc

[nikomatsakis]

I think that the more polished version would basically have some "canned comment" that is added by rustdoc itself. The attribute has this form today:

#[rustc_reservation_impl="permitting this impl would forbid us from adding \
`impl<T> From<!> for T` later; see rust-lang/rust#64715 for details"]

so we could just include that text in rustdoc, perhaps creating a hyperlink for #64715. But it's not like we expect to add a lot of these impls, so a doc comment also seems ok for now.

[GuillaumeGomez]

We tend to implement things when we're somewhat sure that they'll get merged. However, if you need it, we can always talk about it?

[nikomatsakis]

This is how it currently looks, and I think it's "good enough". Going to close the issue.