Skip to content
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

--explain should disambiguate nonexistent from underdocumented error codes #44710

Closed
zackmdavis opened this issue Sep 20, 2017 · 6 comments · Fixed by #69442
Closed

--explain should disambiguate nonexistent from underdocumented error codes #44710

zackmdavis opened this issue Sep 20, 2017 · 6 comments · Fixed by #69442
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@zackmdavis
Copy link
Member

zackmdavis commented Sep 20, 2017

The --explain EXXX functionality (does anyone still uses this now that it's no longer advertised so hard?) gives the same "no extended information" message for real error codes that don't have a "long" explanation written, and nonexistent codes. These messages should be disambiguated (as "no extended information for E0XXX" vs. "E0XXX does not exist" or similar).

$ rustc --explain E0489 # "type/lifetime parameter not in scope here"
error: no extended information for E0489

$ rustc --explain E9999nonexisting
error: no extended information for E9999nonexisting

I thought this would be straightforward to implement: we do keep track of codes without explanations (which get successfully plumbed through to the error index, although the CSS hides them), only to throw that information away when macroexpanding the arrays that ultimately stock the --explain functionality. So this should, you would think, just be a matter of editing expand_build_diagnostic_array to give us a [(&str, Option<&str>)] of code–maybe-explanation pairs instead of a [(&str, &str)] of code–explanation pairs, editing errors::Registry reflect that, and making the driver care.

But when I actually do this (zackmdavis/rust@048dde26e) ... my changes to expand_build_diagnostic_array seem to have absolutely no effect (!?!); we still get a [(&str, &str)] even though I thought my code pretty clearly said otherwise, on more than one day that I've looked at this. I hereby give up in despair, bafflement, and tears.

This issue has been assigned to @jakevossen5 via this comment.

@TimNN TimNN added A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 20, 2017
@Mark-Simulacrum Mark-Simulacrum added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Sep 11, 2019
@Mark-Simulacrum
Copy link
Member

The diagnostic registration is "just" a macro_rules macro now so I suspect this is pretty simple to implement per the suggested path in the issue description; I'm going to mark as E-easy and E-mentor with myself as the mentor.

You'll want to edit the DIAGNOSTICS array definitions here to have the type (&str, Option<&str>) or so and add in the short diagnostics.

Once that's done, if you run ./x.py check you'll likely hit an error in the use site of DIAGNOSTICS from all the crates. Fixing that'll mostly involve fixing up the diagnostics registry in librustc_errors; I would store the long diagnostics and short diagnostics separately, adding a field like short_descriptions: FxHashSet<&'static str>.

Once this is done, there might be a few more type errors to clean up but that should mostly be a matter of making types match up I would hope.

Let me know if you have any questions; here on this issue or feel free to open up a PR with some initial work and I can try and provide feedback there.

@zackmdavis zackmdavis self-assigned this Sep 12, 2019
@Mark-Simulacrum
Copy link
Member

One additional thing that we should probably add as part of this work is an error if the same diagnostic code is registered twice (cc #64426). I imagine this would mostly be a matter of inserting all the codes into an FxHashSet and panicking if there's one inserted twice; an ICE here is fine since it'll definitely be caught in CI.

@jakevossen5
Copy link
Contributor

Hey,

If this still needs fixed, I would love to work on this. I will probably be reaching out before to long for some guidance.

@rustbot claim

@rustbot rustbot assigned rustbot and unassigned zackmdavis Feb 19, 2020
@jakevossen5
Copy link
Contributor

Shoot not sure if I read what was happening in this thread all the way. @zackmdavis are you working on this already?

@zackmdavis
Copy link
Member Author

@jakevossen5 I'm not; go for it! (Although ... I thought I remembered seeing an issue recently where people were discussing moving away from the numeric error codes, but I can't find it. @estebank, was that a real thing?)

@jakevossen5
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants