Skip to content

Migrate rustc_mir_build diagnostics #104417

New issue

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

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

Already on GitHub? # to your account

Merged
merged 18 commits into from
Dec 18, 2022
Merged

Migrate rustc_mir_build diagnostics #104417

merged 18 commits into from
Dec 18, 2022

Conversation

mejrs
Copy link
Contributor

@mejrs mejrs commented Nov 14, 2022

Rebases #100854

The remaining issue is how to better resolve 72bea68

The diagnostic macros seems to generate a broken diagnostic, and I couldn't figure out how to manually format the fluent message, so I hardcoded the format string for now. I'd like pointers to a better fix for this.

Also, I'm not 100% sure I didn't mess up a rebase somewhere 🙂

r? @davidtwco

@rustbot
Copy link
Collaborator

rustbot commented Nov 14, 2022

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @davidtwco (or someone else) soon.

Please see the contribution instructions for more information.

@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 14, 2022
@rustbot
Copy link
Collaborator

rustbot commented Nov 14, 2022

rustc_error_messages was changed

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

@bors

This comment was marked as resolved.

@davidtwco
Copy link
Member

The remaining issue is how to better resolve 72bea68

The diagnostic macros seems to generate a broken diagnostic, and I couldn't figure out how to manually format the fluent message, so I hardcoded the format string for now. I'd like pointers to a better fix for this.

You should be able to fix this by using #[subdiagnostic(eager)] where this subdiagnostic is used.

@rust-log-analyzer

This comment has been minimized.

@mejrs
Copy link
Contributor Author

mejrs commented Nov 15, 2022

You should be able to fix this by using #[subdiagnostic(eager)] where this subdiagnostic is used.

Ah that's good to know. I've fixed that and rebased.

} else {
// FIXME: migration of this diagnostic will require list support
Copy link
Member

Choose a reason for hiding this comment

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

You can try using the list support added in #104047, maybe better as a follow-up so we can land this though.

@davidtwco
Copy link
Member

Apologies for the delay in getting back to this.

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 23, 2022

📌 Commit 2354697 has been approved by davidtwco

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 Nov 23, 2022
Manishearth added a commit to Manishearth/rust that referenced this pull request Nov 23, 2022
Migrate rustc_mir_build diagnostics

Rebases rust-lang#100854

~~The remaining issue is how to better resolve rust-lang@72bea68af4ee2a41c44998916f6a789163f12e7d~~

~~The diagnostic macros seems to generate a broken diagnostic, and I couldn't figure out how to manually format the fluent message, so I hardcoded the format string for now. I'd like pointers to a better fix for this.~~

Also, I'm not 100% sure I didn't mess up a rebase somewhere 🙂

r? `@davidtwco`
@Manishearth
Copy link
Member

@bors r-

#104790 (comment)

@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 Nov 23, 2022
@bors
Copy link
Collaborator

bors commented Nov 24, 2022

☔ The latest upstream changes (presumably #104809) made this pull request unmergeable. Please resolve the merge conflicts.

@mejrs
Copy link
Contributor Author

mejrs commented Nov 26, 2022

Rebased

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 26, 2022
@jyn514
Copy link
Member

jyn514 commented Dec 8, 2022

@bors r=davidtwco

@bors
Copy link
Collaborator

bors commented Dec 8, 2022

📌 Commit 1d324dd has been approved by davidtwco

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 Dec 8, 2022
@mejrs
Copy link
Contributor Author

mejrs commented Dec 17, 2022

I think this fixes the ice that was hit during rollup, but I'm not sure how to test it locally.

@mejrs mejrs marked this pull request as draft December 17, 2022 21:54
@mejrs
Copy link
Contributor Author

mejrs commented Dec 17, 2022

I think this works on dist-riscv64-linux now - see https://github.com/rust-lang/rust/actions/runs/3721937031/jobs/6312424723

@mejrs mejrs marked this pull request as ready for review December 17, 2022 22:31
@Noratrieb
Copy link
Member

@bors r=davidtwco
Let's hope it really works now

@bors
Copy link
Collaborator

bors commented Dec 18, 2022

📌 Commit f7e894c has been approved by davidtwco

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 Dec 18, 2022
@bors
Copy link
Collaborator

bors commented Dec 18, 2022

⌛ Testing commit f7e894c with merge 35a99ee...

@bors
Copy link
Collaborator

bors commented Dec 18, 2022

☀️ Test successful - checks-actions
Approved by: davidtwco
Pushing 35a99ee to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 18, 2022
@bors bors merged commit 35a99ee into rust-lang:master Dec 18, 2022
@rustbot rustbot added this to the 1.68.0 milestone Dec 18, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (35a99ee): comparison URL.

Overall result: ❌✅ regressions and 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)
1.0% [1.0%, 1.0%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.3% [-1.3%, -1.3%] 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)
2.3% [2.3%, 2.3%] 1
Regressions ❌
(secondary)
1.3% [1.3%, 1.3%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 2.3% [2.3%, 2.3%] 1

Cycles

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

# for free to join this conversation on GitHub. Already have an account? # 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-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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