Reduce `ast::ptr::P` to a typedef of `Box` #141603

nnethercote · 2025-05-26T17:00:57Z

As per the MCP at rust-lang/compiler-team#878.

r? @fee1-dead

These date back to 2014. I don't think they're needed any more.

It's barely used, and the places that use it are better if they don't.

A `Vec` is fine, the additional word (vector vs. boxed slice) doesn't matter here.

It's no longer used.

nnethercote · 2025-05-26T21:36:37Z

@bors try @rust-timer queue

bors · 2025-05-26T21:37:47Z

⌛ Trying commit fc6c96f with merge 968f45f...

Reduce `ast::ptr::P` to a typedef of `Box` r? `@ghost`

bors · 2025-05-26T23:44:57Z

☀️ Try build successful - checks-actions
Build commit: 968f45f (968f45f8d7910a081aab4cb3891801bc88a9bba6)

rust-timer · 2025-05-27T01:22:30Z

Finished benchmarking commit (968f45f): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

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

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.2%, -0.1%]	6
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 1.8%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

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

Cycles

Results (secondary -1.3%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.3%	[-1.4%, -1.2%]	2
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 778.13s -> 778.268s (0.02%)
Artifact size: 366.42 MiB -> 366.39 MiB (-0.01%)

nnethercote · 2025-05-27T03:18:18Z

Perf effect is somewhere between neutral and slightly improved.

Keep the `P` constructor function for now, to minimize immediate churn. All the `into_inner` calls are removed, which is nice.

fee1-dead · 2025-05-27T06:29:58Z

If you need a reviewer for after the MCP passes, I can help

nnethercote · 2025-05-27T07:46:21Z

Waiting on approval for rust-lang/compiler-team#878.

rustbot · 2025-06-06T04:35:24Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

nnethercote · 2025-06-06T04:35:42Z

The MCP has been approved, this is ready to proceed.

fee1-dead

LGTM

fee1-dead · 2025-06-06T04:55:53Z

compiler/rustc_builtin_macros/src/test.rs

            // FIXME: Use an 'if let' guard once they are implemented
-            if let ast::StmtKind::Item(i) = stmt.into_inner().kind {
-                (i, true)
-            } else {
-                unreachable!()
-            }
+            if let ast::StmtKind::Item(i) = stmt.kind { (i, true) } else { unreachable!() }


This can be fixed. I'll do so in a followup.

fee1-dead · 2025-06-06T04:56:49Z

compiler/rustc_parse/src/parser/diagnostics.rs

+                        if matches!(inner_pat.clone().kind, PatKind::Ident(..)) =>
                    {
-                        match inner_pat.clone().into_inner().kind {
+                        match inner_pat.clone().kind {


fee1-dead · 2025-06-06T04:58:13Z

@bors r+ rollup

bors · 2025-06-06T04:58:16Z

📌 Commit 991c91f has been approved by fee1-dead

It is now in the queue for this repository.

Rollup of 11 pull requests Successful merges: - #140418 (Reexport types from `c_size_t` in `std`) - #141471 (unsafe keyword docs: emphasize that an unsafe fn in a trait does not get to choose its safety contract) - #141603 (Reduce `ast::ptr::P` to a typedef of `Box`) - #142043 (Verbose suggestion to make param `const`) - #142086 (duduplicate more AST visitor methods) - #142103 (Update `InterpCx::project_field` to take `FieldIdx`) - #142105 (remove extraneous text) - #142112 (fix typo) - #142113 (Reduce confusion of some drop order tests) - #142114 (Compute number of digits instead of relying on constant value for u128 display code) - #142118 (rustc_lexer: typo fix + small cleanups) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #141603 - nnethercote:reduce-P, r=fee1-dead Reduce `ast::ptr::P` to a typedef of `Box` As per the MCP at rust-lang/compiler-team#878. r? `@fee1-dead`

Rollup of 11 pull requests Successful merges: - rust-lang/rust#140418 (Reexport types from `c_size_t` in `std`) - rust-lang/rust#141471 (unsafe keyword docs: emphasize that an unsafe fn in a trait does not get to choose its safety contract) - rust-lang/rust#141603 (Reduce `ast::ptr::P` to a typedef of `Box`) - rust-lang/rust#142043 (Verbose suggestion to make param `const`) - rust-lang/rust#142086 (duduplicate more AST visitor methods) - rust-lang/rust#142103 (Update `InterpCx::project_field` to take `FieldIdx`) - rust-lang/rust#142105 (remove extraneous text) - rust-lang/rust#142112 (fix typo) - rust-lang/rust#142113 (Reduce confusion of some drop order tests) - rust-lang/rust#142114 (Compute number of digits instead of relying on constant value for u128 display code) - rust-lang/rust#142118 (rustc_lexer: typo fix + small cleanups) r? `@ghost` `@rustbot` modify labels: rollup

Reduce `ast::ptr::P` to a typedef of `Box` As per the MCP at rust-lang/compiler-team#878. r? `@fee1-dead`

nnethercote added 5 commits May 27, 2025 02:07

Remove 'static bounds on P.

a35675d

These date back to 2014. I don't think they're needed any more.

Remove P::map.

6973fa0

It's barely used, and the places that use it are better if they don't.

Remove the one use of P<[T]>.

0f285e3

A `Vec` is fine, the additional word (vector vs. boxed slice) doesn't matter here.

Remove support for P<[T]>.

6859750

It's no longer used.

Remove unused P stuff.

c42d1fc

rustbot added the T-compiler label May 26, 2025

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf label May 26, 2025

bors added a commit that referenced this pull request May 26, 2025

Auto merge of #141603 - nnethercote:reduce-P, r=<try>

968f45f

Reduce `ast::ptr::P` to a typedef of `Box` r? `@ghost`

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf label May 27, 2025

nnethercote mentioned this pull request May 27, 2025

Eliminate ast::ptr::P rust-lang/compiler-team#878

Closed

3 tasks

Reduce P<T> to a typedef of Box<T>.

991c91f

Keep the `P` constructor function for now, to minimize immediate churn. All the `into_inner` calls are removed, which is nice.

nnethercote force-pushed the reduce-P branch from fc6c96f to 991c91f Compare May 27, 2025 03:29

rustbot assigned fee1-dead Jun 6, 2025

nnethercote marked this pull request as ready for review June 6, 2025 04:35

rustbot added the S-waiting-on-review label Jun 6, 2025

fee1-dead approved these changes Jun 6, 2025

View reviewed changes

bors removed the S-waiting-on-review label Jun 6, 2025

bors added the S-waiting-on-bors label Jun 6, 2025

GuillaumeGomez mentioned this pull request Jun 6, 2025

Rollup of 11 pull requests #142133

Merged

bors merged commit 93ca0af into rust-lang:master Jun 7, 2025
7 checks passed

rustbot added this to the 1.89.0 milestone Jun 7, 2025

rust-timer added a commit that referenced this pull request Jun 7, 2025

Unrolled build for #141603

4c6f647

Rollup merge of #141603 - nnethercote:reduce-P, r=fee1-dead Reduce `ast::ptr::P` to a typedef of `Box` As per the MCP at rust-lang/compiler-team#878. r? `@fee1-dead`

nnethercote deleted the reduce-P branch June 7, 2025 06:40

flip1995 pushed a commit to flip1995/rust that referenced this pull request Jun 13, 2025

Rollup merge of rust-lang#141603 - nnethercote:reduce-P, r=fee1-dead

507183d

Reduce `ast::ptr::P` to a typedef of `Box` As per the MCP at rust-lang/compiler-team#878. r? `@fee1-dead`

Reduce ast::ptr::P to a typedef of Box #141603

Reduce ast::ptr::P to a typedef of Box #141603

Uh oh!

Conversation

nnethercote commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nnethercote commented May 26, 2025

Uh oh!

This comment has been minimized.

bors commented May 26, 2025

Uh oh!

bors commented May 26, 2025

Uh oh!

This comment has been minimized.

rust-timer commented May 27, 2025

Overall result: ✅ improvements - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

nnethercote commented May 27, 2025

Uh oh!

fee1-dead commented May 27, 2025

Uh oh!

nnethercote commented May 27, 2025

Uh oh!

rustbot commented Jun 6, 2025

Uh oh!

nnethercote commented Jun 6, 2025

Uh oh!

fee1-dead left a comment

Choose a reason for hiding this comment

Uh oh!

fee1-dead Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

fee1-dead Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

fee1-dead commented Jun 6, 2025

Uh oh!

bors commented Jun 6, 2025

Uh oh!

Uh oh!

Uh oh!

Reduce `ast::ptr::P` to a typedef of `Box` #141603

Reduce `ast::ptr::P` to a typedef of `Box` #141603

nnethercote commented May 26, 2025 •

edited

Loading