Auto merge of #101986 - WaffleLapkin:move_lint_note_to_the_bottom, r=estebank

Move lint level source explanation to the bottom

So, uhhhhh

r? `@estebank`

## User-facing change

"note: `#[warn(...)]` on by default" and such are moved to the bottom of the diagnostic:
```diff
-   = note: `#[warn(unsupported_calling_conventions)]` on by default
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, see issue #87678 <https://github.com/rust-lang/rust/issues/87678>
+   = note: `#[warn(unsupported_calling_conventions)]` on by default
```

Why warning is enabled is the least important thing, so it shouldn't be the first note the user reads, IMO.

## Developer-facing change

`struct_span_lint` and similar methods have a different signature.

Before: `..., impl for<'a> FnOnce(LintDiagnosticBuilder<'a, ()>)`
After: `..., impl Into<DiagnosticMessage>, impl for<'a, 'b> FnOnce(&'b mut DiagnosticBuilder<'a, ()>) -> &'b mut DiagnosticBuilder<'a, ()>`

The reason for this is that `struct_span_lint` needs to edit the diagnostic _after_ `decorate` closure is called. This also makes lint code a little bit nicer in my opinion.

Another option is to use `impl for<'a> FnOnce(LintDiagnosticBuilder<'a, ()>) -> DiagnosticBuilder<'a, ()>` altough I don't _really_ see reasons to do `let lint = lint.build(message)` everywhere.

## Subtle problem

By moving the message outside of the closure (that may not be called if the lint is disabled) `format!(...)` is executed earlier, possibly formatting `Ty` which may call a query that trims paths that crashes the compiler if there were no warnings...

I don't think it's that big of a deal, considering that we move from `format!(...)` to `fluent` (which is lazy by-default) anyway, however this required adding a workaround which is unfortunate.

## P.S.

I'm sorry, I do not how to make this PR smaller/easier to review. Changes to the lint API affect SO MUCH 😢

6 files changed, 57 insertions, 59 deletions

diff --git a/compiler/rustc_mir_transform/src/check_const_item_mutation.rs b/compiler/rustc_mir_transform/src/check_const_item_mutation.rs
index 8838b14c53a..fa5f392fa74 100644
--- a/compiler/rustc_mir_transform/src/check_const_item_mutation.rs
+++ b/compiler/rustc_mir_transform/src/check_const_item_mutation.rs
@@ -1,4 +1,4 @@
-use rustc_errors::{DiagnosticBuilder, LintDiagnosticBuilder};
+use rustc_errors::{DiagnosticBuilder, DiagnosticMessage};
 use rustc_middle::mir::visit::Visitor;
 use rustc_middle::mir::*;
 use rustc_middle::ty::TyCtxt;
@@ -63,7 +63,10 @@ impl<'tcx> ConstMutationChecker<'_, 'tcx> {
         place: &Place<'tcx>,
         const_item: DefId,
         location: Location,
-        decorate: impl for<'b> FnOnce(LintDiagnosticBuilder<'b, ()>) -> DiagnosticBuilder<'b, ()>,
+        msg: impl Into<DiagnosticMessage>,
+        decorate: impl for<'a, 'b> FnOnce(
+            &'a mut DiagnosticBuilder<'b, ()>,
+        ) -> &'a mut DiagnosticBuilder<'b, ()>,
     ) {
         // Don't lint on borrowing/assigning when a dereference is involved.
         // If we 'leave' the temporary via a dereference, we must
@@ -84,10 +87,10 @@ impl<'tcx> ConstMutationChecker<'_, 'tcx> {
                 CONST_ITEM_MUTATION,
                 lint_root,
                 source_info.span,
+                msg,
                 |lint| {
                     decorate(lint)
                         .span_note(self.tcx.def_span(const_item), "`const` item defined here")
-                        .emit();
                 },
             );
         }
