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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
