Skip to content

Commit a92413e

Browse files
borsbors
committed
Auto merge of #128506 - compiler-errors:by-move-body, r=<try>
Stop storing a special inner body for the coroutine by-move body for async closures ...and instead, just synthesize an item which is treated mostly normally by the MIR pipeline. This PR does a few things: * We synthesize a new `DefId` for the by-move body of a closure, which has its `mir_built` fed with the output of the `ByMoveBody` MIR transformation, and some other relevant queries. * Remove the special `PassManager` hacks for handling the inner `by_move_body` stored within the coroutine's mir body. Instead, this body is fed like a regular MIR body, so it's goes through all of the `tcx.*_mir` stages normally (build -> promoted -> ...etc... -> optimized) ✨. * Introduce `TyCtxt::is_synthetic_mir` to skip over `mir_borrowck` which is called by `mir_promoted`; borrowck isn't really possible to make work ATM since it heavily relies being called on a body generated from HIR, and is redundant by the construction of the by-move-body. * Remove the `InstanceKind::ByMoveBody` shim, since now we have a "regular" def id, we can just use `InstanceKind::Item`. This also allows us to remove the corresponding hacks from codegen, such as in `fn_sig_for_fn_abi` ✨. Notable remarks: * I know it's kind of weird to be using `DefKind::Closure` here, since it's not a distinct closure but just a new MIR body. I don't believe it really matters, but I could also use a different `DefKind`... maybe one that we could use for synthetic MIR bodies in general?
2 parents 5367673 + bca0ea9 commit a92413e

File tree

25 files changed

+205
-230
lines changed

25 files changed

+205
-230
lines changed

Diff for: ‎compiler/rustc_const_eval/src/interpret/terminator.rs

-1
Original file line numberDiff line numberDiff line change
@@ -608,7 +608,6 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
608608
| ty::InstanceKind::ReifyShim(..)
609609
| ty::InstanceKind::ClosureOnceShim { .. }
610610
| ty::InstanceKind::ConstructCoroutineInClosureShim { .. }
611-
| ty::InstanceKind::CoroutineKindShim { .. }
612611
| ty::InstanceKind::FnPtrShim(..)
613612
| ty::InstanceKind::DropGlue(..)
614613
| ty::InstanceKind::CloneShim(..)

Diff for: ‎compiler/rustc_hir_analysis/src/lib.rs

-4
Original file line numberDiff line numberDiff line change
@@ -199,10 +199,6 @@ pub fn check_crate(tcx: TyCtxt<'_>) {
199199
}
200200
});
201201

202-
// Freeze definitions as we don't add new ones at this point. This improves performance by
203-
// allowing lock-free access to them.
204-
tcx.untracked().definitions.freeze();
205-
206202
// FIXME: Remove this when we implement creating `DefId`s
207203
// for anon constants during their parents' typeck.
208204
// Typeck all body owners in parallel will produce queries

Diff for: ‎compiler/rustc_interface/src/passes.rs

+16
Original file line numberDiff line numberDiff line change
@@ -784,7 +784,22 @@ fn run_required_analyses(tcx: TyCtxt<'_>) {
784784
}
785785
);
786786
});
787+
787788
rustc_hir_analysis::check_crate(tcx);
789+
sess.time("MIR_coroutine_by_move_body", || {
790+
tcx.hir().par_body_owners(|def_id| {
791+
if tcx.needs_coroutine_by_move_body_def_id(def_id) {
792+
tcx.ensure_with_value().coroutine_by_move_body_def_id(def_id);
793+
}
794+
});
795+
});
796+
// Freeze definitions as we don't add new ones at this point.
797+
// We need to wait until now since we synthesize a by-move body
798+
// This improves performance by allowing lock-free access to them.
799+
// FIXME(async_closures): We could force `coroutine_by_move_body_def_id`
800+
// immediately after typeck, then freeze after that.
801+
tcx.untracked().definitions.freeze();
802+
788803
sess.time("MIR_borrow_checking", || {
789804
tcx.hir().par_body_owners(|def_id| {
790805
// Run unsafety check because it's responsible for stealing and
@@ -816,6 +831,7 @@ fn run_required_analyses(tcx: TyCtxt<'_>) {
816831
);
817832
}
818833
});
834+
819835
sess.time("layout_testing", || layout_test::test_layout(tcx));
820836
sess.time("abi_testing", || abi_test::test_abi(tcx));
821837
}

