[libcxx-commits] [PATCH] D144994: [Draft][libc++][modules] Adds std module.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added inline comments.


================
Comment at: libcxx/stdmodules/std-coroutine.cppm:4
+# 1 __FILE__ 1 3
+export module std:coroutine;
+export namespace std {
----------------
philnik wrote:
> These probably have to be reserved names.
They are reserved names. This only works due to the `# 1 __FILE__ 1 3`.
I still like clang to have a cleaner solution.


================
Comment at: libcxx/test/std_modules/language.support/support.coroutines/coroutine.handle/coroutine.handle.capacity/operator_bool.pass.cpp:9
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20, c++20
+// UNSUPPORTED: libcpp-no-coroutines
----------------
philnik wrote:
> 
Good point, but I think these extra tests need to be removed in the end. They are mainly from the original tests.


================
Comment at: libcxx/test/std_modules/language.support/support.coroutines/coroutine.handle/coroutine.handle.capacity/operator_bool.pass.cpp:19-24
+#ifdef TEST_MODULES
+import std;
+#else
+#  include <coroutine>
+#  include <type_traits>
+#endif
----------------
philnik wrote:
> Couldn't we just have a header with `import std;` in it that is always included in the C++ modules build? We have to test `#include`ing the headers and modules together anyways. That would avoid touching every single test and adding a preprocessor conditional.
Interesting point. I think we then need 3 versions:
- module with only an import
- module with import and test whether works with includes
- non-module only using includes.

But this is indeed one of the things we need to think about how we want to tackle it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144994/new/

https://reviews.llvm.org/D144994