Rewrite on_unimplemented format string parser. #139091

mejrs · 2025-03-29T01:09:37Z

This PR rewrites the format string parser for rustc_on_unimplemented and diagnostic::on_unimplemented. I plan on moving this code (and more) into the new attribute parsing system soon and wanted to PR it separately.

This PR introduces some minor differences though:

rustc_on_unimplemented on trait implementations is no longer checked/used - this is actually never used (outside of some tests) so I plan on removing it in the future.
for rustc_on_unimplemented, it introduces the {This} argument in favor of {ThisTraitname} (to be removed later). It'll be easier to parse.
for rustc_on_unimplemented, Self can now consistently be used as a filter, rather than just _Self. It used to not match correctly on for example Self = "[{integer}]"
Some error messages now have better spans.

Fixes #130627

rustbot · 2025-03-29T01:09:41Z

r? @jackh726

rustbot has assigned @jackh726.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2025-03-29T10:46:13Z

This PR changes a file inside tests/crashes. If a crash was fixed, please move into the corresponding ui subdir and add 'Fixes #' to the PR description to autoclose the issue upon merge.

bors · 2025-04-09T06:43:04Z

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

mejrs · 2025-04-09T23:37:28Z

r? @compiler-errors (or reroll)

compiler-errors

ill actually look at this tomorrow, just a few actionable things until then

compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs

compiler-errors · 2025-04-09T23:54:51Z

tests/ui/on-unimplemented/multiple-impls.stderr

@@ -63,9 +60,8 @@ error[E0277]: the trait bound `[i32]: Index<Bar<u32>>` is not satisfied
  --> $DIR/multiple-impls.rs:39:5
   |
 LL |     Index::index(&[] as &[i32], Bar(2u32));
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ on impl for Bar


Can you remove the rustc_on_unimplemented from impls in these test files?

ideally we'd make it a validation error or something to put this on an impl

so I plan on removing it in the future.

why not now? 🤔

for a couple reasons:

in b1bc725 I deleted its only use in std, but it's still present in the beta std. I'm unsure whether raising an error would cause problems.

rather than adding that check to this PR I'd rather do it in a future hir attributes related PR

I'm not intimately familiar with this code so didn't feel like raising new errors.

I'm happy to start raising errors now if you'd prefer.

Well, I'd at least prefer for us to clean up the UI tests so they don't look like they're buggy (i.e. have annotations on impls, but don't use them).

I feel like the easiest way of doing that is just making rustc_on_unimplemented an error then clean up the new errors in the UI tests, but it's up to you I guess.

the "now the ui tests look buggy" argument is compelling to me, i'll experiment with a hard error tomorrow

I've deleted the tests for attribute on impls.

ideally we'd make it a validation error or something to put this on an impl

it's not a validation error to put it anywhere, you can put it on structs, functions, etc. I would like to start erroring on all of that after migrating the attribute itself.

compiler/rustc_span/src/symbol.rs

mejrs · 2025-04-10T14:15:07Z

the Tidy/bootstrap labels are just because it deletes a issue-###.rs test

compiler/rustc_span/src/hygiene.rs

compiler-errors · 2025-04-13T16:42:45Z

One nit, though I didn't review this for perfect parity with the previous behavior, it looks fine to me.

compiler-errors · 2025-04-13T16:42:52Z

@rustbot author

rustbot · 2025-04-13T16:42:58Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

mejrs · 2025-04-13T22:14:23Z

I've fixed the doc comment. Thanks for the review :)

@rustbot ready

mejrs · 2025-04-18T20:51:22Z

@compiler-errors ping 🙏

compiler-errors · 2025-04-18T20:52:08Z

@bors r+

bors · 2025-04-18T20:52:10Z

📌 Commit 4a5369b has been approved by compiler-errors

It is now in the queue for this repository.

…enton Rollup of 8 pull requests Successful merges: - rust-lang#138934 (support config extensions) - rust-lang#139091 (Rewrite on_unimplemented format string parser.) - rust-lang#139753 (Make `#[naked]` an unsafe attribute) - rust-lang#139762 (Don't assemble non-env/bound candidates if projection is rigid) - rust-lang#139834 (Don't canonicalize crate paths) - rust-lang#139868 (Move `pal::env` to `std::sys::env_consts`) - rust-lang#139978 (Add citool command for generating a test dashboard) - rust-lang#139995 (Clean UI tests 4 of n) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#139091 - mejrs:format, r=compiler-errors Rewrite on_unimplemented format string parser. This PR rewrites the format string parser for `rustc_on_unimplemented` and `diagnostic::on_unimplemented`. I plan on moving this code (and more) into the new attribute parsing system soon and wanted to PR it separately. This PR introduces some minor differences though: - `rustc_on_unimplemented` on trait *implementations* is no longer checked/used - this is actually never used (outside of some tests) so I plan on removing it in the future. - for `rustc_on_unimplemented`, it introduces the `{This}` argument in favor of `{ThisTraitname}` (to be removed later). It'll be easier to parse. - for `rustc_on_unimplemented`, `Self` can now consistently be used as a filter, rather than just `_Self`. It used to not match correctly on for example `Self = "[{integer}]"` - Some error messages now have better spans. Fixes rust-lang#130627

…enton Rollup of 8 pull requests Successful merges: - rust-lang#138934 (support config extensions) - rust-lang#139091 (Rewrite on_unimplemented format string parser.) - rust-lang#139753 (Make `#[naked]` an unsafe attribute) - rust-lang#139762 (Don't assemble non-env/bound candidates if projection is rigid) - rust-lang#139834 (Don't canonicalize crate paths) - rust-lang#139868 (Move `pal::env` to `std::sys::env_consts`) - rust-lang#139978 (Add citool command for generating a test dashboard) - rust-lang#139995 (Clean UI tests 4 of n) r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned jackh726 Mar 29, 2025

rustbot added 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 Mar 29, 2025

This comment has been minimized.

Sign in to view

mejrs force-pushed the format branch 2 times, most recently from 8f91772 to 232c722 Compare April 1, 2025 00:47

Move errors

199ee40

mejrs force-pushed the format branch from 232c722 to cb6d018 Compare April 9, 2025 19:48

rustbot assigned compiler-errors and unassigned jackh726 Apr 9, 2025

compiler-errors reviewed Apr 9, 2025

View reviewed changes

rustbot added A-tidy Area: The tidy tool T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Apr 10, 2025

mejrs added 2 commits April 10, 2025 17:28

Write the format string parserand split it from conditions parser

2007c89

Parse condition options into a struct

ba9f51b

mejrs force-pushed the format branch from b064d64 to 63be686 Compare April 10, 2025 15:29

compiler-errors approved these changes Apr 13, 2025

View reviewed changes

compiler/rustc_span/src/hygiene.rs Outdated Show resolved Hide resolved

rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Apr 13, 2025

Documentation and finishing touches

8586cad

mejrs added 4 commits April 14, 2025 00:12

Disable usage on trait impls and aliases

9abaa9d

Raise errors on bad rustc_on_unimplemented format strings again

10ec5cb

Test that Self properly works in filters

40e76c1

Remove rustc_on_unimplemented on impl tests

4a5369b

mejrs force-pushed the format branch from 63be686 to 4a5369b Compare April 13, 2025 22:12

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 Apr 13, 2025

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 18, 2025

ChrisDenton mentioned this pull request Apr 19, 2025

Rollup of 8 pull requests #140043

Merged

bors merged commit aad59a3 into rust-lang:master Apr 20, 2025
6 checks passed

rustbot added this to the 1.88.0 milestone Apr 20, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite on_unimplemented format string parser. #139091

Rewrite on_unimplemented format string parser. #139091

mejrs commented Mar 29, 2025 •

edited

Loading

rustbot commented Mar 29, 2025

This comment has been minimized.

rustbot commented Mar 29, 2025

This comment has been minimized.

This comment has been minimized.

bors commented Apr 9, 2025

mejrs commented Apr 9, 2025

compiler-errors left a comment

compiler-errors Apr 9, 2025

compiler-errors Apr 9, 2025

compiler-errors Apr 9, 2025

mejrs Apr 10, 2025

compiler-errors Apr 10, 2025

mejrs Apr 10, 2025

mejrs Apr 10, 2025

mejrs Apr 10, 2025

mejrs commented Apr 10, 2025

compiler-errors commented Apr 13, 2025

compiler-errors commented Apr 13, 2025

rustbot commented Apr 13, 2025

mejrs commented Apr 13, 2025

mejrs commented Apr 18, 2025

compiler-errors commented Apr 18, 2025

bors commented Apr 18, 2025

Rewrite on_unimplemented format string parser. #139091

Rewrite on_unimplemented format string parser. #139091

Conversation

mejrs commented Mar 29, 2025 • edited Loading

rustbot commented Mar 29, 2025

This comment has been minimized.

rustbot commented Mar 29, 2025

This comment has been minimized.

This comment has been minimized.

bors commented Apr 9, 2025

mejrs commented Apr 9, 2025

compiler-errors left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mejrs commented Apr 10, 2025

compiler-errors commented Apr 13, 2025

compiler-errors commented Apr 13, 2025

rustbot commented Apr 13, 2025

mejrs commented Apr 13, 2025

mejrs commented Apr 18, 2025

compiler-errors commented Apr 18, 2025

bors commented Apr 18, 2025

mejrs commented Mar 29, 2025 •

edited

Loading