introing one-time diagnostics: only emit "lint level defined here" once #37191
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. This is in the matter of rust-lang#24690.
| let span_message = (span, message.to_owned()); | ||
| let already_noted: bool = self.one_time_diagnostics.borrow() | ||
| .contains(&span_message); | ||
| if !already_noted { |
There was a problem hiding this comment.
Nit: you can write
let fresh = self.one_time_diagnostics.borrow_mut().insert(&span_message);
if fresh {
diag_builder.span_note(span, &message);
}| @@ -452,8 +452,7 @@ 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); | |||
| sess.diag_span_note_once(&mut err, span, "lint level defined here"); | |||
There was a problem hiding this comment.
Hmm. So the key is the span+message, but it's possible that the level for many lints is set at one point (e.g., #[allow(warnings)]). Do we want to issue one note per kind of lint, or just one per span? I'm actually not sure =)
There was a problem hiding this comment.
I'm leaning towards one-per-lint (ef6a072).
|
@zackmdavis another point, raised by @jonathandturner, is that if we are emitting in json mode, we probably want to include the duplicates |
Thanks to Niko Matsakis's review for the suggestion.
Jonathan D. Turner pointed out that we don't want to dedup in JSON mode. Since the compile-test runner uses JSON output, we regrettably need to revert the edits to existing tests; one imagines that testing for one-time diagnosticity for humans will have to be added as a UI test. This remains in the matter of rust-lang#24690.
Some lint-level attributes (like `bad-style`, or, more dramatically, `warnings`) can affect more than one lint; it seems fairer to point out the attribute once for each distinct lint affected. Also, a UI test is added. This remains in the matter of rust-lang#24690.
|
@nikomatsakis Not-deduping in JSON mode is implemented in 8d1da84; unfortunately, this means the change in behavior can no longer be detected by the compile-fail tests, but ef6a072 adds a UI test to compensate. |
|
@zackmdavis looks great! thanks =) |
|
@bors r+ |
|
📌 Commit ef6a072 has been approved by |
|
⌛ Testing commit ef6a072 with merge 349394e... |
|
@bors: retry force clean
|
… 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
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_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.
This is in the matter of #24690.
r? @nikomatsakis