Fix handling of allow/deny/forbid(warnings)

Author: FabianWolff

compiler/rustc_errors/src/diagnostic_builder.rs

@@ -28,6 +28,7 @@ struct DiagnosticBuilderInner<'a> {
     handler: &'a Handler,
     diagnostic: Diagnostic,
     allow_suggestions: bool,
+    is_lint: bool,
 }
 
 /// In general, the `DiagnosticBuilder` uses deref to allow access to
@@ -100,7 +101,7 @@ impl<'a> DerefMut for DiagnosticBuilder<'a> {
 impl<'a> DiagnosticBuilder<'a> {
     /// Emit the diagnostic.
     pub fn emit(&mut self) {
-        self.0.handler.emit_diagnostic(&self);
+        self.0.handler.emit_diagnostic_with_ignore(&self, self.0.is_lint);
         self.cancel();
     }
 
@@ -373,6 +374,16 @@ impl<'a> DiagnosticBuilder<'a> {
         self
     }
 
+    /// Set by LintDiagnosticBuilder to distinguish lint warnings from other
+    /// warnings. Necessary because `-A warnings` on the command line turns
+    /// off all regular warnings, but lint warnings can be turned on again
+    /// with #[warn(...)].
+    pub fn is_lint(&mut self, value: bool) -> &mut Self {
+        debug!("is_lint({:?})", value);
+        self.0.is_lint = value;
+        self
+    }
+
     /// Convenience function for internal use, clients should use one of the
     /// `struct_*` methods on [`Handler`].
     crate fn new(handler: &'a Handler, level: Level, message: &str) -> DiagnosticBuilder<'a> {
@@ -399,6 +410,7 @@ impl<'a> DiagnosticBuilder<'a> {
             handler,
             diagnostic,
             allow_suggestions: true,
+            is_lint: false,
         }))
     }
 }

compiler/rustc_errors/src/lib.rs

@@ -553,11 +553,7 @@ impl Handler {
 
     /// Construct a builder at the `Warning` level with the `msg`.
     pub fn struct_warn(&self, msg: &str) -> DiagnosticBuilder<'_> {
-        let mut result = DiagnosticBuilder::new(self, Level::Warning, msg);
-        if !self.flags.can_emit_warnings {
-            result.cancel();
-        }
-        result
+        DiagnosticBuilder::new(self, Level::Warning, msg)
     }
 
     /// Construct a builder at the `Allow` level with the `msg`.
@@ -752,6 +748,14 @@ impl Handler {
         self.inner.borrow_mut().force_print_diagnostic(db)
     }
 
+    pub fn emit_diagnostic_with_ignore(
+        &self,
+        diagnostic: &Diagnostic,
+        ignore_can_emit_warnings: bool,
+    ) {
+        self.inner.borrow_mut().emit_diagnostic_with_ignore(diagnostic, ignore_can_emit_warnings)
+    }
+
     pub fn emit_diagnostic(&self, diagnostic: &Diagnostic) {
         self.inner.borrow_mut().emit_diagnostic(diagnostic)
     }
