Rewrite `inline` attribute parser to use new infrastructure and improve diagnostics for all parsed attributes by jdonszelmann · Pull Request #138165 · rust-lang/rust

jdonszelmann · 2025-03-07T16:07:27Z

This PR:

creates a new parser for inline attributes
creates consistent error messages and error codes between attribute parsers; inline and others
as such changes a few error messages for other attributes to be (in my eyes) much more consistent
tests ast-lowering lints introduced by Infrastructure for lints during attribute parsing, specifically duplicate usages of attributes #138164 since this is now useful for the first time
Coalesce some useless error codes

Builds on top of #138164

jdonszelmann · 2025-03-08T23:37:17Z

compiler/rustc_codegen_ssa/src/codegen_attrs.rs

            return OptimizeAttr::Default;
        };

-        inline_span = Some(attr.span());


This fixes #137950

bors · 2025-03-11T05:03:38Z

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

jdonszelmann · 2025-06-09T22:54:26Z

@rustbot review

jdonszelmann · 2025-06-09T23:21:16Z

ah shit, broke clippy 😭

jdonszelmann · 2025-06-18T09:30:14Z

Yay!

nnethercote · 2025-06-18T10:36:26Z

src/rustdoc-json-types/lib.rs

-// Latest feature: rustdoc JSON: Don't apply #[repr] privacy heuristics
-pub const FORMAT_VERSION: u32 = 46;
+// Latest feature: Pretty printing of inline attributes changed
+pub const FORMAT_VERSION: u32 = 48;


Uh, we skipped 47. Oh well, not much to be done about it now.

@GuillaumeGomez: when you add the (tidy?) check for updating the number and comment in tandem, could you also add a check that the number is being increased by one?

I opened #142677 for that. But that's a good point. Added to my TODO list.

#138165 (comment), earlier it was thaught that someone else was going to do 46->47, to this should do 47->48. I think the takeaways are:

a) Don't try to be too clever and do a double bounce
b) #142601 should fix this. 🤞

I've published rustdoc-types = "0.48.0", with a note that 0.47.0/format_version=47 will never exist. https://github.com/rust-lang/rustdoc-types/blob/trunk/CHANGELOG.md#v0.48.0

rust-timer · 2025-06-18T19:06:17Z

Finished benchmarking commit (1bb3352): comparison URL.

Overall result: ❌ regressions - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

If the regression was expected or you think it can be justified,
please write a comment with sufficient written justification, and add
@rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
If you think that you know of a way to resolve the regression, try to create
a new PR with a fix for the regression.
If you do not understand the regression or you think that it is just noise,
you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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.3%	[0.2%, 0.8%]	12
Regressions ❌ (secondary)	0.5%	[0.2%, 0.8%]	21
Improvements ✅ (primary)	-0.2%	[-0.2%, -0.2%]	2
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.2%]	1
All ❌✅ (primary)	0.3%	[-0.2%, 0.8%]	14

Max RSS (memory usage)

Results (primary -0.8%, secondary -1.0%)

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

	mean	range	count
Regressions ❌ (primary)	7.0%	[7.0%, 7.0%]	1
Regressions ❌ (secondary)	3.4%	[3.4%, 3.4%]	1
Improvements ✅ (primary)	-1.5%	[-2.5%, -0.8%]	11
Improvements ✅ (secondary)	-3.1%	[-3.1%, -3.1%]	2
All ❌✅ (primary)	-0.8%	[-2.5%, 7.0%]	12

Cycles

Results (secondary 4.2%)

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.2%	[4.2%, 4.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

Results (primary 0.1%, secondary 0.2%)

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

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.0%, 0.6%]	87
Regressions ❌ (secondary)	0.2%	[0.0%, 0.8%]	40
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[0.0%, 0.6%]	87

Bootstrap: 693.071s -> 692.01s (-0.15%)
Artifact size: 372.10 MiB -> 372.13 MiB (0.01%)

rewrite `optimize` attribute to use new attribute parsing infrastructure r? `@oli-obk` I'm afraid we'll get quite a few of these PRs in the future. If we get a lot of trivial changes I'll start merging multiple into one PR. They should be easy to review :) Waiting on rust-lang#138165 first

rewrite `optimize` attribute to use new attribute parsing infrastructure r? ``@oli-obk`` I'm afraid we'll get quite a few of these PRs in the future. If we get a lot of trivial changes I'll start merging multiple into one PR. They should be easy to review :) Waiting on rust-lang#138165 first

rewrite `optimize` attribute to use new attribute parsing infrastructure r? ```@oli-obk``` I'm afraid we'll get quite a few of these PRs in the future. If we get a lot of trivial changes I'll start merging multiple into one PR. They should be easy to review :) Waiting on rust-lang#138165 first

therealprof · 2025-06-20T08:30:46Z

@jdonszelmann Is the binary size jump expected? I thought this would only change the infrastructure but not actual inlining?

jdonszelmann · 2025-06-20T08:37:22Z

@oli-obk and I have been looking at this this morning. It has to do with attribute encoding cross crate and I'm working on a fix. The fix is luckily rather simple, but it was definitely an unexpected side effect. I expect to be busy for an hour or so more and then have a pr ready :)

Encode hir attributes cross-crate properly r? `@oli-obk` This should return the lost perf in #138165 cc: `@therealprof`

Rollup merge of #138291 - jdonszelmann:optimize-attr, r=oli-obk rewrite `optimize` attribute to use new attribute parsing infrastructure r? ```@oli-obk``` I'm afraid we'll get quite a few of these PRs in the future. If we get a lot of trivial changes I'll start merging multiple into one PR. They should be easy to review :) Waiting on #138165 first

rylev · 2025-06-24T11:26:03Z

The perf regression has been addressed in #142777

@rustbot label: +perf-regression-triaged

apiraino · 2025-06-24T11:29:14Z

The perf regression has been addressed in #142777

To be precise almost completely addressed, the clap crate still suffers on binary size regression.

jdonszelmann · 2025-06-24T20:37:31Z

@Kobzol has theorized this is noise, saying that the noise threshold isn't very accurate caused by the bimodal nature of the data. It sounds reasonable to me looking at the graph so I think this is actually entirely addressed @rylev

…i-obk Encode hir attributes cross-crate properly r? `@oli-obk` This should return the lost perf in #138165 cc: `@therealprof`

Rewrite `inline` attribute parser to use new infrastructure and improve diagnostics for all parsed attributes r? `@oli-obk` This PR: - creates a new parser for inline attributes - creates consistent error messages and error codes between attribute parsers; inline and others - as such changes a few error messages for other attributes to be (in my eyes) much more consistent - tests ast-lowering lints introduced by rust-lang#138164 since this is now useful for the first time - Coalesce some useless error codes Builds on top of rust-lang#138164 Closes rust-lang#137950

rustbot assigned oli-obk Mar 7, 2025

rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) 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 7, 2025