rsmith added a comment. 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.) If we're not confident of that, how about calling the new feature something ugly like experimental_coroutines_builtins_20170525 or similar? That way, future versions of Clang can stop advertising that feature if we change the design of the builtins, and we can add a feature 'coroutines' later in a non-disruptive way if/when we decide we're happy with them as-is. ================ Comment at: test/Modules/requires-coroutines.mm:1 +// RUN: rm -rf %t +// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify ---------------- EricWF wrote: > Should this test be called `requires-coroutines.cpp` or is using Obj-C++ a > correct thing to do here? You can use a .cpp extension and the Modules TS `import` keyword if you prefer (add `-fmodules-ts` to the clang arguments), or use a .cpp extension and `#pragma clang module import` if you don't want to depend on a second TS in this test. https://reviews.llvm.org/D33538 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
