[libcxx-commits] [PATCH] D156177: [libc++][Modules] Recreate the top level `std` clang module

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

I'm not entirely fond of this approach. So I first like to get a bit more information how this is uses to see whether there are alternative solutions.

@JDevlieghere can you give a bit more information how LLDB uses this module?



================
Comment at: libcxx/include/CMakeLists.txt:655
   __split_buffer
+  __std
   __std_mbstate_t.h
----------------
Is there a reason we need to ship this header to all users of libc++?


================
Comment at: libcxx/include/__std:10
+
+#if !__building_module(std)
+#error "Do not include this header directly, include individual headers instead"
----------------
Where does this `__building_module` come from? I'm not happy with this line since will confuse people. C++23 adds the named module `std`. This has nothing to do with that module.


================
Comment at: libcxx/include/__std:10
+
+#if !__building_module(std)
+#error "Do not include this header directly, include individual headers instead"
----------------
Mordante wrote:
> Where does this `__building_module` come from? I'm not happy with this line since will confuse people. C++23 adds the named module `std`. This has nothing to do with that module.
I think this file name can benefit from something longer. This include is only needed in a very special case.


================
Comment at: libcxx/include/__std:80
+#include <strstream>
+#include <system_error>
+#include <thread>
----------------
When somebody implements the header `text_encoding` will there be a CI tests that fails?

We have the checklist, which you updated, but in general we like to have a red CI when somebody forgets it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156177