Split SpannedTypeVisitor into its own crate, rustc_ty_walk

[joshtriplett]

View all comments

rustc_privacy pulls in all of rustc_ty_utils, but only wants SpannedTypeVisitor and walk_types. Move those to a small rustc_ty_walk crate.

This allows rustc_privacy to start building much sooner.

[rustbot]

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

[rustbot]

r? @fee1-dead

rustbot has assigned @fee1-dead.
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 75 candidates
• Random selection from 18 candidates

[mejrs]

You need to bless bootstrap, see https://github.com/rust-lang/rust/blob/main/src/bootstrap/README.md#testing

[joshtriplett]

@rustbot ready

[mejrs]

I think we should consider splitting up rustc_ty_utils (perhaps poorly named if it's just providing queries now?) further:

crate A: Queries that need rustc_trait_selection
crate B: Queries that only need rustc_middle (or perhaps rustc_infer - haven't thought about what the best split is here)
crate C: What this pr calls rustc_ty_walk - the actual util stuff shared between A and B and other crates. Maybe there's also stuff currently defined in rustc_middle that could be moved here.

[joshtriplett]

@mejrs That sounds reasonable. But also, I'm a big fan of incremental steps; we can always split/merge crates further after this. :)

[mejrs]

@mejrs That sounds reasonable. But also, I'm a big fan of incremental steps; we can always split/merge crates further after this. :)

Yes! Please keep this PR small, I'm not suggesting it is done right here and now (hence, just "considering" 🙂).

[rustbot]

This PR was rebased onto a different main 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.

[joshtriplett]

Rebased.

[fee1-dead]

Thanks!

@bors r+ rollup

[rust-bors]

📌 Commit 0f80d01 has been approved by fee1-dead

It is now in the queue for this repository.

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

Reason for tree closure: manually handling queue due to backlog

[JonathanBrouwer]

@rust-timer build 3a4256c

[rust-timer]

Finished benchmarking commit (3a4256c): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

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.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

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

Max RSS (memory usage)

Results (primary 4.9%, secondary 25.1%)

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

	mean	range	count
Regressions ❌ (primary)	4.9%	[2.0%, 7.9%]	2
Regressions ❌ (secondary)	25.1%	[1.0%, 30.8%]	13
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.9%	[2.0%, 7.9%]	2

Cycles

Results (primary 7.1%, secondary 12.1%)

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

	mean	range	count
Regressions ❌ (primary)	7.1%	[6.7%, 7.5%]	2
Regressions ❌ (secondary)	16.4%	[3.3%, 30.6%]	15
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-9.4%	[-14.3%, -3.6%]	3
All ❌✅ (primary)	7.1%	[6.7%, 7.5%]	2

Binary size

Results (primary 0.0%, 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.0%	[0.0%, 0.0%]	20
Regressions ❌ (secondary)	0.0%	[0.0%, 0.1%]	5
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.0%]	20

Bootstrap: 454.551s -> 457.039s (0.55%)
Artifact size: 399.93 MiB -> 399.13 MiB (-0.20%)

[JonathanBrouwer]

All PRs in the rollup are getting this identical regression, this is most likely a false positive

[joshtriplett]

View all comments

rustc_privacy pulls in all of rustc_ty_utils, but only wants SpannedTypeVisitor and walk_types. Move those to a small rustc_ty_walk crate.

This allows rustc_privacy to start building much sooner.

[rustbot]

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

[rustbot]

r? @fee1-dead

rustbot has assigned @fee1-dead.
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 75 candidates
• Random selection from 18 candidates

[mejrs]

You need to bless bootstrap, see https://github.com/rust-lang/rust/blob/main/src/bootstrap/README.md#testing

[joshtriplett]

@rustbot ready

[mejrs]

I think we should consider splitting up rustc_ty_utils (perhaps poorly named if it's just providing queries now?) further:

crate A: Queries that need rustc_trait_selection
crate B: Queries that only need rustc_middle (or perhaps rustc_infer - haven't thought about what the best split is here)
crate C: What this pr calls rustc_ty_walk - the actual util stuff shared between A and B and other crates. Maybe there's also stuff currently defined in rustc_middle that could be moved here.

[joshtriplett]

@mejrs That sounds reasonable. But also, I'm a big fan of incremental steps; we can always split/merge crates further after this. :)

[mejrs]

@mejrs That sounds reasonable. But also, I'm a big fan of incremental steps; we can always split/merge crates further after this. :)

Yes! Please keep this PR small, I'm not suggesting it is done right here and now (hence, just "considering" 🙂).

[rustbot]

This PR was rebased onto a different main 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.

[joshtriplett]

Rebased.

[fee1-dead]

Thanks!

@bors r+ rollup

[rust-bors]

📌 Commit 0f80d01 has been approved by fee1-dead

It is now in the queue for this repository.

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

Reason for tree closure: manually handling queue due to backlog

[JonathanBrouwer]

@rust-timer build 3a4256c

[rust-timer]

Finished benchmarking commit (3a4256c): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

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.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

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

Max RSS (memory usage)

Results (primary 4.9%, secondary 25.1%)

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

	mean	range	count
Regressions ❌ (primary)	4.9%	[2.0%, 7.9%]	2
Regressions ❌ (secondary)	25.1%	[1.0%, 30.8%]	13
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.9%	[2.0%, 7.9%]	2

Cycles

Results (primary 7.1%, secondary 12.1%)

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

	mean	range	count
Regressions ❌ (primary)	7.1%	[6.7%, 7.5%]	2
Regressions ❌ (secondary)	16.4%	[3.3%, 30.6%]	15
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-9.4%	[-14.3%, -3.6%]	3
All ❌✅ (primary)	7.1%	[6.7%, 7.5%]	2

Binary size

Results (primary 0.0%, 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.0%	[0.0%, 0.0%]	20
Regressions ❌ (secondary)	0.0%	[0.0%, 0.1%]	5
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.0%]	20

Bootstrap: 454.551s -> 457.039s (0.55%)
Artifact size: 399.93 MiB -> 399.13 MiB (-0.20%)

[JonathanBrouwer]

All PRs in the rollup are getting this identical regression, this is most likely a false positive

[joshtriplett]

View all comments

rustc_privacy pulls in all of rustc_ty_utils, but only wants SpannedTypeVisitor and walk_types. Move those to a small rustc_ty_walk crate.

This allows rustc_privacy to start building much sooner.

[rustbot]

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

[rustbot]

r? @fee1-dead

rustbot has assigned @fee1-dead.
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 75 candidates
• Random selection from 18 candidates

[mejrs]

You need to bless bootstrap, see https://github.com/rust-lang/rust/blob/main/src/bootstrap/README.md#testing

[joshtriplett]

@rustbot ready

[mejrs]

I think we should consider splitting up rustc_ty_utils (perhaps poorly named if it's just providing queries now?) further:

crate A: Queries that need rustc_trait_selection
crate B: Queries that only need rustc_middle (or perhaps rustc_infer - haven't thought about what the best split is here)
crate C: What this pr calls rustc_ty_walk - the actual util stuff shared between A and B and other crates. Maybe there's also stuff currently defined in rustc_middle that could be moved here.

[joshtriplett]

@mejrs That sounds reasonable. But also, I'm a big fan of incremental steps; we can always split/merge crates further after this. :)

[mejrs]

@mejrs That sounds reasonable. But also, I'm a big fan of incremental steps; we can always split/merge crates further after this. :)

Yes! Please keep this PR small, I'm not suggesting it is done right here and now (hence, just "considering" 🙂).

[rustbot]

This PR was rebased onto a different main 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.

[joshtriplett]

Rebased.

[fee1-dead]

Thanks!

@bors r+ rollup

[rust-bors]

📌 Commit 0f80d01 has been approved by fee1-dead

It is now in the queue for this repository.

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

Reason for tree closure: manually handling queue due to backlog

[JonathanBrouwer]

@rust-timer build 3a4256c

[rust-timer]

Finished benchmarking commit (3a4256c): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

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.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

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

Max RSS (memory usage)

Results (primary 4.9%, secondary 25.1%)

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

	mean	range	count
Regressions ❌ (primary)	4.9%	[2.0%, 7.9%]	2
Regressions ❌ (secondary)	25.1%	[1.0%, 30.8%]	13
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.9%	[2.0%, 7.9%]	2

Cycles

Results (primary 7.1%, secondary 12.1%)

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

	mean	range	count
Regressions ❌ (primary)	7.1%	[6.7%, 7.5%]	2
Regressions ❌ (secondary)	16.4%	[3.3%, 30.6%]	15
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-9.4%	[-14.3%, -3.6%]	3
All ❌✅ (primary)	7.1%	[6.7%, 7.5%]	2

Binary size

Results (primary 0.0%, 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.0%	[0.0%, 0.0%]	20
Regressions ❌ (secondary)	0.0%	[0.0%, 0.1%]	5
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.0%]	20

Bootstrap: 454.551s -> 457.039s (0.55%)
Artifact size: 399.93 MiB -> 399.13 MiB (-0.20%)

[JonathanBrouwer]

All PRs in the rollup are getting this identical regression, this is most likely a false positive

[joshtriplett]

View all comments

rustc_privacy pulls in all of rustc_ty_utils, but only wants SpannedTypeVisitor and walk_types. Move those to a small rustc_ty_walk crate.

This allows rustc_privacy to start building much sooner.

[rustbot]

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

[rustbot]

r? @fee1-dead

rustbot has assigned @fee1-dead.
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 75 candidates
• Random selection from 18 candidates

[mejrs]

You need to bless bootstrap, see https://github.com/rust-lang/rust/blob/main/src/bootstrap/README.md#testing

[joshtriplett]

@rustbot ready

[mejrs]

I think we should consider splitting up rustc_ty_utils (perhaps poorly named if it's just providing queries now?) further:

crate A: Queries that need rustc_trait_selection
crate B: Queries that only need rustc_middle (or perhaps rustc_infer - haven't thought about what the best split is here)
crate C: What this pr calls rustc_ty_walk - the actual util stuff shared between A and B and other crates. Maybe there's also stuff currently defined in rustc_middle that could be moved here.

[joshtriplett]

@mejrs That sounds reasonable. But also, I'm a big fan of incremental steps; we can always split/merge crates further after this. :)

[mejrs]

@mejrs That sounds reasonable. But also, I'm a big fan of incremental steps; we can always split/merge crates further after this. :)

Yes! Please keep this PR small, I'm not suggesting it is done right here and now (hence, just "considering" 🙂).

[rustbot]

This PR was rebased onto a different main 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.

[joshtriplett]

Rebased.

[fee1-dead]

Thanks!

@bors r+ rollup

[rust-bors]

📌 Commit 0f80d01 has been approved by fee1-dead

It is now in the queue for this repository.

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

Reason for tree closure: manually handling queue due to backlog

[JonathanBrouwer]

@rust-timer build 3a4256c

[rust-timer]

Finished benchmarking commit (3a4256c): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

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.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

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

Max RSS (memory usage)

Results (primary 4.9%, secondary 25.1%)

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

	mean	range	count
Regressions ❌ (primary)	4.9%	[2.0%, 7.9%]	2
Regressions ❌ (secondary)	25.1%	[1.0%, 30.8%]	13
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.9%	[2.0%, 7.9%]	2

Cycles

Results (primary 7.1%, secondary 12.1%)

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

	mean	range	count
Regressions ❌ (primary)	7.1%	[6.7%, 7.5%]	2
Regressions ❌ (secondary)	16.4%	[3.3%, 30.6%]	15
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-9.4%	[-14.3%, -3.6%]	3
All ❌✅ (primary)	7.1%	[6.7%, 7.5%]	2

Binary size

Results (primary 0.0%, 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.0%	[0.0%, 0.0%]	20
Regressions ❌ (secondary)	0.0%	[0.0%, 0.1%]	5
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.0%]	20

Bootstrap: 454.551s -> 457.039s (0.55%)
Artifact size: 399.93 MiB -> 399.13 MiB (-0.20%)

[JonathanBrouwer]

All PRs in the rollup are getting this identical regression, this is most likely a false positive

[joshtriplett]

View all comments

rustc_privacy pulls in all of rustc_ty_utils, but only wants SpannedTypeVisitor and walk_types. Move those to a small rustc_ty_walk crate.

This allows rustc_privacy to start building much sooner.

[rustbot]

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

[rustbot]

r? @fee1-dead

rustbot has assigned @fee1-dead.
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 75 candidates
• Random selection from 18 candidates

[mejrs]

You need to bless bootstrap, see https://github.com/rust-lang/rust/blob/main/src/bootstrap/README.md#testing

[joshtriplett]

@rustbot ready

[mejrs]

I think we should consider splitting up rustc_ty_utils (perhaps poorly named if it's just providing queries now?) further:

crate A: Queries that need rustc_trait_selection
crate B: Queries that only need rustc_middle (or perhaps rustc_infer - haven't thought about what the best split is here)
crate C: What this pr calls rustc_ty_walk - the actual util stuff shared between A and B and other crates. Maybe there's also stuff currently defined in rustc_middle that could be moved here.

[joshtriplett]

@mejrs That sounds reasonable. But also, I'm a big fan of incremental steps; we can always split/merge crates further after this. :)

[mejrs]

@mejrs That sounds reasonable. But also, I'm a big fan of incremental steps; we can always split/merge crates further after this. :)

Yes! Please keep this PR small, I'm not suggesting it is done right here and now (hence, just "considering" 🙂).

[rustbot]

This PR was rebased onto a different main 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.

[joshtriplett]

Rebased.

[fee1-dead]

Thanks!

@bors r+ rollup

[rust-bors]

📌 Commit 0f80d01 has been approved by fee1-dead

It is now in the queue for this repository.

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

Reason for tree closure: manually handling queue due to backlog

[JonathanBrouwer]

@rust-timer build 3a4256c

[rust-timer]

Finished benchmarking commit (3a4256c): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

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.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

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

Max RSS (memory usage)

Results (primary 4.9%, secondary 25.1%)

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

	mean	range	count
Regressions ❌ (primary)	4.9%	[2.0%, 7.9%]	2
Regressions ❌ (secondary)	25.1%	[1.0%, 30.8%]	13
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.9%	[2.0%, 7.9%]	2

Cycles

Results (primary 7.1%, secondary 12.1%)

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

	mean	range	count
Regressions ❌ (primary)	7.1%	[6.7%, 7.5%]	2
Regressions ❌ (secondary)	16.4%	[3.3%, 30.6%]	15
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-9.4%	[-14.3%, -3.6%]	3
All ❌✅ (primary)	7.1%	[6.7%, 7.5%]	2

Binary size

Results (primary 0.0%, 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.0%	[0.0%, 0.0%]	20
Regressions ❌ (secondary)	0.0%	[0.0%, 0.1%]	5
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.0%]	20

Bootstrap: 454.551s -> 457.039s (0.55%)
Artifact size: 399.93 MiB -> 399.13 MiB (-0.20%)

[JonathanBrouwer]

All PRs in the rollup are getting this identical regression, this is most likely a false positive

[joshtriplett]

View all comments

rustc_privacy pulls in all of rustc_ty_utils, but only wants SpannedTypeVisitor and walk_types. Move those to a small rustc_ty_walk crate.

This allows rustc_privacy to start building much sooner.

[rustbot]

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

[rustbot]

r? @fee1-dead

rustbot has assigned @fee1-dead.
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 75 candidates
• Random selection from 18 candidates

[mejrs]

You need to bless bootstrap, see https://github.com/rust-lang/rust/blob/main/src/bootstrap/README.md#testing

[joshtriplett]

@rustbot ready

[mejrs]

I think we should consider splitting up rustc_ty_utils (perhaps poorly named if it's just providing queries now?) further:

crate A: Queries that need rustc_trait_selection
crate B: Queries that only need rustc_middle (or perhaps rustc_infer - haven't thought about what the best split is here)
crate C: What this pr calls rustc_ty_walk - the actual util stuff shared between A and B and other crates. Maybe there's also stuff currently defined in rustc_middle that could be moved here.

[joshtriplett]

@mejrs That sounds reasonable. But also, I'm a big fan of incremental steps; we can always split/merge crates further after this. :)

[mejrs]

@mejrs That sounds reasonable. But also, I'm a big fan of incremental steps; we can always split/merge crates further after this. :)

Yes! Please keep this PR small, I'm not suggesting it is done right here and now (hence, just "considering" 🙂).

[rustbot]

This PR was rebased onto a different main 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.

[joshtriplett]

Rebased.

[fee1-dead]

Thanks!

@bors r+ rollup

[rust-bors]

📌 Commit 0f80d01 has been approved by fee1-dead

It is now in the queue for this repository.

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

Reason for tree closure: manually handling queue due to backlog

[JonathanBrouwer]

@rust-timer build 3a4256c

[rust-timer]

Finished benchmarking commit (3a4256c): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

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.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

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

Max RSS (memory usage)

Results (primary 4.9%, secondary 25.1%)

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

	mean	range	count
Regressions ❌ (primary)	4.9%	[2.0%, 7.9%]	2
Regressions ❌ (secondary)	25.1%	[1.0%, 30.8%]	13
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.9%	[2.0%, 7.9%]	2

Cycles

Results (primary 7.1%, secondary 12.1%)

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

	mean	range	count
Regressions ❌ (primary)	7.1%	[6.7%, 7.5%]	2
Regressions ❌ (secondary)	16.4%	[3.3%, 30.6%]	15
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-9.4%	[-14.3%, -3.6%]	3
All ❌✅ (primary)	7.1%	[6.7%, 7.5%]	2

Binary size

Results (primary 0.0%, 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.0%	[0.0%, 0.0%]	20
Regressions ❌ (secondary)	0.0%	[0.0%, 0.1%]	5
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.0%]	20

Bootstrap: 454.551s -> 457.039s (0.55%)
Artifact size: 399.93 MiB -> 399.13 MiB (-0.20%)

[JonathanBrouwer]

All PRs in the rollup are getting this identical regression, this is most likely a false positive

[joshtriplett]

View all comments

rustc_privacy pulls in all of rustc_ty_utils, but only wants SpannedTypeVisitor and walk_types. Move those to a small rustc_ty_walk crate.

This allows rustc_privacy to start building much sooner.

[rustbot]

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

[rustbot]

r? @fee1-dead

rustbot has assigned @fee1-dead.
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 75 candidates
• Random selection from 18 candidates

[mejrs]

You need to bless bootstrap, see https://github.com/rust-lang/rust/blob/main/src/bootstrap/README.md#testing

[joshtriplett]

@rustbot ready

[mejrs]

I think we should consider splitting up rustc_ty_utils (perhaps poorly named if it's just providing queries now?) further:

crate A: Queries that need rustc_trait_selection
crate B: Queries that only need rustc_middle (or perhaps rustc_infer - haven't thought about what the best split is here)
crate C: What this pr calls rustc_ty_walk - the actual util stuff shared between A and B and other crates. Maybe there's also stuff currently defined in rustc_middle that could be moved here.

[joshtriplett]

@mejrs That sounds reasonable. But also, I'm a big fan of incremental steps; we can always split/merge crates further after this. :)

[mejrs]

@mejrs That sounds reasonable. But also, I'm a big fan of incremental steps; we can always split/merge crates further after this. :)

Yes! Please keep this PR small, I'm not suggesting it is done right here and now (hence, just "considering" 🙂).

[rustbot]

This PR was rebased onto a different main 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.

[joshtriplett]

Rebased.

[fee1-dead]

Thanks!

@bors r+ rollup

[rust-bors]

📌 Commit 0f80d01 has been approved by fee1-dead

It is now in the queue for this repository.

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

Reason for tree closure: manually handling queue due to backlog

[JonathanBrouwer]

@rust-timer build 3a4256c

[rust-timer]

Finished benchmarking commit (3a4256c): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

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.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

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

Max RSS (memory usage)

Results (primary 4.9%, secondary 25.1%)

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

	mean	range	count
Regressions ❌ (primary)	4.9%	[2.0%, 7.9%]	2
Regressions ❌ (secondary)	25.1%	[1.0%, 30.8%]	13
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.9%	[2.0%, 7.9%]	2

Cycles

Results (primary 7.1%, secondary 12.1%)

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

	mean	range	count
Regressions ❌ (primary)	7.1%	[6.7%, 7.5%]	2
Regressions ❌ (secondary)	16.4%	[3.3%, 30.6%]	15
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-9.4%	[-14.3%, -3.6%]	3
All ❌✅ (primary)	7.1%	[6.7%, 7.5%]	2

Binary size

Results (primary 0.0%, 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.0%	[0.0%, 0.0%]	20
Regressions ❌ (secondary)	0.0%	[0.0%, 0.1%]	5
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.0%]	20

Bootstrap: 454.551s -> 457.039s (0.55%)
Artifact size: 399.93 MiB -> 399.13 MiB (-0.20%)

[JonathanBrouwer]

All PRs in the rollup are getting this identical regression, this is most likely a false positive

[joshtriplett]

View all comments

rustc_privacy pulls in all of rustc_ty_utils, but only wants SpannedTypeVisitor and walk_types. Move those to a small rustc_ty_walk crate.

This allows rustc_privacy to start building much sooner.

[rustbot]

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

[rustbot]

r? @fee1-dead

rustbot has assigned @fee1-dead.
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 75 candidates
• Random selection from 18 candidates

[mejrs]

You need to bless bootstrap, see https://github.com/rust-lang/rust/blob/main/src/bootstrap/README.md#testing

[joshtriplett]

@rustbot ready

[mejrs]

I think we should consider splitting up rustc_ty_utils (perhaps poorly named if it's just providing queries now?) further:

crate A: Queries that need rustc_trait_selection
crate B: Queries that only need rustc_middle (or perhaps rustc_infer - haven't thought about what the best split is here)
crate C: What this pr calls rustc_ty_walk - the actual util stuff shared between A and B and other crates. Maybe there's also stuff currently defined in rustc_middle that could be moved here.

[joshtriplett]

@mejrs That sounds reasonable. But also, I'm a big fan of incremental steps; we can always split/merge crates further after this. :)

[mejrs]

@mejrs That sounds reasonable. But also, I'm a big fan of incremental steps; we can always split/merge crates further after this. :)

Yes! Please keep this PR small, I'm not suggesting it is done right here and now (hence, just "considering" 🙂).

[rustbot]

This PR was rebased onto a different main 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.

[joshtriplett]

Rebased.

[fee1-dead]

Thanks!

@bors r+ rollup

[rust-bors]

📌 Commit 0f80d01 has been approved by fee1-dead

It is now in the queue for this repository.

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

Reason for tree closure: manually handling queue due to backlog

[JonathanBrouwer]

@rust-timer build 3a4256c

[rust-timer]

Finished benchmarking commit (3a4256c): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

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.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

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

Max RSS (memory usage)

Results (primary 4.9%, secondary 25.1%)

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

	mean	range	count
Regressions ❌ (primary)	4.9%	[2.0%, 7.9%]	2
Regressions ❌ (secondary)	25.1%	[1.0%, 30.8%]	13
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.9%	[2.0%, 7.9%]	2

Cycles

Results (primary 7.1%, secondary 12.1%)

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

	mean	range	count
Regressions ❌ (primary)	7.1%	[6.7%, 7.5%]	2
Regressions ❌ (secondary)	16.4%	[3.3%, 30.6%]	15
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-9.4%	[-14.3%, -3.6%]	3
All ❌✅ (primary)	7.1%	[6.7%, 7.5%]	2

Binary size

Results (primary 0.0%, 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.0%	[0.0%, 0.0%]	20
Regressions ❌ (secondary)	0.0%	[0.0%, 0.1%]	5
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.0%]	20

Bootstrap: 454.551s -> 457.039s (0.55%)
Artifact size: 399.93 MiB -> 399.13 MiB (-0.20%)

[JonathanBrouwer]

All PRs in the rollup are getting this identical regression, this is most likely a false positive