Auto merge of #87337 - jyn514:lint-error, r=oli-obk,flip1995

Don't abort compilation after giving a lint error

The only reason to use `abort_if_errors` is when the program is so broken that either:
1. later passes get confused and ICE
2. any diagnostics from later passes would be noise

This is never the case for lints, because the compiler has to be able to deal with `allow`-ed lints.
So it can continue to lint and compile even if there are lint errors.

Closes #82761. This is a WIP because I have a feeling it will exit with 0 even if there were lint errors; I don't have a computer that can build rustc locally at the moment.

compiler/rustc_codegen_llvm/src/back/write.rs

@@ -286,7 +286,7 @@ fn report_inline_asm(
         cookie = 0;
     }
     let level = match level {
-        llvm::DiagnosticLevel::Error => Level::Error,
+        llvm::DiagnosticLevel::Error => Level::Error { lint: false },
         llvm::DiagnosticLevel::Warning => Level::Warning,
         llvm::DiagnosticLevel::Note | llvm::DiagnosticLevel::Remark => Level::Note,
     };

compiler/rustc_codegen_ssa/src/back/write.rs

@@ -1757,7 +1757,7 @@ impl SharedEmitterMain {
                     let msg = msg.strip_prefix("error: ").unwrap_or(&msg);
 
                     let mut err = match level {
-                        Level::Error => sess.struct_err(&msg),
+                        Level::Error { lint: false } => sess.struct_err(&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

@@ -66,7 +66,7 @@ fn source_string(file: Lrc<SourceFile>, line: &Line) -> String {
 /// Maps `Diagnostic::Level` to `snippet::AnnotationType`
 fn annotation_type_for_level(level: Level) -> AnnotationType {
     match level {
-        Level::Bug | Level::Fatal | Level::Error => AnnotationType::Error,
+        Level::Bug | Level::Fatal | Level::Error { .. } => AnnotationType::Error,
         Level::Warning => AnnotationType::Warning,
         Level::Note => AnnotationType::Note,
         Level::Help => AnnotationType::Help,

compiler/rustc_errors/src/diagnostic.rs

@@ -114,7 +114,7 @@ impl Diagnostic {
 
     pub fn is_error(&self) -> bool {
         match self.level {
-            Level::Bug | Level::Fatal | Level::Error | Level::FailureNote => true,
+            Level::Bug | Level::Fatal | Level::Error { .. } | Level::FailureNote => true,
 
             Level::Warning | Level::Note | Level::Help | Level::Cancelled | Level::Allow => false,
         }

compiler/rustc_errors/src/lib.rs

@@ -411,6 +411,8 @@ pub struct Handler {
 /// as well as inconsistent state observation.
 struct HandlerInner {
     flags: HandlerFlags,
+    /// The number of lint errors that have been emitted.
+    lint_err_count: usize,
     /// The number of errors that have been emitted, including duplicates.
     ///
     /// This is not necessarily the count that's reported to the user once
@@ -550,6 +552,7 @@ impl Handler {
             flags,
             inner: Lock::new(HandlerInner {
                 flags,
+                lint_err_count: 0,
                 err_count: 0,
                 warn_count: 0,
                 deduplicated_err_count: 0,
@@ -726,7 +729,13 @@ impl Handler {
     /// Construct a builder at the `Error` level with the `msg`.
     // FIXME: This method should be removed (every error should have an associated error code).
     pub fn struct_err(&self, msg: &str) -> DiagnosticBuilder<'_> {
-        DiagnosticBuilder::new(self, Level::Error, msg)
+        DiagnosticBuilder::new(self, Level::Error { lint: false }, msg)
+    }
+
+    /// This should only be used by `rustc_middle::lint::struct_lint_level`. Do not use it for hard errors.
+    #[doc(hidden)]
+    pub fn struct_err_lint(&self, msg: &str) -> DiagnosticBuilder<'_> {
+        DiagnosticBuilder::new(self, Level::Error { lint: true }, msg)
     }
 
     /// Construct a builder at the `Error` level with the `msg` and the `code`.
@@ -790,11 +799,14 @@ impl Handler {
     }
 
     pub fn span_err(&self, span: impl Into<MultiSpan>, msg: &str) {
-        self.emit_diag_at_span(Diagnostic::new(Error, msg), span);
+        self.emit_diag_at_span(Diagnostic::new(Error { lint: false }, msg), span);
     }
 
     pub fn span_err_with_code(&self, span: impl Into<MultiSpan>, msg: &str, code: DiagnosticId) {
-        self.emit_diag_at_span(Diagnostic::new_with_code(Error, Some(code), msg), span);
+        self.emit_diag_at_span(
+            Diagnostic::new_with_code(Error { lint: false }, Some(code), msg),
+            span,
+        );
     }
 
     pub fn span_warn(&self, span: impl Into<MultiSpan>, msg: &str) {
@@ -862,6 +874,9 @@ impl Handler {
     pub fn has_errors(&self) -> bool {
         self.inner.borrow().has_errors()
     }
+    pub fn has_errors_or_lint_errors(&self) -> bool {
+        self.inner.borrow().has_errors_or_lint_errors()
+    }
     pub fn has_errors_or_delayed_span_bugs(&self) -> bool {
         self.inner.borrow().has_errors_or_delayed_span_bugs()
     }
@@ -979,7 +994,11 @@ impl HandlerInner {
             }
         }
         if diagnostic.is_error() {
-            self.bump_err_count();
+            if matches!(diagnostic.level, Level::Error { lint: true }) {
+                self.bump_lint_err_count();
+            } else {
+                self.bump_err_count();
+            }
         } else {
             self.bump_warn_count();
         }
@@ -1073,11 +1092,14 @@ impl HandlerInner {
     fn has_errors(&self) -> bool {
         self.err_count() > 0
     }
+    fn has_errors_or_lint_errors(&self) -> bool {
+        self.has_errors() || self.lint_err_count > 0
+    }
     fn has_errors_or_delayed_span_bugs(&self) -> bool {
         self.has_errors() || !self.delayed_span_bugs.is_empty()
     }
     fn has_any_message(&self) -> bool {
-        self.err_count() > 0 || self.warn_count > 0
+        self.err_count() > 0 || self.lint_err_count > 0 || self.warn_count > 0
     }
 
     fn abort_if_errors(&mut self) {
@@ -1131,7 +1153,7 @@ impl HandlerInner {
     }
 
     fn err(&mut self, msg: &str) {
-        self.emit_error(Error, msg);
+        self.emit_error(Error { lint: false }, msg);
     }
 
     /// Emit an error; level should be `Error` or `Fatal`.
@@ -1167,6 +1189,11 @@ impl HandlerInner {
         }
     }
 
+    fn bump_lint_err_count(&mut self) {
+        self.lint_err_count += 1;
+        self.panic_if_treat_err_as_bug();
+    }
+
     fn bump_err_count(&mut self) {
         self.err_count += 1;
         self.panic_if_treat_err_as_bug();
@@ -1210,7 +1237,10 @@ impl DelayedDiagnostic {
 pub enum Level {
     Bug,
     Fatal,
-    Error,
+    Error {
+        /// If this error comes from a lint, don't abort compilation even when abort_if_errors() is called.
+        lint: bool,
+    },
     Warning,
     Note,
     Help,
@@ -1229,7 +1259,7 @@ impl Level {
     fn color(self) -> ColorSpec {
         let mut spec = ColorSpec::new();
         match self {
-            Bug | Fatal | Error => {
+            Bug | Fatal | Error { .. } => {
                 spec.set_fg(Some(Color::Red)).set_intense(true);
             }
             Warning => {
@@ -1250,7 +1280,7 @@ impl Level {
     pub fn to_str(self) -> &'static str {
         match self {
             Bug => "error: internal compiler error",
-            Fatal | Error => "error",
+            Fatal | Error { .. } => "error",
             Warning => "warning",
             Note => "note",
             Help => "help",

compiler/rustc_expand/src/proc_macro_server.rs

@@ -273,7 +273,7 @@ impl ToInternal<TokenStream> for TokenTree<Group, Punct, Ident, Literal> {
 impl ToInternal<rustc_errors::Level> for Level {
     fn to_internal(self) -> rustc_errors::Level {
         match self {
-            Level::Error => rustc_errors::Level::Error,
+            Level::Error => rustc_errors::Level::Error { lint: false },
             Level::Warning => rustc_errors::Level::Warning,
             Level::Note => rustc_errors::Level::Note,
             Level::Help => rustc_errors::Level::Help,

compiler/rustc_middle/src/lint.rs

@@ -248,8 +248,12 @@ pub fn struct_lint_level<'s, 'd>(
             (Level::Warn, None) => sess.struct_warn(""),
             (Level::ForceWarn, Some(span)) => sess.struct_span_force_warn(span, ""),
             (Level::ForceWarn, None) => sess.struct_force_warn(""),
-            (Level::Deny | Level::Forbid, Some(span)) => sess.struct_span_err(span, ""),
-            (Level::Deny | Level::Forbid, None) => sess.struct_err(""),
+            (Level::Deny | Level::Forbid, Some(span)) => {
+                let mut builder = sess.diagnostic().struct_err_lint("");
+                builder.set_span(span);
+                builder
+            }
+            (Level::Deny | Level::Forbid, None) => sess.diagnostic().struct_err_lint(""),
         };
 
         // If this code originates in a foreign macro, aka something that this crate

compiler/rustc_session/src/session.rs

@@ -411,7 +411,7 @@ impl Session {
         self.diagnostic().abort_if_errors();
     }
     pub fn compile_status(&self) -> Result<(), ErrorReported> {
-        if self.has_errors() {
+        if self.diagnostic().has_errors_or_lint_errors() {
             self.diagnostic().emit_stashed_diagnostics();
             Err(ErrorReported)
         } else {

src/librustdoc/core.rs

@@ -468,11 +468,13 @@ crate fn run_global_ctxt(
         };
         if run {
             debug!("running pass {}", p.pass.name);
-            krate = ctxt.tcx.sess.time(p.pass.name, || (p.pass.run)(krate, &mut ctxt));
+            krate = tcx.sess.time(p.pass.name, || (p.pass.run)(krate, &mut ctxt));
         }
     }
 
-    ctxt.sess().abort_if_errors();
+    if tcx.sess.diagnostic().has_errors_or_lint_errors() {
+        rustc_errors::FatalError.raise();
+    }
 
     let render_options = ctxt.render_options;
     let mut cache = ctxt.cache;

src/librustdoc/doctest.rs

@@ -1,7 +1,7 @@
 use rustc_ast as ast;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::sync::Lrc;
-use rustc_errors::{ColorConfig, ErrorReported};
+use rustc_errors::{ColorConfig, ErrorReported, FatalError};
 use rustc_hir as hir;
 use rustc_hir::def_id::LOCAL_CRATE;
 use rustc_hir::intravisit;
@@ -149,7 +149,9 @@ crate fn run(options: Options) -> Result<(), ErrorReported> {
 
                 collector
             });
-            compiler.session().abort_if_errors();
+            if compiler.session().diagnostic().has_errors_or_lint_errors() {
+                FatalError.raise();
+            }
 
             let unused_extern_reports = collector.unused_extern_reports.clone();
             let compiling_test_count = collector.compiling_test_count.load(Ordering::SeqCst);

src/librustdoc/lib.rs

@@ -776,7 +776,7 @@ fn main_options(options: config::Options) -> MainResult {
             // current architecture.
             let resolver = core::create_resolver(queries, sess);
 
-            if sess.has_errors() {
+            if sess.diagnostic().has_errors_or_lint_errors() {
                 sess.fatal("Compilation failed, aborting rustdoc");
             }
 

src/test/ui-fulldeps/lint-tool-test.rs

@@ -10,10 +10,12 @@
 //~^ WARNING lint name `test_lint` is deprecated and may not have an effect in the future
 //~| WARNING lint name `test_lint` is deprecated and may not have an effect in the future
 //~| WARNING lint name `test_lint` is deprecated and may not have an effect in the future
+//~| WARNING lint name `test_lint` is deprecated and may not have an effect in the future
 #![deny(clippy_group)]
 //~^ WARNING lint name `clippy_group` is deprecated and may not have an effect in the future
 //~| WARNING lint name `clippy_group` is deprecated and may not have an effect in the future
 //~| WARNING lint name `clippy_group` is deprecated and may not have an effect in the future
+//~| WARNING lint name `clippy_group` is deprecated and may not have an effect in the future
 
 fn lintme() { } //~ ERROR item is named 'lintme'
 
@@ -30,6 +32,7 @@ pub fn main() {
 //~^ WARNING lint name `test_group` is deprecated and may not have an effect in the future
 //~| WARNING lint name `test_group` is deprecated and may not have an effect in the future
 //~| WARNING lint name `test_group` is deprecated and may not have an effect in the future
+//~| WARNING lint name `test_group` is deprecated and may not have an effect in the future
 #[deny(this_lint_does_not_exist)] //~ WARNING unknown lint: `this_lint_does_not_exist`
 fn hello() {
     fn lintmetoo() { }

src/test/ui-fulldeps/lint-tool-test.stderr

@@ -7,19 +7,19 @@ LL | #![cfg_attr(foo, warn(test_lint))]
    = note: `#[warn(renamed_and_removed_lints)]` on by default
 
 warning: lint name `clippy_group` is deprecated and may not have an effect in the future.
-  --> $DIR/lint-tool-test.rs:13:9
+  --> $DIR/lint-tool-test.rs:14:9
    |
 LL | #![deny(clippy_group)]
    |         ^^^^^^^^^^^^ help: change it to: `clippy::group`
 
 warning: lint name `test_group` is deprecated and may not have an effect in the future.
-  --> $DIR/lint-tool-test.rs:29:9
+  --> $DIR/lint-tool-test.rs:31:9
    |
 LL | #[allow(test_group)]
    |         ^^^^^^^^^^ help: change it to: `clippy::test_group`
 
 warning: unknown lint: `this_lint_does_not_exist`
-  --> $DIR/lint-tool-test.rs:33:8
+  --> $DIR/lint-tool-test.rs:36:8
    |
 LL | #[deny(this_lint_does_not_exist)]
    |        ^^^^^^^^^^^^^^^^^^^^^^^^
@@ -33,13 +33,13 @@ LL | #![cfg_attr(foo, warn(test_lint))]
    |                       ^^^^^^^^^ help: change it to: `clippy::test_lint`
 
 warning: lint name `clippy_group` is deprecated and may not have an effect in the future.
-  --> $DIR/lint-tool-test.rs:13:9
+  --> $DIR/lint-tool-test.rs:14:9
    |
 LL | #![deny(clippy_group)]
    |         ^^^^^^^^^^^^ help: change it to: `clippy::group`
 
 warning: lint name `test_group` is deprecated and may not have an effect in the future.
-  --> $DIR/lint-tool-test.rs:29:9
+  --> $DIR/lint-tool-test.rs:31:9
    |
 LL | #[allow(test_group)]
    |         ^^^^^^^^^^ help: change it to: `clippy::test_group`
@@ -59,42 +59,60 @@ LL | #![cfg_attr(foo, warn(test_lint))]
    |                       ^^^^^^^^^ help: change it to: `clippy::test_lint`
 
 warning: lint name `clippy_group` is deprecated and may not have an effect in the future.
-  --> $DIR/lint-tool-test.rs:13:9
+  --> $DIR/lint-tool-test.rs:14:9
    |
 LL | #![deny(clippy_group)]
    |         ^^^^^^^^^^^^ help: change it to: `clippy::group`
 
 error: item is named 'lintme'
-  --> $DIR/lint-tool-test.rs:18:1
+  --> $DIR/lint-tool-test.rs:20:1
    |
 LL | fn lintme() { }
    | ^^^^^^^^^^^^^^^
    |
 note: the lint level is defined here
-  --> $DIR/lint-tool-test.rs:13:9
+  --> $DIR/lint-tool-test.rs:14:9
    |
 LL | #![deny(clippy_group)]
    |         ^^^^^^^^^^^^
    = note: `#[deny(clippy::test_lint)]` implied by `#[deny(clippy::group)]`
 
 error: item is named 'lintmetoo'
-  --> $DIR/lint-tool-test.rs:26:5
+  --> $DIR/lint-tool-test.rs:28:5
    |
 LL |     fn lintmetoo() { }
    |     ^^^^^^^^^^^^^^^^^^
    |
 note: the lint level is defined here
-  --> $DIR/lint-tool-test.rs:13:9
+  --> $DIR/lint-tool-test.rs:14:9
    |
 LL | #![deny(clippy_group)]
    |         ^^^^^^^^^^^^
    = note: `#[deny(clippy::test_group)]` implied by `#[deny(clippy::group)]`
 
 warning: lint name `test_group` is deprecated and may not have an effect in the future.
-  --> $DIR/lint-tool-test.rs:29:9
+  --> $DIR/lint-tool-test.rs:31:9
    |
 LL | #[allow(test_group)]
    |         ^^^^^^^^^^ help: change it to: `clippy::test_group`
 
-error: aborting due to 2 previous errors; 11 warnings emitted
+warning: lint name `test_lint` is deprecated and may not have an effect in the future.
+  --> $DIR/lint-tool-test.rs:9:23
+   |
+LL | #![cfg_attr(foo, warn(test_lint))]
+   |                       ^^^^^^^^^ help: change it to: `clippy::test_lint`
+
+warning: lint name `clippy_group` is deprecated and may not have an effect in the future.
+  --> $DIR/lint-tool-test.rs:14:9
+   |
+LL | #![deny(clippy_group)]
+   |         ^^^^^^^^^^^^ help: change it to: `clippy::group`
+
+warning: lint name `test_group` is deprecated and may not have an effect in the future.
+  --> $DIR/lint-tool-test.rs:31:9
+   |
+LL | #[allow(test_group)]
+   |         ^^^^^^^^^^ help: change it to: `clippy::test_group`
+
+error: aborting due to 2 previous errors; 14 warnings emitted
 

src/test/ui/deprecation/deprecation-lint.rs

@@ -278,6 +278,7 @@ mod this_crate {
         let _ = nested::DeprecatedStruct {
             //~^ ERROR use of deprecated struct `this_crate::nested::DeprecatedStruct`: text
             i: 0 //~ ERROR use of deprecated field `this_crate::nested::DeprecatedStruct::i`: text
+            //~| ERROR field `i` of struct `this_crate::nested::DeprecatedStruct` is private
         };
 
         let _ = nested::DeprecatedUnitStruct; //~ ERROR use of deprecated unit struct `this_crate::nested::DeprecatedUnitStruct`: text