Skip to content

Commit

Permalink
Stop using a special inner body for the coroutine by-move body for as…
Browse files Browse the repository at this point in the history
…ync closures
  • Loading branch information
compiler-errors committed Aug 26, 2024
1 parent 515395a commit 4609841
Show file tree
Hide file tree
Showing 40 changed files with 283 additions and 303 deletions.
1 change: 0 additions & 1 deletion compiler/rustc_const_eval/src/interpret/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,6 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
| ty::InstanceKind::ReifyShim(..)
| ty::InstanceKind::ClosureOnceShim { .. }
| ty::InstanceKind::ConstructCoroutineInClosureShim { .. }
| ty::InstanceKind::CoroutineKindShim { .. }
| ty::InstanceKind::FnPtrShim(..)
| ty::InstanceKind::DropGlue(..)
| ty::InstanceKind::CloneShim(..)
Expand Down
11 changes: 9 additions & 2 deletions compiler/rustc_hir/src/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ pub enum DefKind {
/// we treat them all the same, and code which needs to distinguish them can match
/// or `hir::ClosureKind` or `type_of`.
Closure,
/// The definition of a synthetic coroutine body created by the lowering of a
/// coroutine-closure, such as an async closure.
SyntheticCoroutineBody,
}

impl DefKind {
Expand Down Expand Up @@ -177,6 +180,7 @@ impl DefKind {
DefKind::Closure => "closure",
DefKind::ExternCrate => "extern crate",
DefKind::GlobalAsm => "global assembly block",
DefKind::SyntheticCoroutineBody => "synthetic mir body",
}
}

Expand Down Expand Up @@ -236,7 +240,8 @@ impl DefKind {
| DefKind::ForeignMod
| DefKind::GlobalAsm
| DefKind::Impl { .. }
| DefKind::OpaqueTy => None,
| DefKind::OpaqueTy
| DefKind::SyntheticCoroutineBody => None,
}
}

Expand Down Expand Up @@ -276,6 +281,7 @@ impl DefKind {
DefKind::GlobalAsm => DefPathData::GlobalAsm,
DefKind::Impl { .. } => DefPathData::Impl,
DefKind::Closure => DefPathData::Closure,
DefKind::SyntheticCoroutineBody => DefPathData::Closure,
}
}

Expand All @@ -291,7 +297,8 @@ impl DefKind {
| DefKind::AssocFn
| DefKind::Ctor(..)
| DefKind::Closure
| DefKind::Static { .. } => true,
| DefKind::Static { .. }
| DefKind::SyntheticCoroutineBody => true,
DefKind::Mod
| DefKind::Struct
| DefKind::Union
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2174,7 +2174,8 @@ fn lint_redundant_lifetimes<'tcx>(
| DefKind::Field
| DefKind::LifetimeParam
| DefKind::GlobalAsm
| DefKind::Closure => return,
| DefKind::Closure
| DefKind::SyntheticCoroutineBody => return,
}

// The ordering of this lifetime map is a bit subtle.
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_hir_analysis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,6 @@ pub fn check_crate(tcx: TyCtxt<'_>) {
}
});

// Freeze definitions as we don't add new ones at this point. This improves performance by
// allowing lock-free access to them.
tcx.untracked().definitions.freeze();

// FIXME: Remove this when we implement creating `DefId`s
// for anon constants during their parents' typeck.
// Typeck all body owners in parallel will produce queries
Expand Down
16 changes: 16 additions & 0 deletions compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,22 @@ fn run_required_analyses(tcx: TyCtxt<'_>) {
}
);
});

rustc_hir_analysis::check_crate(tcx);
sess.time("MIR_coroutine_by_move_body", || {
tcx.hir().par_body_owners(|def_id| {
if tcx.needs_coroutine_by_move_body_def_id(def_id) {
tcx.ensure_with_value().coroutine_by_move_body_def_id(def_id);
}
});
});
// Freeze definitions as we don't add new ones at this point.
// We need to wait until now since we synthesize a by-move body
// This improves performance by allowing lock-free access to them.
// FIXME(async_closures): We could force `coroutine_by_move_body_def_id`
// immediately after typeck, then freeze after that.
tcx.untracked().definitions.freeze();

