Page MenuHomePhabricator

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

Authored by modocache on Nov 21 2019, 11:47 AM.

Details

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.

Diff Detail

Event Timeline

modocache created this revision.Nov 21 2019, 11:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 21 2019, 11:47 AM
modocache edited the summary of this revision. (Show Details)Nov 21 2019, 11:48 AM
junparser accepted this revision.Mar 29 2020, 10:36 PM

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

This revision is now accepted and ready to land.Mar 29 2020, 10:36 PM

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

This revision was automatically updated to reflect the committed changes.