document + UI test E0208 and make its output more user-friendly

[Ezrashaw]

Cleans up E0208's output a lot. It could actually be useful for someone learning about variance now. I also added a UI test for it in tests/ui/error-codes/ and wrote some docs for it.

r? @GuillaumeGomez another error code, can't be bothered to find the issue :P. Obviously there's some compiler stuff, so you'll have to hand it off.

Part of #61137.

[GuillaumeGomez]

Looks good to me, thanks! As you mentioned, it needs approval from the compiler team too.

r? @estebank

[cjgillot]

This is first a test tool for the compiler.
In this idea, losing the short version of the information makes the test much less useful.

[compiler-errors]

I agree with cjgillot -- these UI tests are specifically checking for [+, -], etc error outputs to assert the variances of an item's generic parameters.

Replacing them with E0208 and a generic error message makes all of them far less useful -- it seems very easy to accidentally --bless a test and not notice the output change, versus // ERROR attributes which need to be "blessed" manually.

Secondly, I don't think a structured suggestion to remove the attribute is necessarily useful.

[Ezrashaw]

@compiler-errors Yeah, I get that and perhaps we can work the old behavior back in somehow. The point though, is that if an error has a code, then it should be documented, tested and usable from a users perspective. This could become helpful for someone learning about variance, etc. Perhaps we can add a note with just the old one like //|~ NOTE [..], do you have to then mark all subdiagnostics?

EDIT: Alternatively, if you think this really should be purely internal then it shouldn't have an error code.

[compiler-errors]

This error is purely internal. It certainly isn't a good candidate for stabilization in the state that it's currently written, not even considering that it's a rustc_ attr and can't even be used by non-nightly users. Perhaps it doesn't need an error code.

--

The problem with //~| NOTE attrs is that we want to point to each variant, so we'd need to do something like:

fn foo<
  T, // NOTE +
  S, // NOTE o
>() {}

Or something... seems like a lot of overhead.

[Ezrashaw]

@compiler-errors Oh, I am absolutely not saying it should be stabilized, just that perhaps it could be used for more than just testing. What about having #[rustc_variance] and #[rustc_variance(terse)] or something like that?

[estebank]

r? @compiler-errors

[compiler-errors]

The point though, is that if an error has a code, then it should be documented, tested and usable from a users perspective.

Well in that case, I'd rather just remove the error code from this error, and keep it fully internal.

If we want to add a new error code and some other variance inspection tool (maybe via a warning rather than an error), then it should be suggested and implemented separately.

[Ezrashaw]

@compiler-errors I've removed the error code from #[rustc_variance]. The error code docs have been updated to reflect that it is no longer emitted but the documentation itself remains. I assume @GuillaumeGomez is ok with this.

@rustbot ready

[compiler-errors]

This is fine then

@bors r+

[bors]

📌 Commit 708861e has been approved by compiler-errors

It is now in the queue for this repository.

[GuillaumeGomez]

Yup I'm fine with it. Sorry didn't see the notification.