[clang][NFC] Augment CoreturnStmt

[urnathan]

Summary

CoreturnStmt needs to keep the operand value distinct from its use in
any return_value call, so that instantiation may rebuild the latter.
But it also needs to keep the operand value separate in the case of
calling return_void. Code generation needs the same check on the
operand value to know whether it is being evaluated in a void context
solely for side effects.

Rather than repeat the (non-trivial) test, add a flag to CoreturnStmt
indicating the void-return case. This simplifies the code generation,
making it harder to get out of sync.

[ChuanqiXu]

This is not correct. Any change to the AST node requires change to Serialization too.

[MaskRay]

I see that a new patch with the same intent/subject has been created: D125880.
Normally the same differential should be used to keep track of all past discussions on a differential.
The "History" tab on the page allows a user to compare two patches to get insights of the evolution of the differential.
It will help archaeology for future contributors: they will be able to understand what approaches have been proven successful and what haven't.

The unwritten reusing-differential rule also applies to patch relands, e.g. D118352/D122256.

clang/include/clang/AST/StmtCXX.h

 ///   <promise>.return_void()
 /// which we name the "promise call".
 class CoreturnStmt : public Stmt {
   SourceLocation CoreturnLoc;
 
   enum SubStmt { Operand, PromiseCall, Count };
   Stmt *SubStmts[SubStmt::Count];
 
+  bool IsVoid : 1; // A void return with non-null operand.
   bool IsImplicit : 1;
 
   friend class ASTStmtReader;
 public:
   CoreturnStmt(SourceLocation CoreturnLoc, Stmt *Operand, Stmt *PromiseCall,
-               bool IsImplicit = false)
-      : Stmt(CoreturnStmtClass), CoreturnLoc(CoreturnLoc),
+               bool IsVoid, bool IsImplicit = false)
+      : Stmt(CoreturnStmtClass), CoreturnLoc(CoreturnLoc), IsVoid(IsVoid),
         IsImplicit(IsImplicit) {
     SubStmts[SubStmt::Operand] = Operand;
     SubStmts[SubStmt::PromiseCall] = PromiseCall;
   }
 
-  CoreturnStmt(EmptyShell) : CoreturnStmt({}, {}, {}) {}
+  CoreturnStmt(EmptyShell) : CoreturnStmt({}, {}, {}, {}) {}
 
   SourceLocation getKeywordLoc() const { return CoreturnLoc; }
 
   /// Retrieve the operand of the 'co_return' statement. Will be nullptr
   /// if none was specified.
   Expr *getOperand() const { return static_cast<Expr*>(SubStmts[Operand]); }
 
   /// Retrieve the promise call that results from this 'co_return'
-  /// statement. Will be nullptr if either the coroutine has not yet been
-  /// finalized or the coroutine has no eventual return type.
+  /// statement. Will encapsulate the operand, unless IsVoid is true.
   Expr *getPromiseCall() const {
     return static_cast<Expr*>(SubStmts[PromiseCall]);
   }
 
   bool isImplicit() const { return IsImplicit; }
   void setIsImplicit(bool value = true) { IsImplicit = value; }
 
+  bool getIsVoid() const { return IsVoid; }
+  void setIsVoid(bool value = true) { IsVoid = value; }
+
   SourceLocation getBeginLoc() const LLVM_READONLY { return CoreturnLoc; }
   SourceLocation getEndLoc() const LLVM_READONLY {
     return getOperand() ? getOperand()->getEndLoc() : getBeginLoc();
   }
 
   child_range children() {
     return child_range(SubStmts, SubStmts + SubStmt::Count);
   }

clang/lib/CodeGen/CGCoroutine.cpp

RValue CodeGenFunction::EmitCoyieldExpr(const CoyieldExpr &E,
                                         AggValueSlot aggSlot,
                                         bool ignoreResult) {
   return emitSuspendExpression(*this, *CurCoro.Data, E, AwaitKind::Yield,
                                aggSlot, ignoreResult, /*forLValue*/false).RV;
 }
 
 void CodeGenFunction::EmitCoreturnStmt(CoreturnStmt const &S) {
   ++CurCoro.Data->CoreturnCount;
-  const Expr *RV = S.getOperand();
-  if (RV && RV->getType()->isVoidType() && !isa<InitListExpr>(RV)) {
-    // Make sure to evaluate the non initlist expression of a co_return
-    // with a void expression for side effects.
+  if (S.getIsVoid()) {
+    // Evaluate the void expression for side effects.
     RunCleanupsScope cleanupScope(*this);
-    EmitIgnoredExpr(RV);
+    EmitIgnoredExpr(S.getOperand());
   }
   EmitStmt(S.getPromiseCall());
   EmitBranchThroughCleanup(CurCoro.Data->FinalJD);
 }
 
 
 #ifndef NDEBUG
 static QualType getCoroutineSuspendExprReturnType(const ASTContext &Ctx,

clang/lib/Sema/SemaCoroutine.cpp

if (E && E->hasPlaceholderType() &&
     if (R.isInvalid()) return StmtError();
     E = R.get();
   }
 
   VarDecl *Promise = FSI->CoroutinePromise;
   if ((E && E->getType()->isDependentType()) ||
       Promise->getType()->isDependentType())
     // It is dependent, do nothing until instantiation time.
-    return new (Context) CoreturnStmt(Loc, E, nullptr, IsImplicit);
+    return new (Context) CoreturnStmt(Loc, E, nullptr,
+                                      /*IsVoid*/ false, IsImplicit);
 
+  bool IsVoid = false;
   ExprResult PC;
   if (E && (isa<InitListExpr>(E) || !E->getType()->isVoidType())) {
     getNamedReturnInfo(E, SimplerImplicitMoveMode::ForceOn);
     PC = buildPromiseCall(*this, Promise, Loc, "return_value", E);
   } else {
     E = MakeFullDiscardedValueExpr(E).get();
+    IsVoid = bool(E);
     PC = buildPromiseCall(*this, Promise, Loc, "return_void", None);
   }
   if (PC.isInvalid())
     return StmtError();
 
   Expr *PCE = ActOnFinishFullExpr(PC.get(), /*DiscardedValue*/ false).get();
-
-  return new (Context) CoreturnStmt(Loc, E, PCE, IsImplicit);
+  return new (Context) CoreturnStmt(Loc, E, PCE, IsVoid, IsImplicit);
 }
 
 /// Look up the std::nothrow object.
 static Expr *buildStdNoThrowDeclRef(Sema &S, SourceLocation Loc) {
   NamespaceDecl *Std = S.getStdNamespace();
   assert(Std && "Should already be diagnosed");
 
   LookupResult Result(S, &S.PP.getIdentifierTable().get("nothrow"), Loc,