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

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

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

> In https://reviews.llvm.org/D33538#765146, @EricWF 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.
> >
> >
> > That's correct. I was mistaken as to why this was needed. I mistook a bug in libc++ for the reason this was needed. 
> >  So I have no need for this patch anymore.
> >
> > Do you still want to land this for the reasons you mentioned?
>
>
> r303936 will break the libc++ modules build if used with an older version of clang that doesn't have the coroutines builtins. If you're OK with that, then we don't need this. But if you intend to support older versions of Clang, then I think you need either this or a different approach (such as splitting out a separate top-level module for the coroutines header) to avoid that problem.


I'll have to investigate this further. I had assumed the builtins were added at the same time as the `__cpp_coroutines` macro, but if that's not the case libc++ could still guard the header correctly using either `__has_builtin` or the newly updated value of `__cpp_coroutines`; but a complete library solution seems possible.

For other users of Clang module maps, though, I see the convince of being able to do this within the module map.


https://reviews.llvm.org/D33538