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: subdiagnostic derive #96468

Merged
merged 9 commits into from
Apr 29, 2022

Conversation

davidtwco
Copy link
Member

@davidtwco davidtwco commented Apr 27, 2022

Add a new macro, #[derive(SessionSubdiagnostic)], which can be applied to structs that represent subdiagnostics, such as labels, notes, helps or suggestions.

#[derive(SessionSubdiagnostic)] can be used with the existing #[derive(SessionDiagnostic)]. All diagnostics implemented using either derive are translatable, and this new derive should make it easier to port existing diagnostics to using these derives.

For example, consider the following subdiagnostic types...

#[derive(SessionSubdiagnostic)]
pub enum ExpectedIdentifierLabel<'tcx> {
    #[label(slug = "parser-expected-identifier")]
    WithoutFound {
        #[primary_span]
        span: Span,
    }
    #[label(slug = "parser-expected-identifier-found")]
    WithFound {
        #[primary_span]
        span: Span,
        found: String,
    }
}

#[derive(SessionSubdiagnostic)]
#[suggestion_verbose(slug = "parser-raw-identifier")]
pub struct RawIdentifierSuggestion<'tcx> {
    #[primary_span]
    span: Span,
    #[applicability]
    applicability: Applicability,
    ident: Ident,
}

...and the corresponding Fluent messages:

parser-expected-identifier = expected identifier

parser-expected-identifier-found = expected identifier, found {$found}

parser-raw-identifier = escape `{$ident}` to use it as an identifier

These can be emitted using the new subdiagnostic function on Diagnostic...

diag.subdiagnostic(ExpectedIdentifierLabel::WithoutFound { span });
diag.subdiagnostic(RawIdentifierSuggestion { span, applicability, ident });

...or as part of a larger #[derive(SessionDiagnostic)]:

#[derive(SessionDiagnostic)]
#[error(slug = "parser-expected-identifier")]
pub struct ExpectedIdentifier {
    #[primary_span]
    span: Span,
    token_descr: String,
    #[subdiagnostic]
    label: ExpectedIdentifierLabel,
    #[subdiagnostic]
    raw_identifier_suggestion: Option<RawIdentifierSuggestion>,
}
sess.emit_err(ExpectedIdentifier { ... });

r? @oli-obk
cc @pvdrz

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 27, 2022
@rust-highfive
Copy link
Collaborator

Some changes occurred in src/tools/clippy.

cc @rust-lang/clippy

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 27, 2022
@rust-log-analyzer

This comment was marked as resolved.

@davidtwco davidtwco force-pushed the diagnostic-translation-subdiagnostic branch from 26e0289 to 4874fa3 Compare April 27, 2022 05:42
@estebank
Copy link
Contributor

One thing I noticed is that you might not be supporting MultiSpans. I think they might be tricky to handle nicely at the macro level (you only have access to tokens, not types), and making it only MultiSpan would be limiting. 🤔

@bors

This comment was marked as resolved.

Change `span_suggestion` (and variants) to take `impl ToString` rather
than `String` for the suggested code, as this simplifies the
requirements on the diagnostic derive.

Signed-off-by: David Wood <david.wood@huawei.com>
Move existing test for session diagnostic derive to a subdirectory.

Signed-off-by: David Wood <david.wood@huawei.com>
Add a new derive, `#[derive(SessionSubdiagnostic)]`, which enables
deriving structs for labels, notes, helps and suggestions.

Signed-off-by: David Wood <david.wood@huawei.com>
Split `SessionDiagnostic` and `SessionSubdiagnostic` derives and the
various helper functions into multiple modules.

Signed-off-by: David Wood <david.wood@huawei.com>
Remove some duplicated code between both diagnostic derives by
introducing helper functions for reporting an error in case of a invalid
attribute.

Signed-off-by: David Wood <david.wood@huawei.com>
`SetOnce` trait was introduced in the subdiagnostic derive to simplify
the code a little bit, re-use it in the diagnostic derive too.

Signed-off-by: David Wood <david.wood@huawei.com>
Documentation comments are always good.

Signed-off-by: David Wood <david.wood@huawei.com>
In the initial implementation of the `SessionSubdiagnostic`, the
`Applicability` of a suggestion can be set both as a field and as part
of the attribute, this commit adds the same support to the original
`SessionDiagnostic` derive.

Signed-off-by: David Wood <david.wood@huawei.com>
Add `#[subdiagnostic]` field attribute to the diagnostic derive which
is applied to fields that have types which use the subdiagnostic derive.

Signed-off-by: David Wood <david.wood@huawei.com>
@davidtwco davidtwco force-pushed the diagnostic-translation-subdiagnostic branch from 4874fa3 to dca8861 Compare April 29, 2022 01:43
@oli-obk
Copy link
Contributor

oli-obk commented Apr 29, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Apr 29, 2022

📌 Commit dca8861 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 Apr 29, 2022
@bors
Copy link
Contributor

bors commented Apr 29, 2022

⌛ Testing commit dca8861 with merge 683c582...

@bors
Copy link
Contributor

bors commented Apr 29, 2022

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 683c582 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 29, 2022
@bors bors merged commit 683c582 into rust-lang:master Apr 29, 2022
@rustbot rustbot added this to the 1.62.0 milestone Apr 29, 2022
@bors bors mentioned this pull request Apr 29, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (683c582): comparison url.

Summary:

  • Primary benchmarks: 😿 relevant regression found
  • Secondary benchmarks: 🎉 relevant improvements found
Regressions 😿
(primary)
Regressions 😿
(secondary)
Improvements 🎉
(primary)
Improvements 🎉
(secondary)
All 😿 🎉
(primary)
count1 1 0 0 2 1
mean2 0.6% N/A N/A -0.4% 0.6%
max 0.6% N/A N/A -0.5% 0.6%

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. number of relevant changes

  2. the arithmetic mean of the percent change

@davidtwco davidtwco deleted the diagnostic-translation-subdiagnostic branch April 30, 2022 03:43
@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 merged-by-bors This PR was explicitly merged by bors. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants