[PATCH] D51868: [libcxx] Build and test fixes for Windows

2018-10-13 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

Libc++ fs works on Windows now?


https://reviews.llvm.org/D51868



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D51868: [libcxx] Build and test fixes for Windows

2018-10-13 Thread Hamza Sood via Phabricator via cfe-commits
hamzasood added inline comments.



Comment at: CMakeLists.txt:550
+endif()
 add_compile_flags_if_supported(
+-Wextra -W -Wwrite-strings

EricWF wrote:
> Couldn't we keep the "-Wall" here? And just add something else that adds 
> "/W4"?
> 
> Also, we don't yet support building with MSVC's frontend. Only clang-cl. Does 
> clang-cl not like this existing code?
The `-Wall` flag gets converted to `-Weverything` by Clang-CL; I don’t think we 
should keep it (it causes thousands of warnings per header).

`/W4` is the equivalent flag on Windows.



Comment at: include/filesystem:1396
 
-  _LIBCPP_FUNC_VIS
   void __create_what(int __num_paths);

EricWF wrote:
> hamzasood wrote:
> > compnerd wrote:
> > > This possibly changes the meaning on other targets.  What was the error 
> > > that this triggered?
> > I've re-uploaded the patch with full context to make this clearer.
> > 
> > That's a member function on an exported type, which I don't think should 
> > have any visibility attributes. The specific issue in this case is that 
> > both the class type and the member function are marked as dllimport, which 
> > causes a compilation error.
> Perhaps part of the problem here is that filesystem builds to a static 
> library, not a shared one.
> 
> None the less, this macro is important once we move the definitions from 
> libc++fs.a to libc++.so.
I don’t think member functions are meant to have function visibility, and I 
wasn’t able to find any other such cases in libc++. That’ll always be a hard 
error on Windows because a dllimport class can’t have dllimport member 
functions.



Comment at: test/support/test_macros.h:147
-#  elif defined(_WIN32)
-#if defined(_MSC_VER) && !defined(__MINGW32__)
-#  define TEST_HAS_C11_FEATURES // Using Microsoft's C Runtime library

EricWF wrote:
> hamzasood wrote:
> > hamzasood wrote:
> > > STL_MSFT wrote:
> > > > compnerd wrote:
> > > > > I think that the condition here is inverted, and should be enabled 
> > > > > for MinGW32.
> > > > What error prompted this? It hasn't caused problems when running 
> > > > libc++'s tests against MSVC's STL (with both C1XX and Clang).
> > > The comment above this says that it's copied from `__config`, but they 
> > > must've gone out of sync at some point because `__config` doesn't have 
> > > that extra section that I deleted.
> > > 
> > > This causes lots of errors when testing libc++. E.g. libc++ isn't 
> > > declaring `std::timespec` on Windows because `_LIBCPP_HAS_C11_FEATURES` 
> > > isn't defined, but the tests think that it's available so they try to use 
> > > it (which causes compilation failures in lots of tests).
> > > 
> > > The better solution here might be to update `__config` to match this, but 
> > > I'm not familiar with some of those platforms so this seemed like the 
> > > safest approach for now.
> > This is a workaround for a libc++ issue rather than an MSVC one. See the 
> > response to @compnerd for the full details.
> 
> What tests are failing?
Any test that requires `TEST_HAS_C11_FEATURES` or `TEST_HAS_TIMESPEC_GET`. 
Those test macros aren’t in sync with the corresponding `__config` ones.



Comment at: utils/libcxx/test/config.py:518
 self.cxx.compile_flags += ['-DNOMINMAX']
+# Disable auto-linking; the library is linked manually when
+# configuring the linker flags.

EricWF wrote:
> I think the correct fix here is to disabling linking libc++ when auto linking 
> is enabled by the headers. Because we want to test that the auto link pragmas 
> work.
In that case, it might be worth adding separate auto-linking tests that 
specifically use Clang-CL. Making it work here would involve defining `_DLL` 
for dynamic builds, but I’m not sure if that would interfere with the CRT.


https://reviews.llvm.org/D51868



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D51868: [libcxx] Build and test fixes for Windows

2018-10-12 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF requested changes to this revision.
EricWF added inline comments.
This revision now requires changes to proceed.



Comment at: CMakeLists.txt:550
+endif()
 add_compile_flags_if_supported(
+-Wextra -W -Wwrite-strings

Couldn't we keep the "-Wall" here? And just add something else that adds "/W4"?

Also, we don't yet support building with MSVC's frontend. Only clang-cl. Does 
clang-cl not like this existing code?



Comment at: include/filesystem:1396
 
-  _LIBCPP_FUNC_VIS
   void __create_what(int __num_paths);

hamzasood wrote:
> compnerd wrote:
> > This possibly changes the meaning on other targets.  What was the error 
> > that this triggered?
> I've re-uploaded the patch with full context to make this clearer.
> 
> That's a member function on an exported type, which I don't think should have 
> any visibility attributes. The specific issue in this case is that both the 
> class type and the member function are marked as dllimport, which causes a 
> compilation error.
Perhaps part of the problem here is that filesystem builds to a static library, 
not a shared one.

None the less, this macro is important once we move the definitions from 
libc++fs.a to libc++.so.



Comment at: test/support/test_macros.h:147
-#  elif defined(_WIN32)
-#if defined(_MSC_VER) && !defined(__MINGW32__)
-#  define TEST_HAS_C11_FEATURES // Using Microsoft's C Runtime library

hamzasood wrote:
> hamzasood wrote:
> > STL_MSFT wrote:
> > > compnerd wrote:
> > > > I think that the condition here is inverted, and should be enabled for 
> > > > MinGW32.
> > > What error prompted this? It hasn't caused problems when running libc++'s 
> > > tests against MSVC's STL (with both C1XX and Clang).
> > The comment above this says that it's copied from `__config`, but they 
> > must've gone out of sync at some point because `__config` doesn't have that 
> > extra section that I deleted.
> > 
> > This causes lots of errors when testing libc++. E.g. libc++ isn't declaring 
> > `std::timespec` on Windows because `_LIBCPP_HAS_C11_FEATURES` isn't 
> > defined, but the tests think that it's available so they try to use it 
> > (which causes compilation failures in lots of tests).
> > 
> > The better solution here might be to update `__config` to match this, but 
> > I'm not familiar with some of those platforms so this seemed like the 
> > safest approach for now.
> This is a workaround for a libc++ issue rather than an MSVC one. See the 
> response to @compnerd for the full details.

What tests are failing?



Comment at: utils/libcxx/test/config.py:518
 self.cxx.compile_flags += ['-DNOMINMAX']
+# Disable auto-linking; the library is linked manually when
+# configuring the linker flags.

I think the correct fix here is to disabling linking libc++ when auto linking 
is enabled by the headers. Because we want to test that the auto link pragmas 
work.


https://reviews.llvm.org/D51868



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D51868: [libcxx] Build and test fixes for Windows

2018-09-19 Thread Hamza Sood via Phabricator via cfe-commits
hamzasood added inline comments.



Comment at: test/support/verbose_assert.h:26
   static_assert(ST == -1, "specialization required for ST != -1");
   static void Print(Tp const&) { std::clog << "Value Not Streamable!\n"; }
 };

mclow.lists wrote:
> > The renaming is to clarify that an `stderr` stream is being selected.
> And yet, there is `clog`, right there.
> 
I guess because that’s logging a programmer error (trying to print a type that 
isn’t streamable) whereas the others are test-related diagnostics? Not 
completely sure though.


https://reviews.llvm.org/D51868



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D51868: [libcxx] Build and test fixes for Windows

2018-09-19 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added inline comments.



Comment at: test/support/verbose_assert.h:26
   static_assert(ST == -1, "specialization required for ST != -1");
   static void Print(Tp const&) { std::clog << "Value Not Streamable!\n"; }
 };