sess.time("MIR_borrow_checking", || {
tcx.hir().par_body_owners(|def_id| {
// Run unsafety check because it's responsible for stealing and
Expand Down Expand Up @@ -816,6 +831,7 @@ fn run_required_analyses(tcx: TyCtxt<'_>) {
);
}
});

sess.time("layout_testing", || layout_test::test_layout(tcx));
sess.time("abi_testing", || abi_test::test_abi(tcx));

Expand Down
39 changes: 28 additions & 11 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,8 @@ fn should_encode_span(def_kind: DefKind) -> bool {
| DefKind::OpaqueTy
| DefKind::Field
| DefKind::Impl { .. }
| DefKind::Closure => true,
| DefKind::Closure
| DefKind::SyntheticCoroutineBody => true,
DefKind::ForeignMod | DefKind::GlobalAsm => false,
}
}
Expand Down Expand Up @@ -902,6 +903,7 @@ fn should_encode_attrs(def_kind: DefKind) -> bool {
// https://github.com/model-checking/kani and is not a performance
// or maintenance issue for us.
DefKind::Closure => true,
DefKind::SyntheticCoroutineBody => false,
DefKind::TyParam
| DefKind::ConstParam
| DefKind::Ctor(..)
Expand Down Expand Up @@ -948,7 +950,8 @@ fn should_encode_expn_that_defined(def_kind: DefKind) -> bool {
| DefKind::Field
| DefKind::LifetimeParam
| DefKind::GlobalAsm
| DefKind::Closure => false,
| DefKind::Closure
| DefKind::SyntheticCoroutineBody => false,
}
}

Expand Down Expand Up @@ -984,7 +987,8 @@ fn should_encode_visibility(def_kind: DefKind) -> bool {
| DefKind::GlobalAsm
| DefKind::Impl { .. }
| DefKind::Closure
| DefKind::ExternCrate => false,
| DefKind::ExternCrate
| DefKind::SyntheticCoroutineBody => false,
}
}

Expand Down Expand Up @@ -1019,7 +1023,8 @@ fn should_encode_stability(def_kind: DefKind) -> bool {
| DefKind::InlineConst
| DefKind::GlobalAsm
| DefKind::Closure
| DefKind::ExternCrate => false,
| DefKind::ExternCrate
| DefKind::SyntheticCoroutineBody => false,
}
}

Expand Down Expand Up @@ -1061,6 +1066,8 @@ fn should_encode_mir(
}
// Coroutines require optimized MIR to compute layout.
DefKind::Closure if tcx.is_coroutine(def_id.to_def_id()) => (false, true),
// FIXME: lol
DefKind::SyntheticCoroutineBody => (false, true),
// Full-fledged functions + closures
DefKind::AssocFn | DefKind::Fn | DefKind::Closure => {
let generics = tcx.generics_of(def_id);
Expand Down Expand Up @@ -1109,7 +1116,8 @@ fn should_encode_variances<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, def_kind: Def
| DefKind::InlineConst
| DefKind::GlobalAsm
| DefKind::Closure
| DefKind::ExternCrate => false,
| DefKind::ExternCrate
| DefKind::SyntheticCoroutineBody => false,
DefKind::TyAlias => tcx.type_alias_is_lazy(def_id),
}
}
Expand Down Expand Up @@ -1137,7 +1145,8 @@ fn should_encode_generics(def_kind: DefKind) -> bool {
| DefKind::Impl { .. }
| DefKind::Field
| DefKind::TyParam
| DefKind::Closure => true,
| DefKind::Closure
| DefKind::SyntheticCoroutineBody => true,
DefKind::Mod
| DefKind::ForeignMod
| DefKind::ConstParam
Expand Down Expand Up @@ -1168,7 +1177,8 @@ fn should_encode_type(tcx: TyCtxt<'_>, def_id: LocalDefId, def_kind: DefKind) ->
| DefKind::Closure
| DefKind::ConstParam
| DefKind::AnonConst
| DefKind::InlineConst => true,
| DefKind::InlineConst
| DefKind::SyntheticCoroutineBody => true,

DefKind::OpaqueTy => {
let origin = tcx.opaque_type_origin(def_id);
Expand Down Expand Up @@ -1240,7 +1250,8 @@ fn should_encode_fn_sig(def_kind: DefKind) -> bool {
| DefKind::Use
| DefKind::LifetimeParam
| DefKind::GlobalAsm
| DefKind::ExternCrate => false,
| DefKind::ExternCrate
| DefKind::SyntheticCoroutineBody => false,
}
}

Expand Down Expand Up @@ -1277,7 +1288,8 @@ fn should_encode_constness(def_kind: DefKind) -> bool {
| DefKind::Use
| DefKind::LifetimeParam
| DefKind::GlobalAsm
| DefKind::ExternCrate => false,
| DefKind::ExternCrate
| DefKind::SyntheticCoroutineBody => false,
}
}

Expand Down Expand Up @@ -1310,7 +1322,8 @@ fn should_encode_const(def_kind: DefKind) -> bool {
| DefKind::Use
| DefKind::LifetimeParam
| DefKind::GlobalAsm
| DefKind::ExternCrate => false,
| DefKind::ExternCrate
| DefKind::SyntheticCoroutineBody => false,
}
}

