[libcxx] Build and test fixes for Windows

[hamzasood]

Summary

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. <new> 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 <new>. The end result is that nothing actually gets declared and so we're missing a few declarations, which causes lots of compilation errors.

[compnerd]

include/filesystem

1396

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

test/support/test_macros.h

147

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

[mclow.lists]

test/support/verbose_assert.h

24

Why the renaming here?

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

[STL_MSFT]

test/support/test_macros.h

147

What error prompted this? It hasn't caused problems when running libc++'s tests against MSVC's STL (with both C1XX and Clang).

[hamzasood]

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.

[hamzasood]

include/filesystem

1396

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.

test/support/test_macros.h

147

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.

147

This is a workaround for a libc++ issue rather than an MSVC one. See the response to @compnerd for the full details.

test/support/verbose_assert.h

24

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.

[mclow.lists]

test/support/verbose_assert.h

26

The renaming is to clarify that an stderr stream is being selected.

And yet, there is clog, right there.

[hamzasood]

test/support/verbose_assert.h

26

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.

[EricWF]

CMakeLists.txt

550

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?

include/filesystem

1396

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.

test/support/test_macros.h

147

What tests are failing?

utils/libcxx/test/config.py

518

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.

[hamzasood]

CMakeLists.txt

550

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.

include/filesystem

1396

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.

test/support/test_macros.h

147

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.

utils/libcxx/test/config.py

518

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.

[xbolva00]

Libc++ fs works on Windows now?

CMakeLists.txt

 #===============================================================================
 include(CMakeDependentOption)
 include(HandleCompilerRT)
 
 # Basic options ---------------------------------------------------------------
 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" $<NOT:WIN32>)
 set(ENABLE_FILESYSTEM_DEFAULT ${LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY})
 if (WIN32)
   set(ENABLE_FILESYSTEM_DEFAULT OFF)
 endif()
 option(LIBCXX_ENABLE_FILESYSTEM "Build filesystem as part of libc++fs.a"
     ${ENABLE_FILESYSTEM_DEFAULT})
 option(LIBCXX_INCLUDE_TESTS "Build the libc++ tests." ${LLVM_INCLUDE_TESTS})
 

 add_definitions(-D_LIBCPP_BUILDING_LIBRARY)
 
 if (NOT LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS)
   add_definitions(-D_LIBCPP_DISABLE_NEW_DELETE_DEFINITIONS)
 endif()
 
 # 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")
     add_compile_flags_if_supported(
         -Wno-user-defined-literals
         -Wno-covered-switch-default)
 elseif("${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU")
     add_compile_flags_if_supported(

include/filesystem

public:
 
   ~filesystem_error() override; // key function
 
   _LIBCPP_INLINE_VISIBILITY
   const char* what() const noexcept override {
     return __storage_->__what_.c_str();
   }
 
-  _LIBCPP_FUNC_VIS
   void __create_what(int __num_paths);
 
 private:
   struct _Storage {
     _LIBCPP_INLINE_VISIBILITY
     _Storage(const path& __p1, const path& __p2) : __p1_(__p1), __p2_(__p2) {}
 
     path __p1_;
     path __p2_;
     string __what_;
   };

test/std/strings/c.strings/cuchar.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // XFAIL: *
+//
+// The MSVC stdlib has cuchar, which causes this test to pass unexpectedly.
+// UNSUPPORTED: windows
 
 // <cuchar>
 
 #include <cuchar>
 
 int main()
 {
 }

test/support/test_macros.h

 #      if _LIBCPP_GLIBC_PREREQ(2, 17)
 #        define TEST_HAS_TIMESPEC_GET
 #        define TEST_HAS_C11_FEATURES
 #      endif
 #    else // defined(_LIBCPP_HAS_MUSL_LIBC)
 #      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
 
 /* Features that were introduced in C++14 */
 #if TEST_STD_VER >= 14
 #define TEST_HAS_EXTENDED_CONSTEXPR
 #define TEST_HAS_VARIABLE_TEMPLATES
 #endif

test/support/verbose_assert.h

 std::true_type IsStreamableImp(int);
 template <class Stream, class Tp> std::false_type IsStreamableImp(long);
 
 template <class Stream, class Tp>
 struct IsStreamable : decltype(IsStreamableImp<Stream, Tp>(0)) {};
 
 template <class Tp, int ST = (IsStreamable<decltype(std::cerr), Tp>::value ? 1
         : (IsStreamable<decltype(std::wcerr), Tp>::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 <class Tp>
-struct SelectStream<Tp, 1> {
+struct SelectErrStream<Tp, 1> {
   static void Print(Tp const& val) { std::cerr << val; }
 };
 
 template <class Tp>
-struct SelectStream<Tp, 2> {
+struct SelectErrStream<Tp, 2> {
   static void Print(Tp const& val) { std::wcerr << val; }
 };
 
 struct AssertData {
   AssertData(const char* xcheck, const char* xfile, const char* xfunc,
              unsigned long xline, bool xpassed = true)
       : passed(xpassed), check(xcheck), file(xfile), func(xfunc), line(xline),
         msg() {}
public:
   }
 
   class LogType {
     friend class AssertHandler;
 
     template <class Tp>
     friend LogType& operator<<(LogType& log, Tp const& value) {
       if (!log.is_disabled) {
-        SelectStream<Tp>::Print(value);
+        SelectErrStream<Tp>::Print(value);
       }
       return log;
     }
 
     friend LogType& operator<<(LogType& log, EndLType* m) {
       if (!log.is_disabled) {
-        SelectStream<EndLType*>::Print(m);
+        std::cerr << m;
       }
       return log;
     }
 
   private:
     LogType(bool disable) : is_disabled(disable) {}
     bool is_disabled;
 

utils/libcxx/test/config.py

def configure_compile_flags(self):
         self.cxx.compile_flags += shlex.split(compile_flags_str)
         if self.is_windows:
             # FIXME: Can we remove this?
             self.cxx.compile_flags += ['-D_CRT_SECURE_NO_WARNINGS']
             # Required so that tests using min/max don't fail on Windows,
             # 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)
 
     def configure_default_compile_flags(self):
         # Try and get the std version from the command line. Fall back to
         # default given in lit.site.cfg is not present. If default is not
         # present then force c++11.