even more cleanups for `rustc_builtin_macros` by cyrgani · Pull Request #162925 · rust-lang/rust

cyrgani · 2026-09-17T20:30:16Z

Followup to #162234 with more cleanups and perf improvements.

rustbot · 2026-09-17T20:30:19Z

Changes to the code generated for builtin derived traits.

rustbot · 2026-09-17T20:30:22Z

rustbot has assigned @mati865.
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

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler
compiler expanded to 77 candidates
Random selection from 21 candidates

cyrgani · 2026-09-17T20:33:54Z

@@ -2,15 +2,15 @@ macro-stats ====================================================================
 macro-stats MACRO EXPANSION STATS: macro_stats
 macro-stats Macro Name                         Uses      Lines  Avg Lines      Bytes  Avg Bytes
 macro-stats -----------------------------------------------------------------------------------
-macro-stats #[derive(Clone)]                      8         67        8.4      1_879      234.9
+macro-stats #[derive(Clone)]                      8         67        8.4      1_895      236.9


this test shows a slight increase in size, but that is only because the types in the test all have one-or-two-letter names

View changes since the review

clarfonthey · 2026-09-17T20:33:55Z

@bors try @rust-timer queue

even more cleanups for `rustc_builtin_macros`

rust-bors · 2026-09-17T21:32:39Z

☀️ Try build successful (CI)
Build commit: 34797f1 (34797f1780452184438a4934a50dd0fbab5bb644)
Base parent: 76af58f (76af58f665c5cd76672e64ca118efbb0101fb224)

nnethercote · 2026-09-17T21:35:41Z

@cyrgani: The rustc_builtin_macros code has always been hard to navigate, so it's cool that you are making it better :)

rust-timer · 2026-09-18T11:21:41Z

Finished benchmarking commit (34797f1): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

@bors rollup=never rustc-perf
@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
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.2%	[-0.2%, -0.2%]	3
Improvements ✅ (secondary)	-0.3%	[-0.4%, -0.2%]	9
All ❌✅ (primary)	-0.2%	[-0.2%, -0.2%]	3

Max RSS (memory usage)

Results (primary 3.9%, 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)	15.2%	[15.2%, 15.2%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.7%	[-2.9%, -0.6%]	2
Improvements ✅ (secondary)	-4.2%	[-4.7%, -3.6%]	2
All ❌✅ (primary)	3.9%	[-2.9%, 15.2%]	3

Cycles

Results (primary 1.7%, secondary 22.4%)

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

	mean	range	count
Regressions ❌ (primary)	1.7%	[1.7%, 1.7%]	1
Regressions ❌ (secondary)	22.4%	[9.9%, 35.8%]	11
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.7%	[1.7%, 1.7%]	1

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 498.345s -> 497.629s (-0.14%)
Artifact size: 408.96 MiB -> 408.89 MiB (-0.02%)

nnethercote

Mostly looks good. Changing the return types to Self is particularly nice.

I'm not convinced that the "remove PathKind, use ast::Path for all local paths directly" commit is an improvement. What's the rationale? I see it removes the PathKind enum, but now some paths go via Path and some via AstTy, which is a bit confusing.

View changes since this review

nnethercote · 2026-09-19T04:12:04Z

            });
-            let nonself_args =
-                nonself_arg_tys.into_iter().map(|(name, ty)| cx.param(span, name, ty));
+            let mut nonself_args = Vec::new();


Instead of a Vec that is pushed onto, nonself_args could be an iter + map.

I think that would avoid the need for the "construct args directly" commit.

Makes sense, this is now part of the "stop passing nonself_arg_tys around" commit.

cyrgani · 2026-09-19T18:42:43Z

The motivation for removing PathKind etc. is that I want to remove this local version of Path entirely and just use ast::Path everywhere. But it probably makes more sense to do all those changes in the same PR, so I'll remove most of that commit for now.

nnethercote · 2026-09-19T23:55:35Z

Thanks!

@bors r+

rust-bors · 2026-09-19T23:55:37Z

📌 Commit 072a61a has been approved by nnethercote

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 5. This pull request will be tested once the tree is reopened.

Reason for tree closure: GitHub outage

