Encode coroutine_by_move_body_def_id in crate metadata#130201
Merged
bors merged 3 commits intorust-lang:masterfrom Sep 17, 2024
Merged
Encode coroutine_by_move_body_def_id in crate metadata#130201bors merged 3 commits intorust-lang:masterfrom
coroutine_by_move_body_def_id in crate metadata#130201bors merged 3 commits intorust-lang:masterfrom
Conversation
Collaborator
rustbot
added
S-waiting-on-review
Collaborator
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
This comment has been minimized.
This comment has been minimized.
compiler-errors
force-pushed
the
foreign-synthetic-body
branch
from
5fe639c to
5b07148
Compare
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
hahah this is blocked on #130199, that's where the ci test failure comes from. |
Collaborator
|
☔ The latest upstream changes (presumably #130200) made this pull request unmergeable. Please resolve the merge conflicts. |
compiler-errors
force-pushed
the
foreign-synthetic-body
branch
from
5b07148 to
4af4369
Compare
This comment has been minimized.
This comment has been minimized.
compiler-errors
force-pushed
the
foreign-synthetic-body
branch
from
4af4369 to
af1ca77
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…DefKind::SyntheticCoroutineBody
compiler-errors
force-pushed
the
foreign-synthetic-body
branch
from
5c374a4 to
4beb1cf
Compare
Contributor
|
@bors r+ |
Collaborator
bors
added
S-waiting-on-bors
fee1-dead
added a commit
to fee1-dead-contrib/rust
that referenced
this pull request
Sep 17, 2024
…body, r=lcnr Encode `coroutine_by_move_body_def_id` in crate metadata We synthesize the MIR for a by-move body for the `FnOnce` implementation of async closures. It can be accessed with the `coroutine_by_move_body_def_id` query. We weren't encoding this query in the metadata though, nor were we properly recording that synthetic MIR in `mir_keys`, so the `optimized_mir` wasn't getting encoded either! Stacked on top is a fix to consider `DefKind::SyntheticCoroutineBody` to return true in several places I missed. Specifically, we should consider the def-kind in `fn DefKind::is_fn_like()`, since that's what we were using to make sure we ensure `query mir_inliner_callees` before the MIR gets stolen for the body. This led to some CI failures that were caught by miri but which I added a test for.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Sep 17, 2024
…fee1-dead Rollup of 8 pull requests Successful merges: - rust-lang#128961 (Fix rust-lang#128930: Print documentation of CLI options missing their arg) - rust-lang#129073 (Relate receiver invariantly in method probe for `Mode::Path`) - rust-lang#129674 (Add new_cyclic_in for Rc and Arc) - rust-lang#130201 (Encode `coroutine_by_move_body_def_id` in crate metadata) - rust-lang#130275 (Don't call `extern_crate` when local crate name is the same as a dependency and we have a trait error) - rust-lang#130440 (Don't ICE in `opaque_hidden_inferred_bound` lint for RPITIT in trait with no default method body) - rust-lang#130454 (tests: allow trunc/select instructions to be missing) - rust-lang#130458 (`rustc_codegen_ssa` cleanups) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Sep 17, 2024
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#128535 (Format `std::env::consts` docstrings with markdown backticks) - rust-lang#128961 (Fix rust-lang#128930: Print documentation of CLI options missing their arg) - rust-lang#129988 (Use `Vec` in `rustc_interface::Config::locale_resources`) - rust-lang#130201 (Encode `coroutine_by_move_body_def_id` in crate metadata) - rust-lang#130275 (Don't call `extern_crate` when local crate name is the same as a dependency and we have a trait error) - rust-lang#130314 (Use the same precedence for all macro-like exprs) - rust-lang#130440 (Don't ICE in `opaque_hidden_inferred_bound` lint for RPITIT in trait with no default method body) - rust-lang#130458 (`rustc_codegen_ssa` cleanups) - rust-lang#130469 (Mark `where_clauses_object_safety` as removed) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Sep 17, 2024
Rollup merge of rust-lang#130201 - compiler-errors:foreign-synthetic-body, r=lcnr Encode `coroutine_by_move_body_def_id` in crate metadata We synthesize the MIR for a by-move body for the `FnOnce` implementation of async closures. It can be accessed with the `coroutine_by_move_body_def_id` query. We weren't encoding this query in the metadata though, nor were we properly recording that synthetic MIR in `mir_keys`, so the `optimized_mir` wasn't getting encoded either! Stacked on top is a fix to consider `DefKind::SyntheticCoroutineBody` to return true in several places I missed. Specifically, we should consider the def-kind in `fn DefKind::is_fn_like()`, since that's what we were using to make sure we ensure `query mir_inliner_callees` before the MIR gets stolen for the body. This led to some CI failures that were caught by miri but which I added a test for.
eggyal
added a commit
to eggyal/rust
that referenced
this pull request
Feb 6, 2026
Ever since rust-lang#130201, the `mir_keys` query generates additional by-move MIR bodies (that aren't in the HIR) for coroutine-closures. However, computing such bodies before typeck is complete can lead to cycles. Ever since rust-lang#103172, in order to compute a function's ABI (which happens during CTFE), it may be necessary to look at the function's body in order to deduce certain codegen optimization attributes for its indirectly-passed parameters beyond what can be determined purely from its signature (namely today `ArgAttribute::ReadOnly` and `ArgAttribute::CapturesNone`). Prior to this patch, such parameter deduction is bypassed for functions that have no body by calling upon the `is_mir_available` query, which in turn calls upon `mir_keys`, and cycles can ensue. This patch breaks such cycles by using instead a new query, `is_mir_for_hir_available`, which only considers MIR bodies from the HIR without calling upon `mir_keys`.
eggyal
added a commit
to eggyal/rust
that referenced
this pull request
Feb 6, 2026
Ever since `rust-lang#130201`, the `mir_keys` query generates additional by-move MIR bodies (that aren't in the HIR) for coroutine-closures. However, computing such bodies before typeck is complete can lead to cycles. Ever since `rust-lang#103172`, in order to compute a function's ABI (which happens during CTFE), it may be necessary to look at the function's body in order to deduce certain codegen optimization attributes for its indirectly-passed parameters beyond what can be determined purely from its signature (namely today `ArgAttribute::ReadOnly` and `ArgAttribute::CapturesNone`). Prior to this patch, such parameter deduction is bypassed for functions that have no body by calling upon the `is_mir_available` query, which in turn calls upon `mir_keys`, and cycles can ensue. This patch breaks such cycles by using instead a new query, `is_mir_for_hir_available`, which only considers MIR bodies from the HIR without calling upon `mir_keys`.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
We synthesize the MIR for a by-move body for the
FnOnceimplementation of async closures. It can be accessed with thecoroutine_by_move_body_def_idquery. We weren't encoding this query in the metadata though, nor were we properly recording that synthetic MIR inmir_keys, so theoptimized_mirwasn't getting encoded either!Stacked on top is a fix to consider
DefKind::SyntheticCoroutineBodyto return true in several places I missed. Specifically, we should consider the def-kind infn DefKind::is_fn_like(), since that's what we were using to make sure we ensurequery mir_inliner_calleesbefore the MIR gets stolen for the body. This led to some CI failures that were caught by miri but which I added a test for.