This is an archive of the discontinued LLVM Phabricator instance.

[coroutines] Add allocation and deallocation substatements.
ClosedPublic

Authored by GorNishanov on Oct 21 2016, 12:51 PM.

Details

Diff Detail

Event Timeline

GorNishanov retitled this revision from to [coroutines] Add allocation and deallocation substatements..
GorNishanov updated this object.
GorNishanov added a reviewer: rsmith.
GorNishanov added subscribers: llvm-commits, EricWF.
ABataev added inline comments.
lib/Sema/SemaCoroutine.cpp
165

id does not fllow LLVM coding rules. Must be Id

169

true argument must have adjacent comment with the name of the parameter, like /*Param=*/true

171

auto *BuiltInDecl

174–175

I believe you can drop the last nullptr arg, it is defaulted. And I think it's better to make it VK_LValue

409

auto *PointeeRD

472–473

The last nullptr can be dropped

481

OpDeleteQualType

494

const auto *OpDeleteType

496–498

No need for braces in one-line substatement

501

Drop last nullptr

Thank you for review! Applied suggested changes.

GorNishanov marked 10 inline comments as done.

Formatting changes (NFC)

rsmith accepted this revision.Oct 27 2016, 12:22 AM
rsmith edited edge metadata.

LGTM with minor changes.

lib/CodeGen/CGCoroutine.cpp
86–87

Can this really happen? The CoroutineBodyStmt should be the first thing we emitted into a new function, so it seems like we shouldn't have pre-existing coro data yet.

lib/Sema/SemaCoroutine.cpp
422

This should be outside the if.

This revision is now accepted and ready to land.Oct 27 2016, 12:22 AM
GorNishanov edited edge metadata.

Thank you for review! Applied suggested changes and preparing to land.

GorNishanov marked 2 inline comments as done.Oct 27 2016, 6:47 AM
GorNishanov added inline comments.
lib/CodeGen/CGCoroutine.cpp
86–87

Yep. You are right! I took it out and made createCoroData return void, since nobody cares about its result anymore.

GorNishanov marked an inline comment as done.

Merged prior to landing. Incoming ...

This revision was automatically updated to reflect the committed changes.