> The renaming is to clarify that an `stderr` stream is being selected.
And yet, there is `clog`, right there.



https://reviews.llvm.org/D51868



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D51868: [libcxx] Build and test fixes for Windows

2018-09-19 Thread Hamza Sood via Phabricator via cfe-commits
hamzasood added inline comments.



Comment at: include/filesystem:1396
 
-  _LIBCPP_FUNC_VIS
   void __create_what(int __num_paths);

compnerd wrote:
> This possibly changes the meaning on other targets.  What was the error that 
> this triggered?
I've re-uploaded the patch with full context to make this clearer.

That's a member function on an exported type, which I don't think should have 
any visibility attributes. The specific issue in this case is that both the 
class type and the member function are marked as dllimport, which causes a 
compilation error.



Comment at: test/support/test_macros.h:147
-#  elif defined(_WIN32)
-#if defined(_MSC_VER) && !defined(__MINGW32__)
-#  define TEST_HAS_C11_FEATURES // Using Microsoft's C Runtime library

STL_MSFT wrote:
> compnerd wrote:
> > I think that the condition here is inverted, and should be enabled for 
> > MinGW32.
> What error prompted this? It hasn't caused problems when running libc++'s 
> tests against MSVC's STL (with both C1XX and Clang).
The comment above this says that it's copied from `__config`, but they must've 
gone out of sync at some point because `__config` doesn't have that extra 
section that I deleted.