even more cleanups for `rustc_builtin_macros` Followup to #162234 with more cleanups and perf improvements.

jhpratt · 2026-09-20T05:02:13Z

stalled

@bors yield

@bors try jobs=test-x86_64-gnu-parallel-frontend

rust-bors · 2026-09-20T05:02:22Z

Auto build was cancelled. Cancelled workflows:

https://github.com/rust-lang/rust/actions/runs/35477985498

The next pull request likely to be tested is #163043.

even more cleanups for `rustc_builtin_macros` try-job: test-x86_64-gnu-parallel-frontend

even more cleanups for `rustc_builtin_macros` Followup to #162234 with more cleanups and perf improvements.

rust-bors · 2026-09-20T06:10:35Z

☀️ Try build successful (CI)
Build commit: dd284bd (dd284bd5a6314d55a02debd1652ee6aadffb2997)
Base parent: feaadee (feaadeeaca7db0594da854e7c8c07495341c7439)

Zalathar · 2026-09-20T09:56:30Z

The test-various job looks stuck.

@bors yield
@bors try jobs=test-various

rust-bors · 2026-09-20T09:56:32Z

Auto build was cancelled. Cancelled workflows:

https://github.com/rust-lang/rust/actions/runs/35492698450

The next pull request likely to be tested is #163043.

even more cleanups for `rustc_builtin_macros` try-job: test-various

rust-bors · 2026-09-20T11:59:49Z

☀️ Try build successful (CI)
Build commit: 7a3dc2c (7a3dc2c2d77da28feed11a6dae035f3739dee995)
Base parent: feaadee (feaadeeaca7db0594da854e7c8c07495341c7439)

rust-bors · 2026-09-20T20:18:23Z

☀️ Test successful - CI
Approved by: nnethercote
Duration: 3h 7m 8s
Pushing bba5310 to main...

github-actions · 2026-09-20T20:21:32Z

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 12c4d3f (parent) -> bba5310 (this PR)

Test differences

No test diffs found

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard bba531001d4de6d7f49693e0836a2668ca063282 --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

test-x86_64-gnu-miri: 44m 57s -> 1h 23m (+86.0%)
test-i686-gnu-2: 1h -> 1h 39m (+64.8%)
test-x86_64-gnu-llvm-21-1: 57m 27s -> 34m 14s (-40.4%)
dist-apple-various: 1h 36m -> 2h 14m (+40.0%)
dist-various-2: 39m 14s -> 53m 28s (+36.3%)
test-x86_64-gnu-llvm-21-2: 1h 39m -> 1h 3m (-35.9%)
test-x86_64-gnu: 1h 54m -> 2h 33m (+34.7%)
dist-sparcv9-solaris: 1h 15m -> 1h 41m (+34.3%)
dist-i586-gnu-i586-i686-musl: 21m 16s -> 28m 13s (+32.7%)
test-x86_64-msvc-2: 1h 59m -> 2h 37m (+31.9%)

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 · 2026-09-20T21:03:10Z

Finished benchmarking commit (bba5310): comparison URL.

Overall result: ❌✅ regressions and improvements - 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)	0.6%	[0.5%, 0.7%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.3%	[-0.3%, -0.2%]	5
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -2.3%, secondary -2.4%)

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

Cycles

Results (primary 1.8%, secondary 6.0%)

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

	mean	range	count