Diff for: ‎compiler/rustc_metadata/src/rmeta/encoder.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -1366,7 +1366,11 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
13661366
let def_span = tcx.def_span(local_id);
13671367
record!(self.tables.def_span[def_id] <- def_span);
13681368
}
1369-
if should_encode_attrs(def_kind) {
1369+
// FIXME(async_closures): We should just use `tcx.attrs` rather than going
1370+
// through the HIR. Historically, though, this has been inefficient apparently.
1371+
// For now, it's kind of pointless to fix, because coroutine-closures' coroutine
1372+
// bodies have no attrs anyways.
1373+
if should_encode_attrs(def_kind) && !tcx.is_synthetic_mir(local_id) {
13701374
self.encode_attrs(local_id);
13711375
}
13721376
if should_encode_expn_that_defined(def_kind) {

Diff for: ‎compiler/rustc_middle/src/mir/mod.rs

-17
Original file line numberDiff line numberDiff line change
@@ -267,18 +267,6 @@ pub struct CoroutineInfo<'tcx> {
267267
/// Coroutine drop glue. This field is populated after the state transform pass.
268268
pub coroutine_drop: Option<Body<'tcx>>,
269269

270-
/// The body of the coroutine, modified to take its upvars by move rather than by ref.
271-
///
272-
/// This is used by coroutine-closures, which must return a different flavor of coroutine
273-
/// when called using `AsyncFnOnce::call_once`. It is produced by the `ByMoveBody` pass which
274-
/// is run right after building the initial MIR, and will only be populated for coroutines
275-
/// which come out of the async closure desugaring.
276-
///
277-
/// This body should be processed in lockstep with the containing body -- any optimization
278-
/// passes, etc, should be applied to this body as well. This is done automatically if
279-
/// using `run_passes`.
280-
pub by_move_body: Option<Body<'tcx>>,
281-
282270
/// The layout of a coroutine. This field is populated after the state transform pass.
283271
pub coroutine_layout: Option<CoroutineLayout<'tcx>>,
284272

@@ -298,7 +286,6 @@ impl<'tcx> CoroutineInfo<'tcx> {
298286
coroutine_kind,
299287
yield_ty: Some(yield_ty),
300288
resume_ty: Some(resume_ty),
301-
by_move_body: None,
302289
coroutine_drop: None,
303290
coroutine_layout: None,
304291
}
@@ -665,10 +652,6 @@ impl<'tcx> Body<'tcx> {
665652
self.coroutine.as_ref().and_then(|coroutine| coroutine.coroutine_drop.as_ref())
666653
}
667654

668-
pub fn coroutine_by_move_body(&self) -> Option<&Body<'tcx>> {
669-
self.coroutine.as_ref()?.by_move_body.as_ref()
670-
}
671-
672655
#[inline]
673656
pub fn coroutine_kind(&self) -> Option<CoroutineKind> {
674657
self.coroutine.as_ref().map(|coroutine| coroutine.coroutine_kind)

Diff for: ‎compiler/rustc_middle/src/mir/mono.rs

-1
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,6 @@ impl<'tcx> CodegenUnit<'tcx> {
415415
| InstanceKind::Virtual(..)
416416
| InstanceKind::ClosureOnceShim { .. }
417417
| InstanceKind::ConstructCoroutineInClosureShim { .. }
418-
| InstanceKind::CoroutineKindShim { .. }
419418
| InstanceKind::DropGlue(..)
420419
| InstanceKind::CloneShim(..)
421420
| InstanceKind::ThreadLocalShim(..)

Diff for: ‎compiler/rustc_middle/src/mir/visit.rs

-1
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,6 @@ macro_rules! make_mir_visitor {
349349
coroutine_closure_def_id: _def_id,
350350
receiver_by_ref: _,
351351
} |
352-
ty::InstanceKind::CoroutineKindShim { coroutine_def_id: _def_id } |
353352
ty::InstanceKind::AsyncDropGlueCtorShim(_def_id, None) |
354353
ty::InstanceKind::DropGlue(_def_id, None) => {}
355354

Diff for: ‎compiler/rustc_middle/src/query/mod.rs

+20
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,7 @@ rustc_queries! {
327327
query predicates_of(key: DefId) -> ty::GenericPredicates<'tcx> {
328328
desc { |tcx| "computing predicates of `{}`", tcx.def_path_str(key) }
329329
cache_on_disk_if { key.is_local() }
330+
feedable
330331
}
331332

332333
query opaque_types_defined_by(
@@ -488,13 +489,19 @@ rustc_queries! {
488489
separate_provide_extern
489490
}
490491

492+
query is_synthetic_mir(key: LocalDefId) -> bool {
493+
desc { |tcx| "checking if `{}` is synthetic", tcx.def_path_str(key) }
494+
feedable
495+
}
496+
491497
/// Build the MIR for a given `DefId` and prepare it for const qualification.
492498
///
493499
/// See the [rustc dev guide] for more info.
494500
///
495501
/// [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/mir/construction.html
496502
query mir_built(key: LocalDefId) -> &'tcx Steal<mir::Body<'tcx>> {
497503
desc { |tcx| "building MIR for `{}`", tcx.def_path_str(key) }
504+
feedable
498505
}
499506

500507
/// Try to build an abstract representation of the given constant.
@@ -739,6 +746,7 @@ rustc_queries! {
739746
query constness(key: DefId) -> hir::Constness {
740747
desc { |tcx| "checking if item is const: `{}`", tcx.def_path_str(key) }
741748
separate_provide_extern
749+
feedable
742750
}
743751

744752
query asyncness(key: DefId) -> ty::Asyncness {
@@ -757,10 +765,22 @@ rustc_queries! {
757765
desc { |tcx| "checking if item is promotable: `{}`", tcx.def_path_str(key) }
758766
}
759767

768+
/// The body of the coroutine, modified to take its upvars by move rather than by ref.
769+
///
770+
/// This is used by coroutine-closures, which must return a different flavor of coroutine
771+
/// when called using `AsyncFnOnce::call_once`. It is produced by the `ByMoveBody` pass which
772+
/// is run right after building the initial MIR, and will only be populated for coroutines
773+
/// which come out of the async closure desugaring.
774+
query coroutine_by_move_body_def_id(def_id: DefId) -> DefId {
775+
desc { |tcx| "looking up the coroutine by-move body for `{}`", tcx.def_path_str(def_id) }
776+
separate_provide_extern
777+
}
778+
760779
/// Returns `Some(coroutine_kind)` if the node pointed to by `def_id` is a coroutine.
761780
query coroutine_kind(def_id: DefId) -> Option<hir::CoroutineKind> {
762781
desc { |tcx| "looking up coroutine kind of `{}`", tcx.def_path_str(def_id) }
763782
separate_provide_extern
783+
feedable
764784
}
765785

766786
query coroutine_for_closure(def_id: DefId) -> DefId {

Diff for: ‎compiler/rustc_middle/src/ty/context.rs

+12
Original file line numberDiff line numberDiff line change
@@ -3176,6 +3176,18 @@ impl<'tcx> TyCtxt<'tcx> {
31763176
pub fn impl_polarity(self, def_id: impl IntoQueryParam<DefId>) -> ty::ImplPolarity {
31773177
self.impl_trait_header(def_id).map_or(ty::ImplPolarity::Positive, |h| h.polarity)
31783178
}
3179+
3180+
pub fn needs_coroutine_by_move_body_def_id(self, def_id: LocalDefId) -> bool {
3181+
if let Some(hir::CoroutineKind::Desugared(_, hir::CoroutineSource::Closure)) =
3182+
self.coroutine_kind(def_id)
3183+
&& let ty::Coroutine(_, args) = self.type_of(def_id).instantiate_identity().kind()
3184+
&& args.as_coroutine().kind_ty().to_opt_closure_kind() != Some(ty::ClosureKind::FnOnce)
3185+
{
3186+
true
3187+
} else {
3188+
false
3189+
}
3190+
}
31793191
}
31803192

31813193
/// Parameter attributes that can only be determined by examining the body of a function instead

Diff for: ‎compiler/rustc_middle/src/ty/instance.rs

+3-13
Original file line numberDiff line numberDiff line change
@@ -141,14 +141,6 @@ pub enum InstanceKind<'tcx> {
141141
receiver_by_ref: bool,
142142
},
143143

144-
/// `<[coroutine] as Future>::poll`, but for coroutines produced when `AsyncFnOnce`
145-
/// is called on a coroutine-closure whose closure kind greater than `FnOnce`, or
146-
/// similarly for `AsyncFnMut`.
147-
///
148-
/// This will select the body that is produced by the `ByMoveBody` transform, and thus
149-
/// take and use all of its upvars by-move rather than by-ref.
150-
CoroutineKindShim { coroutine_def_id: DefId },
151-
152144
/// Compiler-generated accessor for thread locals which returns a reference to the thread local
153145
/// the `DefId` defines. This is used to export thread locals from dylibs on platforms lacking
154146
/// native support.
@@ -248,7 +240,6 @@ impl<'tcx> InstanceKind<'tcx> {
248240
coroutine_closure_def_id: def_id,
249241
receiver_by_ref: _,
250242
}
251-
| ty::InstanceKind::CoroutineKindShim { coroutine_def_id: def_id }
252243
| InstanceKind::DropGlue(def_id, _)
253244
| InstanceKind::CloneShim(def_id, _)
254245
| InstanceKind::FnPtrAddrShim(def_id, _)
@@ -270,7 +261,6 @@ impl<'tcx> InstanceKind<'tcx> {
270261
| InstanceKind::Intrinsic(..)
271262
| InstanceKind::ClosureOnceShim { .. }
272263
| ty::InstanceKind::ConstructCoroutineInClosureShim { .. }
273-
| ty::InstanceKind::CoroutineKindShim { .. }
274264
| InstanceKind::DropGlue(..)
275265
| InstanceKind::AsyncDropGlueCtorShim(..)
276266
| InstanceKind::CloneShim(..)
@@ -377,7 +367,6 @@ impl<'tcx> InstanceKind<'tcx> {
377367
| InstanceKind::AsyncDropGlueCtorShim(_, Some(_)) => false,
378368
InstanceKind::ClosureOnceShim { .. }
379369
| InstanceKind::ConstructCoroutineInClosureShim { .. }
380-
| InstanceKind::CoroutineKindShim { .. }
381370
| InstanceKind::DropGlue(..)
382371
| InstanceKind::AsyncDropGlueCtorShim(..)
383372
| InstanceKind::Item(_)
@@ -452,7 +441,6 @@ pub fn fmt_instance(
452441
InstanceKind::FnPtrShim(_, ty) => write!(f, " - shim({ty})"),
453442
InstanceKind::ClosureOnceShim { .. } => write!(f, " - shim"),
454443
InstanceKind::ConstructCoroutineInClosureShim { .. } => write!(f, " - shim"),
455-
InstanceKind::CoroutineKindShim { .. } => write!(f, " - shim"),
456444
InstanceKind::DropGlue(_, None) => write!(f, " - shim(None)"),
457445
InstanceKind::DropGlue(_, Some(ty)) => write!(f, " - shim(Some({ty}))"),
458446
InstanceKind::CloneShim(_, ty) => write!(f, " - shim({ty})"),
@@ -850,7 +838,9 @@ impl<'tcx> Instance<'tcx> {
850838
Some(Instance { def: ty::InstanceKind::Item(coroutine_def_id), args })
851839
} else {
852840
Some(Instance {
853-
def: ty::InstanceKind::CoroutineKindShim { coroutine_def_id },
841+
def: ty::InstanceKind::Item(
842+
tcx.coroutine_by_move_body_def_id(coroutine_def_id),
843+
),
854844
args,
855845
})
856846
}

Diff for: ‎compiler/rustc_middle/src/ty/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1755,7 +1755,6 @@ impl<'tcx> TyCtxt<'tcx> {
17551755
| ty::InstanceKind::Virtual(..)
17561756
| ty::InstanceKind::ClosureOnceShim { .. }
17571757
| ty::InstanceKind::ConstructCoroutineInClosureShim { .. }
1758-
| ty::InstanceKind::CoroutineKindShim { .. }
17591758
| ty::InstanceKind::DropGlue(..)
17601759
| ty::InstanceKind::CloneShim(..)
17611760
| ty::InstanceKind::ThreadLocalShim(..)
@@ -1873,7 +1872,8 @@ impl<'tcx> TyCtxt<'tcx> {
18731872
identity_kind_ty.to_opt_closure_kind(),
18741873
Some(ClosureKind::Fn | ClosureKind::FnMut)
18751874
);
1876-
mir.coroutine_by_move_body().unwrap().coroutine_layout_raw()
1875+
self.optimized_mir(self.coroutine_by_move_body_def_id(def_id))
1876+
.coroutine_layout_raw()
18771877
}
18781878
}
18791879
}

Diff for: ‎compiler/rustc_mir_transform/src/coroutine.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353
mod by_move_body;
5454
use std::{iter, ops};
5555

56-
pub use by_move_body::ByMoveBody;
56+
pub use by_move_body::coroutine_by_move_body_def_id;
5757
use rustc_data_structures::fx::FxHashSet;
5858
use rustc_errors::pluralize;
5959
use rustc_hir as hir;

0 commit comments

Comments
 (0)