Revert "Remove the witness type from coroutine args" #145193
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit e976578.
rustbot
added
S-waiting-on-review
compiler-errors
added
the
beta-nominated
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
☔ The latest upstream changes (presumably #145210) made this pull request unmergeable. Please resolve the merge conflicts. |
lcnr
reviewed
Aug 11, 2025
|
|
||
| fn main() { | ||
| require_send(process()); | ||
| } |
There was a problem hiding this comment.
why did this error?
We've got
coroutine<'a>: Send- requires
witness<'a>: Send - requires
Box<Pin<coroutine<'!0>>: Send? - requires
coroutine<'!0>: Send - requires
witness<'!0>: Send - requires
Box<Pin<coroutine<'!1>>: Send? - ....
But with this revert we get
coroutine<'a, witness<'a>>: Send- requires
witness<'a>: Send - requires
Box<Pin<coroutine<'!0, witness<'!1>>>>: Send? - requires
coroutine<'!0, witness<'!1>>: Send - requires
witness<'!1>: Send - requires
Box<Pin<coroutine<'!2, witness<'!3>>>>: Send? - ....
Zalathar
added a commit
to Zalathar/rust
that referenced
this pull request
Aug 11, 2025
…=lcnr
Ignore coroutine witness type region args in auto trait confirmation
## The problem
Consider code like:
```
async fn process<'a>() {
Box::pin(process()).await;
}
fn require_send(_: impl Send) {}
fn main() {
require_send(process());
}
```
When proving that the coroutine `{coroutine@process}::<'?0>: Send`, we end up instantiating a nested goal `{witness@process}::<'?0>: Send` by synthesizing a witness type from the coroutine's args:
Proving a coroutine witness type implements an auto trait requires looking up the coroutine's witness types. The witness types are a binder that look like `for<'r> { Pin<Box<{coroutine@process}::<'r>>> }`. We instantiate this binder with placeholders and prove `Send` on the witness types. This ends up eventually needing to prove something like `{coroutine@process}::<'!1>: Send`. Repeat this process, and we end up in an overflow during fulfillment, since fulfillment does not use freshening.
This can be visualized with a trait stack that ends up looking like:
* `{coroutine@process}::<'?0>: Send`
* `{witness@process}::<'?0>: Send`
* `Pin<Box<{coroutine@process}::<'!1>>>: Send`
* `{coroutine@process}::<'!1>: Send`
* ...
* `{coroutine@process}::<'!2>: Send`
* `{witness@process}::<'!2>: Send`
* ...
* overflow!
The problem here specifically comes from the first step: synthesizing a witness type from the coroutine's args.
## Why wasn't this an issue before?
Specifically, before 63f6845, this wasn't an issue because we were instead extracting the witness from the coroutine type itself. It turns out that given some `{coroutine@process}::<'?0>`, the witness type was actually something like `{witness@process}::<'erased>`!
So why do we end up with a witness type with `'erased` in its args? This is due to the fact that opaque type inference erases all regions from the witness. This is actually explicitly part of opaque type inference -- changing this to actually visit the witness types actually replicates this overflow even with 63f6845 reverted:
https://github.com/rust-lang/rust/blob/ca77504943887037504c7fc0b9bf06dab3910373/compiler/rustc_borrowck/src/type_check/opaque_types.rs#L303-L313
To better understand this difference and how it avoids a cycle, if you look at the trait stack before 63f6845, we end up with something like:
* `{coroutine@process}::<'?0>: Send`
* `{witness@process}::<'erased>: Send` **<-- THIS CHANGED**
* `Pin<Box<{coroutine@process}::<'!1>>>: Send`
* `{coroutine@process}::<'!1>: Send`
* ...
* `{coroutine@process}::<'erased>: Send` **<-- THIS CHANGED**
* `{witness@process}::<'erased>: Send` **<-- THIS CHANGED**
* coinductive cycle! 🎉
## So what's the fix?
This hack replicates the behavior in opaque type inference to erase regions from the witness type, but instead erasing the regions during auto trait confirmation. This is kinda a hack, but is sound. It does not need to be replicated in the new trait solver, of course.
---
I hope this explanation makes sense.
We could beta backport this instead of the revert rust-lang#145193, but then I'd like to un-revert that on master in this PR along with landing this this hack. Thoughts?
r? lcnr
rust-timer
added a commit
that referenced
this pull request
Aug 11, 2025
Rollup merge of #145194 - compiler-errors:coro-witness-re, r=lcnr Ignore coroutine witness type region args in auto trait confirmation ## The problem Consider code like: ``` async fn process<'a>() { Box::pin(process()).await; } fn require_send(_: impl Send) {} fn main() { require_send(process()); } ``` When proving that the coroutine `{coroutine@process}::<'?0>: Send`, we end up instantiating a nested goal `{witness@process}::<'?0>: Send` by synthesizing a witness type from the coroutine's args: Proving a coroutine witness type implements an auto trait requires looking up the coroutine's witness types. The witness types are a binder that look like `for<'r> { Pin<Box<{coroutine@process}::<'r>>> }`. We instantiate this binder with placeholders and prove `Send` on the witness types. This ends up eventually needing to prove something like `{coroutine@process}::<'!1>: Send`. Repeat this process, and we end up in an overflow during fulfillment, since fulfillment does not use freshening. This can be visualized with a trait stack that ends up looking like: * `{coroutine@process}::<'?0>: Send` * `{witness@process}::<'?0>: Send` * `Pin<Box<{coroutine@process}::<'!1>>>: Send` * `{coroutine@process}::<'!1>: Send` * ... * `{coroutine@process}::<'!2>: Send` * `{witness@process}::<'!2>: Send` * ... * overflow! The problem here specifically comes from the first step: synthesizing a witness type from the coroutine's args. ## Why wasn't this an issue before? Specifically, before 63f6845, this wasn't an issue because we were instead extracting the witness from the coroutine type itself. It turns out that given some `{coroutine@process}::<'?0>`, the witness type was actually something like `{witness@process}::<'erased>`! So why do we end up with a witness type with `'erased` in its args? This is due to the fact that opaque type inference erases all regions from the witness. This is actually explicitly part of opaque type inference -- changing this to actually visit the witness types actually replicates this overflow even with 63f6845 reverted: https://github.com/rust-lang/rust/blob/ca77504943887037504c7fc0b9bf06dab3910373/compiler/rustc_borrowck/src/type_check/opaque_types.rs#L303-L313 To better understand this difference and how it avoids a cycle, if you look at the trait stack before 63f6845, we end up with something like: * `{coroutine@process}::<'?0>: Send` * `{witness@process}::<'erased>: Send` **<-- THIS CHANGED** * `Pin<Box<{coroutine@process}::<'!1>>>: Send` * `{coroutine@process}::<'!1>: Send` * ... * `{coroutine@process}::<'erased>: Send` **<-- THIS CHANGED** * `{witness@process}::<'erased>: Send` **<-- THIS CHANGED** * coinductive cycle! 🎉 ## So what's the fix? This hack replicates the behavior in opaque type inference to erase regions from the witness type, but instead erasing the regions during auto trait confirmation. This is kinda a hack, but is sound. It does not need to be replicated in the new trait solver, of course. --- I hope this explanation makes sense. We could beta backport this instead of the revert #145193, but then I'd like to un-revert that on master in this PR along with landing this this hack. Thoughts? r? lcnr
|
Closing in favor of the other fix |
rustbot
removed
the
S-waiting-on-review
apiraino
removed
the
beta-nominated
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Revert 63f6845 (last commit in #144458 -- the whole thing doesn't need to be reverted) to fix #145151.
r? lcnr