Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

async: Give predictable name to binding generated from .await expressions. #95011

Merged
merged 1 commit into from
Mar 31, 2022

Conversation

michaelwoerister
Copy link
Member

This name makes it to debuginfo and allows debuggers to identify such bindings and their captured versions in suspended async fns.

This will be useful for async stack traces, as discussed in https://internals.rust-lang.org/t/async-debugging-logical-stack-traces-setting-goals-collecting-examples/15547.

I don't know if this needs some discussion by @rust-lang/compiler, e.g. about the name of the binding (__awaitee) or about the fact that this PR introduces a (soft) guarantee about a compiler generated name. Although, regarding the later, I think the same reasoning applies here as it does for debuginfo in general.

r? @tmandry

@michaelwoerister michaelwoerister added A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) A-async-await Area: Async & Await WG-async Working group: Async & await T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 16, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 16, 2022
@tmandry
Copy link
Member

tmandry commented Mar 29, 2022

I'm wondering if it's possible to make the name something not writeable by the user like $awaitee, though I don't know what our limitations are wrt debuginfo. I don't think it matters though because this would always shadow any user named variables.

@bors r+

@bors
Copy link
Contributor

bors commented Mar 29, 2022

📌 Commit daa47488a7e2eff1bece9e3e26eb2eaa72f70126 has been approved by tmandry

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 29, 2022
@bors
Copy link
Contributor

bors commented Mar 30, 2022

⌛ Testing commit daa47488a7e2eff1bece9e3e26eb2eaa72f70126 with merge 636998919c20fa752c1842488c057cdd94e3a5c4...

@bors
Copy link
Contributor

bors commented Mar 30, 2022

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 30, 2022
@rust-log-analyzer

This comment has been minimized.

…it expressions.

This name makes it to debuginfo and allows debuggers to identify such bindings and
their captured versions in suspended async fns.
@michaelwoerister
Copy link
Member Author

I'm wondering if it's possible to make the name something not writeable by the user like $awaitee, though I don't know what our limitations are wrt debuginfo. I don't think it matters though because this would always shadow any user named variables.

Yeah, I've been wondering about that too. But I think it's better to stick to something that's a valid identifier in most contexts. The leading double underscore is a pretty strong convention for reserved names (at least in the C world).

@michaelwoerister
Copy link
Member Author

Fixed the test case so it works on MSVC too.
@bors r=tmandry

@bors
Copy link
Contributor

bors commented Mar 30, 2022

📌 Commit 78e27e2 has been approved by tmandry

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 30, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 30, 2022
…mandry

async: Give predictable name to binding generated from .await expressions.

This name makes it to debuginfo and allows debuggers to identify such bindings and their captured versions in suspended async fns.

This will be useful for async stack traces, as discussed in https://internals.rust-lang.org/t/async-debugging-logical-stack-traces-setting-goals-collecting-examples/15547.

I don't know if this needs some discussion by `@rust-lang/compiler,` e.g. about the name of the binding (`__awaitee`) or about the fact that this PR introduces a (soft) guarantee about a compiler generated name. Although, regarding the later, I think the same reasoning applies here as it does for debuginfo in general.

r? `@tmandry`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 30, 2022
…mandry

async: Give predictable name to binding generated from .await expressions.

This name makes it to debuginfo and allows debuggers to identify such bindings and their captured versions in suspended async fns.

This will be useful for async stack traces, as discussed in https://internals.rust-lang.org/t/async-debugging-logical-stack-traces-setting-goals-collecting-examples/15547.

I don't know if this needs some discussion by ``@rust-lang/compiler,`` e.g. about the name of the binding (`__awaitee`) or about the fact that this PR introduces a (soft) guarantee about a compiler generated name. Although, regarding the later, I think the same reasoning applies here as it does for debuginfo in general.

r? ``@tmandry``
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 30, 2022
…mandry

async: Give predictable name to binding generated from .await expressions.

This name makes it to debuginfo and allows debuggers to identify such bindings and their captured versions in suspended async fns.

This will be useful for async stack traces, as discussed in https://internals.rust-lang.org/t/async-debugging-logical-stack-traces-setting-goals-collecting-examples/15547.

I don't know if this needs some discussion by ```@rust-lang/compiler,``` e.g. about the name of the binding (`__awaitee`) or about the fact that this PR introduces a (soft) guarantee about a compiler generated name. Although, regarding the later, I think the same reasoning applies here as it does for debuginfo in general.

r? ```@tmandry```
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 31, 2022
Rollup of 6 pull requests

Successful merges:

 - rust-lang#93901 (Stabilize native library modifier syntax and the `whole-archive` modifier specifically)
 - rust-lang#94806 (Fix `cargo run tidy`)
 - rust-lang#94869 (Add the generic_associated_types_extended feature)
 - rust-lang#95011 (async: Give predictable name to binding generated from .await expressions.)
 - rust-lang#95251 (Reduce max hash in raw strings from u16 to u8)
 - rust-lang#95298 (Fix double drop of allocator in IntoIter impl of Vec)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1c3657b into rust-lang:master Mar 31, 2022
@rustbot rustbot added this to the 1.61.0 milestone Mar 31, 2022
flip1995 pushed a commit to flip1995/rust that referenced this pull request Apr 7, 2022
Rollup of 6 pull requests

Successful merges:

 - rust-lang#93901 (Stabilize native library modifier syntax and the `whole-archive` modifier specifically)
 - rust-lang#94806 (Fix `cargo run tidy`)
 - rust-lang#94869 (Add the generic_associated_types_extended feature)
 - rust-lang#95011 (async: Give predictable name to binding generated from .await expressions.)
 - rust-lang#95251 (Reduce max hash in raw strings from u16 to u8)
 - rust-lang#95298 (Fix double drop of allocator in IntoIter impl of Vec)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) 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. WG-async Working group: Async & await
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants