Rollup merge of #139153 - compiler-errors:incr-comp-closure, r=oli-obk

Encode synthetic by-move coroutine body with a different `DefPathData`

See the included test. In the first revision rpass1, we have an async closure `{closure#0}` which has a coroutine as a child `{closure#0}::{closure#0}`. We synthesize a by-move coroutine body, which is `{closure#0}::{closure#1}` which depends on the mir_built query, which depends on the typeck query.

In the second revision rpass2, we've replaced the coroutine-closure by a closure with two children closure. Notably, the def path of the second child closure is the same as the synthetic def id from the last revision: `{closure#0}::{closure#1}`. When type-checking this closure, we end up trying to compute its def_span, which tries to fetch it from the incremental cache; this will try to force the dependencies from the last run, which ends up forcing the mir_built query, which ends up forcing the typeck query, which ends up with a query cycle.

The problem here is that we really should never have used the same `DefPathData` for the synthetic by-move coroutine body, since it's not a closure. Changing the `DefPathData` will mean that we can see that the def ids are distinct, which means we won't try to look up the closure's def span from the incremental cache, which will properly skip replaying the node's dependencies and avoid a query cycle.

Fixes #139142

compiler/rustc_hir/src/def.rs

@@ -294,7 +294,7 @@ impl DefKind {
             DefKind::GlobalAsm => DefPathData::GlobalAsm,
             DefKind::Impl { .. } => DefPathData::Impl,
             DefKind::Closure => DefPathData::Closure,
-            DefKind::SyntheticCoroutineBody => DefPathData::Closure,
+            DefKind::SyntheticCoroutineBody => DefPathData::SyntheticCoroutineBody,
         }
     }
 

compiler/rustc_hir/src/definitions.rs

@@ -291,6 +291,8 @@ pub enum DefPathData {
     /// An existential `impl Trait` type node.
     /// Argument position `impl Trait` have a `TypeNs` with their pretty-printed name.
     OpaqueTy,
+    /// A synthetic body for a coroutine's by-move body.
+    SyntheticCoroutineBody,
 }
 
 impl Definitions {
@@ -415,8 +417,16 @@ impl DefPathData {
 
             ValueNs(name) | MacroNs(name) | LifetimeNs(name) => Some(name),
 
-            Impl | ForeignMod | CrateRoot | Use | GlobalAsm | Closure | Ctor | AnonConst
+            Impl
-            | OpaqueTy => None,
+            | ForeignMod
+            | CrateRoot
+            | Use
+            | GlobalAsm
+            | Closure
+            | Ctor
+            | AnonConst
+            | OpaqueTy
+            | SyntheticCoroutineBody => None,
         }
     }
 
@@ -441,6 +451,7 @@ impl DefPathData {
             Ctor => DefPathDataName::Anon { namespace: sym::constructor },
             AnonConst => DefPathDataName::Anon { namespace: sym::constant },
             OpaqueTy => DefPathDataName::Anon { namespace: sym::opaque },
+            SyntheticCoroutineBody => DefPathDataName::Anon { namespace: sym::synthetic },
         }
     }
 }

compiler/rustc_middle/src/ty/context.rs

