about summary refs log tree commit diff
path: root/compiler/rustc_passes/src/check_attr.rs
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-10-01 10:44:25 +0000
committerbors <bors@rust-lang.org>2022-10-01 10:44:25 +0000
commit744e397d8855f7da87d70aa8d0bd9e0f5f0b51a1 (patch)
tree1721987352b5f0a8548fc46984821d974b661934 /compiler/rustc_passes/src/check_attr.rs
parent277bb6653b55475b5fbce6309e9852fa2100dabe (diff)
parentb5b3ffe3fc9cfb524a6432ec60a0fc95c514d2e1 (diff)
downloadrust-744e397d8855f7da87d70aa8d0bd9e0f5f0b51a1.tar.gz
rust-744e397d8855f7da87d70aa8d0bd9e0f5f0b51a1.zip
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 😢
Diffstat (limited to 'compiler/rustc_passes/src/check_attr.rs')
-rw-r--r--compiler/rustc_passes/src/check_attr.rs53
1 files changed, 31 insertions, 22 deletions
diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs
index 897a0db930c..87433538512 100644
--- a/compiler/rustc_passes/src/check_attr.rs
+++ b/compiler/rustc_passes/src/check_attr.rs
@@ -370,10 +370,13 @@ impl CheckAttrVisitor<'_> {
                 b.push_str(&(allowed_target.to_string() + "s"));
                 b
             });
-            self.tcx.struct_span_lint_hir(UNUSED_ATTRIBUTES, hir_id, attr.span, |lint| {
-                lint.build(&format!("`#[{name}]` only has an effect on {}", supported_names))
-                    .emit();
-            });
+            self.tcx.struct_span_lint_hir(
+                UNUSED_ATTRIBUTES,
+                hir_id,
+                attr.span,
+                &format!("`#[{name}]` only has an effect on {}", supported_names),
+                |lint| lint,
+            );
         }
     }
 
@@ -877,25 +880,31 @@ impl CheckAttrVisitor<'_> {
         hir_id: HirId,
     ) -> bool {
         if hir_id != CRATE_HIR_ID {
-            self.tcx.struct_span_lint_hir(INVALID_DOC_ATTRIBUTES, hir_id, meta.span(), |lint| {
-                let mut err = lint.build(fluent::passes::attr_crate_level);
-                if attr.style == AttrStyle::Outer
-                    && self.tcx.hir().get_parent_item(hir_id) == CRATE_OWNER_ID
-                {
-                    if let Ok(mut src) = self.tcx.sess.source_map().span_to_snippet(attr.span) {
-                        src.insert(1, '!');
-                        err.span_suggestion_verbose(
-                            attr.span,
-                            fluent::passes::suggestion,
-                            src,
-                            Applicability::MaybeIncorrect,
-                        );
-                    } else {
-                        err.span_help(attr.span, fluent::passes::help);
+            self.tcx.struct_span_lint_hir(
+                INVALID_DOC_ATTRIBUTES,
+                hir_id,
+                meta.span(),
+                fluent::passes::attr_crate_level,
+                |err| {
+                    if attr.style == AttrStyle::Outer
+                        && self.tcx.hir().get_parent_item(hir_id) == CRATE_OWNER_ID
+                    {
+                        if let Ok(mut src) = self.tcx.sess.source_map().span_to_snippet(attr.span) {
+                            src.insert(1, '!');
+                            err.span_suggestion_verbose(
+                                attr.span,
+                                fluent::passes::suggestion,
+                                src,
+                                Applicability::MaybeIncorrect,
+                            );
+                        } else {
+                            err.span_help(attr.span, fluent::passes::help);
+                        }
                     }
-                }
-                err.note(fluent::passes::note).emit();
-            });
+                    err.note(fluent::passes::note);
+                    err
+                },
+            );
             return false;
         }
         true