Auto merge of rust-lang#119286 - jyn514:linker-output, r=bjorn3

show linker output even if the linker succeeds

- show stderr by default
- show stdout if `--verbose` is passed
- remove both from RUSTC_LOG
- hide the linker cli args unless `--verbose` is passed

fixes rust-lang#83436. fixes rust-lang#38206. fixes rust-lang#109979. helps with rust-lang#46998. cc https://rust-lang.zulipchat.com/#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/uplift.20some.20-Zverbose.20calls.20and.20rename.20to.E2.80.A6.20compiler-team.23706/near/408986134

this is based on rust-lang#119129 for convenience so i didn't have to duplicate the changes around saving `--verbose` in rust-lang@cb6d033#diff-7a49efa20548d6806dbe1c66dd4dc445fda18fcbbf1709520cadecc4841aae12

r? `@bjorn3`

compiler/rustc_codegen_ssa/messages.ftl

@@ -160,6 +160,8 @@ codegen_ssa_linker_file_stem = couldn't extract file stem from specified linker
 codegen_ssa_linker_not_found = linker `{$linker_path}` not found
     .note = {$error}
 
+codegen_ssa_linker_output = {$inner}
+
 codegen_ssa_linker_unsupported_modifier = `as-needed` modifier not supported for current linker
 
 codegen_ssa_linking_failed = linking with `{$linker_path}` failed: {$exit_status}

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -731,6 +731,14 @@ fn link_dwarf_object<'a>(
     }
 }
 
+#[derive(Diagnostic)]
+#[diag(codegen_ssa_linker_output)]
+/// Translating this is kind of useless. We don't pass translation flags to the linker, so we'd just
+/// end up with inconsistent languages within the same diagnostic.
+struct LinkerOutput {
+    inner: String,
+}
+
 /// Create a dynamic library or executable.
 ///
 /// This will invoke the system linker/cc to create the resulting file. This links to all upstream
@@ -936,12 +944,12 @@ fn link_natively<'a>(
                 let mut output = prog.stderr.clone();
                 output.extend_from_slice(&prog.stdout);
                 let escaped_output = escape_linker_output(&output, flavor);
-                // FIXME: Add UI tests for this error.
                 let err = errors::LinkingFailed {
                     linker_path: &linker_path,
                     exit_status: prog.status,
                     command: &cmd,
                     escaped_output,
+                    verbose: sess.opts.verbose,
                 };
                 sess.dcx().emit_err(err);
                 // If MSVC's `link.exe` was expected but the return code
@@ -982,8 +990,22 @@ fn link_natively<'a>(
 
                 sess.dcx().abort_if_errors();
             }
