This is an archive of the discontinued LLVM Phabricator instance.

[Sema] isValidCoroutineContext FIXME and citations
ClosedPublic

Authored by modocache on Jun 23 2018, 8:46 AM.

Details

Summary

Add citations to the Coroutines TS to the isValidCoroutineContext
function, as well as a FIXME and test for [expr.await]p2, which states
a co_await expression cannot be used in a default argument.

Test Plan: check-clang

Diff Detail

Event Timeline

modocache created this revision.Jun 23 2018, 8:46 AM

Also @GorNishanov I'm curious about your two cents on whether comments like these are valuable. If you think they are I may add a few more with post-commit review.

modocache added inline comments.Jun 23 2018, 8:49 AM
lib/Sema/SemaCoroutine.cpp
260

@GorNishanov Is there anything in the TS that states copy and move assignment operators shall not include await or yield expressions? These were added D25292 but I'm not sure whether I'm missing something in the TS text, or if maybe this language was in a prior revision of the TS.

GorNishanov accepted this revision.Jun 23 2018, 10:00 AM
GorNishanov added a subscriber: rsmith.

LGTM

lib/Sema/SemaCoroutine.cpp
260

Yes. N4499/[special] said:

A special member function shall not be a coroutine.

I think @rsmith wanted to relax it, but, I am not sure if he had a use case in mind.

I am thinking putting the restriction from N4499 back.
My approach is if in doubt, be more restrictive initially, then, we can relax if use cases are discovered. It will be a non-breaking change.

This revision is now accepted and ready to land.Jun 23 2018, 10:00 AM

Great, thanks for the review! I added a reference to N4499.

modocache marked 2 inline comments as done.Jun 23 2018, 10:33 AM
This revision was automatically updated to reflect the committed changes.