Repository navigation
Closed as not planned
Labels
A-diagnosticsArea: Messages for errors, warnings, and lintsArea: Messages for errors, warnings, and lintsA-docsArea: Documentation for any part of the project, including the compiler, standard library, and toolsArea: Documentation for any part of the project, including the compiler, standard library, and toolsA-suggestion-diagnosticsArea: Suggestions generated by the compiler applied by `cargo fix`Area: Suggestions generated by the compiler applied by `cargo fix`C-enhancementCategory: An issue proposing an enhancement or a PR with one.Category: An issue proposing an enhancement or a PR with one.D-confusingDiagnostics: Confusing error or lint that should be reworked.Diagnostics: Confusing error or lint that should be reworked.D-newcomer-roadblockDiagnostics: Confusing error or lint; hard to understand for new users.Diagnostics: Confusing error or lint; hard to understand for new users.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.
Description
originally described in blog post: https://www.ultrasaurus.com/2019/10/rust-whats-a-tuple-variant/
sample code:
fn get_num() -> Result<i32, &'static str> {
let result = 4;
OK(result)
}
error message:
error[E0425]: cannot find function `OK` in this scope
--> examples/tuple.rs:13:3
|
13 | OK(result)
| ^^ help: a tuple variant with a similar name exists: `Ok`
Error message suggestions:
- From @estebank playground
- point to line 2 (even if defined in standard library) and the compiled code which differs by case (addressed in Point at local similarly named element and tweak references to variants #65421, Point at the span for the definition of crate foreign ADTs #65912)
- include additional context in the error: "a similar name exists:
Ok(which is a tuple variant ofenum Result)" -- then the reference to "tuple variant" has context and there's a clear reference to look up in the docs
- special error for a difference in captilization (from @estebank via tweet) (addressed in Bring attention to suggestions when the only difference is capitalization #65398)
- change help text to make clear that this symbol is not recognized (when I ran into this, I thought at first that perhaps I had created a conflicting definition of a tuple variant that was somehow defined elsewhere), perhaps something like "help:
OKhas not yet been defined, perhaps you meant to refer to:Ok, which has a similar name" (addressed in Point at local similarly named element and tweak references to variants #65421) - call out the specific difference when suggesting similar term -- colors are really nice for visually-oriented people, but not super accessible. Could also include "O[K/k]" which would really help when difference is embedded in a string "foo::ba[r/z]::App" (example from @estebank tweet) -- see related Highlight only relevant parts of type path in type errors #57413
General terminology suggestions:
- stop referring to "tuple-like enum" and "tuple-like struct" and instead come up with a more precise term, such as "named tuple" (or "typed tuple" but I like the former better) which could be declared as a struct or as part of an enum.
- refer to the struct / enum use of tuples in the book, here: https://doc.rust-lang.org/1.7.0/book/primitive-types.html#tuples link to Tuple structs and create a similar titled sub-section of enum and include a link to that
- standardize naming of struct and enum and create logical parallel: unit enum / unit struct OR enum unit variant / struct unit variant (I prefer the former with fewer words)
- when you standardize on the names, consider annotate them here:
enum Message {
Quit, // unit enum
ChangeColor(i32, i32, i32), // tuple enum
Move { x: i32, y: i32 }, // struct enum
Write(String), // also a tuple enum
}
(by the way, the above example is very good serving as both an introduction to the section content and an easy syntax reference that is quick to find later)
Reacted by Zzzen
Reacted by Nicole L, Jean Mertz, Dennis Collinson, Esteban Kuber, Zach Wegrzyniak and Squirrel
Metadata
Metadata
Assignees
Labels
A-diagnosticsArea: Messages for errors, warnings, and lintsArea: Messages for errors, warnings, and lintsA-docsArea: Documentation for any part of the project, including the compiler, standard library, and toolsArea: Documentation for any part of the project, including the compiler, standard library, and toolsA-suggestion-diagnosticsArea: Suggestions generated by the compiler applied by `cargo fix`Area: Suggestions generated by the compiler applied by `cargo fix`C-enhancementCategory: An issue proposing an enhancement or a PR with one.Category: An issue proposing an enhancement or a PR with one.D-confusingDiagnostics: Confusing error or lint that should be reworked.Diagnostics: Confusing error or lint that should be reworked.D-newcomer-roadblockDiagnostics: Confusing error or lint; hard to understand for new users.Diagnostics: Confusing error or lint; hard to understand for new users.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.
Activity
estebank commentedon Oct 13, 2019
Tagging the docs team for the latter half of the ticket.
Centril commentedon Oct 13, 2019
A "unit enum" suggests an enum with a single variant rather than the unit variant of an enum; I would call this a "enum unit variant", which is albeit more verbose, also more clear.
ultrasaurus commentedon Oct 13, 2019
Missed one suggestion.. the first part of the error message is a bit misleading since it implies that
Ok()is a function.Instead of:
it could be something like this:
ultrasaurus commentedon Oct 13, 2019
@Centril would it be typical/common for someone create an enum with a single variant? seems like that would be unlikely in normal practice. If so, I would use more words to describe the less common use case, such as "enum with single variant"
Centril commentedon Oct 13, 2019
No it's not usual but e.g. "tuple enum" still makes a category mistake by conflating the data type itself with its constructors (the variant in this case). I think that paints a misleading mental model of the language, which is avoidable by including an additional word.
ultrasaurus commentedon Oct 13, 2019
hmm... I was reading "tuple" in "tuple enum" as an adjective -- a descriptor (to me) doesn't necessarily change the data type.
However, I agree that this is an important clarification (that I missed in early readings of the docs) about the difference between an enum and a struct, where enum variants are all the same type and the equivalent struct syntax creates multiple types. In practice, I wasn't confused, but wasn't really conscious of this nuance in a way I could have spoken about it.
FYI: in case it is useful, I ran into an example of "unit enum" used in the wild: https://users.rust-lang.org/t/what-is-a-unit-variant/18468/2
estebank commentedon Oct 14, 2019
Looking a bit into this, procuring the
Spanforstd::option::Option::Someis proving to be difficult here because although we have the variant'sDefIdavailable inlibrustc_resolve, where we emit this error, we do not have access to thehir::Map, so we can't get the span that way. Trying to use the definitionIdent'sSpanis also not feasible because everything that comes fromstd::prelude::v1::*has aDUMMY_SP. I think that the only way to support this would be to delay the resolution error until a later stage where thehir::Mapis available to properly display the "Result::Okdefined here" label. On the other hand, we can fix this in a minimally intrusive way today for things defined in the current crate.@petrochenkov what do you think is the feasibility of having the prelude imports carry correct
Spans pointing at thestdlocations? It seems to me that that approach would be incorrect.Rollup merge of rust-lang#65398 - estebank:capitalization-only, r=varkor
petrochenkov commentedon Oct 15, 2019
@estebank
It's more about spans for all extern items than about spans from std prelude specifically.
I tried to do that in #63149, but backed out the change due to some non-reproducible order in diagnostics. (Perhaps it's already fixed?)
The dummy spans are used instead of actual spans in
fn build_reduced_graph_for_external_crate_res.7 remaining items
ultrasaurus commentedon Oct 17, 2019
@estebank that would have definitely helped!
If you can put some kind of break between the code from diff files, I think that would be more readable.
Just in case it's nearby, the "cannot find function..." seems to conflict with "similarly named tuple variant".... would it be possible to change to "cannot find function or enum tuple variant..." at the top of your screenshot.
Thanks for your attention to this detail!
estebank commentedon Oct 17, 2019
The change made the PR size balloon in size, but I do think that this error and all others that were changed are now more accurate.
ultrasaurus commentedon Oct 17, 2019
This is great!
It would have helped me if "tuple variant" had the word "enum" near it. Would it be correct to say "enum tuple variant" ?
Not intending to be nit-picky, just providing feedback. The given context should provide enough breadcrumbs for a novice :)
Centril commentedon Oct 17, 2019
Yeah it's correct. It just has some redundancy because a tuple variant always belongs to an enum. That said, redundancy can be useful.
estebank commentedon Oct 17, 2019
Googling "tuple variant rust" leads me to believe that not mentioning it could be a stumbling block for a newcomer:
vs "enum tuple variant rust":
That being said, I'm tempted at tackling that in a more methodical way with
[t-doc].Rollup merge of rust-lang#65421 - estebank:variants, r=petrochenkov
Auto merge of #65421 - estebank:variants, r=petrochenkov
Dylan-DPC commentedon May 23, 2024
It appears that this error message is no longer confusing and should be fine to close this issue. If there are any improvements, it will be better in a new issue.