bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

Rollup merge of #97757 - xFrednet:rfc-2383-expect-with-force-warn, r=wesleywiser,flip1995

Browse source 
Support lint expectations for `--force-warn` lints (RFC 2383)

Rustc has a `--force-warn` flag, which overrides lint level attributes and forces the diagnostics to always be warn. This means, that for lint expectations, the diagnostic can't be suppressed as usual. This also means that the expectation would not be fulfilled, even if a lint had been triggered in the expected scope.

This PR now also tracks the expectation ID in the `ForceWarn` level. I've also made some minor adjustments, to possibly catch more bugs and make the whole implementation more robust.

This will probably conflict with https://github.com/rust-lang/rust/pull/97718. That PR should ideally be reviewed and merged first. The conflict itself will be trivial to fix.

---

r? `@wesleywiser`

cc: `@flip1995` since you've helped with the initial review and also discussed this topic with me. 🙃

Follow-up of: https://github.com/rust-lang/rust/pull/87835

Issue: https://github.com/rust-lang/rust/issues/85549

Yeah, and that's it.
This commit is contained in:
Matthias Krüger 2022-06-16 09:10:20 +02:00 • committed by GitHub
commit 95be954af4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 317 additions and 71 deletions
Download patch file Download diff file Expand all files Collapse all files

96
compiler/rustc_errors/src/lib.rs
View file