Expand Down Expand Up @@ -1366,6 +1379,10 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
let def_span = tcx.def_span(local_id);
record!(self.tables.def_span[def_id] <- def_span);
}
// FIXME(async_closures): We should just use `tcx.attrs` rather than going
// through the HIR. Historically, though, this has been inefficient apparently.
// For now, it's kind of pointless to fix, because coroutine-closures' coroutine
// bodies have no attrs anyways.
if should_encode_attrs(def_kind) {
self.encode_attrs(local_id);
}
Expand Down Expand Up @@ -1458,7 +1475,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
record!(self.tables.associated_type_for_effects[def_id] <- assoc_def_id);
}
}
if def_kind == DefKind::Closure
if let DefKind::Closure | DefKind::SyntheticCoroutineBody = def_kind
&& let Some(coroutine_kind) = self.tcx.coroutine_kind(def_id)
{
self.tables.coroutine_kind.set(def_id.index, Some(coroutine_kind))
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_metadata/src/rmeta/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ fixed_size_enum! {
( Macro(MacroKind::Bang) )
( Macro(MacroKind::Attr) )
( Macro(MacroKind::Derive) )
( SyntheticCoroutineBody )
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ impl<'hir> Map<'hir> {
}
DefKind::InlineConst => BodyOwnerKind::Const { inline: true },
DefKind::Ctor(..) | DefKind::Fn | DefKind::AssocFn => BodyOwnerKind::Fn,
DefKind::Closure => BodyOwnerKind::Closure,
DefKind::Closure | DefKind::SyntheticCoroutineBody => BodyOwnerKind::Closure,
DefKind::Static { safety: _, mutability, nested: false } => {
BodyOwnerKind::Static(mutability)
}
Expand Down
17 changes: 0 additions & 17 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,18 +267,6 @@ pub struct CoroutineInfo<'tcx> {
/// Coroutine drop glue. This field is populated after the state transform pass.
pub coroutine_drop: Option<Body<'tcx>>,

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

/// The layout of a coroutine. This field is populated after the state transform pass.
pub coroutine_layout: Option<CoroutineLayout<'tcx>>,

Expand All @@ -298,7 +286,6 @@ impl<'tcx> CoroutineInfo<'tcx> {
coroutine_kind,
yield_ty: Some(yield_ty),
resume_ty: Some(resume_ty),
by_move_body: None,
coroutine_drop: None,
coroutine_layout: None,
}
Expand Down Expand Up @@ -665,10 +652,6 @@ impl<'tcx> Body<'tcx> {
self.coroutine.as_ref().and_then(|coroutine| coroutine.coroutine_drop.as_ref())
}

pub fn coroutine_by_move_body(&self) -> Option<&Body<'tcx>> {
self.coroutine.as_ref()?.by_move_body.as_ref()
}

#[inline]
pub fn coroutine_kind(&self) -> Option<CoroutineKind> {
self.coroutine.as_ref().map(|coroutine| coroutine.coroutine_kind)
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_middle/src/mir/mono.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,6 @@ impl<'tcx> CodegenUnit<'tcx> {
| InstanceKind::Virtual(..)
| InstanceKind::ClosureOnceShim { .. }
| InstanceKind::ConstructCoroutineInClosureShim { .. }
| InstanceKind::CoroutineKindShim { .. }
| InstanceKind::DropGlue(..)
| InstanceKind::CloneShim(..)
| InstanceKind::ThreadLocalShim(..)
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_middle/src/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,6 @@ macro_rules! make_mir_visitor {
coroutine_closure_def_id: _def_id,
receiver_by_ref: _,
} |
ty::InstanceKind::CoroutineKindShim { coroutine_def_id: _def_id } |
ty::InstanceKind::AsyncDropGlueCtorShim(_def_id, None) |
ty::InstanceKind::DropGlue(_def_id, None) => {}

Expand Down
15 changes: 15 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ rustc_queries! {
query predicates_of(key: DefId) -> ty::GenericPredicates<'tcx> {
desc { |tcx| "computing predicates of `{}`", tcx.def_path_str(key) }
cache_on_disk_if { key.is_local() }
feedable
}

query opaque_types_defined_by(
Expand Down Expand Up @@ -498,6 +499,7 @@ rustc_queries! {
/// [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/mir/construction.html
query mir_built(key: LocalDefId) -> &'tcx Steal<mir::Body<'tcx>> {
desc { |tcx| "building MIR for `{}`", tcx.def_path_str(key) }
feedable
}

/// Try to build an abstract representation of the given constant.
Expand Down Expand Up @@ -742,6 +744,7 @@ rustc_queries! {
query constness(key: DefId) -> hir::Constness {
desc { |tcx| "checking if item is const: `{}`", tcx.def_path_str(key) }
separate_provide_extern
feedable
}

query asyncness(key: DefId) -> ty::Asyncness {
Expand All @@ -760,10 +763,22 @@ rustc_queries! {
desc { |tcx| "checking if item is promotable: `{}`", tcx.def_path_str(key) }
}

/// The body of the coroutine, modified to take its upvars by move rather than by ref.
///
/// This is used by coroutine-closures, which must return a different flavor of coroutine
/// when called using `AsyncFnOnce::call_once`. It is produced by the `ByMoveBody` pass which
/// is run right after building the initial MIR, and will only be populated for coroutines
/// which come out of the async closure desugaring.
query coroutine_by_move_body_def_id(def_id: DefId) -> DefId {
desc { |tcx| "looking up the coroutine by-move body for `{}`", tcx.def_path_str(def_id) }
separate_provide_extern
}

/// Returns `Some(coroutine_kind)` if the node pointed to by `def_id` is a coroutine.
query coroutine_kind(def_id: DefId) -> Option<hir::CoroutineKind> {
desc { |tcx| "looking up coroutine kind of `{}`", tcx.def_path_str(def_id) }
separate_provide_extern
feedable
}

query coroutine_for_closure(def_id: DefId) -> DefId {
Expand Down
18 changes: 18 additions & 0 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1579,6 +1579,12 @@ impl<'tcx> TyCtxt<'tcx> {
)
}

// Whether the body owner is synthetic, which in this case means it does not correspond to
// meaningful HIR. This is currently used to skip over MIR borrowck.
pub fn is_synthetic_mir(self, def_id: impl Into<DefId>) -> bool {
matches!(self.def_kind(def_id.into()), DefKind::SyntheticCoroutineBody)
}

/// Returns `true` if the node pointed to by `def_id` is a general coroutine that implements `Coroutine`.
/// This means it is neither an `async` or `gen` construct.
pub fn is_general_coroutine(self, def_id: DefId) -> bool {
Expand Down Expand Up @@ -3168,6 +3174,18 @@ impl<'tcx> TyCtxt<'tcx> {
self.impl_trait_header(def_id).map_or(ty::ImplPolarity::Positive, |h| h.polarity)
}

pub fn needs_coroutine_by_move_body_def_id(self, def_id: LocalDefId) -> bool {
if let Some(hir::CoroutineKind::Desugared(_, hir::CoroutineSource::Closure)) =
self.coroutine_kind(def_id)
&& let ty::Coroutine(_, args) = self.type_of(def_id).instantiate_identity().kind()
&& args.as_coroutine().kind_ty().to_opt_closure_kind() != Some(ty::ClosureKind::FnOnce)
{
true
} else {
false
}
}

/// Whether this is a trait implementation that has `#[diagnostic::do_not_recommend]`
pub fn do_not_recommend_impl(self, def_id: DefId) -> bool {
matches!(self.def_kind(def_id), DefKind::Impl { of_trait: true })
Expand Down
Loading

0 comments on commit 4609841

Please sign in to comment.