Skip to content

rustc_builtin_macros cleanup, part 8 - #163485

Merged
rust-bors[bot] merged 16 commits into
rust-lang:mainfrom
cyrgani:builtin-macros-8
Oct 21, 2026
Merged

rust-bors[bot] merged 16 commits into
rust-lang:mainfrom
cyrgani:builtin-macros-8

Conversation

@cyrgani

@cyrgani cyrgani commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

View all comments

This PR is roughly split into three parts:

  • The first few commits add helpers to ExtCtxt to both construct some common AST elements in an easier way. They also change some Ident arguments to Symbol to avoid the annoying call-site pattern of repeating the span (cx.some_method(span, Ident::new(name, span), ...)).
  • The next few commits focus on the derive infrastructure cleanup a bit.
  • The further following commits make Path construction faster and remove the new_path remnant of the old local path version.

Previous PR: #163389

r? @nnethercote

@rustbot rustbot added 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 Sep 29, 2026
@jieyouxu

Copy link
Copy Markdown
Member

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

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

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Sep 29, 2026
[PERF] `rustc_builtin_macros` cleanup, part 8
Comment thread compiler/rustc_expand/src/base.rs Outdated
@rust-bors

rust-bors Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 0bcf40d (0bcf40dc0c6b1a2253f428ebaa94e0e54aa98617)
Base parent: b46c90c (b46c90c5619a16a0c7e4d0f4e9e1ad4db4cf106e)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (0bcf40d): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@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
Improvements ✅
(secondary)
-0.2% [-0.3%, -0.2%] 7
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary 0.7%, secondary 2.1%)

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

mean range count
Regressions ❌
(primary)
2.2% [2.2%, 2.2%] 1
Regressions ❌
(secondary)
3.6% [3.4%, 3.8%] 3
Improvements ✅
(primary)
-0.9% [-0.9%, -0.9%] 1
Improvements ✅
(secondary)
-2.3% [-2.3%, -2.3%] 1
All ❌✅ (primary) 0.7% [-0.9%, 2.2%] 2

Cycles

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

Binary size

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

Bootstrap: 491.711s -> 491.207s (-0.10%)
Artifact size: 406.38 MiB -> 406.39 MiB (0.00%)

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

This comment has been minimized.

@cyrgani cyrgani changed the title [PERF] rustc_builtin_macros cleanup, part 8 rustc_builtin_macros cleanup, part 8 Oct 9, 2026
@cyrgani
cyrgani marked this pull request as ready for review October 9, 2026 12:23
@rustbot

rustbot commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in compiler/rustc_builtin_macros/src/autodiff.rs

cc @ZuseZ4

@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 Oct 9, 2026
Comment thread compiler/rustc_builtin_macros/src/deriving/generic/mod.rs Outdated

@nnethercote nnethercote left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think leaving path_std! and pathvec! seems fine, though perhaps the latter should be renamed path_vec!.

I don't have strong opinions about the substructure change, though the extra error messages are annoying.

View changes since this review

Comment thread compiler/rustc_expand/src/build.rs Outdated
Comment thread compiler/rustc_builtin_macros/src/alloc_error_handler.rs Outdated
Comment thread compiler/rustc_expand/src/build.rs Outdated
@nnethercote

Copy link
Copy Markdown
Contributor

@petrochenkov: do you have opinions about the first commit ("rename Legacy* macro kinds to Ast*")?

@nnethercote nnethercote 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 Oct 16, 2026
@petrochenkov

Copy link
Copy Markdown
Contributor

@petrochenkov: do you have opinions about the first commit ("rename Legacy* macro kinds to Ast*")?

Yes :D #163485 (comment)

As I understand it, this would only be needed if the crate defining the derive macros (which is `core`) was using edition 2015.
Since `core` uses edition 2024, there is in this case no difference between global and local and we can skip the leading `::`.
Previously, `std_path` converted the given `&[Symbol]` to a `Vec<Ident>` by prepending a `$crate::` and mapping each `Symbol` to an `Ident`.
This commit changes the functions that previously took `Vec<Ident>` to directly take `Path` instead, and updates `std_path` to also directly return a `Path`. This allows directly prepending the `$crate::` to the resulting `segments`.
It also allows constructing a `Path` from `&[Ident]` instead of `Vec<Ident>`.
`new_path` is just a obsolete trivial wrapper around `cx.std_path_all` now.
Passing around `type_ident` everywhere is what the old code did as well, but it's actually only needed in two places.
And rename other places that were called `ident` but took a `Symbol` to `name` too.
@cyrgani

cyrgani commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor Author

I added a commit that uses name instead of ident where appropriate and removed the two commits for the path macros and the broader use of Substructure for now.
@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 Oct 20, 2026
@nnethercote

Copy link
Copy Markdown
Contributor

Thanks!

@bors r+

@rust-bors

rust-bors Bot commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

📌 Commit a85d97f 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: spurious failures

@rust-bors 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 Oct 21, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Oct 21, 2026
…rcote

`rustc_builtin_macros` cleanup, part 8

This PR is roughly split into three parts:
- The first few commits add helpers to `ExtCtxt` to both construct some common AST elements in an easier way. They also change some `Ident` arguments to `Symbol` to avoid the annoying call-site pattern of repeating the span (`cx.some_method(span, Ident::new(name, span), ...)`).
- The next few commits focus on the derive infrastructure cleanup a bit.
- The further following commits make `Path` construction faster and remove the `new_path` remnant of the old local path version.

Previous PR: rust-lang#163389

r? @nnethercote
rust-bors Bot pushed a commit that referenced this pull request Oct 21, 2026
…uwer

Rollup of 8 pull requests

Successful merges:

 - #162460 (Feat - `#[inline(always)] + #[target_feature(enable = "....")]` #2)
 - #163485 (`rustc_builtin_macros` cleanup, part 8)
 - #164578 (Don't ignore unknown LLVM diagnostics)
 - #162369 (Document the `recursion_limit` built-in attribute)
 - #162840 (tier 3 target policy: target CPU and features must be supported by upstream backend)
 - #164330 (Skip redundant monomorphization checks)
 - #164656 (stabilize ExitCodeExt for Windows)
 - #164660 (Touch up attribute docs)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Oct 21, 2026
…rcote

`rustc_builtin_macros` cleanup, part 8

This PR is roughly split into three parts:
- The first few commits add helpers to `ExtCtxt` to both construct some common AST elements in an easier way. They also change some `Ident` arguments to `Symbol` to avoid the annoying call-site pattern of repeating the span (`cx.some_method(span, Ident::new(name, span), ...)`).
- The next few commits focus on the derive infrastructure cleanup a bit.
- The further following commits make `Path` construction faster and remove the `new_path` remnant of the old local path version.

Previous PR: rust-lang#163389

r? @nnethercote
@rust-bors

rust-bors Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

⌛ Testing commit a85d97f with merge ae9d6d5...

Workflow: https://github.com/rust-lang/rust/actions/runs/39634220880

rust-bors Bot pushed a commit that referenced this pull request Oct 21, 2026
`rustc_builtin_macros` cleanup, part 8

This PR is roughly split into three parts: 
- The first few commits add helpers to `ExtCtxt` to both construct some common AST elements in an easier way. They also change some `Ident` arguments to `Symbol` to avoid the annoying call-site pattern of repeating the span (`cx.some_method(span, Ident::new(name, span), ...)`).
- The next few commits focus on the derive infrastructure cleanup a bit.
- The further following commits make `Path` construction faster and remove the `new_path` remnant of the old local path version.

Previous PR: #163389

r? @nnethercote
@JonathanBrouwer

Copy link
Copy Markdown
Member

@bors yield

@rust-bors

rust-bors Bot commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Auto build was cancelled. Cancelled workflows:

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

rust-bors Bot pushed a commit that referenced this pull request Oct 21, 2026
…uwer

Rollup of 13 pull requests

Successful merges:

 - #162460 (Feat - `#[inline(always)] + #[target_feature(enable = "....")]` #2)
 - #163485 (`rustc_builtin_macros` cleanup, part 8)
 - #164457 (Add `vec_visualizer` lldb test)
 - #161253 (Implement `AsMut<T>` and `AsRef<T>` for `!`.)
 - #162369 (Document the `recursion_limit` built-in attribute)
 - #162840 (tier 3 target policy: target CPU and features must be supported by upstream backend)
 - #164329 (use default_field_values in declare_lint)
 - #164330 (Skip redundant monomorphization checks)
 - #164527 (doc: added addtional docs for Write)
 - #164656 (stabilize ExitCodeExt for Windows)
 - #164660 (Touch up attribute docs)
 - #164681 (tidy: use repository-relative path for style checks)
 - #164688 (re-enable test_dir_remove_dir test on QNX)
rust-bors Bot pushed a commit that referenced this pull request Oct 21, 2026
Rollup merge of #163485 - cyrgani:builtin-macros-8, r=nnethercote

`rustc_builtin_macros` cleanup, part 8

This PR is roughly split into three parts:
- The first few commits add helpers to `ExtCtxt` to both construct some common AST elements in an easier way. They also change some `Ident` arguments to `Symbol` to avoid the annoying call-site pattern of repeating the span (`cx.some_method(span, Ident::new(name, span), ...)`).
- The next few commits focus on the derive infrastructure cleanup a bit.
- The further following commits make `Path` construction faster and remove the `new_path` remnant of the old local path version.

Previous PR: #163389

r? @nnethercote
@rust-bors
rust-bors Bot merged commit 2ede6da into rust-lang:main Oct 21, 2026
14 of 15 checks passed
@rustbot rustbot added this to the 1.101.0 milestone Oct 21, 2026
@cyrgani
cyrgani deleted the builtin-macros-8 branch October 21, 2026 20:15
@rust-timer

Copy link
Copy Markdown
Collaborator

Note

This PR was benchmarked as part of triage of its containing rollup: triage URL.

Finished benchmarking commit (6351b4b): comparison URL.

Overall result: ✅ 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
Improvements ✅
(primary)
-0.1% [-0.1%, -0.1%] 2
Improvements ✅
(secondary)
-0.2% [-0.3%, -0.1%] 11
All ❌✅ (primary) -0.1% [-0.1%, -0.1%] 2

Max RSS (memory usage)

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

Cycles

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

Binary size

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

Artifact size: 406.57 MiB -> 406.49 MiB (-0.02%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

7 participants