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

Author: xFrednet

compiler/rustc_codegen_llvm/src/back/write.rs

@@ -340,7 +340,7 @@ fn report_inline_asm(
     }
     let level = match level {
         llvm::DiagnosticLevel::Error => Level::Error { lint: false },
-        llvm::DiagnosticLevel::Warning => Level::Warning,
+        llvm::DiagnosticLevel::Warning => Level::Warning(None),
         llvm::DiagnosticLevel::Note | llvm::DiagnosticLevel::Remark => Level::Note,
     };
     cgcx.diag_emitter.inline_asm_error(cookie as u32, msg, level, source);

compiler/rustc_codegen_ssa/src/back/write.rs

@@ -1761,7 +1761,7 @@ impl SharedEmitterMain {
 
                     let mut err = match level {
                         Level::Error { lint: false } => sess.struct_err(msg).forget_guarantee(),
-                        Level::Warning => sess.struct_warn(msg),
+                        Level::Warning(_) => sess.struct_warn(msg),
                         Level::Note => sess.struct_note_without_error(msg),
                         _ => bug!("Invalid inline asm diagnostic level"),
                     };

compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs

@@ -87,7 +87,7 @@ fn annotation_type_for_level(level: Level) -> AnnotationType {
         Level::Bug | Level::DelayedBug | Level::Fatal | Level::Error { .. } => {
             AnnotationType::Error
         }
-        Level::Warning => AnnotationType::Warning,
+        Level::Warning(_) => AnnotationType::Warning,
         Level::Note | Level::OnceNote => AnnotationType::Note,
         Level::Help => AnnotationType::Help,
         // FIXME(#59346): Not sure how to map this level

compiler/rustc_errors/src/diagnostic.rs

@@ -208,7 +208,7 @@ impl Diagnostic {
             | Level::Error { .. }
             | Level::FailureNote => true,
 
-            Level::Warning
+            Level::Warning(_)
             | Level::Note
             | Level::OnceNote
             | Level::Help
@@ -221,7 +221,9 @@ impl Diagnostic {
         &mut self,
         unstable_to_stable: &FxHashMap<LintExpectationId, LintExpectationId>,
     ) {
-        if let Level::Expect(expectation_id) = &mut self.level {
+        if let Level::Expect(expectation_id) | Level::Warning(Some(expectation_id)) =
+            &mut self.level
+        {
             if expectation_id.is_stable() {
                 return;
             }
@@ -445,7 +447,7 @@ impl Diagnostic {
 
     /// Add a warning attached to this diagnostic.
     pub fn warn(&mut self, msg: impl Into<SubdiagnosticMessage>) -> &mut Self {
-        self.sub(Level::Warning, msg, MultiSpan::new(), None);
+        self.sub(Level::Warning(None), msg, MultiSpan::new(), None);
         self
     }
 
@@ -456,7 +458,7 @@ impl Diagnostic {
         sp: S,
         msg: impl Into<SubdiagnosticMessage>,
     ) -> &mut Self {
-        self.sub(Level::Warning, msg, sp.into(), None);
+        self.sub(Level::Warning(None), msg, sp.into(), None);
         self
     }
 

compiler/rustc_errors/src/json.rs

@@ -154,7 +154,7 @@ impl Emitter for JsonEmitter {
             .into_iter()
             .map(|mut diag| {
                 if diag.level == crate::Level::Allow {
-                    diag.level = crate::Level::Warning;
+                    diag.level = crate::Level::Warning(None);
                 }
                 FutureBreakageItem { diagnostic: Diagnostic::from_errors_diagnostic(&diag, self) }
             })

compiler/rustc_errors/src/lib.rs

@@ -658,6 +658,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`.
     pub fn struct_span_allow(
         &self,
@@ -688,7 +705,21 @@ impl Handler {
     /// * `can_emit_warnings` is `true`
     /// * `is_force_warn` was set in `DiagnosticId::Lint`
     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`.
@@ -842,7 +873,7 @@ impl Handler {
     }
 
     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);
     }
 
     pub fn span_warn_with_code(
@@ -851,7 +882,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>) -> ! {
@@ -905,7 +936,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();
     }
 
@@ -1010,13 +1041,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);
             }
         }
 
@@ -1066,6 +1094,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`
@@ -1082,7 +1119,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()
         {
@@ -1092,22 +1134,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;
         }
 
@@ -1144,7 +1173,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;
             }
         }
@@ -1197,7 +1226,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) => {
@@ -1430,7 +1459,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,
@@ -1453,7 +1485,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 => {
@@ -1472,7 +1504,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",
@@ -1487,7 +1519,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,
         }
     }

compiler/rustc_expand/src/proc_macro_server.rs

@@ -267,7 +267,7 @@ impl ToInternal<rustc_errors::Level> for Level {
     fn to_internal(self) -> rustc_errors::Level {
         match self {
             Level::Error => rustc_errors::Level::Error { lint: false },
-            Level::Warning => rustc_errors::Level::Warning,
+            Level::Warning => rustc_errors::Level::Warning(None),
             Level::Note => rustc_errors::Level::Note,
             Level::Help => rustc_errors::Level::Help,
             _ => unreachable!("unknown proc_macro::Level variant: {:?}", self),

compiler/rustc_lint/src/context.rs

@@ -324,7 +324,7 @@ impl LintStore {
         registered_tools: &RegisteredTools,
     ) {
         let (tool_name, lint_name_only) = parse_lint_and_tool_name(lint_name);
-        if lint_name_only == crate::WARNINGS.name_lower() && level == Level::ForceWarn {
+        if lint_name_only == crate::WARNINGS.name_lower() && matches!(level, Level::ForceWarn(_)) {
             struct_span_err!(
                 sess,
                 DUMMY_SP,
@@ -375,7 +375,7 @@ impl LintStore {
                 match level {
                     Level::Allow => "-A",
                     Level::Warn => "-W",
-                    Level::ForceWarn => "--force-warn",
+                    Level::ForceWarn(_) => "--force-warn",
                     Level::Deny => "-D",
                     Level::Forbid => "-F",
                     Level::Expect(_) => {

compiler/rustc_lint/src/expect.rs

@@ -19,16 +19,16 @@ fn check_expectations(tcx: TyCtxt<'_>, tool_filter: Option<Symbol>) {
     let lint_expectations = &tcx.lint_levels(()).lint_expectations;
 
     for (id, expectation) in lint_expectations {
-        if !fulfilled_expectations.contains(id)
-            && tool_filter.map_or(true, |filter| expectation.lint_tool == Some(filter))
-        {
-            // This check will always be true, since `lint_expectations` only
-            // holds stable ids
-            if let LintExpectationId::Stable { hir_id, .. } = id {
+        // This check will always be true, since `lint_expectations` only
+        // holds stable ids
+        if let LintExpectationId::Stable { hir_id, .. } = id {
+            if !fulfilled_expectations.contains(&id)
+                && tool_filter.map_or(true, |filter| expectation.lint_tool == Some(filter))
+            {
                 emit_unfulfilled_expectation_lint(tcx, *hir_id, expectation);
-            } else {
-                unreachable!("at this stage all `LintExpectationId`s are stable");
             }
+        } else {
+            unreachable!("at this stage all `LintExpectationId`s are stable");
         }
     }
 }

compiler/rustc_lint/src/levels.rs

@@ -117,7 +117,9 @@ impl<'s> LintLevelsBuilder<'s> {
             };
             for id in ids {
                 // ForceWarn and Forbid cannot be overridden
-                if let Some((Level::ForceWarn | Level::Forbid, _)) = self.current_specs().get(&id) {
+                if let Some((Level::ForceWarn(_) | Level::Forbid, _)) =
+                    self.current_specs().get(&id)
+                {
                     continue;
                 }
 
@@ -226,11 +228,18 @@ impl<'s> LintLevelsBuilder<'s> {
             return;
         }
 
-        if let Level::ForceWarn = old_level {
-            self.current_specs_mut().insert(id, (old_level, old_src));
-        } else {
-            self.current_specs_mut().insert(id, (level, src));
-        }
+        match (old_level, level) {
+            // If the new level is an expectation store it in `ForceWarn`
+            (Level::ForceWarn(_), Level::Expect(expectation_id)) => self
+                .current_specs_mut()
+                .insert(id, (Level::ForceWarn(Some(expectation_id)), old_src)),
+            // Keep `ForceWarn` level but drop the expectation
+            (Level::ForceWarn(_), _) => {
+                self.current_specs_mut().insert(id, (Level::ForceWarn(None), old_src))
+            }
+            // Set the lint level as normal
+            _ => self.current_specs_mut().insert(id, (level, src)),
+        };
     }
 
     /// Pushes a list of AST lint attributes onto this context.
@@ -269,6 +278,7 @@ impl<'s> LintLevelsBuilder<'s> {
 
             let level = match Level::from_attr(attr) {
                 None => continue,
+                // This is the only lint level with a `LintExpectationId` that can be created from an attribute
                 Some(Level::Expect(unstable_id)) if let Some(hir_id) = source_hir_id => {
                     let stable_id = self.create_stable_id(unstable_id, hir_id, attr_index);