Auto merge of #153968 - jyn514:jyn/linker-warn-by-default, r=mati865

linker-messages is warn-by-default again

cc #136096 

I ended up keeping it a lint and adding an option for lints to ignore `-Dwarnings` (there was already a lint that did that actually, it was just hard-coded in rustc_middle instead of in rustc_lint_defs like I'd expect). This allows people to actually see the warnings without them failing the build in CI.

Author: bors

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -692,6 +692,15 @@ fn is_windows_gnu_ld(sess: &Session) -> bool {
     sess.target.is_like_windows
         && !sess.target.is_like_msvc
         && matches!(flavor, LinkerFlavor::Gnu(_, Lld::No))
+        && sess.target.options.cfg_abi != CfgAbi::Llvm
+}
+
+fn is_windows_gnu_clang(sess: &Session) -> bool {
+    let (_, flavor) = linker_and_flavor(sess);
+    sess.target.is_like_windows
+        && !sess.target.is_like_msvc
+        && matches!(flavor, LinkerFlavor::Gnu(Cc::Yes, Lld::No))
+        && sess.target.options.cfg_abi == CfgAbi::Llvm
 }
 
 fn report_linker_output(sess: &Session, levels: CodegenLintLevels, stdout: &[u8], stderr: &[u8]) {
@@ -720,9 +729,10 @@ fn report_linker_output(sess: &Session, levels: CodegenLintLevels, stdout: &[u8]
         escaped_stdout = for_each(&stdout, |line, output| {
             // Hide some progress messages from link.exe that we don't care about.
             // See https://github.com/chromium/chromium/blob/bfa41e41145ffc85f041384280caf2949bb7bd72/build/toolchain/win/tool_wrapper.py#L144-L146
-            if line.starts_with("   Creating library")
-                || line.starts_with("Generating code")
-                || line.starts_with("Finished generating code")
+            let trimmed = line.trim_start();
+            if trimmed.starts_with("Creating library")
+                || trimmed.starts_with("Generating code")
+                || trimmed.starts_with("Finished generating code")
             {
                 linker_info += line;
                 linker_info += "\r\n";
@@ -781,7 +791,18 @@ fn report_linker_output(sess: &Session, levels: CodegenLintLevels, stdout: &[u8]
                 *output += "\n"
             }
         });
-    }
+    } else if is_windows_gnu_clang(sess) {
+        info!("inferred Windows Clang (GNU ABI)");
+        escaped_stderr = for_each(&stderr, |line, output| {
+            if line.contains("argument unused during compilation: '-nolibc'") {
+                linker_info += line;
+                linker_info += "\n";
+            } else {
+                *output += line;
+                *output += "\n"
+            }
+        });
+    };
 
     let lint_msg = |msg| {
         emit_lint_base(
@@ -805,10 +826,10 @@ fn report_linker_output(sess: &Session, levels: CodegenLintLevels, stdout: &[u8]
             .strip_prefix("Warning: ")
             .unwrap_or(&escaped_stderr)
             .replace(": warning: ", ": ");
-        lint_msg(format!("linker stderr: {escaped_stderr}"));
+        lint_msg(format!("linker stderr: {}", escaped_stderr.trim_end()));
     }
     if !escaped_stdout.is_empty() {
-        lint_msg(format!("linker stdout: {}", escaped_stdout))
+        lint_msg(format!("linker stdout: {}", escaped_stdout.trim_end()))
     }
     if !linker_info.is_empty() {
         lint_info(linker_info);

compiler/rustc_lint_defs/src/builtin.rs

@@ -193,6 +193,11 @@ declare_lint! {
         reason: fcw!(FutureReleaseError #81670),
         report_in_deps: true,
     };
+    // We exempt `FORBIDDEN_LINT_GROUPS` from `-Dwarnings` because it specifically
+    // triggers in cases (like #80988) where you have `forbid(warnings)`,
+    // and so if we turned that into an error, it'd defeat the purpose of the
+    // future compatibility warning.
+    ignore_deny_warnings
 }
 
 declare_lint! {
@@ -4073,8 +4078,12 @@ declare_lint! {
     /// and actionable warning of similar quality to our other diagnostics. See this tracking
     /// issue for more details: <https://github.com/rust-lang/rust/issues/136096>.
     pub LINKER_MESSAGES,
-    Allow,
-    "warnings emitted at runtime by the target-specific linker program"
+    Warn,
+    "warnings emitted at runtime by the target-specific linker program",
+    // Linker messages don't live up to the high standard people expect of rustc's errors.
+    // Prevent `-D warnings` from applying to it.
+    // It's still possible to pass `-D linker-messages` specifically.
+    ignore_deny_warnings
 }
 
 declare_lint! {

compiler/rustc_lint_defs/src/lib.rs

@@ -343,6 +343,9 @@ pub struct Lint {
     /// `true` if this lint should not be filtered out under any circustamces
     /// (e.g. the unknown_attributes lint)
     pub eval_always: bool,
+
+    /// `true` if this lint is unaffected by `-D warnings`
+    pub ignore_deny_warnings: bool,
 }
 
 /// Extra information for a future incompatibility lint.
@@ -558,6 +561,7 @@ impl Lint {
             feature_gate: None,
             crate_level_only: false,
             eval_always: false,
+            ignore_deny_warnings: false,
         }
     }
 

compiler/rustc_middle/src/lint.rs

@@ -7,8 +7,9 @@ use rustc_hir::{HirId, ItemLocalId};
 use rustc_lint_defs::EditionFcw;
 use rustc_macros::{Decodable, Encodable, HashStable};
 use rustc_session::Session;
-use rustc_session::lint::builtin::{self, FORBIDDEN_LINT_GROUPS};
-use rustc_session::lint::{FutureIncompatibilityReason, Level, Lint, LintExpectationId, LintId};
+use rustc_session::lint::{
+    FutureIncompatibilityReason, Level, Lint, LintExpectationId, LintId, builtin,
+};
 use rustc_span::{DUMMY_SP, ExpnKind, Span, Symbol, kw};
 use tracing::instrument;
 
@@ -89,17 +90,30 @@ pub fn reveal_actual_level(
         level.unwrap_or_else(|| (lint.lint.default_level(sess.edition()), None));
 
     // If we're about to issue a warning, check at the last minute for any
-    // directives against the warnings "lint". If, for example, there's an
+    // directives against the `warnings` lint group. If, for example, there's an
     // `allow(warnings)` in scope then we want to respect that instead.
-    //
-    // We exempt `FORBIDDEN_LINT_GROUPS` from this because it specifically
-    // triggers in cases (like #80988) where you have `forbid(warnings)`,
-    // and so if we turned that into an error, it'd defeat the purpose of the
-    // future compatibility warning.
-    if level == Level::Warn && lint != LintId::of(FORBIDDEN_LINT_GROUPS) {
+    if level == Level::Warn {
         let (warnings_level, warnings_src) = probe_for_lint_level(LintId::of(builtin::WARNINGS));
         if let Some((configured_warning_level, configured_lint_id)) = warnings_level {
-            if configured_warning_level != Level::Warn {
+            let respect_warnings_lint_group = match configured_warning_level {
+                // -Wwarnings is a no-op.
+                Level::Warn => false,
+                // Some warnings cannot be denied from the `warnings` lint group, only individually.
+                Level::Deny | Level::Forbid => !lint.lint.ignore_deny_warnings,
+                // All warnings respect -Awarnings.
+                Level::Allow => true,
+                // Not sure what the right behavior is here, but, sure, why not.
+                // See tests/ui/lint/rfc-2383-lint-reason/expect_warnings.rs.
+                Level::Expect => true,
+                Level::ForceWarn => {
+                    sess.dcx().span_delayed_bug(
+                        warnings_src.span(),
+                        "cannot --force-warn the `warnings` lint group",
+                    );
+                    false
+                }
+            };
+            if respect_warnings_lint_group {
                 level = configured_warning_level;
                 lint_id = configured_lint_id;
                 *src = warnings_src;
@@ -291,6 +305,18 @@ fn explain_lint_level_source(
             }
         }
     }
+
+    if let Some(warnings_group) = sess
+        .opts
+        .lint_opts
+        .iter()
+        .find_map(|(opt, level)| (opt == "warnings").then_some(level))
+        .copied()
+        && warnings_group >= Level::Deny
+        && level < warnings_group
+    {
+        err.note_once(format!("the `{name}` lint ignores `-D warnings`"));
+    }
 }
 
 /// The innermost function for emitting lints implementing the [`trait@Diagnostic`] trait.

src/bootstrap/src/core/builder/cargo.rs

@@ -344,6 +344,10 @@ impl Cargo {
             self.rustflags.arg("-Clink-arg=-gz");
         }
 
+        // Ignore linker warnings for now. These are complicated to fix and don't affect the build.
+        // FIXME: we should really investigate these...
+        self.rustflags.arg("-Alinker-messages");
+
         // Throughout the build Cargo can execute a number of build scripts
         // compiling C/C++ code and we need to pass compilers, archivers, flags, etc
         // obtained previously to those build scripts.

src/tools/compiletest/src/directives/directive_names.rs

@@ -143,6 +143,7 @@ pub(crate) const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
     "ignore-x86_64",
     "ignore-x86_64-apple-darwin",
     "ignore-x86_64-pc-windows-gnu",
+    "ignore-x86_64-pc-windows-gnullvm",
     "ignore-x86_64-unknown-linux-gnu",
     "incremental",
     "known-bug",
@@ -268,6 +269,7 @@ pub(crate) const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
     "only-x86_64-apple-darwin",
     "only-x86_64-fortanix-unknown-sgx",
     "only-x86_64-pc-windows-gnu",
+    "only-x86_64-pc-windows-gnullvm",
     "only-x86_64-pc-windows-msvc",
     "only-x86_64-unknown-linux-gnu",
     "pp-exact",

tests/run-make/linker-warning/deny-linker-lint.txt

@@ -0,0 +1,6 @@
+error: linker stderr: bar
+  |
+  = note: requested on the command line with `-D linker-messages`
+
+error: aborting due to 1 previous error
+

tests/run-make/linker-warning/deny-warnings.txt

@@ -0,0 +1,7 @@
+warning: linker stderr: bar
+  |
+  = note: requested on the command line with `-W linker-messages`
+  = note: the `linker_messages` lint ignores `-D warnings`
+
+warning: 1 warning emitted
+

tests/run-make/linker-warning/rmake.rs

@@ -37,6 +37,19 @@ fn main() {
         .run()
         .assert_stderr_contains("warning: linker stdout: foo");
 
+    // Make sure it respects `-D linker-messages`
+    let out = run_rustc().link_arg("run_make_warn").arg("-Dlinker-messages").run_fail();
+    out.assert_stderr_contains("error: linker stderr: bar");
+    diff()
+        .expected_file("deny-linker-lint.txt")
+        .actual_text("(linker warning)", out.stderr())
+        .run();
+
+    // Make sure that linker warnings don't fail the build when `-D warnings` is present.
+    let out = run_rustc().link_arg("run_make_warn").arg("-Dwarnings").run();
+    out.assert_stderr_contains("warning: linker stderr: bar");
+    diff().expected_file("deny-warnings.txt").actual_text("(linker warning)", out.stderr()).run();
+
     // Make sure we short-circuit this new path if the linker exits with an error
     // (so the diagnostic is less verbose)
     run_rustc().link_arg("run_make_error").run_fail().assert_stderr_contains("note: error: baz");

tests/ui/linking/no-libc-mingw-llvm.rs

@@ -0,0 +1,8 @@
+//@ build-pass
+//@ only-x86_64-pc-windows-gnullvm
+#![allow(linker_messages)]
+#![warn(linker_info)]
+
+//~? WARN argument unused
+
+fn main() {}