Unrolled build for #156875

Rollup merge of #156875 - cjgillot:yield-drop-resume, r=oli-obk

Correct and document semantics of `yield` terminator

The current implementation of dataflow for the yield terminator confuses the `drop` target with unwinding. Which is wrong, in particular when implementing async drops.

r? @RalfJung

Author: rust-timer

compiler/rustc_middle/src/mir/syntax.rs

@@ -872,20 +872,25 @@ pub enum TerminatorKind<'tcx> {
     /// Marks a suspend point.
     ///
     /// Like `Return` terminators in coroutine bodies, this computes `value` and then a
-    /// `CoroutineState::Yielded(value)` as if by `Aggregate` rvalue. That value is then assigned to
-    /// the return place of the function calling this one, and execution continues in the calling
-    /// function. When next invoked with the same first argument, execution of this function
-    /// continues at the `resume` basic block, with the second argument written to the `resume_arg`
-    /// place. If the coroutine is dropped before then, the `drop` basic block is invoked.
+    /// `CoroutineState::Yielded(value)` as if by `Aggregate` rvalue. That value is then assigned
+    /// to the return place provided by the caller function, and execution continues in this caller
+    /// function.
+    ///
+    /// When the coroutine is resumed/polled, execution of this function continues at the `resume`
+    /// basic block, the `resume_arg` place is evaluated and the second argument to `resume/poll`
+    /// is written to it.
+    ///
+    /// If the coroutine is dropped before then, execution of this function continues at the `drop`
+    /// basic block and the `resume_arg` place expression is evaluated. For async drop, the second
+    /// argument to the destructor `resume/poll` method is written to `resume_arg`. For synchronous
+    /// drops, uninitialized bytes are written to `resume_arg`.
     ///
     /// Note that coroutines can be (unstably) cloned under certain conditions, which means that
     /// this terminator can **return multiple times**! MIR optimizations that reorder code into
     /// different basic blocks needs to be aware of that.
     /// See <https://github.com/rust-lang/rust/issues/95360>.
     ///
     /// Not permitted in bodies that are not coroutine bodies, or after coroutine lowering.
-    ///
-    /// **Needs clarification**: What about the evaluation order of the `resume_arg` and `value`?
     Yield {
         /// The value to return.
         value: Operand<'tcx>,

compiler/rustc_middle/src/mir/terminator.rs

@@ -674,7 +674,7 @@ impl<'tcx> TerminatorKind<'tcx> {
     }
 }
 
