[PATCH] D59619: Allow disabling of filesystem library.

Thu Mar 21 09:40:17 PDT 2019

ldionne added inline comments.

================
Comment at: libcxx/trunk/utils/libcxx/test/config.py:438

+        if not self.get_lit_bool('enable_filesystem', default=True):
+            self.config.available_features.add('c++filesystem-disabled')
----------------
EricWF wrote:
> ldionne wrote:
> > Instead, I think this should be dependent on the platform. I don't see a reason why this would be different from` dylib-has-no-filesystem`.
> That information is specified at configure time and the build system should respect that choice. I see no value auto-magically detecting it here. If they don't want it to run, they should disable it at configure time.
The way I see things, the lit test suite is disjoint from the CMake build configuration. When we run the lit test suite, we run it against a concrete release of the standard library (i.e. release by a vendor shipping the library). It just happens to be very convenient to pretend that libc++-as-shipped-with-LLVM is something that makes sense to test against because it gives us a sane workflow. This is why I'd rather not use switches to turn this-or-that on/off, but instead have the test suite know about various implementation characteristics and work with that.

In other words, instead of saying `c++filesystem-disabled`, just don't build filesystem into the dylib, and then have a way of telling the test suite that the library you're testing (libc++ trunk without filesystem support) has no support for filesystem.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59619