@@ -794,6 +798,14 @@ impl HandlerInner {
     }
 
     fn emit_diagnostic(&mut self, diagnostic: &Diagnostic) {
+        self.emit_diagnostic_with_ignore(diagnostic, false);
+    }
+
+    fn emit_diagnostic_with_ignore(
+        &mut self,
+        diagnostic: &Diagnostic,
+        ignore_can_emit_warnings: bool,
+    ) {
         if diagnostic.cancelled() {
             return;
         }
@@ -802,7 +814,8 @@ impl HandlerInner {
             self.future_breakage_diagnostics.push(diagnostic.clone());
         }
 
-        if diagnostic.level == Warning && !self.flags.can_emit_warnings {
+        if diagnostic.level == Warning && !self.flags.can_emit_warnings && !ignore_can_emit_warnings
+        {
             if diagnostic.has_future_breakage() {
                 (*TRACK_DIAGNOSTICS)(diagnostic);
             }

compiler/rustc_lint/src/levels.rs

@@ -87,8 +87,8 @@ impl<'s> LintLevelsBuilder<'s> {
         self.sets.list.push(LintSet::CommandLine { specs: FxHashMap::default() });
         self.sets.lint_cap = sess.opts.lint_cap.unwrap_or(Level::Forbid);
 
-        for &(ref lint_name, level) in &sess.opts.lint_opts {
-            store.check_lint_name_cmdline(sess, &lint_name, Some(level));
+        for (position, &(ref lint_name, level)) in (0u32..).zip(sess.opts.lint_opts.iter()) {
+            store.check_lint_name_cmdline(sess, &lint_name, level);
             let orig_level = level;
 
             // If the cap is less than this specified level, e.g., if we've got
@@ -97,14 +97,15 @@ impl<'s> LintLevelsBuilder<'s> {
             let level = cmp::min(level, self.sets.lint_cap);
 
             let lint_flag_val = Symbol::intern(lint_name);
+            let src = LintLevelSource::CommandLine(lint_flag_val, orig_level, position);
 
             let ids = match store.find_lints(&lint_name) {
                 Ok(ids) => ids,
                 Err(_) => continue, // errors handled in check_lint_name_cmdline above
             };
+
             for id in ids {
                 self.check_gated_lint(id, DUMMY_SP);
-                let src = LintLevelSource::CommandLine(lint_flag_val, orig_level);
                 let specs = match self.sets.list.last().unwrap() {
                     LintSet::CommandLine { ref specs } => specs,
                     _ => unreachable!(),
@@ -132,6 +133,22 @@ impl<'s> LintLevelsBuilder<'s> {
         self.sets.list.push(LintSet::CommandLine { specs });
     }
 
+    fn get_current_depth(&self) -> u32 {
+        let mut idx = self.cur;
+        let mut depth = 0;
+        loop {
+            match &self.sets.list[idx as usize] {
+                LintSet::CommandLine { .. } => {
+                    return depth;
+                }
+                LintSet::Node { parent, .. } => {
+                    depth += 1;
+                    idx = *parent;
+                }
+            }
+        }
+    }
+
     fn check_before_insert_spec(
         &self,
         specs: &FxHashMap<LintId, LevelAndSource>,
@@ -157,8 +174,8 @@ impl<'s> LintLevelsBuilder<'s> {
                 let id_name = id.lint.name_lower();
                 let fcw_warning = match old_src {
                     LintLevelSource::Default => false,
-                    LintLevelSource::Node(symbol, _, _) => self.store.is_lint_group(symbol),
-                    LintLevelSource::CommandLine(symbol, _) => self.store.is_lint_group(symbol),
+                    LintLevelSource::Node(symbol, ..) => self.store.is_lint_group(symbol),
+                    LintLevelSource::CommandLine(symbol, ..) => self.store.is_lint_group(symbol),
                     LintLevelSource::ForceWarn(_symbol) => {
                         bug!("forced warn lint returned a forbid lint level")
                     }
@@ -177,13 +194,13 @@ impl<'s> LintLevelsBuilder<'s> {
                                 id.to_string()
                             ));
                         }
-                        LintLevelSource::Node(_, forbid_source_span, reason) => {
+                        LintLevelSource::Node(_, forbid_source_span, reason, ..) => {
                             diag_builder.span_label(forbid_source_span, "`forbid` level set here");
                             if let Some(rationale) = reason {
                                 diag_builder.note(&rationale.as_str());
                             }
                         }
-                        LintLevelSource::CommandLine(_, _) => {
+                        LintLevelSource::CommandLine(..) => {
                             diag_builder.note("`forbid` lint level was set on command line");
                         }
                         _ => bug!("forced warn lint returned a forbid lint level"),
@@ -263,7 +280,7 @@ impl<'s> LintLevelsBuilder<'s> {
         let mut specs = FxHashMap::default();
         let sess = self.sess;
         let bad_attr = |span| struct_span_err!(sess, span, E0452, "malformed lint attribute input");
-        for attr in attrs {
+        for (attr_pos, attr) in (0u32..).zip(attrs.iter()) {
             let level = match Level::from_symbol(attr.name_or_empty()) {
                 None => continue,
                 Some(lvl) => lvl,
@@ -374,7 +391,10 @@ impl<'s> LintLevelsBuilder<'s> {
                             meta_item.path.segments.last().expect("empty lint name").ident.name,
                             sp,
                             reason,
+                            self.get_current_depth(),
+                            attr_pos,
                         );
+
                         for &id in *ids {
                             self.check_gated_lint(id, attr.span);
                             self.insert_spec(&mut specs, id, (level, src));
@@ -389,6 +409,8 @@ impl<'s> LintLevelsBuilder<'s> {
                                     Symbol::intern(complete_name),
                                     sp,
                                     reason,
+                                    self.get_current_depth(),
+                                    attr_pos,
                                 );
                                 for id in ids {
                                     self.insert_spec(&mut specs, *id, (level, src));
@@ -425,6 +447,8 @@ impl<'s> LintLevelsBuilder<'s> {
                                     Symbol::intern(&new_lint_name),
                                     sp,
                                     reason,
+                                    self.get_current_depth(),
+                                    attr_pos,
                                 );
                                 for id in ids {
                                     self.insert_spec(&mut specs, *id, (level, src));
@@ -495,7 +519,13 @@ impl<'s> LintLevelsBuilder<'s> {
                     // Ignore any errors or warnings that happen because the new name is inaccurate
                     // NOTE: `new_name` already includes the tool name, so we don't have to add it again.
                     if let CheckLintNameResult::Ok(ids) = store.check_lint_name(&new_name, None) {
-                        let src = LintLevelSource::Node(Symbol::intern(&new_name), sp, reason);
+                        let src = LintLevelSource::Node(
+                            Symbol::intern(&new_name),
+                            sp,
+                            reason,
+                            self.get_current_depth(),
+                            attr_pos,
+                        );
                         for &id in ids {
                             self.check_gated_lint(id, attr.span);
                             self.insert_spec(&mut specs, id, (level, src));
@@ -514,7 +544,7 @@ impl<'s> LintLevelsBuilder<'s> {
                 }
 
                 let (lint_attr_name, lint_attr_span) = match *src {
-                    LintLevelSource::Node(name, span, _) => (name, span),
+                    LintLevelSource::Node(name, span, _, _, _) => (name, span),
                     _ => continue,
                 };
 

compiler/rustc_middle/src/lint.rs

@@ -22,12 +22,18 @@ pub enum LintLevelSource {
     Default,
 
     /// Lint level was set by an attribute.
-    Node(Symbol, Span, Option<Symbol> /* RFC 2383 reason */),
+    Node(
+        Symbol,
+        Span,
+        Option<Symbol>, /* RFC 2383 reason */
+        u32,            /* depth */
+        u32,            /* position */
+    ),
 
     /// Lint level was set by a command-line flag.
     /// The provided `Level` is the level specified on the command line.
     /// (The actual level may be lower due to `--cap-lints`.)
-    CommandLine(Symbol, Level),
+    CommandLine(Symbol, Level, u32 /* position */),
 
     /// Lint is being forced to warn no matter what.
     ForceWarn(Symbol),
@@ -37,20 +43,41 @@ impl LintLevelSource {
     pub fn name(&self) -> Symbol {
         match *self {
             LintLevelSource::Default => symbol::kw::Default,
-            LintLevelSource::Node(name, _, _) => name,
-            LintLevelSource::CommandLine(name, _) => name,
+            LintLevelSource::Node(name, ..) => name,
+            LintLevelSource::CommandLine(name, ..) => name,
             LintLevelSource::ForceWarn(name) => name,
         }
     }
 
     pub fn span(&self) -> Span {
         match *self {
             LintLevelSource::Default => DUMMY_SP,
-            LintLevelSource::Node(_, span, _) => span,
-            LintLevelSource::CommandLine(_, _) => DUMMY_SP,
+            LintLevelSource::Node(_, span, ..) => span,
+            LintLevelSource::CommandLine(..) => DUMMY_SP,
             LintLevelSource::ForceWarn(_) => DUMMY_SP,
         }
     }
+
+    /// Whether a lint level specified at `a` can be overridden by one at `b`.
+    pub fn can_override(a: &Self, b: &Self) -> bool {
+        match (a, b) {
+            (LintLevelSource::Default, LintLevelSource::Default) => false,
+            (LintLevelSource::ForceWarn(_), LintLevelSource::ForceWarn(_)) => false,
+            (_, LintLevelSource::ForceWarn(_)) => true,
+            (LintLevelSource::ForceWarn(_), _) => false,
+            (LintLevelSource::Default, _) => true,
+            (_, LintLevelSource::Default) => false,
+            (LintLevelSource::CommandLine(.., pos_a), LintLevelSource::CommandLine(.., pos_b)) => {
+                pos_a < pos_b
+            }
+            (LintLevelSource::CommandLine(..), LintLevelSource::Node(..)) => true,
+            (LintLevelSource::Node(..), LintLevelSource::CommandLine(..)) => false,
+            (
+                LintLevelSource::Node(.., depth_a, pos_a),
+                LintLevelSource::Node(.., depth_b, pos_b),
+            ) => (depth_a < depth_b) || (depth_a == depth_b && pos_a < pos_b),
+        }
+    }
 }
 
 /// A tuple of a lint level and its source.
@@ -116,9 +143,11 @@ impl LintLevelSets {
             let (warnings_level, warnings_src) =
                 self.get_lint_id_level(LintId::of(builtin::WARNINGS), idx, aux);
             if let Some(configured_warning_level) = warnings_level {
-                if configured_warning_level != Level::Warn {
-                    level = configured_warning_level;
-                    src = warnings_src;
+                if LintLevelSource::can_override(&src, &warnings_src) {
+                    if configured_warning_level != Level::Warn {
+                        level = configured_warning_level;
+                        src = warnings_src;
+                    }
                 }
             }
         }
@@ -227,6 +256,7 @@ impl<'a> LintDiagnosticBuilder<'a> {
     /// Return the inner DiagnosticBuilder, first setting the primary message to `msg`.
     pub fn build(mut self, msg: &str) -> DiagnosticBuilder<'a> {
         self.0.set_primary_message(msg);
+        self.0.is_lint(true);
         self.0
     }
 
@@ -310,7 +340,7 @@ pub fn struct_lint_level<'s, 'd>(
                     &format!("`#[{}({})]` on by default", level.as_str(), name),
                 );
             }
-            LintLevelSource::CommandLine(lint_flag_val, orig_level) => {
+            LintLevelSource::CommandLine(lint_flag_val, orig_level, _) => {
                 let flag = match orig_level {
                     Level::Warn => "-W",
                     Level::Deny => "-D",
@@ -339,7 +369,7 @@ pub fn struct_lint_level<'s, 'd>(
                     );
                 }
             }
-            LintLevelSource::Node(lint_attr_name, src, reason) => {
+            LintLevelSource::Node(lint_attr_name, src, reason, _, _) => {
                 if let Some(rationale) = reason {
                     err.note(&rationale.as_str());
                 }

compiler/rustc_session/src/session.rs

@@ -1266,8 +1266,11 @@ pub fn build_session(
         .lint_opts
         .iter()
         .filter(|&&(ref key, _)| *key == "warnings")
-        .map(|&(_, ref level)| *level == lint::Allow)
+        // We are only interested in the last one, because there might be a chain
+        // of `-A warnings -D warnings -W warnings ...`, and only the last one
+        // should have an effect
         .last()
+        .map(|&(_, ref level)| *level == lint::Allow)
         .unwrap_or(false);
     let cap_lints_allow = sopts.lint_cap.map_or(false, |cap| cap == lint::Allow);
     let can_emit_warnings = !(warnings_allow || cap_lints_allow);

src/test/ui/lint/issue-41941.rs

@@ -0,0 +1,10 @@
+// Checks whether exceptions can be added to #![deny(warnings)].
+// check-pass
+
+#![deny(warnings)]
+#![warn(unused_variables)]
+
+fn main() {
+    let a = 5;
+    //~^ WARNING: unused
+}

src/test/ui/lint/issue-41941.stderr

@@ -0,0 +1,14 @@
+warning: unused variable: `a`
+  --> $DIR/issue-41941.rs:8:9
+   |
+LL |     let a = 5;
+   |         ^ help: if this is intentional, prefix it with an underscore: `_a`
+   |
+note: the lint level is defined here
+  --> $DIR/issue-41941.rs:5:9
+   |
+LL | #![warn(unused_variables)]
+   |         ^^^^^^^^^^^^^^^^
+
+warning: 1 warning emitted
+

src/test/ui/lint/issue-70819-command-line-F.rs

@@ -0,0 +1,7 @@
+// compile-flags: -Z deduplicate-diagnostics=yes -F unused -D forbidden_lint_groups -A unused
+//~^ ERROR: allow(unused) incompatible
+//~| WARNING: this was previously accepted
+//~| ERROR: allow(unused) incompatible
+//~| WARNING: this was previously accepted
+
+fn main() {}