Always compute coroutine layout for eagerly emitting recursive layout errors #136073

compiler-errors · 2025-01-25T23:20:56Z

Detect recursive coroutine layouts even if we don't detect opaque type recursion in the new solver. This is for two reasons:

It helps us detect (bad) recursive async function calls in the new solver, which due to its approach to normalization causes us to not detect this via a recursive RPIT (since the opaques are more eagerly revealed in the opaque body).
- Fixes recursive async functions no longer error in check builds trait-system-refactor-initiative#137.
It helps us detect (bad) recursive async functions behind AFITs. See the AFIT test that changed for the old solver too.
It also greatly simplifies the recursive impl trait check, since I can remove some jankness around how it handles coroutines.

compiler-errors · 2025-01-25T23:21:05Z

@bors try @rust-timer queue

bors · 2025-01-25T23:22:16Z

⌛ Trying commit 9db9cf2 with merge c104bc5...

…s, r=<try> Always compute coroutine layout when checking for recursive opaques r? `@ghost`

bors · 2025-01-26T01:11:12Z

☀️ Try build successful - checks-actions
Build commit: c104bc5 (c104bc51797b06eb29cd2555f79c3ac9f394784c)

rust-timer · 2025-01-26T03:21:45Z

Finished benchmarking commit (c104bc5): comparison URL.

Overall result: ❌ regressions - please read the text below

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.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@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)	2.6%	[0.4%, 5.9%]	12
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 1.3%, secondary 2.4%)

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.3%	[0.7%, 2.0%]	8
Regressions ❌ (secondary)	2.4%	[2.1%, 2.7%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.3%	[0.7%, 2.0%]	8

Cycles

Results (secondary 4.1%)

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)	4.1%	[2.5%, 7.0%]	6
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 771.158s -> 772.043s (0.11%)
Artifact size: 325.82 MiB -> 325.82 MiB (0.00%)

compiler-errors · 2025-01-26T03:36:49Z

I still think we should land this even tho it has regressions on some pathological tests.

We need it to detect some recursive coroutines in the new solver.

I'll add more info, add some tests, and take this out of draft.

compiler-errors · 2025-01-27T19:16:04Z

@bors try @rust-timer queue

bors · 2025-01-27T19:17:15Z

⌛ Trying commit d75d180 with merge 313ad1f...

…s, r=<try> Always compute coroutine layout when checking for recursive opaques r? `@ghost`

bors · 2025-01-27T21:05:43Z

☀️ Try build successful - checks-actions
Build commit: 313ad1f (313ad1f53edbaaf88cd78023244a12e4af14c352)

rust-timer · 2025-01-27T23:06:18Z

Finished benchmarking commit (313ad1f): comparison URL.

Overall result: ❌ regressions - please read the text below

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.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@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)	4.7%	[0.5%, 8.0%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (secondary 2.2%)

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

Cycles

Results (secondary 8.4%)

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)	8.4%	[7.4%, 9.4%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 772.206s -> 774.281s (0.27%)
Artifact size: 328.24 MiB -> 328.18 MiB (-0.02%)

compiler-errors · 2025-01-27T23:10:24Z

This is only a regression for check builds on a pathological test for coroutines. Notably it doesn't affect full builds (at least from what I could tell from the results).

I think that we should otherwise land this; see PR description for why.

compiler-errors · 2025-01-27T23:10:47Z

r? @oli-obk (or @lcnr if they're around)

oli-obk · 2025-01-28T08:43:09Z

nice!

@bors r+

bors · 2025-01-28T08:43:12Z

📌 Commit d75d180 has been approved by oli-obk

It is now in the queue for this repository.

bors · 2025-01-30T18:16:06Z

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

bors · 2025-02-01T23:42:43Z

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

git checkout recursive-coro-always (switch to your branch)
git fetch upstream master (retrieve the latest master)
git rebase upstream/master -p (rebase on top of it)
Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
git push self recursive-coro-always --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message

Auto-merging compiler/rustc_interface/src/passes.rs
Auto-merging compiler/rustc_hir_analysis/src/check/check.rs
CONFLICT (content): Merge conflict in compiler/rustc_hir_analysis/src/check/check.rs
Automatic merge failed; fix conflicts and then commit the result.

compiler-errors · 2025-02-02T15:03:27Z

rebased

@bors r=oli-obk

bors · 2025-02-02T15:03:29Z

📌 Commit eedfece has been approved by oli-obk

It is now in the queue for this repository.

Noratrieb · 2025-02-03T11:55:19Z

@bors rollup=maybe basically perf neutral and queue is full

…ays, r=oli-obk Always compute coroutine layout for eagerly emitting recursive layout errors Detect recursive coroutine layouts even if we don't detect opaque type recursion in the new solver. This is for two reasons: 1. It helps us detect (bad) recursive async function calls in the new solver, which due to its approach to normalization causes us to not detect this via a recursive RPIT (since the opaques are more eagerly revealed in the opaque body). * Fixes rust-lang/trait-system-refactor-initiative#137. 2. It helps us detect (bad) recursive async functions behind AFITs. See the AFIT test that changed for the old solver too. 3. It also greatly simplifies the recursive impl trait check, since I can remove some jankness around how it handles coroutines.

matthiaskrgr · 2025-02-03T20:12:21Z

looks like this needs another rebase
@bors r-

lcnr · 2025-02-06T10:48:55Z

@bors r=oli-obk

bors · 2025-02-06T10:48:57Z

📌 Commit d0b0b02 has been approved by oli-obk

It is now in the queue for this repository.

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#136073 (Always compute coroutine layout for eagerly emitting recursive layout errors) - rust-lang#136235 (Pretty print pattern type values with transmute if they don't satisfy their pattern) - rust-lang#136311 (Ensure that we never try to monomorphize the upcasting or vtable calls of impossible dyn types) - rust-lang#136315 (Use short ty string for binop and unop errors) - rust-lang#136393 (Fix accidentally not emitting overflowing literals lints anymore in patterns) - rust-lang#136435 (Simplify some code for lowering THIR patterns) - rust-lang#136630 (Change two std process tests to not output to std{out,err}, and fix test suite stat reset in bootstrap CI test rendering) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 7 pull requests Successful merges: - rust-lang#136073 (Always compute coroutine layout for eagerly emitting recursive layout errors) - rust-lang#136235 (Pretty print pattern type values with transmute if they don't satisfy their pattern) - rust-lang#136311 (Ensure that we never try to monomorphize the upcasting or vtable calls of impossible dyn types) - rust-lang#136315 (Use short ty string for binop and unop errors) - rust-lang#136393 (Fix accidentally not emitting overflowing literals lints anymore in patterns) - rust-lang#136435 (Simplify some code for lowering THIR patterns) - rust-lang#136630 (Change two std process tests to not output to std{out,err}, and fix test suite stat reset in bootstrap CI test rendering) r? `@ghost` `@rustbot` modify labels: rollup try-job: aarch64-gnu-debug

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#136073 (Always compute coroutine layout for eagerly emitting recursive layout errors) - rust-lang#136235 (Pretty print pattern type values with transmute if they don't satisfy their pattern) - rust-lang#136311 (Ensure that we never try to monomorphize the upcasting or vtable calls of impossible dyn types) - rust-lang#136315 (Use short ty string for binop and unop errors) - rust-lang#136393 (Fix accidentally not emitting overflowing literals lints anymore in patterns) - rust-lang#136435 (Simplify some code for lowering THIR patterns) - rust-lang#136630 (Change two std process tests to not output to std{out,err}, and fix test suite stat reset in bootstrap CI test rendering) r? `@ghost` `@rustbot` modify labels: rollup try-job: aarch64-gnu-debug

Rollup merge of rust-lang#136073 - compiler-errors:recursive-coro-always, r=oli-obk Always compute coroutine layout for eagerly emitting recursive layout errors Detect recursive coroutine layouts even if we don't detect opaque type recursion in the new solver. This is for two reasons: 1. It helps us detect (bad) recursive async function calls in the new solver, which due to its approach to normalization causes us to not detect this via a recursive RPIT (since the opaques are more eagerly revealed in the opaque body). * Fixes rust-lang/trait-system-refactor-initiative#137. 2. It helps us detect (bad) recursive async functions behind AFITs. See the AFIT test that changed for the old solver too. 3. It also greatly simplifies the recursive impl trait check, since I can remove some jankness around how it handles coroutines.

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#136073 (Always compute coroutine layout for eagerly emitting recursive layout errors) - rust-lang#136235 (Pretty print pattern type values with transmute if they don't satisfy their pattern) - rust-lang#136311 (Ensure that we never try to monomorphize the upcasting or vtable calls of impossible dyn types) - rust-lang#136315 (Use short ty string for binop and unop errors) - rust-lang#136393 (Fix accidentally not emitting overflowing literals lints anymore in patterns) - rust-lang#136435 (Simplify some code for lowering THIR patterns) - rust-lang#136630 (Change two std process tests to not output to std{out,err}, and fix test suite stat reset in bootstrap CI test rendering) r? `@ghost` `@rustbot` modify labels: rollup try-job: aarch64-gnu-debug

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 Jan 25, 2025

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 Jan 25, 2025

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 25, 2025

Auto merge of rust-lang#136073 - compiler-errors:recursive-coro-alway…

c104bc5

…s, r=<try> Always compute coroutine layout when checking for recursive opaques r? `@ghost`

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jan 26, 2025

compiler-errors force-pushed the recursive-coro-always branch from 9db9cf2 to d75d180 Compare January 27, 2025 19:15

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 Jan 27, 2025

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 27, 2025

Auto merge of rust-lang#136073 - compiler-errors:recursive-coro-alway…

313ad1f

…s, r=<try> Always compute coroutine layout when checking for recursive opaques r? `@ghost`

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 Jan 27, 2025

compiler-errors marked this pull request as ready for review January 27, 2025 23:10

rustbot assigned oli-obk Jan 27, 2025

compiler-errors changed the title ~~Always compute coroutine layout when checking for recursive opaques~~ Always compute coroutine layout for eagerly emitting recursive layout errors Jan 27, 2025

compiler-errors mentioned this pull request Jan 28, 2025

Properly deeply normalize in the next solver #136074

Merged

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 30, 2025

compiler-errors force-pushed the recursive-coro-always branch from d75d180 to eedfece Compare February 2, 2025 15:01

bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 2, 2025

matthiaskrgr mentioned this pull request Feb 3, 2025

Rollup of 9 pull requests #136505

Closed

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 3, 2025

Eagerly detect coroutine recursion pre-mono when possible

d0b0b02

compiler-errors force-pushed the recursive-coro-always branch from eedfece to d0b0b02 Compare February 5, 2025 18:36

bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 6, 2025

matthiaskrgr mentioned this pull request Feb 6, 2025

Rollup of 7 pull requests #136641

Merged

bors merged commit 7ca2936 into rust-lang:master Feb 6, 2025
6 checks passed

rustbot added this to the 1.86.0 milestone Feb 6, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Always compute coroutine layout for eagerly emitting recursive layout errors #136073

Always compute coroutine layout for eagerly emitting recursive layout errors #136073

compiler-errors commented Jan 25, 2025 •

edited

Loading

compiler-errors commented Jan 25, 2025

This comment has been minimized.

bors commented Jan 25, 2025

bors commented Jan 26, 2025

This comment has been minimized.

rust-timer commented Jan 26, 2025

compiler-errors commented Jan 26, 2025

compiler-errors commented Jan 27, 2025

This comment has been minimized.

bors commented Jan 27, 2025

bors commented Jan 27, 2025

This comment has been minimized.

rust-timer commented Jan 27, 2025

compiler-errors commented Jan 27, 2025 •

edited

Loading

compiler-errors commented Jan 27, 2025

oli-obk commented Jan 28, 2025

bors commented Jan 28, 2025

bors commented Jan 30, 2025

bors commented Feb 1, 2025

compiler-errors commented Feb 2, 2025

bors commented Feb 2, 2025

Noratrieb commented Feb 3, 2025

matthiaskrgr commented Feb 3, 2025

lcnr commented Feb 6, 2025

bors commented Feb 6, 2025

Always compute coroutine layout for eagerly emitting recursive layout errors #136073

Always compute coroutine layout for eagerly emitting recursive layout errors #136073

Conversation

compiler-errors commented Jan 25, 2025 • edited Loading

compiler-errors commented Jan 25, 2025

This comment has been minimized.

bors commented Jan 25, 2025

bors commented Jan 26, 2025

This comment has been minimized.

rust-timer commented Jan 26, 2025

Overall result: ❌ regressions - please read the text below

compiler-errors commented Jan 26, 2025

compiler-errors commented Jan 27, 2025

This comment has been minimized.

bors commented Jan 27, 2025

bors commented Jan 27, 2025

This comment has been minimized.

rust-timer commented Jan 27, 2025

Overall result: ❌ regressions - please read the text below

compiler-errors commented Jan 27, 2025 • edited Loading

compiler-errors commented Jan 27, 2025

oli-obk commented Jan 28, 2025

bors commented Jan 28, 2025

bors commented Jan 30, 2025

bors commented Feb 1, 2025

compiler-errors commented Feb 2, 2025

bors commented Feb 2, 2025

Noratrieb commented Feb 3, 2025

matthiaskrgr commented Feb 3, 2025

lcnr commented Feb 6, 2025

bors commented Feb 6, 2025

compiler-errors commented Jan 25, 2025 •

edited

Loading

compiler-errors commented Jan 27, 2025 •

edited

Loading