Avoid generating by-move coroutine MIR during parameter deduction

The `mir_keys` query generates additional by-move MIR bodies (that
aren't in the HIR) for coroutine-closures. However, computing such
bodies (indeed, merely determining whether such a body needs to be
generated) before typeck is complete can lead to cycles.

In order to compute a function's ABI (which happens amongst other things
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 (eg in trait or extern declarations) 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_available_for_hir`, which itself calls another new query
`mir_keys_for_hir` that only considers MIR bodies for items that exist
in the HIR (ie without generating the additional by-move MIR bodies for
coroutine closures).  Clearly theis cannot change overall behaviour, as
`is_mir_available` could never have been called with the `DefId` of an
as-yet not generated item.

In order to minimize duplicated effort, the existing `mir_keys` query
reuses the result from the new `mir_keys_for_hir` query, modifying it
only if there are coroutine-closures for which by-move MIR bodies need
be generated.

Author: eggyal

compiler/rustc_data_structures/src/stable_hasher.rs

@@ -1,3 +1,4 @@
+use std::borrow::Cow;
 use std::hash::{BuildHasher, Hash, Hasher};
 use std::marker::PhantomData;
 use std::mem;
@@ -501,6 +502,16 @@ where
     }
 }
 
+impl<'a, T, CTX> HashStable<CTX> for Cow<'a, T>
+where
+    T: HashStable<CTX> + ?Sized + ToOwned,
+{
+    #[inline]
+    fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) {
+        (**self).hash_stable(ctx, hasher);
+    }
+}
+
 impl<T, CTX> HashStable<CTX> for ::std::mem::Discriminant<T> {
     #[inline]
     fn hash_stable(&self, _: &mut CTX, hasher: &mut StableHasher) {

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -1877,7 +1877,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
             && tcx.sess.opts.optimize != OptLevel::No
             && tcx.sess.opts.incremental.is_none()
         {
-            for &local_def_id in tcx.mir_keys(()) {
+            for &local_def_id in &**tcx.mir_keys(()) {
                 if let DefKind::AssocFn | DefKind::Fn = tcx.def_kind(local_def_id) {
                     record_array!(self.tables.deduced_param_attrs[local_def_id.to_def_id()] <-
                         self.tcx.deduced_param_attrs(local_def_id.to_def_id()));
@@ -2284,7 +2284,7 @@ fn prefetch_mir(tcx: TyCtxt<'_>) {
     }
 
     let reachable_set = tcx.reachable_set(());
-    par_for_each_in(tcx.mir_keys(()), |&&def_id| {
+    par_for_each_in(&**tcx.mir_keys(()), |&&def_id| {
         if tcx.is_trivial_const(def_id) {
             return;
         }

compiler/rustc_middle/src/queries.rs

@@ -62,6 +62,7 @@
 
 #![allow(unused_parens)]
 
+use std::borrow::Cow;
 use std::ffi::OsStr;
 use std::mem;
 use std::path::PathBuf;
@@ -623,10 +624,18 @@ rustc_queries! {
         desc { |tcx| "building THIR for `{}`", tcx.def_path_str(key) }
     }
 
-    /// Set of all the `DefId`s in this crate that have MIR associated with
-    /// them. This includes all the body owners, but also things like struct
-    /// constructors.
-    query mir_keys(_: ()) -> &'tcx rustc_data_structures::fx::FxIndexSet<LocalDefId> {
+    /// Set of all the `DefId`s in this crate that have MIR associated with them owing to their
+    /// presence in the HIR. This includes all the body owners, but also things like struct
+    /// constructors. Unlike `mir_keys`, this query will not include the generated by-move MIR
+    /// bodies of coroutine closures.
+    query mir_keys_for_hir(_: ()) -> &'tcx rustc_data_structures::fx::FxIndexSet<LocalDefId> {
+        arena_cache
+        desc { "getting a list of all mir_keys for hir items" }
+    }
+
+    /// Set of all the `DefId`s in this crate that have MIR associated with them. This includes all
+    /// the body owners, but also things like struct constructors.
+    query mir_keys(_: ()) -> &'tcx Cow<'tcx, rustc_data_structures::fx::FxIndexSet<LocalDefId>> {
         arena_cache
         desc { "getting a list of all mir_keys" }
     }
@@ -1588,6 +1597,17 @@ rustc_queries! {
         separate_provide_extern
     }
 
+    /// Answers whether the HIR item with the given `key` has MIR available. Unlike
+    /// `is_mir_available`, this query will not answer `true` for the generated by-move MIR bodies
+    /// of coroutine closures.
+    query is_mir_available_for_hir(key: DefId) -> bool {
+        desc { |tcx| "checking if HIR item has MIR available: `{}`", tcx.def_path_str(key) }
+        cache_on_disk_if { key.is_local() }
+        separate_provide_extern
+    }
+
+    /// Answers whether MIR is available for the given `key`. Unlike `is_mir_available_for_hir`,
+    /// this query also answers `true` for the generated by-move MIR bodies of coroutine closures.
     query is_mir_available(key: DefId) -> bool {
         desc { |tcx| "checking if item has MIR available: `{}`", tcx.def_path_str(key) }
         cache_on_disk_if { key.is_local() }

compiler/rustc_mir_transform/src/deduce_param_attrs.rs

@@ -189,7 +189,7 @@ pub(super) fn deduced_param_attrs<'tcx>(
     }
 
     // Don't deduce any attributes for functions that have no MIR.
-    if !tcx.is_mir_available(def_id) {
+    if !tcx.is_mir_available_for_hir(def_id) {
         return &[];
     }
 

compiler/rustc_mir_transform/src/lib.rs

@@ -35,6 +35,7 @@ use tracing::debug;
 #[macro_use]
 mod pass_manager;
 
+use std::borrow::Cow;
 use std::sync::LazyLock;
 
 use pass_manager::{self as pm, Lint, MirLint, MirPass, WithMinOptLevel};
@@ -219,6 +220,7 @@ pub fn provide(providers: &mut Providers) {
     shim::provide(&mut providers.queries);
     cross_crate_inline::provide(&mut providers.queries);
     providers.queries = query::Providers {
+        mir_keys_for_hir,
         mir_keys,
         mir_built,
         mir_const_qualif,
@@ -228,6 +230,7 @@ pub fn provide(providers: &mut Providers) {
         mir_coroutine_witnesses: coroutine::mir_coroutine_witnesses,
         optimized_mir,
         check_liveness: liveness::check_liveness,
+        is_mir_available_for_hir,
         is_mir_available,
         mir_callgraph_cyclic: inline::cycle::mir_callgraph_cyclic,
         mir_inliner_callees: inline::cycle::mir_inliner_callees,
@@ -316,30 +319,18 @@ fn take_array<T, const N: usize>(b: &mut Box<[T]>) -> Result<[T; N], Box<[T]>> {
     Ok(*b)
 }
 
-fn is_mir_available(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
-    tcx.mir_keys(()).contains(&def_id)
+fn is_mir_available_for_hir(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
+    tcx.mir_keys_for_hir(()).contains(&def_id)
 }
 
-/// Finds the full set of `DefId`s within the current crate that have
-/// MIR associated with them.
-fn mir_keys(tcx: TyCtxt<'_>, (): ()) -> FxIndexSet<LocalDefId> {
+fn mir_keys_for_hir(tcx: TyCtxt<'_>, (): ()) -> FxIndexSet<LocalDefId> {
     // All body-owners have MIR associated with them.
     let mut set: FxIndexSet<_> = tcx.hir_body_owners().collect();
 
     // Remove the fake bodies for `global_asm!`, since they're not useful
     // to be emitted (`--emit=mir`) or encoded (in metadata).
     set.retain(|&def_id| !matches!(tcx.def_kind(def_id), DefKind::GlobalAsm));
 
-    // Coroutine-closures (e.g. async closures) have an additional by-move MIR
-    // body that isn't in the HIR.
-    for body_owner in tcx.hir_body_owners() {
-        if let DefKind::Closure = tcx.def_kind(body_owner)
-            && tcx.needs_coroutine_by_move_body_def_id(body_owner.to_def_id())
-        {
-            set.insert(tcx.coroutine_by_move_body_def_id(body_owner).expect_local());
-        }
-    }
-
     // tuple struct/variant constructors have MIR, but they don't have a BodyId,
     // so we need to build them separately.
     for item in tcx.hir_crate_items(()).free_items() {
@@ -355,6 +346,28 @@ fn mir_keys(tcx: TyCtxt<'_>, (): ()) -> FxIndexSet<LocalDefId> {
     set
 }
 
+fn is_mir_available(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
+    tcx.mir_keys(()).contains(&def_id)
+}
+
+/// Finds the full set of `DefId`s within the current crate that have
+/// MIR associated with them.
+fn mir_keys(tcx: TyCtxt<'_>, (): ()) -> Cow<'_, FxIndexSet<LocalDefId>> {
+    let mut set = Cow::Borrowed(tcx.mir_keys_for_hir(()));
+
+    // Coroutine-closures (e.g. async closures) have an additional by-move MIR
+    // body that isn't in the HIR.
+    for body_owner in tcx.hir_body_owners() {
+        if let DefKind::Closure = tcx.def_kind(body_owner)
+            && tcx.needs_coroutine_by_move_body_def_id(body_owner.to_def_id())
+        {
+            set.to_mut().insert(tcx.coroutine_by_move_body_def_id(body_owner).expect_local());
+        }
+    }
+
+    set
+}
+
 fn mir_const_qualif(tcx: TyCtxt<'_>, def: LocalDefId) -> ConstQualifs {
     // N.B., this `borrow()` is guaranteed to be valid (i.e., the value
     // cannot yet be stolen), because `mir_promoted()`, which steals

tests/ui/coroutine/avoid-query-cycle-in-ctfe.rs

@@ -0,0 +1,18 @@
+//! Items whose type depends on CTFE (such as the async closure/coroutine beneath, whose type
+//! depends upon evaluating `do_nothing`) should not cause a query cycle owing to the deduction of
+//! the function's parameter attributes, which are only required for codegen and not for CTFE.
+//!
+//! Regression test for https://github.com/rust-lang/rust/issues/151748
+//@ compile-flags: -O
+//@ edition: 2018
+//@ check-pass
+
+fn main() {
+    let _ = async || {
+        let COMPLEX_CONSTANT = ();
+    };
+}
+
+const fn do_nothing() {}
+
+const COMPLEX_CONSTANT: () = do_nothing();