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

Kind-less SessionDiagnostic derive #100765

Merged
merged 4 commits into from
Aug 21, 2022

Conversation

Xiretza
Copy link
Contributor

@Xiretza Xiretza commented Aug 19, 2022

From #100730 (comment):

Hm, maybe we should expose some sess.struct_$SOMETHING (like struct_diagnostic?) that is generic over EmissionGuarantee, then make the SessionDiagnostic derive generic, i.e.

impl<'tcx> SessionDiagnostic for UnusedGenericParams {
  fn into_diagnostic<T: EmissionGuarantee>( .. ) -> DiagnosticBuilder<'tcx, T> {
    let mut diag = sess.struct_diagnostic(rustc_errors:..);
    ..
  }
}

Discussed on Zulip.

@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(rust-highfive has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 19, 2022
@rustbot
Copy link
Collaborator

rustbot commented Aug 19, 2022

rustc_macros::diagnostics was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 19, 2022
@Xiretza Xiretza force-pushed the session-diagnostic-unification branch from b868bb8 to 9eec2c4 Compare August 19, 2022 17:15
@Xiretza Xiretza mentioned this pull request Aug 19, 2022
84 tasks
@Xiretza Xiretza force-pushed the session-diagnostic-unification branch 2 times, most recently from 4e04b57 to 450989f Compare August 19, 2022 18:06
@compiler-errors
Copy link
Member

r? @compiler-errors

@compiler-errors
Copy link
Member

This is exactly what I wanted to see! 😄 I'll wait until CI is ready and then approve.

@compiler-errors
Copy link
Member

Also, we probably need to adjust the rustc dev guide docs once again.

@Xiretza
Copy link
Contributor Author

Xiretza commented Aug 19, 2022

That's good to hear! :D The PR will conflict with all the diagnostics migration PRs currently open, so I suppose it will need a rollup=never with a relatively high priority or it'll fail bors until the end of time.

@Xiretza
Copy link
Contributor Author

Xiretza commented Aug 19, 2022

Also, we probably need to adjust the rustc dev guide docs once again.

Already drafted: rust-lang/rustc-dev-guide#1440

@compiler-errors
Copy link
Member

The PR will conflict with all the diagnostics migration PRs currently open, so I suppose it will need a rollup=never with a relatively high priority or it'll fail bors until the end of time.

Yup makes sense. I was gonna wait until CI is green then approve.

Already drafted: rust-lang/rustc-dev-guide#1440

You're wonderful!

@compiler-errors
Copy link
Member

@bors r+ p=1 rollup=never

(will likely not merge unless p=1)

@bors
Copy link
Contributor

bors commented Aug 19, 2022

📌 Commit 450989f9ede869f8e432f9c1f64d3d055bfa041a has been approved by compiler-errors

It is now in the queue for this repository.

@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 Aug 19, 2022
@bors
Copy link
Contributor

bors commented Aug 19, 2022

⌛ Testing commit 450989f9ede869f8e432f9c1f64d3d055bfa041a with merge 8175a9f41cc3944935dcc70a5eb751ffc371d3a9...

@ChrisDenton
Copy link
Member

ChrisDenton commented Aug 20, 2022

CI has stalled.

EDIT: Sorry for the noise. The CI was still going after 5hrs without movement from a Windows builder for at least 3hrs.

@ChrisDenton

This comment was marked as resolved.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 20, 2022
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 21, 2022
@compiler-errors
Copy link
Member

Wait... this needs rebase.

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 21, 2022
This unifies the struct_{warn,error,fatal}() methods in one generic
method.
Deriving SessionDiagnostic on a type no longer forces that diagnostic to
be one of warning, error, or fatal. The level is instead decided when
the struct is passed to the respective Handler::emit_*() method.
@Xiretza Xiretza force-pushed the session-diagnostic-unification branch from dc8d944 to 7f3a6fd Compare August 21, 2022 07:18
@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Aug 21, 2022

📌 Commit 7f3a6fd has been approved by compiler-errors

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 21, 2022
@bors
Copy link
Contributor

bors commented Aug 21, 2022

⌛ Testing commit 7f3a6fd with merge 4b695f7...

@bors
Copy link
Contributor

bors commented Aug 21, 2022

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 4b695f7 to master...

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (4b695f7): comparison url.

Instruction count

  • Primary benchmarks: ✅ relevant improvements found
  • Secondary benchmarks: no relevant changes found
mean1 max count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.8% -0.9% 5
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.8% -0.9% 5

Max RSS (memory usage)

Results
  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.9% 2.9% 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.4% -3.3% 2
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

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

@rustbot label: -perf-regression

Footnotes

  1. the arithmetic mean of the percent change 2

  2. number of relevant changes 2

@Xiretza Xiretza deleted the session-diagnostic-unification branch August 21, 2022 15:34
@davidtwco
Copy link
Member

Thanks for working on this!

JhonnyBillM added a commit to JhonnyBillM/rust that referenced this pull request Aug 30, 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.

10 participants