This is an archive of the discontinued LLVM Phabricator instance.

[coroutines] Build and pass coroutine_handle to await_suspend.
ClosedPublic

Authored by GorNishanov on Nov 4 2016, 4:55 PM.

Details

Summary

This patch adds passing a coroutine_handle object to await_suspend calls.

It builds the coroutine_handle using coroutine_handle<PromiseType>::from_address(__builtin_coro_frame()).

Diff Detail

Event Timeline

EricWF updated this revision to Diff 76955.Nov 4 2016, 4:55 PM
EricWF retitled this revision from to [coroutines] Build and pass coroutine_handle to await_suspend..
EricWF updated this object.
EricWF added reviewers: rsmith, GorNishanov.
EricWF added a subscriber: cfe-commits.
EricWF updated this revision to Diff 88852.Feb 16 2017, 8:56 PM
  • Update so that it merges with D26057
GorNishanov commandeered this revision.Mar 6 2017, 11:56 AM
GorNishanov edited reviewers, added: EricWF; removed: GorNishanov.
GorNishanov edited edge metadata.

Comandeering this patch.
Last week Richard gave me some feedback in person on this code (in the full coroutine implementation not this patch in particular). I am going to implement the feedback and adjust this patch and resubmit for review.

GorNishanov updated this revision to Diff 90861.Mar 7 2017, 8:32 AM

Per's @rsmith feedback in Kona

  • Added diagnostic if from_address is missing from coroutine_handle
  • Switch to using BuildDeclarationNameExpr in buildCoroutineHandle
GorNishanov updated this revision to Diff 90862.Mar 7 2017, 8:38 AM
  • removed '&' that snicked near Location parameter
  • reordered a few lines to minimize the diff from Eric's version
EricWF edited edge metadata.Mar 8 2017, 1:37 PM

This LGTM, but I don't know enough Clang to be 100% on it. Perhaps somebody else could give it a quick second look?

lib/Sema/SemaCoroutine.cpp
159

Should this diagnostic be renamed since it's general to both coroutine_traits and coroutine_handle?

test/SemaCXX/coroutines.cpp
549

Nit, put the // expected-note directly on the co_return line.

EricWF accepted this revision.Mar 8 2017, 1:41 PM

On second thought if @rsmith already reviewed most of this offline then I feel comfortable giving it the thumbs up.

This revision is now accepted and ready to land.Mar 8 2017, 1:41 PM
EricWF added a comment.Mar 8 2017, 3:38 PM

@GorNishanov You forgot to update test/SemaCXX/coreturn.cpp.

GorNishanov updated this revision to Diff 91117.Mar 8 2017, 5:47 PM
  • Addressed nits
  • Improved error messages even further
  • Merge on top of the trunk

Preparing to land

GorNishanov updated this revision to Diff 91119.Mar 8 2017, 6:37 PM

preparing to land