[libcxx-commits] [PATCH] D151792: [libc++][NFC] Granularise <thread> header

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.

LGTM w/ a few comments and green CI.



================
Comment at: libcxx/include/__thread/formatter.h:30-32
+#ifdef _LIBCPP_HAS_NO_THREADS
+#  error "<thread> is not supported since libc++ has been configured without support for threads."
+#endif
----------------
I think I would only keep this error message in the top-level `<thread>` header. That's just for consistency with e.g. `<filesystem>`. (here and everywhere else)


================
Comment at: libcxx/include/thread:93
+#include <__thread/formatter.h>
 #include <__thread/poll_with_backoff.h>
+#include <__thread/this_thread.h>
----------------
Let's remove those includes since they don't contribute to the public API of `<thread>`.

At first we tried to systematically include all headers from `<__foo/*.h>` inside `<foo>`, but we noticed it didn't work (I think it was causing circular dependency issues but I think @Mordante might remember more context) so now I guess the only consistent thing we can do is include only the ones that contribute to the public API of `<foo>`.


================
Comment at: libcxx/include/thread:100-102
+// Some tests rely on these chrono dependencies
+#include <__chrono/steady_clock.h>
+#include <__chrono/time_point.h>
----------------
Let's fix those tests instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151792