[libcxx-commits] [PATCH] D94921: [libc++] Make LIBCXX_ENABLE_FILESYSTEM fully consistent

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue Jan 19 11:12:21 PST 2021

ldionne accepted this revision as: libc++.
ldionne added inline comments.

Comment at: libcxx/include/filesystem:255
+# error "The Filesystem library is not supported by this configuration of libc++"
rnk wrote:
> What do you think of allowing users to feature test with `__has_include(<filesystem>)`? This could be implemented as part of the installation rules for include/*, but that adds some complexity, and it will not work when using libc++ from a build directory. It seems nice-to-have to me, but not super important.
Using `__has_include` to test whether features are enabled is an anti-pattern, and I think we shouldn't do anything towards making it "better". For example, people try to check whether filesystem is enabled in C++17 by checking for `#if __has_include(<filesystem>)`, but that never worked (and couldn't work unless we installed different headers for different dialects). This here is the same thing IMO.

The right way to test is to use feature-test macros. I think we should instead make sure that feature-test macros respect `_LIBCPP_HAS_NO_FILESYSTEM_LIBRARY`, so that you can check for `__cpp_lib_filesystem` and have it return the right answer.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list