@@ -102,10 +105,8 @@ impl<'tcx> Visitor<'tcx> for ConstMutationChecker<'_, 'tcx> {
             // so emitting a lint would be redundant.
             if !lhs.projection.is_empty() {
                 if let Some(def_id) = self.is_const_item_without_destructor(lhs.local) {
-                    self.lint_const_item_usage(&lhs, def_id, loc, |lint| {
-                        let mut lint = lint.build("attempting to modify a `const` item");
-                        lint.note("each usage of a `const` item creates a new temporary; the original `const` item will not be modified");
-                        lint
+                    self.lint_const_item_usage(&lhs, def_id, loc, "attempting to modify a `const` item",|lint| {
+                        lint.note("each usage of a `const` item creates a new temporary; the original `const` item will not be modified")
                     })
                 }
             }
@@ -137,8 +138,7 @@ impl<'tcx> Visitor<'tcx> for ConstMutationChecker<'_, 'tcx> {
                 });
                 let lint_loc =
                     if method_did.is_some() { self.body.terminator_loc(loc.block) } else { loc };
-                self.lint_const_item_usage(place, def_id, lint_loc, |lint| {
-                    let mut lint = lint.build("taking a mutable reference to a `const` item");
+                self.lint_const_item_usage(place, def_id, lint_loc, "taking a mutable reference to a `const` item", |lint| {
                     lint
                         .note("each usage of a `const` item creates a new temporary")
                         .note("the mutable reference will refer to this temporary, not the original `const` item");
diff --git a/compiler/rustc_mir_transform/src/check_packed_ref.rs b/compiler/rustc_mir_transform/src/check_packed_ref.rs
index 3b7ba3f9a67..51abcf51189 100644
--- a/compiler/rustc_mir_transform/src/check_packed_ref.rs
+++ b/compiler/rustc_mir_transform/src/check_packed_ref.rs
@@ -33,21 +33,27 @@ struct PackedRefChecker<'a, 'tcx> {
 fn unsafe_derive_on_repr_packed(tcx: TyCtxt<'_>, def_id: LocalDefId) {
     let lint_hir_id = tcx.hir().local_def_id_to_hir_id(def_id);
 
-    tcx.struct_span_lint_hir(UNALIGNED_REFERENCES, lint_hir_id, tcx.def_span(def_id), |lint| {
-        // FIXME: when we make this a hard error, this should have its
-        // own error code.
-        let extra = if tcx.generics_of(def_id).own_requires_monomorphization() {
-            "with type or const parameters"
-        } else {
-            "that does not derive `Copy`"
-        };
-        let message = format!(
-            "`{}` can't be derived on this `#[repr(packed)]` struct {}",
-            tcx.item_name(tcx.trait_id_of_impl(def_id.to_def_id()).expect("derived trait name")),
-            extra
-        );
-        lint.build(message).emit();
-    });
+    // FIXME: when we make this a hard error, this should have its
+    // own error code.
+
+    let extra = if tcx.generics_of(def_id).own_requires_monomorphization() {
+        "with type or const parameters"
+    } else {
+        "that does not derive `Copy`"
+    };
+    let message = format!(
+        "`{}` can't be derived on this `#[repr(packed)]` struct {}",
+        tcx.item_name(tcx.trait_id_of_impl(def_id.to_def_id()).expect("derived trait name")),
+        extra
+    );
+
+    tcx.struct_span_lint_hir(
+        UNALIGNED_REFERENCES,
+        lint_hir_id,
+        tcx.def_span(def_id),
+        message,
+        |lint| lint,
+    );
 }
 
 impl<'tcx> Visitor<'tcx> for PackedRefChecker<'_, 'tcx> {
@@ -86,8 +92,9 @@ impl<'tcx> Visitor<'tcx> for PackedRefChecker<'_, 'tcx> {
                         UNALIGNED_REFERENCES,
                         lint_root,
                         source_info.span,
+                        "reference to packed field is unaligned",
                         |lint| {
-                            lint.build("reference to packed field is unaligned")
+                            lint
                                 .note(
                                     "fields of packed structs are not properly aligned, and creating \
                                     a misaligned reference is undefined behavior (even if that \
@@ -98,7 +105,6 @@ impl<'tcx> Visitor<'tcx> for PackedRefChecker<'_, 'tcx> {
                                     reference with a raw pointer and use `read_unaligned`/`write_unaligned` \
                                     (loads and stores via `*p` must be properly aligned even when using raw pointers)"
                                 )
-                                .emit();
                         },
                     );
                 }
diff --git a/compiler/rustc_mir_transform/src/check_unsafety.rs b/compiler/rustc_mir_transform/src/check_unsafety.rs
index beff19a3ab2..4730be1244b 100644
--- a/compiler/rustc_mir_transform/src/check_unsafety.rs
+++ b/compiler/rustc_mir_transform/src/check_unsafety.rs
@@ -489,21 +489,20 @@ fn unsafety_check_result<'tcx>(
 
 fn report_unused_unsafe(tcx: TyCtxt<'_>, kind: UnusedUnsafe, id: HirId) {
     let span = tcx.sess.source_map().guess_head_span(tcx.hir().span(id));
-    tcx.struct_span_lint_hir(UNUSED_UNSAFE, id, span, |lint| {
-        let msg = "unnecessary `unsafe` block";
-        let mut db = lint.build(msg);
-        db.span_label(span, msg);
+    let msg = "unnecessary `unsafe` block";
+    tcx.struct_span_lint_hir(UNUSED_UNSAFE, id, span, msg, |lint| {
+        lint.span_label(span, msg);
         match kind {
             UnusedUnsafe::Unused => {}
             UnusedUnsafe::InUnsafeBlock(id) => {
-                db.span_label(
+                lint.span_label(
                     tcx.sess.source_map().guess_head_span(tcx.hir().span(id)),
                     "because it's nested under this `unsafe` block",
                 );
             }
         }
 
-        db.emit();
+        lint
     });
 }
 
@@ -543,15 +542,8 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: LocalDefId) {
                 UNSAFE_OP_IN_UNSAFE_FN,
                 lint_root,
                 source_info.span,
-                |lint| {
-                    lint.build(&format!(
-                        "{} is unsafe and requires unsafe block (error E0133)",
-                        description,
-                    ))
-                    .span_label(source_info.span, description)
-                    .note(note)
-                    .emit();
-                },
+                format!("{} is unsafe and requires unsafe block (error E0133)", description,),
+                |lint| lint.span_label(source_info.span, description).note(note),
             ),
         }
     }
diff --git a/compiler/rustc_mir_transform/src/const_prop_lint.rs b/compiler/rustc_mir_transform/src/const_prop_lint.rs
index 973f55437ee..cda3702c83d 100644
--- a/compiler/rustc_mir_transform/src/const_prop_lint.rs
+++ b/compiler/rustc_mir_transform/src/const_prop_lint.rs
@@ -347,10 +347,8 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
         panic: AssertKind<impl std::fmt::Debug>,
     ) {
         if let Some(lint_root) = self.lint_root(source_info) {
-            self.tcx.struct_span_lint_hir(lint, lint_root, source_info.span, |lint| {
-                let mut err = lint.build(message);
-                err.span_label(source_info.span, format!("{:?}", panic));
-                err.emit();
+            self.tcx.struct_span_lint_hir(lint, lint_root, source_info.span, message, |lint| {
+                lint.span_label(source_info.span, format!("{:?}", panic))
             });
         }
     }
diff --git a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs
index 7522a50a8c6..1244c18020d 100644
--- a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs
+++ b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs
@@ -106,14 +106,12 @@ fn has_ffi_unwind_calls(tcx: TyCtxt<'_>, local_def_id: LocalDefId) -> bool {
                 .lint_root;
             let span = terminator.source_info.span;
 
-            tcx.struct_span_lint_hir(FFI_UNWIND_CALLS, lint_root, span, |lint| {
-                let msg = match fn_def_id {
-                    Some(_) => "call to foreign function with FFI-unwind ABI",
-                    None => "call to function pointer with FFI-unwind ABI",
-                };
-                let mut db = lint.build(msg);
-                db.span_label(span, msg);
-                db.emit();
+            let msg = match fn_def_id {
+                Some(_) => "call to foreign function with FFI-unwind ABI",
+                None => "call to function pointer with FFI-unwind ABI",
+            };
+            tcx.struct_span_lint_hir(FFI_UNWIND_CALLS, lint_root, span, msg, |lint| {
+                lint.span_label(span, msg)
             });
 
             tainted = true;
diff --git a/compiler/rustc_mir_transform/src/function_item_references.rs b/compiler/rustc_mir_transform/src/function_item_references.rs
index 0568eb2ffa6..469566694a3 100644
--- a/compiler/rustc_mir_transform/src/function_item_references.rs
+++ b/compiler/rustc_mir_transform/src/function_item_references.rs
@@ -179,11 +179,15 @@ impl<'tcx> FunctionItemRefChecker<'_, 'tcx> {
         let num_args = fn_sig.inputs().map_bound(|inputs| inputs.len()).skip_binder();
         let variadic = if fn_sig.c_variadic() { ", ..." } else { "" };
         let ret = if fn_sig.output().skip_binder().is_unit() { "" } else { " -> _" };
-        self.tcx.struct_span_lint_hir(FUNCTION_ITEM_REFERENCES, lint_root, span, |lint| {
-            lint.build("taking a reference to a function item does not give a function pointer")
-                .span_suggestion(
+        self.tcx.struct_span_lint_hir(
+            FUNCTION_ITEM_REFERENCES,
+            lint_root,
+            span,
+            "taking a reference to a function item does not give a function pointer",
+            |lint| {
+                lint.span_suggestion(
                     span,
-                    &format!("cast `{}` to obtain a function pointer", ident),
+                    format!("cast `{}` to obtain a function pointer", ident),
                     format!(
                         "{} as {}{}fn({}{}){}",
                         if params.is_empty() { ident } else { format!("{}::<{}>", ident, params) },
@@ -195,7 +199,7 @@ impl<'tcx> FunctionItemRefChecker<'_, 'tcx> {
                     ),
                     Applicability::Unspecified,
                 )
-                .emit();
-        });
+            },
+        );
     }
 }