-#[derive(Copy, Clone, Debug)]
+#[derive(Debug)]
 pub enum TerminatorEdges<'mir, 'tcx> {
     /// For terminators that have no successor, like `return`.
     None,
@@ -686,7 +686,7 @@ pub enum TerminatorEdges<'mir, 'tcx> {
     Double(BasicBlock, BasicBlock),
     /// Special action for `Yield`, `Call` and `InlineAsm` terminators.
     AssignOnReturn {
-        return_: &'mir [BasicBlock],
+        return_: Box<[BasicBlock]>,
         /// The cleanup block, if it exists.
         cleanup: Option<BasicBlock>,
         place: CallReturnPlaces<'mir, 'tcx>,
@@ -755,27 +755,21 @@ impl<'tcx> TerminatorKind<'tcx> {
                 TerminatorEdges::Double(real_target, imaginary_target)
             }
 
-            Yield { resume: ref target, drop, resume_arg, value: _ } => {
+            Yield { resume: target, drop, resume_arg, value: _ } => {
                 TerminatorEdges::AssignOnReturn {
-                    return_: slice::from_ref(target),
-                    cleanup: drop,
+                    return_: [target].into_iter().chain(drop.into_iter()).collect(),
+                    cleanup: None,
                     place: CallReturnPlaces::Yield(resume_arg),
                 }
             }
 
-            Call {
-                unwind,
-                destination,
-                ref target,
-                func: _,
-                args: _,
-                fn_span: _,
-                call_source: _,
-            } => TerminatorEdges::AssignOnReturn {
-                return_: target.as_ref().map(slice::from_ref).unwrap_or_default(),
-                cleanup: unwind.cleanup_block(),
-                place: CallReturnPlaces::Call(destination),
-            },
+            Call { unwind, destination, target, func: _, args: _, fn_span: _, call_source: _ } => {
+                TerminatorEdges::AssignOnReturn {
+                    return_: target.into_iter().collect(),
+                    cleanup: unwind.cleanup_block(),
+                    place: CallReturnPlaces::Call(destination),
+                }
+            }
 
             InlineAsm {
                 asm_macro: _,
@@ -786,7 +780,7 @@ impl<'tcx> TerminatorKind<'tcx> {
                 ref targets,
                 unwind,
             } => TerminatorEdges::AssignOnReturn {
-                return_: targets,
+                return_: targets.to_owned(),
                 cleanup: unwind.cleanup_block(),
                 place: CallReturnPlaces::InlineAsm(operands),
             },

compiler/rustc_mir_dataflow/src/framework/direction.rs

@@ -101,11 +101,13 @@ impl Direction for Backward {
                     propagate(pred, &tmp);
                 }
 
-                mir::TerminatorKind::Yield { resume, resume_arg, .. } if resume == block => {
+                mir::TerminatorKind::Yield { resume, drop, resume_arg, .. }
+                    if resume == block || drop == Some(block) =>
+                {
                     let mut tmp = exit_state.clone();
                     analysis.apply_call_return_effect(
                         &mut tmp,
-                        resume,
+                        block,
                         CallReturnPlaces::Yield(resume_arg),
                     );
                     propagate(pred, &tmp);
@@ -275,7 +277,7 @@ impl Direction for Forward {
 
                 if !return_.is_empty() {
                     analysis.apply_call_return_effect(exit_state, block, place);
-                    for &target in return_ {
+                    for target in return_ {
                         propagate(target, exit_state);
                     }
                 }

compiler/rustc_mir_transform/src/coroutine.rs

@@ -1302,13 +1302,21 @@ fn create_coroutine_resume_function<'tcx>(
 enum Operation {
     Resume,
     Drop,
+    AsyncDrop,
 }
 
 impl Operation {
     fn target_block(self, point: &SuspensionPoint<'_>) -> Option<BasicBlock> {
         match self {
             Operation::Resume => Some(point.resume),
-            Operation::Drop => point.drop,
+            Operation::Drop | Operation::AsyncDrop => point.drop,
+        }
+    }
+
+    fn resume_place<'tcx>(self, point: &SuspensionPoint<'tcx>) -> Option<Place<'tcx>> {
+        match self {
+            Operation::Resume | Operation::AsyncDrop => Some(point.resume_arg),
+            Operation::Drop => None,
         }
     }
 }
@@ -1339,12 +1347,14 @@ fn create_cases<'tcx>(
                     }
                 }
 
-                if operation == Operation::Resume && point.resume_arg != CTX_ARG.into() {
-                    // Move the resume argument to the destination place of the `Yield` terminator
+                // Move the resume argument to the destination place of the `Yield` terminator
+                if let Some(resume_arg) = operation.resume_place(point)
+                    && resume_arg != CTX_ARG.into()
+                {
                     statements.push(Statement::new(
                         source_info,
                         StatementKind::Assign(Box::new((
-                            point.resume_arg,
+                            resume_arg,
                             Rvalue::Use(Operand::Move(CTX_ARG.into()), WithRetag::Yes),
                         ))),
                     ));

compiler/rustc_mir_transform/src/coroutine/drop.rs

@@ -646,7 +646,7 @@ pub(super) fn create_coroutine_drop_shim_async<'tcx>(
 
     let source_info = SourceInfo::outermost(body.span);
 
-    let mut cases = create_cases(&mut body, transform, Operation::Drop);
+    let mut cases = create_cases(&mut body, transform, Operation::AsyncDrop);
 
     cases.insert(0, (CoroutineArgs::UNRESUMED, drop_clean));
 

tests/mir-opt/coroutine/async_drop_live_dead.a-{closure#0}.coroutine_drop_async.0.panic-abort.mir

@@ -86,10 +86,12 @@ fn a::{closure#0}(_1: Pin<&mut {async fn body of a<T>()}>, _2: &mut Context<'_>)
     }
 
     bb12: {
+        _9 = move _2;
         goto -> bb4;
     }
 
     bb13: {
+        _13 = move _2;
         goto -> bb4;
     }
 

tests/mir-opt/coroutine/async_drop_live_dead.a-{closure#0}.coroutine_drop_async.0.panic-unwind.mir

@@ -105,10 +105,12 @@ fn a::{closure#0}(_1: Pin<&mut {async fn body of a<T>()}>, _2: &mut Context<'_>)
     }
 
     bb16: {
+        _9 = move _2;
         goto -> bb7;
     }
 
     bb17: {
+        _13 = move _2;
         goto -> bb7;
     }