only emit "lint level defined here" the first time #34084
Conversation
We introduce a new `one_time_diagnostics` field on `rustc::session::Session` to hold a hashset of diagnostic messages we've set once but don't want to see again (as uniquified by span and message text), "lint level defined here" being the motivating example dealt with here. It is the responsibility of the caller setting a diagnostic to decide whether to add to `one_time_diagnostics`; there are other situations where we likely do want it to be possible for a note to appear twice on the same span with the same message (e.g., "prior assignment occurs here"). This is in the matter of rust-lang#24690.
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (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. |
| @@ -462,7 +462,13 @@ pub fn raw_struct_lint<'a>(sess: &'a Session, | |||
|
|
|||
| if let Some(span) = def { | |||
| let explanation = "lint level defined here"; | |||
| err.span_note(span, &explanation); | |||
| let span_explanation = (span, explanation.to_owned()); | |||
| let already_noted: bool = sess.one_time_diagnostics.borrow() | |||
There was a problem hiding this comment.
I think it'd be nice to offer a method that encapsulates this "check-and-add" pattern rather than modifying the field directly (in general, I think it's best practice to hide a RefCell behind methods in any case, but in particular here).
For example, this code might look like:
err.span_note_once(span, explanation);and then this method would do the check.
|
@zackmdavis sorry for delay, I've been on vacation (still am). The PR seems good but I left a comment on one way to improve it. |
|
Closing due to inactivity, but feel free to resubmit with comments addressed! |
|
Niko: Thanks; sorry to have bothered you on vacation! (I tagged you because someone on IRC said you'd been working on lints, but maybe I should have just let the bot decide.) @alexcrichton understood; I've unfortunately been pretty busy lately, but will try to take another look on yhe weekend. |
|
@zackmdavis no worries! Feel free to ping a PR if it languishes for awhile as well, I'm sure others would also be more than willing to review :) |
|
@zackmdavis not to worry. I'm back now anyhow (and I was trying to keep up with PRs...easier said than done though). |
Well, not this weekend (lots of non- |
… r=nikomatsakis introing one-time diagnostics: only emit "lint level defined here" once This is a revised resubmission of PR #34084 (which was closed due to inactivity on account of time constraints on the author's part). --- We introduce a new `one_time_diagnostics` field on `rustc::session::Session` to hold a hashset of diagnostic messages we've set once but don't want to see again (as uniquified by span and message text), "lint level defined here" being the motivating example dealt with here. This is in the matter of #24690. --- r? @nikomatsakis
We introduce a new
one_time_diagnosticsfield onrustc::session::Sessionto hold a hashset of diagnostic messages we'veset once but don't want to see again (as uniquified by span and message
text), "lint level defined here" being the motivating example dealt with
here. It is the responsibility of the caller setting a diagnostic to
decide whether to add to
one_time_diagnostics; there are othersituations where we likely do want it to be possible for a note to
appear twice on the same span with the same message (e.g., "prior
assignment occurs here").
Fixes #24690.
I get 83 debuginfo-gdb failures when I run the tests locally, but I am given to understand that it's possible that this may be a problem with my environment rather than the proposed changes; our friend Travis can be trusted to let us know.
Thanks to @mitaa for the pointer on where to get started and to @eddyb in
#rustcfor advice onconfigureoptions, CI, and my possible gdb problem.r? @nikomatsakis