[coroutines] Don't build promise init with no args

[modocache]

Summary

In the case of a coroutine that takes no arguments,
Sema::buildCoroutinePromise constructs a list-initialization
(clang::InitializationKind::InitKind::IK_DirectList) of the
promise variable, using a list of empty arguments. So, if one were to
dump the promise VarDecl immediately after Sema::ActOnCoroutineBodyStart
calls checkCoroutineContext, for a coroutine function that takes no
arguments, they'd see the following:

VarDecl 0xb514490 <test.cpp:26:3> col:3 __promise '<dependent type>' callinit
`-ParenListExpr 0xb514510 <col:3> 'NULL TYPE'

But after this patch, the ParenListExpr is no longer constructed, and
the promise variable uses default initialization
(clang::InitializationKind::InitKind::IK_Default):

VarDecl 0x63100012dae0 <test.cpp:26:3> col:3 __promise '<dependent type>'

As far as I know, there's no case in which list-initialization with no
arguments differs from default initialization, but if I'm wrong please
let me know (and I'll add a test case that demonstrates the change --
but as-is I can't think of a functional test case for this). I think both
comply with the wording of C++20 [dcl.fct.def.coroutine]p5:

_promise-constructor-arguments_ is determined as follows: overload
resolution is performed on a promise constructor call created by
assembling an argument list with lvalues p1 ... pn. If a viable
constructor is found (12.4.2), then _promise-constructor-arguments_
is (p1, ... , pn), otherwise _promise-constructor-arguments_ is
empty.

Still, I think this patch is an improvement regardless, because it
reduces the size of the AST.

[junparser]

LGTM

[junparser]

I'm not sure whether I can approve this. If not, @modocache please ignore this.

[modocache]

Of course, your approval is very welcome! 😄 I'll go ahead and land this today, thanks for the review!

clang/lib/Sema/SemaCoroutine.cpp

VarDecl *Sema::buildCoroutinePromise(SourceLocation Loc) {
   auto *VD = VarDecl::Create(Context, FD, FD->getLocation(), FD->getLocation(),
                              &PP.getIdentifierTable().get("__promise"), T,
                              Context.getTrivialTypeSourceInfo(T, Loc), SC_None);
   CheckVariableDeclarationType(VD);
   if (VD->isInvalidDecl())
     return nullptr;
 
   auto *ScopeInfo = getCurFunction();
-  // Build a list of arguments, based on the coroutine functions arguments,
-  // that will be passed to the promise type's constructor.
+
+  // Build a list of arguments, based on the coroutine function's arguments,
+  // that if present will be passed to the promise type's constructor.
   llvm::SmallVector<Expr *, 4> CtorArgExprs;
 
   // Add implicit object parameter.
   if (auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
     if (MD->isInstance() && !isLambdaCallOperator(MD)) {
       ExprResult ThisExpr = ActOnCXXThis(Loc);
       if (ThisExpr.isInvalid())
         return nullptr;
       ThisExpr = CreateBuiltinUnaryOp(Loc, UO_Deref, ThisExpr.get());
       if (ThisExpr.isInvalid())
         return nullptr;
       CtorArgExprs.push_back(ThisExpr.get());
     }
   }
 
+  // Add the coroutine function's parameters.
   auto &Moves = ScopeInfo->CoroutineParameterMoves;
   for (auto *PD : FD->parameters()) {
     if (PD->getType()->isDependentType())
       continue;
 
     auto RefExpr = ExprEmpty();
     auto Move = Moves.find(PD);
     assert(Move != Moves.end() &&
            "Coroutine function parameter not inserted into move map");
     // If a reference to the function parameter exists in the coroutine
     // frame, use that reference.
     auto *MoveDecl =
         cast<VarDecl>(cast<DeclStmt>(Move->second)->getSingleDecl());
     RefExpr =
         BuildDeclRefExpr(MoveDecl, MoveDecl->getType().getNonReferenceType(),
                          ExprValueKind::VK_LValue, FD->getLocation());
     if (RefExpr.isInvalid())
       return nullptr;
     CtorArgExprs.push_back(RefExpr.get());
   }
 
+  // If we have a non-zero number of constructor arguments, try to use them.
+  // Otherwise, fall back to the promise type's default constructor.
+  if (!CtorArgExprs.empty()) {
     // Create an initialization sequence for the promise type using the
     // constructor arguments, wrapped in a parenthesized list expression.
     Expr *PLE = ParenListExpr::Create(Context, FD->getLocation(),
                                       CtorArgExprs, FD->getLocation());
     InitializedEntity Entity = InitializedEntity::InitializeVariable(VD);
     InitializationKind Kind = InitializationKind::CreateForInit(
         VD->getLocation(), /*DirectInit=*/true, PLE);
     InitializationSequence InitSeq(*this, Entity, Kind, CtorArgExprs,
                                    /*TopLevelOfInitList=*/false,
                                    /*TreatUnavailableAsInvalid=*/false);
 
     // Attempt to initialize the promise type with the arguments.
     // If that fails, fall back to the promise type's default constructor.
     if (InitSeq) {
       ExprResult Result = InitSeq.Perform(*this, Entity, Kind, CtorArgExprs);
       if (Result.isInvalid()) {
         VD->setInvalidDecl();
       } else if (Result.get()) {
         VD->setInit(MaybeCreateExprWithCleanups(Result.get()));
         VD->setInitStyle(VarDecl::CallInit);
         CheckCompleteVariableDeclaration(VD);
       }
     } else
       ActOnUninitializedDecl(VD);
+  } else
+    ActOnUninitializedDecl(VD);
 
   FD->addDecl(VD);
   return VD;
 }
 
 /// Check that this is a context in which a coroutine suspension can appear.
 static FunctionScopeInfo *checkCoroutineContext(Sema &S, SourceLocation Loc,
                                                 StringRef Keyword,