@@ -1930,10 +1930,10 @@ impl<'tcx> TyCtxt<'tcx> {
         // As a consequence, this LocalDefId is always re-created before it is needed by the incr.
         // comp. engine itself.
         //
-        // This call also writes to the value of `source_span` and `expn_that_defined` queries.
+        // This call also writes to the value of the `source_span` query.
         // This is fine because:
-        // - those queries are `eval_always` so we won't miss their result changing;
+        // - that query is `eval_always` so we won't miss its result changing;
-        // - this write will have happened before these queries are called.
+        // - this write will have happened before that query is called.
         let def_id = self.untracked.definitions.write().create_def(parent, data);
 
         // This function modifies `self.definitions` using a side-effect.

compiler/rustc_middle/src/ty/print/mod.rs

@@ -139,8 +139,7 @@ pub trait Printer<'tcx>: Sized {
 
                     match key.disambiguated_data.data {
                         DefPathData::Closure => {
-                            // FIXME(async_closures): This is somewhat ugly.
+                            // We need to additionally print the `kind` field of a coroutine if
-                            // We need to additionally print the `kind` field of a closure if
                             // it is desugared from a coroutine-closure.
                             if let Some(hir::CoroutineKind::Desugared(
                                 _,
@@ -156,6 +155,10 @@ pub trait Printer<'tcx>: Sized {
                                 // Closures' own generics are only captures, don't print them.
                             }
                         }
+                        DefPathData::SyntheticCoroutineBody => {
+                            // Synthetic coroutine bodies have no distinct generics, since like
+                            // closures they're all just internal state of the coroutine.
+                        }
                         // This covers both `DefKind::AnonConst` and `DefKind::InlineConst`.
                         // Anon consts doesn't have their own generics, and inline consts' own
                         // generics are their inferred types, so don't print them.

compiler/rustc_query_system/src/dep_graph/graph.rs

@@ -66,6 +66,7 @@ pub struct MarkFrame<'a> {
     parent: Option<&'a MarkFrame<'a>>,
 }
 
+#[derive(Debug)]
 pub(super) enum DepNodeColor {
     Red,
     Green(DepNodeIndex),
@@ -909,7 +910,7 @@ impl<D: Deps> DepGraphData<D> {
                 self.try_mark_previous_green(qcx, parent_dep_node_index, dep_dep_node, frame);
 
             if node_index.is_some() {
-                debug!("managed to MARK dependency {dep_dep_node:?} as green",);
+                debug!("managed to MARK dependency {dep_dep_node:?} as green");
                 return Some(());
             }
         }
@@ -930,7 +931,7 @@ impl<D: Deps> DepGraphData<D> {
                 return Some(());
             }
             Some(DepNodeColor::Red) => {
-                debug!("dependency {dep_dep_node:?} was red after forcing",);
+                debug!("dependency {dep_dep_node:?} was red after forcing");
                 return None;
             }
             None => {}
@@ -950,7 +951,7 @@ impl<D: Deps> DepGraphData<D> {
         // invalid state will not be persisted to the
         // incremental compilation cache because of
         // compilation errors being present.
-        debug!("dependency {dep_dep_node:?} resulted in compilation error",);
+        debug!("dependency {dep_dep_node:?} resulted in compilation error");
         return None;
     }
 

compiler/rustc_sanitizers/src/cfi/typeid/itanium_cxx_abi/encode.rs

@@ -716,6 +716,7 @@ fn encode_ty_name(tcx: TyCtxt<'_>, def_id: DefId) -> String {
             hir::definitions::DefPathData::Ctor => "c",
             hir::definitions::DefPathData::AnonConst => "k",
             hir::definitions::DefPathData::OpaqueTy => "i",
+            hir::definitions::DefPathData::SyntheticCoroutineBody => "s",
             hir::definitions::DefPathData::CrateRoot
             | hir::definitions::DefPathData::Use
             | hir::definitions::DefPathData::GlobalAsm

compiler/rustc_symbol_mangling/src/legacy.rs

@@ -28,7 +28,10 @@ pub(super) fn mangle<'tcx>(
     loop {
         let key = tcx.def_key(ty_def_id);
         match key.disambiguated_data.data {
-            DefPathData::TypeNs(_) | DefPathData::ValueNs(_) | DefPathData::Closure => {
+            DefPathData::TypeNs(_)
+            | DefPathData::ValueNs(_)
+            | DefPathData::Closure
+            | DefPathData::SyntheticCoroutineBody => {
                 instance_ty = tcx.type_of(ty_def_id).instantiate_identity();
                 debug!(?instance_ty);
                 break;

compiler/rustc_symbol_mangling/src/v0.rs

@@ -850,6 +850,7 @@ impl<'tcx> Printer<'tcx> for SymbolMangler<'tcx> {
             DefPathData::Ctor => 'c',
             DefPathData::AnonConst => 'k',
             DefPathData::OpaqueTy => 'i',
+            DefPathData::SyntheticCoroutineBody => 's',
 
             // These should never show up as `path_append` arguments.
             DefPathData::CrateRoot

tests/coverage/async_closure.cov-map

@@ -38,6 +38,15 @@ Number of file 0 mappings: 1
 - Code(Counter(0)) at (prev + 11, 35) to (start + 0, 36)
 Highest counter ID seen: c0
 
+Function name: async_closure::main::{closure#0}
+Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 23, 00, 24]
+Number of files: 1
+- file 0 => global file 1
+Number of expressions: 0
+Number of file 0 mappings: 1
+- Code(Counter(0)) at (prev + 11, 35) to (start + 0, 36)
+Highest counter ID seen: c0
+
 Function name: async_closure::main::{closure#0}::{closure#0}::<i16>
 Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 22, 00, 24]
 Number of files: 1
@@ -47,12 +56,3 @@ Number of file 0 mappings: 1
 - Code(Counter(0)) at (prev + 11, 34) to (start + 0, 36)
 Highest counter ID seen: c0
 
-Function name: async_closure::main::{closure#0}::{closure#1}::<i32>
-Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 23, 00, 24]
-Number of files: 1
-- file 0 => global file 1
-Number of expressions: 0
-Number of file 0 mappings: 1
-- Code(Counter(0)) at (prev + 11, 35) to (start + 0, 36)
-Highest counter ID seen: c0
-

tests/coverage/async_closure.coverage

@@ -14,7 +14,7 @@
   | async_closure::main::{closure#0}:
   |   LL|      1|    let async_closure = async || {};
   ------------------
-  | async_closure::main::{closure#0}::{closure#1}::<i32>:
+  | async_closure::main::{closure#0}:
   |   LL|      1|    let async_closure = async || {};
   ------------------
    LL|      1|    executor::block_on(async_closure());

tests/incremental/user-written-closure-synthetic-closure-conflict.rs

@@ -0,0 +1,15 @@
+//@ revisions: rpass1 rpass2
+//@ edition: 2024
+
+#![allow(unused)]
+
+fn main() {
+    #[cfg(rpass1)]
+    async || {};
+
+    #[cfg(rpass2)]
+    || {
+        || ();
+        || ();
+    };
+}

tests/mir-opt/async_closure_fake_read_for_by_move.foo-{closure#0}-{synthetic#0}.built.after.mir

@@ -1,6 +1,6 @@
-// MIR for `foo::{closure#0}::{closure#1}` after built
+// MIR for `foo::{closure#0}::{synthetic#0}` after built
 
-fn foo::{closure#0}::{closure#1}(_1: {async closure body@$DIR/async_closure_fake_read_for_by_move.rs:12:27: 15:6}, _2: ResumeTy) -> ()
+fn foo::{closure#0}::{synthetic#0}(_1: {async closure body@$DIR/async_closure_fake_read_for_by_move.rs:12:27: 15:6}, _2: ResumeTy) -> ()
 yields ()
  {
     debug _task_context => _2;

tests/mir-opt/async_closure_fake_read_for_by_move.rs

@@ -7,7 +7,7 @@ enum Foo {
 }
 
 // EMIT_MIR async_closure_fake_read_for_by_move.foo-{closure#0}-{closure#0}.built.after.mir
-// EMIT_MIR async_closure_fake_read_for_by_move.foo-{closure#0}-{closure#1}.built.after.mir
+// EMIT_MIR async_closure_fake_read_for_by_move.foo-{closure#0}-{synthetic#0}.built.after.mir
 fn foo(f: &Foo) {
     let x = async move || match f {
         Foo::Bar if true => {}

tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{synthetic#0}.built.after.mir

@@ -1,6 +1,6 @@
-// MIR for `main::{closure#0}::{closure#0}::{closure#1}` after built
+// MIR for `main::{closure#0}::{closure#0}::{synthetic#0}` after built
 
-fn main::{closure#0}::{closure#0}::{closure#1}(_1: {async closure body@$DIR/async_closure_shims.rs:53:53: 56:10}, _2: ResumeTy) -> ()
+fn main::{closure#0}::{closure#0}::{synthetic#0}(_1: {async closure body@$DIR/async_closure_shims.rs:53:53: 56:10}, _2: ResumeTy) -> ()
 yields ()
  {
     debug _task_context => _2;

tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{synthetic#0}.built.after.mir

@@ -1,6 +1,6 @@
-// MIR for `main::{closure#0}::{closure#1}::{closure#1}` after built
+// MIR for `main::{closure#0}::{closure#1}::{synthetic#0}` after built
 
-fn main::{closure#0}::{closure#1}::{closure#1}(_1: {async closure body@$DIR/async_closure_shims.rs:62:48: 65:10}, _2: ResumeTy) -> ()
+fn main::{closure#0}::{closure#1}::{synthetic#0}(_1: {async closure body@$DIR/async_closure_shims.rs:62:48: 65:10}, _2: ResumeTy) -> ()
 yields ()
  {
     debug _task_context => _2;

tests/mir-opt/async_closure_shims.rs

@@ -42,11 +42,11 @@ async fn call_normal_mut<F: Future<Output = ()>>(f: &mut impl FnMut(i32) -> F) {
 
 // EMIT_MIR async_closure_shims.main-{closure#0}-{closure#0}.coroutine_closure_by_move.0.mir
 // EMIT_MIR async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.built.after.mir
-// EMIT_MIR async_closure_shims.main-{closure#0}-{closure#0}-{closure#1}.built.after.mir
+// EMIT_MIR async_closure_shims.main-{closure#0}-{closure#0}-{synthetic#0}.built.after.mir
 // EMIT_MIR async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_ref.0.mir
 // EMIT_MIR async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_move.0.mir
 // EMIT_MIR async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.built.after.mir
-// EMIT_MIR async_closure_shims.main-{closure#0}-{closure#1}-{closure#1}.built.after.mir
+// EMIT_MIR async_closure_shims.main-{closure#0}-{closure#1}-{synthetic#0}.built.after.mir
 pub fn main() {
     block_on(async {
         let b = 2i32;

tests/ui/stable-mir-print/async-closure.stdout

@@ -56,7 +56,7 @@ fn foo::{closure#0}::{closure#0}(_1: Pin<&mut {async closure body@$DIR/async-clo
         unreachable;
     }
 }
-fn foo::{closure#0}::{closure#1}(_1: Pin<&mut {async closure body@$DIR/async-closure.rs:9:22: 11:6}>, _2: &mut Context<'_>) -> Poll<()> {
+fn foo::{closure#0}::{synthetic#0}(_1: Pin<&mut {async closure body@$DIR/async-closure.rs:9:22: 11:6}>, _2: &mut Context<'_>) -> Poll<()> {
     let mut _0: Poll<()>;
     let  _3: i32;
     let mut _4: &i32;