Improve E0308 error message wording #70242
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @cramertj (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
|
r? @Dylan-DPC |
|
I tried to edit the same files that had to be edited in #69139, which was the last PR to update this error message. Apparently the tests hardcode the whole error message several times, so likewise, the changed phrasing needed to be copy-pasted several times. |
|
Critiquing my own PR: I'm mildly concerned that "on the one hand" (as an idiom) is going to unnecessarily confuse English-as-a-second-language readers. I think it's clearer that way for proficient English speakers, but that shouldn't outweigh other users' needs. So I'm definitely open to removing those if other folks feel the same way about it. |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
Looks like I didn't edit the test files correctly -- am I supposed to properly type CR, LF, and CR+LF symbols for those tests? I'm not sure I want to guess and check since it took a bit over an hour to run the tests... And it would be tricky to type that correctly. |
|
Did you run tidy ? |
No, I actually don't know what that is. Will do if I can understand how to. Edit: I found the instructions here: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#pull-requests
I'm running that now. (Sorry I missed that. I tried to skim I caught an extra stray quote marks in a few places in the original commit. Fixed (amended the commit) and force-pushed. Maybe it will pass this time. |
|
https://rustc-dev-guide.rust-lang.org/conventions.html?highlight=tidy#formatting-and-the-tidy-script run |
|
Both commits (one before amending and one after amending) appear to pass the My output is like this: |
|
It passed, woo! 🎉 |
|
Just looked up the technical meaning of expressions in Rust. If I understand correctly, in the example snippet:
The expression would be I think the original wording is clear if you already understand the definition of an "expression" as a technical term meaning roughly "something that is a value, or which can be evaluated". I think my updated wording is still correct, but less bare-bones, or arguably less focused and concise. However, I hope the added context and hints for reasoning about the error are worth it. I would welcome some technical review that the updated wording is still accurate, and feedback on whether it would be considered an actual improvement by someone who knows the language well. I opened this PR as a beginner to Rust, and I am okay with it not being merged if it doesn't better-explain the error. I'm also thinking about if I understand the message well enough to split it into more sentences. |
|
Something like this would properly break up the sentences to be more manageable:
Edit: The default integer type is
|
|
Another attempt at simplifying:
The sentence also works without "as set by the author" and "as deduced by the compiler" which allows it to be even shorter:
(Though, IMO, this short version is harder to comprehend for a total programming newbie like I apparently was a couple of hours ago.) |
|
Technical details: The error message can be triggered by any combination of explicit or inferred types that don't match. Examples:
All these attempts at assignment trigger E0308. I'm afraid the effort to provide a "common scenario" is just confusing, whereas the core problem or idea is just that "two types didn't match." Edit to add: If anything needs to be explained here, it's just that Rust is statically typed and won't allow the same variable to take on two different types within the same program. |
|
@bors r+ rollup |
|
📌 Commit 99ae428 has been approved by |
bors
added
S-waiting-on-bors
|
Dont worry. Looks good to me |
Rollup of 11 pull requests Successful merges: - rust-lang#67761 (Move the dep_graph construction to a dedicated crate.) - rust-lang#69740 (Replace some desc logic in librustc_lint with article_and_desc) - rust-lang#69981 (Evaluate repeat expression lengths as late as possible) - rust-lang#70087 (Remove const eval loop detector) - rust-lang#70242 (Improve E0308 error message wording) - rust-lang#70264 (Fix invalid suggestion on `&mut` iterators yielding `&` references) - rust-lang#70267 (get rid of ConstPropUnsupported; use ZST marker structs instead) - rust-lang#70277 (Remove `ReClosureBound`) - rust-lang#70283 (Add regression test for rust-lang#70155.) - rust-lang#70294 (Account for bad placeholder types in where clauses) - rust-lang#70309 (Clean up E0452 explanation) Failed merges: r? @ghost
|
It seems too late for this, but in light of my better understanding the error, I came up with this wording:
I prefer it for its simplicity, clarity and tone. Should I open another PR if I want to suggest that wording instead? Edit: I see the earlier wording as I had it in my branch is in |
| variable. | ||
| variable. It can happen in several cases, the most common being a mismatch | ||
| between the type that the compiler inferred for a variable based on its | ||
| initializing expression, on the one hand, and the type the author explicitly |
There was a problem hiding this comment.
Is this a typo? on the one hand
There was a problem hiding this comment.
Just my two cents, on one hand and on the other hand may not be easily understood by non-native english speakers. Maybe it's just me but I learned that late, and is easier to be understood.
There was a problem hiding this comment.
I meant to contribute a better message with no "on the other hand." But I got side-tracked and honestly a bit discouraged, since there's nobody to workshop good messages with, and no standards of what a good error message looks like.
I'd like there to be a working group to bounce ideas off of, and/or a style guide.
Here's what I had, roughly:
This error occurs when the compiler was unable to infer the concrete type of a variable.
It can occur for several cases; The most common case is a mismatch between
two types: the type explicitly assigned to a variable by the [code's] author,
and the type the compiler infers for an expression assigned to that variable.
Possibly a "hint" line about dynamic typing versus static typing:
In Rust each declared variable is assigned a static type. Dynamic typing is not supported.
See prior discussion here if you want: https://users.rust-lang.org/t/looking-for-feedback-on-an-improved-error-message-for-e0308/40004
I actually think the phrasing can be much simplified.
Any two mis-matching types assigned to the same variable cause this error. Regardless of explicit or inferred, at or after initialization, etc... Just... two non-matching types, one variable.
(Apparently there is a thing called shadowing, but if I read correctly, I believe it just spins up a new variable with a different compiled identity, but re-using the same name in source code. New instance of the variable can have a new type. It's still static typing under the hood, though. I am afraid of over-simplifying, and ending up with a message that is inaccurate regarding shadowing.)
There was a problem hiding this comment.
This error occurs when the compiler was unable to infer the concrete type of a variable.
It can occur for several cases; The most common case is a mismatch between
two types: the type explicitly assigned to a variable by the [code's] author,
and the type the compiler infers for an expression assigned to that variable.
That is better than the merged one I think but still kinda long-winded, I think documentation should be short, concise and understandable. What I would say a good example is IETF RFC docs, rust docs is good enough for me.
There was a problem hiding this comment.
Thanks for the reply.
I have some things to do at the moment, but I will post that version as a second PR when I get the chance. Hopefully before a new stable release.
It could probably be shortened, mostly by removing the "common example" of when this error occurs. (It is kind of arbitrary, and I think it might be overly specific when the error is more general than that.) But I don't want to be too drastic while I am still getting used to contributing to Rust.
Posting this for reference since you mentioned IETF RFCs as being a good example: https://tools.ietf.org/html/rfc7322 The IETF RFC Style Guide.
Maybe that can be an influence for a Rust docs "style guide" or best practices guide. No time now, but planning to look that over later.
…bank Improve E0308 error message wording again Hello again, I recently did this PR: rust-lang#70242 I felt the error message could be further improved, so I made [a post on the Rust community forum](https://users.rust-lang.org/t/looking-for-feedback-on-an-improved-error-message-for-e0308/40004) to ask for feedback. (Also, there were some comments on my original PR that I took into consideration as well.) This PR is my attempt to take all the feedback into account and propose a better and simplified error message that should still be accurate. Its main benefit is having simpler grammar, and hopefully being easier to read and understand. Thanks to everyone who commented and gave feedback, and thank you for taking a look at this PR.
…bank Improve E0308 error message wording again Hello again, I recently did this PR: rust-lang#70242 I felt the error message could be further improved, so I made [a post on the Rust community forum](https://users.rust-lang.org/t/looking-for-feedback-on-an-improved-error-message-for-e0308/40004) to ask for feedback. (Also, there were some comments on my original PR that I took into consideration as well.) This PR is my attempt to take all the feedback into account and propose a better and simplified error message that should still be accurate. Its main benefit is having simpler grammar, and hopefully being easier to read and understand. Thanks to everyone who commented and gave feedback, and thank you for taking a look at this PR.
Hi folks,
I made a post on Reddit about how (IMO) the docs/error messages can be a bit intimidating, one thing led to another, and I was encouraged to submit a Pull Request if I felt I could re-phrase the error message that I used as an example.
So that's this Pull Request. Open to any feedback or style changes, and I understand this is subjective.
(On another note: I am happy to see this message was recently improved in
master, so it's already better than it is in stable Rust 1.42.0.)Ideally the last sentence could be split into at least two: [sentence explaining the inferred type.] [Sentence explaining explicit type.] [Sentence that summarizes that "this is bad," and why.]
But I'm not sure how to do so; I'm wary of writing something that turns out to be technically incorrect.