-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Tweak auto trait errors #137831
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
base: master
Are you sure you want to change the base?
Tweak auto trait errors #137831
Conversation
7cd126f
to
585fb6f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that all of these removals are clear enough that we should probably make all of them hidden. A suggestion that says "remove the parameters" doesn't need to take 4 extra lines to render the removal of those parameters; it's pretty clear by the name and the span that's being highlighted what the user needs to do here, and really the only thing we want to make sure is that they're still suggestions so they can be applicable.
In general, I think instead of making what I'll call "unambiguous removal" suggestions verbose, it's probably best to make them hidden.
Make suggestions to remove params and super traits verbose. ``` error[E0567]: auto traits cannot have generic parameters --> $DIR/auto-trait-validation.rs:6:19 | LL | auto trait Generic<T> {} | -------^^^ | | | auto trait cannot have generic parameters | help: remove the parameters | LL - auto trait Generic<T> {} LL + auto trait Generic {} | error[E0568]: auto traits cannot have super traits or lifetime bounds --> $DIR/auto-trait-validation.rs:8:20 | LL | auto trait Bound : Copy {} | ----- ^^^^ | | | auto traits cannot have super traits or lifetime bounds | help: remove the super traits or lifetime bounds | LL - auto trait Bound : Copy {} LL + auto trait Bound {} | ``` Make suggestion to remove associated items hidden. ``` error[E0380]: auto traits cannot have associated items --> $DIR/issue-23080.rs:5:8 | LL | unsafe auto trait Trait { | ----- auto traits cannot have associated items LL | fn method(&self) { | ^^^^^^ | = help: remove the associated items ``` Make spans smaller.
The only caveat that I have is that not everyone is well versed on the jargon we use. For generic parameters, that'd be weird, but for super traits, that can conceivably be true (although what would a person be doing adding super traits without knowing what they are called, but I digress). The other consideration that I have is the likelihood of an error being seen in the first place. A really uncommon diagnostic doesn't need to be as short as necessary. Having said that, I agree: suggestions that are clear from the message only for the vast majority of people should not be rendering the code to make the diagnostic shorter. The only thing we should be super careful about is to make sure that all "hidden" suggestions have a |
I'm not totally sure if I agree with this part. For diagnostics that we expect users to never see (auto traits are almost never likely to be stabilized any time soon), I think we should be optimizing these diagnostics for the people who we do expect to see them (e.g. people working on the type system, or experimenting with the unstable feature). So I'd still prefer shorter presentations for these, since anyone who's going to be messing with auto traits either knows what a supertrait is, or can and should find out what they are (if anything because without knowing what it is, they'll be in the dark by blindly applying the suggestion). |
I believe this is still waiting on adding a tracking issue for majorly refactoring diagnostic suggestion styles that I asked for in #t-compiler/wg-diagnostics > Why are we making suggestions verbose? @ 💬 @rustbot author |
Make suggestions to remove params and super traits verbose.
Make suggestion to remove associated items hidden.
Make spans smaller.
r? @compiler-errors