@ -660,6 +660,23 @@ impl Handler {
result
}
/// Construct a builder at the `Warning` level at the given `span` and with the `msg`.
/// The `id` is used for lint emissions which should also fulfill a lint expectation.
///
/// Attempting to `.emit()` the builder will only emit if either:
/// * `can_emit_warnings` is `true`
/// * `is_force_warn` was set in `DiagnosticId::Lint`
pub fn struct_span_warn_with_expectation(
&self,
span: impl Into<MultiSpan>,
msg: impl Into<DiagnosticMessage>,
id: LintExpectationId,
) -> DiagnosticBuilder<'_, ()> {
let mut result = self.struct_warn_with_expectation(msg, id);
result.set_span(span);
result
}
/// Construct a builder at the `Allow` level at the given `span` and with the `msg`.
#[cfg_attr(not(bootstrap), rustc_lint_diagnostics)]
pub fn struct_span_allow(
@ -693,7 +710,21 @@ impl Handler {
/// * `is_force_warn` was set in `DiagnosticId::Lint`
#[cfg_attr(not(bootstrap), rustc_lint_diagnostics)]
pub fn struct_warn(&self, msg: impl Into<DiagnosticMessage>) -> DiagnosticBuilder<'_, ()> {
DiagnosticBuilder::new(self, Level::Warning, msg)
DiagnosticBuilder::new(self, Level::Warning(None), msg)
}
/// Construct a builder at the `Warning` level with the `msg`. The `id` is used for
/// lint emissions which should also fulfill a lint expectation.
///
/// Attempting to `.emit()` the builder will only emit if either:
/// * `can_emit_warnings` is `true`
/// * `is_force_warn` was set in `DiagnosticId::Lint`
pub fn struct_warn_with_expectation(
&self,
msg: impl Into<DiagnosticMessage>,
id: LintExpectationId,
) -> DiagnosticBuilder<'_, ()> {
DiagnosticBuilder::new(self, Level::Warning(Some(id)), msg)
}
/// Construct a builder at the `Allow` level with the `msg`.
@ -864,7 +895,7 @@ impl Handler {
#[cfg_attr(not(bootstrap), rustc_lint_diagnostics)]
pub fn span_warn(&self, span: impl Into<MultiSpan>, msg: impl Into<DiagnosticMessage>) {
self.emit_diag_at_span(Diagnostic::new(Warning, msg), span);
self.emit_diag_at_span(Diagnostic::new(Warning(None), msg), span);
}
#[cfg_attr(not(bootstrap), rustc_lint_diagnostics)]
@ -874,7 +905,7 @@ impl Handler {
msg: impl Into<DiagnosticMessage>,
code: DiagnosticId,
) {
self.emit_diag_at_span(Diagnostic::new_with_code(Warning, Some(code), msg), span);
self.emit_diag_at_span(Diagnostic::new_with_code(Warning(None), Some(code), msg), span);
}
pub fn span_bug(&self, span: impl Into<MultiSpan>, msg: impl Into<DiagnosticMessage>) -> ! {
@ -928,7 +959,7 @@ impl Handler {
}
pub fn warn(&self, msg: impl Into<DiagnosticMessage>) {
let mut db = DiagnosticBuilder::new(self, Warning, msg);
let mut db = DiagnosticBuilder::new(self, Warning(None), msg);
db.emit();
}
@ -1033,13 +1064,10 @@ impl Handler {
for mut diag in diags.into_iter() {
diag.update_unstable_expectation_id(unstable_to_stable);
let stable_id = diag
.level
.get_expectation_id()
.expect("all diagnostics inside `unstable_expect_diagnostics` must have a `LintExpectationId`");
inner.fulfilled_expectations.insert(stable_id);
(*TRACK_DIAGNOSTICS)(&diag);
// Here the diagnostic is given back to `emit_diagnostic` where it was first
// intercepted. Now it should be processed as usual, since the unstable expectation
// id is now stable.
inner.emit_diagnostic(&mut diag);
}
}
@ -1089,6 +1117,15 @@ impl HandlerInner {
// FIXME(eddyb) this should ideally take `diagnostic` by value.
fn emit_diagnostic(&mut self, diagnostic: &mut Diagnostic) -> Option<ErrorGuaranteed> {
// The `LintExpectationId` can be stable or unstable depending on when it was created.
// Diagnostics created before the definition of `HirId`s are unstable and can not yet
// be stored. Instead, they are buffered until the `LintExpectationId` is replaced by
// a stable one by the `LintLevelsBuilder`.
if let Some(LintExpectationId::Unstable { .. }) = diagnostic.level.get_expectation_id() {
self.unstable_expect_diagnostics.push(diagnostic.clone());
return None;
}
if diagnostic.level == Level::DelayedBug {
// FIXME(eddyb) this should check for `has_errors` and stop pushing
// once *any* errors were emitted (and truncate `delayed_span_bugs`
@ -1105,7 +1142,12 @@ impl HandlerInner {
self.future_breakage_diagnostics.push(diagnostic.clone());
}
if diagnostic.level == Warning
if let Some(expectation_id) = diagnostic.level.get_expectation_id() {
self.suppressed_expected_diag = true;
self.fulfilled_expectations.insert(expectation_id);
}
if matches!(diagnostic.level, Warning(_))
&& !self.flags.can_emit_warnings
&& !diagnostic.is_force_warn()
{
@ -1115,22 +1157,9 @@ impl HandlerInner {
return None;
}
// The `LintExpectationId` can be stable or unstable depending on when it was created.
// Diagnostics created before the definition of `HirId`s are unstable and can not yet
// be stored. Instead, they are buffered until the `LintExpectationId` is replaced by
// a stable one by the `LintLevelsBuilder`.
if let Level::Expect(LintExpectationId::Unstable { .. }) = diagnostic.level {
self.unstable_expect_diagnostics.push(diagnostic.clone());
return None;
}
(*TRACK_DIAGNOSTICS)(diagnostic);
if let Level::Expect(expectation_id) = diagnostic.level {
self.suppressed_expected_diag = true;
self.fulfilled_expectations.insert(expectation_id);
return None;
} else if diagnostic.level == Allow {
if matches!(diagnostic.level, Level::Expect(_) | Level::Allow) {
return None;
}
@ -1167,7 +1196,7 @@ impl HandlerInner {
self.emitter.emit_diagnostic(&diagnostic);
if diagnostic.is_error() {
self.deduplicated_err_count += 1;
} else if diagnostic.level == Warning {
} else if let Warning(_) = diagnostic.level {
self.deduplicated_warn_count += 1;
}
}
@ -1220,7 +1249,7 @@ impl HandlerInner {
match (errors.len(), warnings.len()) {
(0, 0) => return,
(0, _) => self.emitter.emit_diagnostic(&Diagnostic::new(
Level::Warning,
Level::Warning(None),
DiagnosticMessage::Str(warnings),
)),
(_, 0) => {
@ -1453,7 +1482,10 @@ pub enum Level {
/// If this error comes from a lint, don't abort compilation even when abort_if_errors() is called.
lint: bool,
},
Warning,
/// This [`LintExpectationId`] is used for expected lint diagnostics, which should
/// also emit a warning due to the `force-warn` flag. In all other cases this should
/// be `None`.
Warning(Option<LintExpectationId>),
Note,
/// A note that is only emitted once.
OnceNote,
@ -1476,7 +1508,7 @@ impl Level {
Bug | DelayedBug | Fatal | Error { .. } => {
spec.set_fg(Some(Color::Red)).set_intense(true);
}
Warning => {
Warning(_) => {
spec.set_fg(Some(Color::Yellow)).set_intense(cfg!(windows));
}
Note | OnceNote => {
@ -1495,7 +1527,7 @@ impl Level {
match self {
Bug | DelayedBug => "error: internal compiler error",
Fatal | Error { .. } => "error",
Warning => "warning",
Warning(_) => "warning",
Note | OnceNote => "note",
Help => "help",
FailureNote => "failure-note",
@ -1510,7 +1542,7 @@ impl Level {
pub fn get_expectation_id(&self) -> Option<LintExpectationId> {
match self {
Level::Expect(id) => Some(*id),
Level::Expect(id) | Level::Warning(Some(id)) => Some(*id),
_ => None,
}
}