Migrate more of rustc_parse to SessionDiagnostic by Xiretza · Pull Request #101619 · rust-lang/rust

Xiretza · 2022-09-09T17:15:36Z

Still far from complete, but I thought I'd add a checkpoint here because rebasing was starting to get annoying.

rust-highfive · 2022-09-09T17:15:41Z

r? @davidtwco

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

rustbot · 2022-09-09T17:15:41Z

rustc_error_messages was changed

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

compiler/rustc_parse/src/parser/expr.rs

davidtwco · 2022-09-12T10:52:41Z

compiler/rustc_parse/src/parser/expr.rs

I wonder if we should keep sess.create_err given that it does exactly this and this might be more discoverable? Or vice-versa?

compiler/rustc_parse/src/parser/expr.rs

compiler/rustc_parse/src/parser/attr.rs

compiler/rustc_parse/src/parser/diagnostics.rs

Xiretza · 2022-09-13T17:55:38Z

I'll rebase this and address the review comments once #101558 has landed.

compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs

rustbot · 2022-09-21T18:31:13Z

rustc_macros::diagnostics was changed

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

davidtwco

Apologies for the delay in reviewing this, this looks good to me, I've left one or two comments about places where it looks like we're interpolating English but those have FIXMEs in the source so maybe you're still to address them?

compiler/rustc_error_messages/locales/en-US/parser.ftl

davidtwco

r=me after rebase

Xiretza · 2022-09-26T17:57:30Z

Hm, can/should I do this?

@bors r=davidtwco

Edit: yep, not surprised. I thought I had seen a non-Member do it the other day, so I was curious :)

bors · 2022-09-26T17:57:31Z

@Xiretza: 🔑 Insufficient privileges: Not in reviewers

davidtwco · 2022-09-27T09:31:37Z

Hm, can/should I do this?

@bors r=davidtwco

Edit: yep, not surprised. I thought I had seen a non-Member do it the other day, so I was curious :)

Sorry, it was just a note to myself or anyone else with privileges should they stumble upon this - I'll r+ when rebased again.

This function is only ever called when the `#` has already been consumed, no need to produce an error message here.

davidtwco · 2022-09-28T08:59:47Z

@bors r+

bors · 2022-09-28T08:59:50Z

📌 Commit d7c6457 has been approved by davidtwco

It is now in the queue for this repository.

bors · 2022-09-28T11:11:46Z

⌛ Testing commit d7c6457 with merge 09ae784...

bors · 2022-09-28T14:05:59Z

☀️ Test successful - checks-actions
Approved by: davidtwco
Pushing 09ae784 to master...

rust-timer · 2022-09-28T16:34:03Z

Finished benchmarking commit (09ae784): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean¹	range	count²
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.2%	[-2.2%, -2.2%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean¹	range	count²
Regressions ❌ (primary)	3.4%	[3.4%, 3.4%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.4%	[3.4%, 3.4%]	1

Cycles

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

the arithmetic mean of the percent change ↩ ↩²
number of relevant changes ↩ ↩²

…ics, r=davidtwco Migrate more of rustc_parse to SessionDiagnostic Still far from complete, but I thought I'd add a checkpoint here because rebasing was starting to get annoying.

rust-highfive assigned davidtwco Sep 9, 2022

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 Sep 9, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 9, 2022

davidtwco requested changes Sep 12, 2022

View reviewed changes