[libcxx-commits] [PATCH] D144322: [libc++][Modules] Make top level modules for all C++ headers with OS/clang versions

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added inline comments.


================
Comment at: libcxx/include/algorithm:1924-1931
 #if !defined(_LIBCPP_REMOVE_TRANSITIVE_INCLUDES) && _LIBCPP_STD_VER <= 17
-#  include <chrono>
+#  if !__has_feature(modules)
+// <chrono> creates an unbrekable module cycle from
+// <algorithm> -> <chrono> -> <format> -> <locale> ->
+// <mutex> -> <system_error> -> <string> -> <algorithm>
+#    include <chrono>
+#  endif
----------------
ldionne wrote:
> Instead, I would rather take the hit and unconditionally remove that transitive include in all standard modes. That's something we could do in LLVM 17, the impact is likely not a deal breaker and we have a good incentive to do it.
I would prefer to do that in a separate patch and notify the libc++ vendors and announce it on Discourse. (And obviously it should be in the release notes.) The same for the other transitive includes removals.

Edit I see @ldionne wrote something similar in a different comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144322