[libcxx-commits] [PATCH] D115578: [libc++][NFC] Granularize <filesystem>

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/filesystem:268
 #include <version>
-
-#if !defined(_LIBCPP_HAS_NO_LOCALIZATION)
-# include <locale>
-# include <iomanip> // for quoted
-#endif
+#include <system_error>
 
----------------
I see your repeated updates fiddling with `#include`s here, and I imagine they're to fix test failures with Modules? I believe that you should actually leave //all// of the public headers `#include`d here, i.e. please re-add `<chrono>` and `<cstddef>` and `<cstdlib>` and `<iosfwd>` and....
(And re-alphabetize; you've got `<system_error>` below `<version>` right now.)

That will make this refactoring truly NFC.

Then, feel free to follow up with a PR to remove all these now-unused headers... but knowing that that will break anyone who's currently using `<filesystem>` with Modules and accidentally depending on these transitive includes, so it's not "NFC", it's just a good idea.  In that same PR, you would also update any libcxx/test/ files that are currently relying on these transitive includes to "Include What You Use".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115578