[PATCH] D33538: [coroutines] Support "coroutines" feature in module map requires clause

[Richard Smith via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D33538#765062, @rsmith wrote:

> In https://reviews.llvm.org/D33538#765045, @rsmith wrote:
>
> > Do we need to conditionalize this part of libc++? Nothing in the <coroutines> header appears to need compiler support.
>
>
> Oh wait, I see what's going on. You're not testing for whether coroutines is enabled, you're testing for whether the `__builtin_coro_*` builtins exist. Are we sufficiently confident that those aren't going to change that we're prepared to make libc++ rely on this? (If we change the signature of those builtins in the future, then new versions of clang would stop being able to build old versions of the libc++ module.)


On reflection, I think this is fine. If the signatures of the builtins change, and the user builds with `-fmodules` `-fcoroutines-ts` and libc++, and there's version skew between libc++ and clang, they'll get a compile error. That's mostly the expected outcome; the issue is that we'd produce this compile error *even if* they never `#include <experimental/coroutines>`, because building the complete libc++ module would fail in that situation.

If we're worried about that, we could split the coroutines header out of the main libc++ module into its own top-level module, but I don't think we need to worry too much about rejecting code that uses `-fcoroutines-ts` but never actually uses a coroutine.


https://reviews.llvm.org/D33538