maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

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

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.

Reviewers: GorNishanov, rsmith, lewissbaker

Subscribers: EricWF, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D70555
This commit is contained in:
Brian Gesiak 2019-11-21 13:51:44 -05:00
parent 3a5192098c
commit 627e01feb7
1 changed files with 30 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
clang/lib/Sema/SemaCoroutine.cpp
View File

@ -502,8 +502,9 @@ VarDecl *Sema::buildCoroutinePromise(SourceLocation Loc) {
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.
@ -519,6 +520,7 @@ VarDecl *Sema::buildCoroutinePromise(SourceLocation Loc) {
}
}
// Add the coroutine function's parameters.
auto &Moves = ScopeInfo->CoroutineParameterMoves;
for (auto *PD : FD->parameters()) {
if (PD->getType()->isDependentType())
@ -540,28 +542,33 @@ VarDecl *Sema::buildCoroutinePromise(SourceLocation Loc) {
CtorArgExprs.push_back(RefExpr.get());
}
// 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);
// 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);
}
// 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);