-            info!("linker stderr:\n{}", escape_string(&prog.stderr));
-            info!("linker stdout:\n{}", escape_string(&prog.stdout));
+
+            if !prog.stderr.is_empty() {
+                // We already print `warning:` at the start of the diagnostic. Remove it from the linker output if present.
+                let stderr = escape_string(&prog.stderr);
+                debug!("original stderr: {stderr}");
+                let stderr = stderr
+                    .strip_prefix("warning: ")
+                    .unwrap_or(&stderr)
+                    .replace(": warning: ", ": ");
+                sess.dcx().emit_warn(LinkerOutput { inner: format!("linker stderr: {stderr}") });
+            }
+            if !prog.stdout.is_empty() && sess.opts.verbose {
+                sess.dcx().emit_warn(LinkerOutput {
+                    inner: format!("linker stdout: {}", escape_string(&prog.stdout)),
+                });
+            }
         }
         Err(e) => {
             let linker_not_found = e.kind() == io::ErrorKind::NotFound;

compiler/rustc_codegen_ssa/src/errors.rs

@@ -340,6 +340,7 @@ pub struct LinkingFailed<'a> {
     pub exit_status: ExitStatus,
     pub command: &'a Command,
     pub escaped_output: String,
+    pub verbose: bool,
 }
 
 impl<G: EmissionGuarantee> IntoDiagnostic<'_, G> for LinkingFailed<'_> {
@@ -350,7 +351,13 @@ impl<G: EmissionGuarantee> IntoDiagnostic<'_, G> for LinkingFailed<'_> {
 
         let contains_undefined_ref = self.escaped_output.contains("undefined reference to");
 
-        diag.note(format!("{:?}", self.command)).note(self.escaped_output);
+        if self.verbose {
+            diag.note(format!("{:?}", self.command));
+        } else {
+            diag.note("use `--verbose` to show all linker arguments");
+        }
+
+        diag.note(self.escaped_output);
 
         // Trying to match an error from OS linkers
         // which by now we have no way to translate.

compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs

@@ -249,7 +249,10 @@ impl DiagnosticDeriveVariantBuilder {
         let mut field_binding = binding_info.binding.clone();
         field_binding.set_span(field.ty.span());
 
-        let ident = field.ident.as_ref().unwrap();
+        let Some(ident) = field.ident.as_ref() else {
+            span_err(field.span().unwrap(), "tuple structs are not supported").emit();
+            return TokenStream::new();
+        };
         let ident = format_ident!("{}", ident); // strip `r#` prefix, if present
 
         quote! {

compiler/rustc_macros/src/diagnostics/error.rs

@@ -55,7 +55,7 @@ fn path_to_string(path: &syn::Path) -> String {
 /// Returns an error diagnostic on span `span` with msg `msg`.
 #[must_use]
 pub(crate) fn span_err<T: Into<String>>(span: impl MultiSpan, msg: T) -> Diagnostic {
-    Diagnostic::spanned(span, Level::Error, msg)
+    Diagnostic::spanned(span, Level::Error, format!("derive(Diagnostic): {}", msg.into()))
 }
 
 /// Emit a diagnostic on span `$span` with msg `$msg` (optionally performing additional decoration

compiler/rustc_macros/src/diagnostics/utils.rs

@@ -242,7 +242,7 @@ impl<T> SetOnce<T> for SpannedOption<T> {
                 *self = Some((value, span));
             }
             Some((_, prev_span)) => {
-                span_err(span, "specified multiple times")
+                span_err(span, "attribute specified multiple times")
                     .span_note(*prev_span, "previously specified here")
                     .emit();
             }

src/etc/cat-and-grep.sh

@@ -33,7 +33,6 @@ while getopts ':vieh' OPTION; do
     case "$OPTION" in
         v)
             INVERT=1
-            ERROR_MSG='should not be found'
             ;;
         i)
             GREPFLAGS="i$GREPFLAGS"

src/tools/compiletest/src/runtest.rs

@@ -2135,6 +2135,10 @@ impl<'test> TestCx<'test> {
         self.config.target.contains("vxworks") && !self.is_vxworks_pure_static()
     }
 
+    fn has_aux_dir(&self) -> bool {
+        !self.props.aux_builds.is_empty() || !self.props.aux_crates.is_empty()
+    }
+
     fn build_all_auxiliary(&self, rustc: &mut Command) -> PathBuf {
         let aux_dir = self.aux_output_dir_name();
 
@@ -2612,7 +2616,11 @@ impl<'test> TestCx<'test> {
         }
 
         if let LinkToAux::Yes = link_to_aux {
-            rustc.arg("-L").arg(self.aux_output_dir_name());
+            // if we pass an `-L` argument to a directory that doesn't exist,
+            // macOS ld emits warnings which disrupt the .stderr files
+            if self.has_aux_dir() {
+                rustc.arg("-L").arg(self.aux_output_dir_name());
+            }
         }
 
         rustc.args(&self.props.compile_flags);

tests/run-make/link-args-order/Makefile

@@ -6,5 +6,5 @@ RUSTC_FLAGS = -C linker-flavor=ld -C link-arg=a -C link-args="b c" -C link-args=
 RUSTC_FLAGS_PRE = -C linker-flavor=ld -Z pre-link-arg=a -Z pre-link-args="b c" -Z pre-link-args="d e" -Z pre-link-arg=f
 
 all:
-	$(RUSTC) $(RUSTC_FLAGS) empty.rs 2>&1 | $(CGREP) '"a" "b" "c" "d" "e" "f"'
-	$(RUSTC) $(RUSTC_FLAGS_PRE) empty.rs 2>&1 | $(CGREP) '"a" "b" "c" "d" "e" "f"'
+	$(RUSTC) $(RUSTC_FLAGS) empty.rs --print=link-args | $(CGREP) '"a" "b" "c" "d" "e" "f"'
+	$(RUSTC) $(RUSTC_FLAGS_PRE) empty.rs --print=link-args | $(CGREP) '"a" "b" "c" "d" "e" "f"'

tests/run-make/link-dedup/Makefile

@@ -6,7 +6,7 @@ all:
 	$(RUSTC) depa.rs
 	$(RUSTC) depb.rs
 	$(RUSTC) depc.rs
-	$(RUSTC) empty.rs --cfg bar 2>&1 | $(CGREP) '"-ltesta" "-ltestb" "-ltesta"'
-	$(RUSTC) empty.rs 2>&1 | $(CGREP) '"-ltesta"'
-	$(RUSTC) empty.rs 2>&1 | $(CGREP) -v '"-ltestb"'
-	$(RUSTC) empty.rs 2>&1 | $(CGREP) -v '"-ltesta" "-ltesta" "-ltesta"'
+	$(RUSTC) empty.rs --cfg bar --print=link-args | $(CGREP) '"-ltesta" "-ltestb" "-ltesta"'
+	$(RUSTC) empty.rs --print=link-args | $(CGREP) '"-ltesta"'
+	$(RUSTC) empty.rs --print=link-args | $(CGREP) -v '"-ltestb"'
+	$(RUSTC) empty.rs --print=link-args | $(CGREP) -v '"-ltesta" "-ltesta" "-ltesta"'

tests/run-make/linker-warning/Makefile

@@ -0,0 +1,25 @@
+include ../tools.mk
+
+RUN_RUSTC := $(RUSTC_ORIGINAL) main.rs -o $(TMPDIR)/main -C linker=./fake-linker.sh
+
+all: succeeds_with_warnings errors linker_args
+
+succeeds_with_warnings:
+	# Run rustc with our fake linker, and make sure it shows warnings
+	$(RUN_RUSTC) -C link-arg=run_make_warn 2>&1 | $(CGREP) "warning: linker stderr: bar"
+
+	# Make sure it shows stdout, but only when --verbose is passed
+	$(RUN_RUSTC) -C link-arg=run_make_info --verbose 2>&1 | $(CGREP) "warning: linker stdout: foo"
+	$(RUN_RUSTC) -C link-arg=run_make_info 2>&1 | $(CGREP) -v "warning: linker stdout: foo"
+
+errors:
+	# Make sure we short-circuit this new path if the linker exits with an error (so the diagnostic is less verbose)
+	rm -f $(TMPDIR)/main
+	$(RUN_RUSTC) -C link-arg=run_make_error 2>&1 | $(CGREP) "note: error: baz"
+	! [ -e $(TMPDIR)/main ]
+
+linker_args:
+	# Make sure we don't show the linker args unless `--verbose` is passed
+	$(RUN_RUSTC) --verbose -C link-arg=run_make_error 2>&1 | $(CGREP) -e "PATH=.*fake-linker.sh.*run_make_error"
+	$(RUN_RUSTC) -C link-arg=run_make_error 2>&1 | $(CGREP) -v -e "PATH=.*fake-linker.sh.*run_make_error"
+

tests/run-make/linker-warning/fake-linker.sh

@@ -0,0 +1,17 @@
+#!/bin/sh
+
+code=0
+while ! [ $# = 0 ]; do
+    case "$1" in
+        run_make_info) echo "foo"
+            ;;
+        run_make_warn) echo "warning: bar" >&2
+            ;;
+        run_make_error) echo "error: baz" >&2; code=1
+            ;;
+        *) ;;   # rustc passes lots of args we don't care about
+    esac
+    shift
+done
+
+exit $code

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

@@ -0,0 +1 @@
+fn main() {}