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

Stop storing a special inner body for the coroutine by-move body for async closures #128506

Merged
merged 2 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
14 changes: 14 additions & 0 deletions compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,20 @@ 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.
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 +829,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
34 changes: 23 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,7 @@ fn should_encode_mir(
}
// Coroutines require optimized MIR to compute layout.
DefKind::Closure if tcx.is_coroutine(def_id.to_def_id()) => (false, true),
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 +1115,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 +1144,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 +1176,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 +1249,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 +1287,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 +1321,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 @@ -1458,7 +1470,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>>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use the same device for drop body?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably. I could investigate that as a follow-up.


/// 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
Loading