Regressions ❌ (primary)	1.8%	[1.3%, 2.4%]	2
Regressions ❌ (secondary)	6.0%	[6.0%, 6.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.8%	[1.3%, 2.4%]	2

Binary size

Results (primary 0.0%, secondary 0.1%)

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

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

Bootstrap: 496.862s -> 496.585s (-0.06%)
Artifact size: 408.74 MiB -> 408.87 MiB (0.03%)

cyrgani added 5 commits September 17, 2026 08:02

put the check for empty assert_fields_are_eq at the right spot

f438cf8

micro cleanups

a51fdcb

remove use_foldl arg

08961a7

split the cs_fold closure into three subclosures

b6d0760

stop passing explicit_self around

8ca7a1c

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 Sep 17, 2026

rustbot assigned mati865 Sep 17, 2026

cyrgani commented Sep 17, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 17, 2026

rust-bors Bot pushed a commit that referenced this pull request Sep 17, 2026

Auto merge of #162925 - cyrgani:builtin-macros-4, r=<try>

34797f1

even more cleanups for `rustc_builtin_macros`

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 18, 2026

nnethercote assigned nnethercote and unassigned mati865 Sep 19, 2026

nnethercote reviewed Sep 19, 2026

View reviewed changes

cyrgani added 4 commits September 19, 2026 13:27

stop passing nonself_arg_tys around

3b433cb

optimize Path::to_path

d7a86e4

just use Self instead of writing the type out

c50e0f0

remove only recursively used arguments

072a61a

rust-cloud-vms Bot force-pushed the builtin-macros-4 branch from c1b6f1c to 072a61a Compare September 19, 2026 18:42

rust-bors Bot 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 Sep 19, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Sep 20, 2026

Auto merge of #162925 - cyrgani:builtin-macros-4, r=nnethercote

b0e93f1

even more cleanups for `rustc_builtin_macros` Followup to #162234 with more cleanups and perf improvements.

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Sep 20, 2026

Auto merge of #162925 - cyrgani:builtin-macros-4, r=<try>

dd284bd

even more cleanups for `rustc_builtin_macros` try-job: test-x86_64-gnu-parallel-frontend

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Sep 20, 2026

Auto merge of #162925 - cyrgani:builtin-macros-4, r=nnethercote

8c9f275

even more cleanups for `rustc_builtin_macros` Followup to #162234 with more cleanups and perf improvements.

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Sep 20, 2026

Auto merge of #162925 - cyrgani:builtin-macros-4, r=<try>

7a3dc2c

even more cleanups for `rustc_builtin_macros` try-job: test-various

This comment has been minimized.

Sign in to view

rust-bors Bot added merged-by-bors This PR was explicitly merged by bors. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 20, 2026

rust-bors Bot merged commit bba5310 into rust-lang:main Sep 20, 2026
15 checks passed

rustbot added this to the 1.100.0 milestone Sep 20, 2026

This was referenced Sep 20, 2026

expanding #[rustc_trivial_field_reads] to more targets, place on Derive and Clone #160666

Draft

Do not continue past rustc_resolve when encountering duplicated items #162831

Open

cyrgani deleted the builtin-macros-4 branch September 20, 2026 20:21

cyrgani mentioned this pull request Sep 20, 2026

rustc_builtin_macros cleanup, part 5 #163080

Open

Uh oh!

Conversation

cyrgani commented Sep 17, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Sep 17, 2026

Uh oh!

rustbot commented Sep 17, 2026

Uh oh!

cyrgani Sep 17, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clarfonthey commented Sep 17, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors Bot commented Sep 17, 2026

Uh oh!

This comment has been minimized.

nnethercote commented Sep 17, 2026

Uh oh!

rust-timer commented Sep 18, 2026

Overall result: ✅ improvements - no action needed

Uh oh!

nnethercote left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cyrgani commented Sep 19, 2026

Uh oh!

nnethercote commented Sep 19, 2026

Uh oh!

rust-bors Bot commented Sep 19, 2026

Uh oh!

This comment has been minimized.

jhpratt commented Sep 20, 2026

Uh oh!

rust-bors Bot commented Sep 20, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors Bot commented Sep 20, 2026

Uh oh!

Zalathar commented Sep 20, 2026

Uh oh!

rust-bors Bot commented Sep 20, 2026

Uh oh!

This comment has been minimized.

rust-bors Bot commented Sep 20, 2026

Uh oh!

This comment has been minimized.

rust-bors Bot commented Sep 20, 2026

Uh oh!

Uh oh!

github-actions Bot commented Sep 20, 2026

Test differences

Job duration changes

Uh oh!

rust-timer commented Sep 20, 2026

Overall result: ❌✅ regressions and improvements - no action needed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

cyrgani commented Sep 17, 2026 •

edited by rustbot

Loading

cyrgani Sep 17, 2026 •

edited by rustbot

Loading

nnethercote left a comment •

edited

Loading