Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

macros: LintDiagnostic derive #98884

Merged
merged 6 commits into from
Jul 6, 2022

Conversation

davidtwco
Copy link
Member

@davidtwco davidtwco commented Jul 4, 2022

  • Move LintDiagnosticBuilder into rustc_errors so that a diagnostic derive can refer to it.
  • Introduce a DecorateLint trait, which is equivalent to SessionDiagnostic or AddToDiagnostic but for lints. Necessary without making more changes to the lint infrastructure as DecorateLint takes a LintDiagnosticBuilder and re-uses all of the existing logic for determining what type of diagnostic a lint should be emitted as (e.g. error/warning).
  • Various refactorings of the diagnostic derive machinery (extracting build_field_mapping helper and moving sess field out of the DiagnosticDeriveBuilder).
  • Introduce a LintDiagnostic derive macro that works almost exactly like the SessionDiagnostic derive macro except that it derives a DecorateLint implementation instead. A new derive is necessary for this because SessionDiagnostic is intended for when the generated code creates the diagnostic. AddToDiagnostic could have been used but it would have required more changes to the lint machinery.

At time of opening this pull request, ignore all of the commits from #98624, it's just the last few commits that are new.

r? @oli-obk

@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jul 4, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 4, 2022
@rustbot
Copy link
Collaborator

rustbot commented Jul 4, 2022

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rust-log-analyzer

This comment was marked as resolved.

@davidtwco davidtwco force-pushed the translation-on-lints-derive branch from 17770ff to 1a011dc Compare July 4, 2022 12:03
Signed-off-by: David Wood <david.wood@huawei.com>
Signed-off-by: David Wood <david.wood@huawei.com>
Add a new trait to be generated by diagnostic derives which uses a
`LintDiagnosticBuilder`.

Signed-off-by: David Wood <david.wood@huawei.com>
Move the logic for building a field mapping (which is used by the
building of format strings in `suggestion` annotations) into a helper
function.

Signed-off-by: David Wood <david.wood@huawei.com>
`sess` field of `SessionDiagnosticDeriveBuilder` is never actually used
in the builder's member functions, so it doesn't need to be a field.

Signed-off-by: David Wood <david.wood@huawei.com>
`SessionDiagnostic` isn't suitable for use on lints as whether or not it
creates an error or a warning is decided at compile-time by the macro,
whereas lints decide this at runtime based on the location of the lint
being reported (as it will depend on the user's `allow`/`deny`
attributes, etc). Re-using most of the machinery for
`SessionDiagnostic`, this macro introduces a `LintDiagnostic` derive
which implements a `DecorateLint` trait, taking a
`LintDiagnosticBuilder` and adding to the lint according to the
diagnostic struct.
@davidtwco davidtwco force-pushed the translation-on-lints-derive branch from 1a011dc to 9d864c8 Compare July 5, 2022 15:01
Comment on lines -1556 to 1571
cx.struct_span_lint(INVALID_ATOMIC_ORDERING, fail_order_arg.span, |diag| {
diag.build(fluent::lint::atomic_ordering_invalid)
.set_arg("method", method)
.span_label(fail_order_arg.span, fluent::lint::label)
.help(fluent::lint::help)
.emit();
});
#[derive(LintDiagnostic)]
#[lint(lint::atomic_ordering_invalid)]
#[help]
struct InvalidAtomicOrderingDiag {
method: Symbol,
#[label]
fail_order_arg_span: Span,
}

cx.emit_spanned_lint(
INVALID_ATOMIC_ORDERING,
fail_order_arg.span,
InvalidAtomicOrderingDiag { method, fail_order_arg_span: fail_order_arg.span },
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @rust-lang/clippy this may allow some separation of lint logic and lint emitting in clippy, too

@oli-obk
Copy link
Contributor

oli-obk commented Jul 6, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Jul 6, 2022

📌 Commit 9d864c8 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 6, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 6, 2022
Rollup of 5 pull requests

Successful merges:

 - rust-lang#98881 (Only compute DefKind through the query.)
 - rust-lang#98884 (macros: `LintDiagnostic` derive)
 - rust-lang#98964 (fix typo in function name)
 - rust-lang#98967 (fix typo in note about multiple inaccessible type aliases)
 - rust-lang#98968 (assert Scalar sanity)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit df1f415 into rust-lang:master Jul 6, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 6, 2022
@davidtwco davidtwco deleted the translation-on-lints-derive branch July 6, 2022 18:49
@compiler-errors compiler-errors added the A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic label Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants