Rewrite the new attribute argument parser #144689

JonathanBrouwer · 2025-07-30T16:34:30Z

Fixes #143940

This rewrites the parser, should improve performance and maintainability.
This can be reviewed commit by commit

jdonszelmann · 2025-07-30T18:47:19Z

@bors try @rust-timer queue

rust-bors · 2025-07-30T18:47:23Z

⌛ Trying commit 7b3bb12 with merge acc46aa…

To cancel the try build, run the command @bors try cancel.

Rewrite the new attribute parser

compiler/rustc_attr_parsing/src/parser.rs

tests/ui/attributes/key-value-expansion-on-mac.rs

jdonszelmann · 2025-07-30T19:09:24Z

tests/ui/attributes/malformed-fn-align.stderr

@@ -51,12 +37,26 @@ error[E0589]: invalid alignment value: not a power of two
 LL | #[rustc_align(0)]
   |               ^

+error: expected unsuffixed literal, found `-`


it would be pretty cool if we could explain here what an unsuffixed literal is. Maybe give an example of one with a short explanation. It's not required, but does teach the language better through diagnostics which is always cool

I improved the error a lot :)

error: expected a literal (`1u8`, `1.0f32`, `"string"`, etc.) here, found `-` --> $DIR/malformed-fn-align.rs:29:15 | LL | #[rustc_align(-1)] | ^ | help: negative numbers are not literals, try removing the `-` sign | LL - #[rustc_align(-1)] LL + #[rustc_align(1)] |

tests/ui/attributes/malformed-fn-align.stderr

tests/ui/attributes/nonterminal-expansion.rs

tests/ui/parser/bad-lit-suffixes.stderr

rust-bors · 2025-07-30T21:03:08Z

☀️ Try build successful (CI)
Build commit: acc46aa (acc46aa5d053362bb20ae7f352cba2635ef61120, parent: e5e79f8bd428d0b8d26e8240d718b134ef297459)

rust-timer · 2025-07-30T22:13:56Z

Finished benchmarking commit (acc46aa): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	0.2%	[0.2%, 0.2%]	1
Regressions ❌ (secondary)	0.8%	[0.0%, 1.0%]	7
Improvements ✅ (primary)	-0.3%	[-0.5%, -0.1%]	12
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.1%]	12
All ❌✅ (primary)	-0.2%	[-0.5%, 0.2%]	13

Max RSS (memory usage)

Results (secondary -1.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.0%	[2.0%, 2.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.4%	[-2.9%, -2.0%]	3
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary -0.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	4.7%	[4.7%, 4.7%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.1%	[-3.7%, -2.5%]	2
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 469.342s -> 468.873s (-0.10%)
Artifact size: 376.86 MiB -> 376.85 MiB (-0.00%)

lqd · 2025-07-31T11:42:24Z

(match-stress looks currently noisy to me)

jdonszelmann · 2025-07-31T12:37:56Z

Mhm, we came to the same conclusion (@JonathanBrouwer )

JonathanBrouwer · 2025-07-31T13:41:24Z

tests/ui/issues/issue-32782.rs

@@ -4,7 +4,7 @@ macro_rules! bar (

 macro_rules! foo (
    () => (
-        #[allow_internal_unstable] //~ ERROR allow_internal_unstable side-steps
+        #[allow_internal_unstable()] //~ ERROR allow_internal_unstable side-steps


BREAKING CHANGE:
Previously, attributes on macro calls were partially checked for correct syntax (only what check_builtin_meta_item can check).
After this PR, the attributes are fully parsed.

In any case, these attributes are always completely ignored, except for #[cfg], which was always completely parsed and still is and is not affected by this breaking change.

JonathanBrouwer · 2025-07-31T13:42:24Z

compiler/rustc_parse/src/parser/mod.rs

@@ -1095,7 +1095,7 @@ impl<'a> Parser<'a> {
    /// Parses a comma-separated sequence delimited by parentheses (e.g. `(x, y)`).
    /// The function `f` must consume tokens until reaching the next separator or
    /// closing bracket.
-    fn parse_paren_comma_seq<T>(
+    pub fn parse_paren_comma_seq<T>(


I've had to make quite a few of these functions public, how problematic is that?
I don't think it matters right, or at least it's better than the alternative of copying them over

JonathanBrouwer · 2025-07-31T13:47:17Z

@rustbot ready
r? @jdonszelmann

rustbot · 2025-07-31T13:47:24Z

r? @SparrowLii

rustbot has assigned @SparrowLii.
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-07-31T13:47:25Z

jdonszelmann is currently at their maximum review capacity.
They may take a while to respond.

JonathanBrouwer · 2025-08-19T08:52:21Z

@rustbot author
:c will take a look later

rustbot · 2025-08-19T08:52:25Z

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

JonathanBrouwer · 2025-08-20T10:10:51Z

@rustbot ready
@jdonszelmann

bors · 2025-08-22T03:07:22Z

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

rustbot · 2025-08-22T06:59:06Z

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

jdonszelmann · 2025-08-22T09:28:53Z

@bors r+

bors · 2025-08-22T09:28:55Z

📌 Commit ec5b2cc has been approved by jdonszelmann

It is now in the queue for this repository.

bors · 2025-08-22T10:27:16Z

⌛ Testing commit ec5b2cc with merge f5703d5...

bors · 2025-08-22T13:40:32Z

☀️ Test successful - checks-actions
Approved by: jdonszelmann
Pushing f5703d5 to master...

github-actions · 2025-08-22T13:43:25Z

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 831e291 (parent) -> f5703d5 (this PR)

Test differences

Show 31 test diffs

Stage 1

errors::verify_parse_asm_expected_comma_183: [missing] -> pass (J0)
errors::verify_parse_asm_expected_comma_184: pass -> [missing] (J0)
errors::verify_parse_asm_expected_other_184: [missing] -> pass (J0)
errors::verify_parse_asm_expected_other_185: pass -> [missing] (J0)
errors::verify_parse_asm_expected_register_class_or_explicit_register_187: [missing] -> pass (J0)
errors::verify_parse_asm_expected_register_class_or_explicit_register_188: pass -> [missing] (J0)
errors::verify_parse_asm_expected_string_literal_186: [missing] -> pass (J0)
errors::verify_parse_asm_expected_string_literal_187: pass -> [missing] (J0)
errors::verify_parse_asm_non_abi_185: [missing] -> pass (J0)
errors::verify_parse_asm_non_abi_186: pass -> [missing] (J0)
errors::verify_parse_asm_requires_template_182: [missing] -> pass (J0)
errors::verify_parse_asm_requires_template_183: pass -> [missing] (J0)
errors::verify_parse_asm_sym_no_path_181: [missing] -> pass (J0)
errors::verify_parse_asm_sym_no_path_182: pass -> [missing] (J0)
errors::verify_parse_asm_underscore_input_180: [missing] -> pass (J0)
errors::verify_parse_asm_underscore_input_181: pass -> [missing] (J0)
errors::verify_parse_asm_unsupported_operand_179: [missing] -> pass (J0)
errors::verify_parse_asm_unsupported_operand_180: pass -> [missing] (J0)
errors::verify_parse_binder_and_polarity_177: [missing] -> pass (J0)
errors::verify_parse_binder_and_polarity_178: pass -> [missing] (J0)
errors::verify_parse_binder_before_modifiers_176: [missing] -> pass (J0)
errors::verify_parse_binder_before_modifiers_177: pass -> [missing] (J0)
errors::verify_parse_invalid_attr_unsafe_176: pass -> [missing] (J0)
errors::verify_parse_modifiers_and_polarity_178: [missing] -> pass (J0)
errors::verify_parse_modifiers_and_polarity_179: pass -> [missing] (J0)
session_diagnostics::verify_attr_parsing_invalid_attr_unsafe_44: [missing] -> pass (J0)
session_diagnostics::verify_attr_parsing_suffixed_literal_in_attribute_45: [missing] -> pass (J0)
[ui] tests/ui/parser/attribute/attr-incomplete.rs: [missing] -> pass (J1)

Stage 2

[ui] tests/ui/parser/attribute/attr-incomplete.rs: [missing] -> pass (J2)

Additionally, 2 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard f5703d5dd3eec176f86a6e9bf7e668b48b84eabb --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

dist-aarch64-apple: 6805.9s -> 7685.4s (12.9%)
x86_64-gnu-llvm-19: 2736.4s -> 2434.2s (-11.0%)
x86_64-rust-for-linux: 2960.4s -> 2689.6s (-9.1%)
dist-x86_64-illumos: 6304.7s -> 5842.7s (-7.3%)
aarch64-msvc-2: 4942.2s -> 5277.0s (6.8%)
x86_64-gnu-aux: 6746.3s -> 6307.6s (-6.5%)
test-various: 4961.2s -> 4647.0s (-6.3%)
dist-x86_64-apple: 7374.5s -> 6907.5s (-6.3%)
aarch64-apple: 6416.0s -> 6034.4s (-5.9%)
dist-apple-various: 4984.6s -> 4707.4s (-5.6%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

rust-timer · 2025-08-22T14:58:55Z

Finished benchmarking commit (f5703d5): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.1%	[0.7%, 1.4%]	8
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 0.2%, secondary 2.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	1.4%	[1.4%, 1.4%]	1
Regressions ❌ (secondary)	2.7%	[2.2%, 3.9%]	7
Improvements ✅ (primary)	-1.0%	[-1.0%, -1.0%]	1
Improvements ✅ (secondary)	-0.7%	[-1.0%, -0.4%]	2
All ❌✅ (primary)	0.2%	[-1.0%, 1.4%]	2

Cycles

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

Binary size

Results (secondary -0.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

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

Bootstrap: 466.749s -> 466.339s (-0.09%)
Artifact size: 378.24 MiB -> 378.27 MiB (0.01%)

rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 30, 2025

JonathanBrouwer force-pushed the share_parse_path branch from 799d605 to 7b3bb12 Compare July 30, 2025 16:42

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Jul 30, 2025

Auto merge of #144689 - JonathanBrouwer:share_parse_path, r=<try>

acc46aa

Rewrite the new attribute parser

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 30, 2025

jdonszelmann reviewed Jul 30, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jul 30, 2025

JonathanBrouwer force-pushed the share_parse_path branch 2 times, most recently from 4c9a573 to 244cefa Compare July 31, 2025 13:39

JonathanBrouwer commented Jul 31, 2025

View reviewed changes

JonathanBrouwer force-pushed the share_parse_path branch from 244cefa to c0f407d Compare July 31, 2025 13:46

JonathanBrouwer marked this pull request as ready for review July 31, 2025 13:47

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 Jul 31, 2025

rustbot assigned SparrowLii and jdonszelmann Jul 31, 2025

This comment has been minimized.

Sign in to view

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 Aug 19, 2025

JonathanBrouwer force-pushed the share_parse_path branch from b203b21 to 3170d9d Compare August 20, 2025 08:16

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 Aug 20, 2025

JonathanBrouwer added 4 commits August 22, 2025 08:37

Move validate_attr to rustc_attr_parsing

21d3189

Rewrite the new attribute parser

549314b

Use the new attribute parser throughout the codebase

17e34f6

Updated uitests for new parser

ec5b2cc

JonathanBrouwer force-pushed the share_parse_path branch from 3170d9d to ec5b2cc Compare August 22, 2025 06:59

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

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 22, 2025

bors merged commit f5703d5 into rust-lang:master Aug 22, 2025
11 checks passed

rustbot added this to the 1.91.0 milestone Aug 22, 2025

bors mentioned this pull request Aug 22, 2025

fix: incorrect error message for string literal suffixes #145602

Open

rustbot removed the perf-regression Performance regression. label Aug 22, 2025

matthiaskrgr mentioned this pull request Aug 23, 2025

itertools 0.9: error: '#[inline]' attribute cannot be used on macro calls #145779

Closed

Rewrite the new attribute argument parser #144689

Rewrite the new attribute argument parser #144689

Uh oh!

Conversation

JonathanBrouwer commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jdonszelmann commented Jul 30, 2025

Uh oh!

This comment has been minimized.

rust-bors bot commented Jul 30, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rust-bors bot commented Jul 30, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Jul 30, 2025

Overall result: ❌✅ regressions and improvements - please read the text below

Uh oh!

lqd commented Jul 31, 2025

Uh oh!

jdonszelmann commented Jul 31, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer commented Jul 31, 2025

Uh oh!

rustbot commented Jul 31, 2025

Uh oh!

rustbot commented Jul 31, 2025

Uh oh!

This comment has been minimized.

JonathanBrouwer commented Aug 19, 2025

Uh oh!

rustbot commented Aug 19, 2025

Uh oh!

JonathanBrouwer commented Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors commented Aug 22, 2025

Uh oh!

rustbot commented Aug 22, 2025

Uh oh!

jdonszelmann commented Aug 22, 2025

Uh oh!

bors commented Aug 22, 2025

Uh oh!

bors commented Aug 22, 2025

Uh oh!

bors commented Aug 22, 2025

Uh oh!

Uh oh!

github-actions bot commented Aug 22, 2025

Test differences

Stage 1

Stage 2

Job duration changes

Uh oh!

rust-timer commented Aug 22, 2025

Overall result: ❌ regressions - no action needed

Uh oh!

Uh oh!

JonathanBrouwer commented Jul 30, 2025 •

edited

Loading

JonathanBrouwer commented Aug 20, 2025 •

edited

Loading