This causes lots of errors when testing libc++. E.g. libc++ isn't declaring 
`std::timespec` on Windows because `_LIBCPP_HAS_C11_FEATURES` isn't defined, 
but the tests think that it's available so they try to use it (which causes 
compilation failures in lots of tests).

The better solution here might be to update `__config` to match this, but I'm 
not familiar with some of those platforms so this seemed like the safest 
approach for now.



Comment at: test/support/test_macros.h:147
-#  elif defined(_WIN32)
-#if defined(_MSC_VER) && !defined(__MINGW32__)
-#  define TEST_HAS_C11_FEATURES // Using Microsoft's C Runtime library

hamzasood wrote:
> STL_MSFT wrote:
> > compnerd wrote:
> > > I think that the condition here is inverted, and should be enabled for 
> > > MinGW32.
> > What error prompted this? It hasn't caused problems when running libc++'s 
> > tests against MSVC's STL (with both C1XX and Clang).
> The comment above this says that it's copied from `__config`, but they 
> must've gone out of sync at some point because `__config` doesn't have that 
> extra section that I deleted.
> 
> This causes lots of errors when testing libc++. E.g. libc++ isn't declaring 
> `std::timespec` on Windows because `_LIBCPP_HAS_C11_FEATURES` isn't defined, 
> but the tests think that it's available so they try to use it (which causes 
> compilation failures in lots of tests).
> 
> The better solution here might be to update `__config` to match this, but I'm 
> not familiar with some of those platforms so this seemed like the safest 
> approach for now.
This is a workaround for a libc++ issue rather than an MSVC one. See the 
response to @compnerd for the full details.



Comment at: test/support/verbose_assert.h:24
 : (IsStreamable::value ? 2 : -1))>
-struct SelectStream {
+struct SelectErrStream {
   static_assert(ST == -1, "specialization required for ST != -1");

mclow.lists wrote:
> Why the renaming here? 
> 
> What's the deal with sometimes using `clog` and sometimes `cerr`?
> (I know, you didn't do this, but ???)
> 
Some of the casting that goes on in that template triggers an error under 
`-fno-ms-extensions` when given a function pointer (in this case: `std::endl`). 
Selecting between a standard/wide stream isn't needed for `std::endl` as it can 
go to either, so I've bypassed the selection by sending it straight to `cerr`.

The renaming is to clarify that an `stderr` stream (and nothing else) is being 
selected, which sort of justifies going directly to `cerr`.


https://reviews.llvm.org/D51868



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D51868: [libcxx] Build and test fixes for Windows

2018-09-19 Thread Hamza Sood via Phabricator via cfe-commits
hamzasood updated this revision to Diff 166124.
hamzasood added a comment.
Herald added a subscriber: christof.

I've added a fix for another related issue: the tests would fail to link if 
libc++ is built only as a DLL.

The auto-linking doesn't look for a dynamic library unless `_DLL` is defined 
(which I think is done by passing `/MD` to CL), but the test runner doesn't use 
CL-style commands. This could either be solved by either manually defining that 
macro or just disabling auto-linking. The test runner already configures the 
linker correctly so disabling auto-linking seemed like the simplest option.


https://reviews.llvm.org/D51868

Files:
  CMakeLists.txt
  include/filesystem
  test/std/strings/c.strings/cuchar.pass.cpp
  test/support/test_macros.h
  test/support/verbose_assert.h
  utils/libcxx/test/config.py

Index: utils/libcxx/test/config.py
===
--- utils/libcxx/test/config.py
+++ utils/libcxx/test/config.py
@@ -515,6 +515,10 @@
 # and so that those tests don't have to be changed to tolerate
 # this insanity.
 self.cxx.compile_flags += ['-DNOMINMAX']
+# Disable auto-linking; the library is linked manually when
+# configuring the linker flags.
+if self.cxx_stdlib_under_test == 'libc++':
+self.cxx.compile_flags += ['-D_LIBCPP_NO_AUTO_LINK']
 additional_flags = self.get_lit_conf('test_compiler_flags')
 if additional_flags:
 self.cxx.compile_flags += shlex.split(additional_flags)
Index: test/support/verbose_assert.h
===
--- test/support/verbose_assert.h
+++ test/support/verbose_assert.h
@@ -21,18 +21,18 @@
 
 template ::value ? 1
 : (IsStreamable::value ? 2 : -1))>
-struct SelectStream {
+struct SelectErrStream {
   static_assert(ST == -1, "specialization required for ST != -1");
   static void Print(Tp const&) { std::clog << "Value Not Streamable!\n"; }
 };
 
 template 
-struct SelectStream {
+struct SelectErrStream {
   static void Print(Tp const& val) { std::cerr << val; }
 };
 
 template 
-struct SelectStream {
+struct SelectErrStream {
   static void Print(Tp const& val) { std::wcerr << val; }
 };
 
@@ -86,14 +86,14 @@
 template 
 friend LogType& operator<<(LogType& log, Tp const& value) {
   if (!log.is_disabled) {
-SelectStream::Print(value);
+SelectErrStream::Print(value);
   }
   return log;
 }
 
 friend LogType& operator<<(LogType& log, EndLType* m) {
   if (!log.is_disabled) {
-SelectStream::Print(m);
+std::cerr << m;
   }
   return log;
 }
Index: test/support/test_macros.h
===
--- test/support/test_macros.h
+++ test/support/test_macros.h
@@ -143,11 +143,6 @@
 #  define TEST_HAS_C11_FEATURES
 #  define TEST_HAS_TIMESPEC_GET
 #endif
-#  elif defined(_WIN32)
-#if defined(_MSC_VER) && !defined(__MINGW32__)
-#  define TEST_HAS_C11_FEATURES // Using Microsoft's C Runtime library
-#  define TEST_HAS_TIMESPEC_GET
-#endif
 #  endif
 #endif
 
Index: test/std/strings/c.strings/cuchar.pass.cpp
===
--- test/std/strings/c.strings/cuchar.pass.cpp
+++ test/std/strings/c.strings/cuchar.pass.cpp
@@ -8,6 +8,9 @@
 //===--===//
 //
 // XFAIL: *
+//
+// The MSVC stdlib has cuchar, which causes this test to pass unexpectedly.
+// UNSUPPORTED: windows
 
 // 
 
Index: include/filesystem
===
--- include/filesystem
+++ include/filesystem
@@ -1390,7 +1390,6 @@
 return __storage_->__what_.c_str();
   }
 
-  _LIBCPP_FUNC_VIS
   void __create_what(int __num_paths);
 
 private:
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -67,7 +67,7 @@
 option(LIBCXX_ENABLE_ASSERTIONS "Enable assertions independent of build mode." OFF)
 option(LIBCXX_ENABLE_SHARED "Build libc++ as a shared library." ON)
 option(LIBCXX_ENABLE_STATIC "Build libc++ as a static library." ON)
-option(LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY "Build libc++experimental.a" ON)
+option(LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY "Build libc++experimental.a" $)
 set(ENABLE_FILESYSTEM_DEFAULT ${LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY})
 if (WIN32)
   set(ENABLE_FILESYSTEM_DEFAULT OFF)
@@ -542,8 +542,13 @@
 
 # Warning flags ===
 add_definitions(-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+if (MSVC)
+add_compile_flags(/W4)
+else()
+add_compile_flags_if_supported(-Wall)
+endif()
 add_compile_flags_if_supported(
--Wall -Wextra -W -Wwrite-strings
+-Wextra -W -Wwrite-strings
 -Wno-unused-parameter -Wno-long-long
 

[PATCH] D51868: [libcxx] Build and test fixes for Windows

2018-09-18 Thread Stephan T. Lavavej via Phabricator via cfe-commits
STL_MSFT added inline comments.



Comment at: test/support/test_macros.h:147
-#  elif defined(_WIN32)
-#if defined(_MSC_VER) && !defined(__MINGW32__)
-#  define TEST_HAS_C11_FEATURES // Using Microsoft's C Runtime library

compnerd wrote:
> I think that the condition here is inverted, and should be enabled for 
> MinGW32.
What error prompted this? It hasn't caused problems when running libc++'s tests 
against MSVC's STL (with both C1XX and Clang).


Repository:
  rCXX libc++

https://reviews.llvm.org/D51868



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D51868: [libcxx] Build and test fixes for Windows

2018-09-18 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added inline comments.



Comment at: test/support/verbose_assert.h:24
 : (IsStreamable::value ? 2 : -1))>
-struct SelectStream {
+struct SelectErrStream {
   static_assert(ST == -1, "specialization required for ST != -1");

Why the renaming here? 

What's the deal with sometimes using `clog` and sometimes `cerr`?
(I know, you didn't do this, but ???)



Repository:
  rCXX libc++

https://reviews.llvm.org/D51868



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D51868: [libcxx] Build and test fixes for Windows

2018-09-18 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd added inline comments.
Herald added a subscriber: libcxx-commits.



Comment at: include/filesystem:1396
 
-  _LIBCPP_FUNC_VIS
   void __create_what(int __num_paths);

This possibly changes the meaning on other targets.  What was the error that 
this triggered?



Comment at: test/support/test_macros.h:147
-#  elif defined(_WIN32)
-#if defined(_MSC_VER) && !defined(__MINGW32__)
-#  define TEST_HAS_C11_FEATURES // Using Microsoft's C Runtime library

I think that the condition here is inverted, and should be enabled for MinGW32.


Repository:
  rCXX libc++

https://reviews.llvm.org/D51868



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D51868: [libcxx] Build and test fixes for Windows

2018-09-10 Thread Hamza Sood via Phabricator via cfe-commits
hamzasood created this revision.
hamzasood added reviewers: rnk, EricWF, compnerd, smeenai.
Herald added subscribers: cfe-commits, ldionne, mgorny.

The patch fixes a few issues that arise when using libc++ on Windows.
Specifically:

1. The CMake configuration added `-Wall` to the compilation, which actually 
means `-Weverything` in MSVC. This led to several thousand warnings per file.
2. The experimental library was enabled by default. It doesn't really work 
(yet) on Windows, and in fact the build docs suggest to disable it, so I don't 
think that was a sensible default.
3. There were some other errors that caused compilation errors in some of the 
tests.

I've identified a few other issues, but I'm not sure how best to fix them:

1. `support/win32/locale_win32.h` includes `xlocinfo.h`, which ends up 
including `yvals_core.h`. That MSVC header defines feature test macros, among 
other things, that clash with macros defined in libc++. This is causing lots of 
test errors.
2. `` includes the ucrt header `new.h` in the hope that it'll declare 
`get_new_handler` etc. but `new.h` really just goes back and includes ``. 
The end result is that nothing actually gets declared and so we're missing a 
few declarations, which causes lots of compilation errors.


Repository:
  rCXX libc++

https://reviews.llvm.org/D51868

Files:
  CMakeLists.txt
  include/filesystem
  test/std/strings/c.strings/cuchar.pass.cpp
  test/support/test_macros.h
  test/support/verbose_assert.h

Index: test/support/verbose_assert.h
===
--- test/support/verbose_assert.h
+++ test/support/verbose_assert.h
@@ -21,18 +21,18 @@
 
 template ::value ? 1
 : (IsStreamable::value ? 2 : -1))>
-struct SelectStream {
+struct SelectErrStream {
   static_assert(ST == -1, "specialization required for ST != -1");
   static void Print(Tp const&) { std::clog << "Value Not Streamable!\n"; }
 };
 
 template 
-struct SelectStream {
+struct SelectErrStream {
   static void Print(Tp const& val) { std::cerr << val; }
 };
 
 template 
-struct SelectStream {
+struct SelectErrStream {
   static void Print(Tp const& val) { std::wcerr << val; }
 };
 
@@ -86,14 +86,14 @@
 template 
 friend LogType& operator<<(LogType& log, Tp const& value) {
   if (!log.is_disabled) {
-SelectStream::Print(value);
+SelectErrStream::Print(value);
   }
   return log;
 }
 
 friend LogType& operator<<(LogType& log, EndLType* m) {
   if (!log.is_disabled) {
-SelectStream::Print(m);
+std::cerr << m;
   }
   return log;
 }
Index: test/support/test_macros.h
===
--- test/support/test_macros.h
+++ test/support/test_macros.h
@@ -143,11 +143,6 @@
 #  define TEST_HAS_C11_FEATURES
 #  define TEST_HAS_TIMESPEC_GET
 #endif
-#  elif defined(_WIN32)
-#if defined(_MSC_VER) && !defined(__MINGW32__)
-#  define TEST_HAS_C11_FEATURES // Using Microsoft's C Runtime library
-#  define TEST_HAS_TIMESPEC_GET
-#endif
 #  endif
 #endif
 
Index: test/std/strings/c.strings/cuchar.pass.cpp
===
--- test/std/strings/c.strings/cuchar.pass.cpp
+++ test/std/strings/c.strings/cuchar.pass.cpp
@@ -8,6 +8,9 @@
 //===--===//
 //
 // XFAIL: *
+//
+// The MSVC stdlib has cuchar, which causes this test to pass unexpectedly.
+// UNSUPPORTED: windows
 
 // 
 
Index: include/filesystem
===
--- include/filesystem
+++ include/filesystem
@@ -1393,7 +1393,6 @@
 return __storage_->__what_.c_str();
   }
 
-  _LIBCPP_FUNC_VIS
   void __create_what(int __num_paths);
 
 private:
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -67,7 +67,7 @@
 option(LIBCXX_ENABLE_ASSERTIONS "Enable assertions independent of build mode." OFF)
 option(LIBCXX_ENABLE_SHARED "Build libc++ as a shared library." ON)
 option(LIBCXX_ENABLE_STATIC "Build libc++ as a static library." ON)
-option(LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY "Build libc++experimental.a" ON)
+option(LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY "Build libc++experimental.a" $)
 set(ENABLE_FILESYSTEM_DEFAULT ${LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY})
 if (WIN32)
   set(ENABLE_FILESYSTEM_DEFAULT OFF)
@@ -542,8 +542,13 @@
 
 # Warning flags ===
 add_definitions(-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+if (MSVC)
+add_compile_flags(/W4)
+else()
+add_compile_flags_if_supported(-Wall)
+endif()
 add_compile_flags_if_supported(
--Wall -Wextra -W -Wwrite-strings
+-Wextra -W -Wwrite-strings
 -Wno-unused-parameter -Wno-long-long
 -Werror=return-type)
 if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang")