[libcxx] [test] Make filesystem tests not rely on libc++ internals

[mstorsjo]

Summary

As part of https://reviews.llvm.org/D119036
(506cf6dc048835c598b654e43ed8f723a42e39ba), -DNOMINMAX was
dropped from the Windows CI configurations, replaced with a
block with _LIBCPP_PUSH_MACROS, #include <__undef_macros>
and _LIBCPP_POP_MACROS (and
ADDITIONAL_COMPILE_FLAGS: -DNOMINMAX left in two tests).

However, this workaround breaks the running the libc++ tests
against a different C++ standard library than libc++, as those
macros and that header are libc++ internals.

This works around it by simply doing #undef min and #undef max
(when on Windows), making the tests work on other C++ standard
libraries again.

[ldionne]

libcxx/test/support/filesystem_test_helper.h

34

Instead, could we guard against those macros by doing something like (std::numeric_limits<large_file_offset_t>::max)()?

Otherwise, we could also add -DNOMINMAX in the testing config when we're on Windows. I'm not a huge fan of undef'ing those macros here in this file, it seems kind of ad-hoc.

[ldionne]

libcxx/test/support/filesystem_test_helper.h

34

Yeah, FWIW, I think this is where I made a mistake: https://reviews.llvm.org/D119036#3301636

I should have left -DNOMINMAX in place, at least on Windows.

[mstorsjo]

libcxx/test/support/filesystem_test_helper.h

34

Yeah I don't mind putting that define back.

[mstorsjo]

Reinstate -DNOMINMAX instead of using libc++ specifics in filesystem_test_helper.h.

[mstorsjo]

Add an extra -Wno-macro-redefined in a test that includes libcxx internal headers (which also define NOMINMAX). Will push later if CI is passing.

libcxx/test/configs/llvm-libc++-shared-clangcl.cfg.in

 # This testing configuration handles running the test suite against LLVM's libc++
 # using a DLL, with Clang-cl on Windows.
 
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
 config.substitutions.append(('%{flags}', '--driver-mode=g++'))
 config.substitutions.append(('%{compile_flags}',
-    '-nostdinc++ -isystem %{include} -isystem %{target-include} -I %{libcxx}/test/support -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_STDIO_ISO_WIDE_SPECIFIERS'
+    '-nostdinc++ -isystem %{include} -isystem %{target-include} -I %{libcxx}/test/support -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_STDIO_ISO_WIDE_SPECIFIERS -DNOMINMAX'
 ))
 config.substitutions.append(('%{link_flags}',
     '-nostdlib -L %{lib} -lc++ -lmsvcrt -lmsvcprt -loldnames'
 ))
 config.substitutions.append(('%{exec}',
     '%{executor} --execdir %T --env PATH=%{lib} -- '
 ))
 

libcxx/test/configs/llvm-libc++-static-clangcl.cfg.in

 # This testing configuration handles running the test suite against LLVM's libc++
 # using a static library, with Clang-cl on Windows.
 
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
 config.substitutions.append(('%{flags}', '--driver-mode=g++'))
 config.substitutions.append(('%{compile_flags}',
-    '-nostdinc++ -isystem %{include} -isystem %{target-include} -I %{libcxx}/test/support -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_STDIO_ISO_WIDE_SPECIFIERS'
+    '-nostdinc++ -isystem %{include} -isystem %{target-include} -I %{libcxx}/test/support -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_STDIO_ISO_WIDE_SPECIFIERS -DNOMINMAX'
 ))
 config.substitutions.append(('%{link_flags}',
     '-nostdlib -L %{lib} -llibc++ -lmsvcrt -lmsvcprt -loldnames'
 ))
 config.substitutions.append(('%{exec}',
     '%{executor} --execdir %T --env PATH=%{lib} -- '
 ))
 

libcxx/test/libcxx/input.output/filesystems/convert_file_time.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // UNSUPPORTED: c++03, c++11
 
 // <filesystem>
 
 // typedef TrivialClock file_time_type;
 
-// ADDITIONAL_COMPILE_FLAGS: -I %S/../../../../src/filesystem
+// ADDITIONAL_COMPILE_FLAGS: -I %S/../../../../src/filesystem -Wno-macro-redefined
 
 #include <cassert>
 #include <chrono>
 #include <cstddef>
 #include <filesystem>
 #include <limits>
 #include <string>
 #include <type_traits>

libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.obs/last_write_time.pass.cpp

 
 // <filesystem>
 
 // class directory_entry
 
 // file_time_type last_write_time() const;
 // file_time_type last_write_time(error_code const&) const noexcept;
 
-// Disable min() and max() macros in <windows.h> on Windows.
-// ADDITIONAL_COMPILE_FLAGS: -DNOMINMAX
-
 #include "filesystem_include.h"
 #include <type_traits>
 #include <cassert>
 
 #include "filesystem_test_helper.h"
 #include "rapid-cxx-test.h"
 
 #include "test_macros.h"

libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp

 // <filesystem>
 
 // file_time_type last_write_time(const path& p);
 // file_time_type last_write_time(const path& p, std::error_code& ec) noexcept;
 // void last_write_time(const path& p, file_time_type new_time);
 // void last_write_time(const path& p, file_time_type new_type,
 //                      std::error_code& ec) noexcept;
 
-// Disable min() and max() macros in <windows.h> on Windows.
-// ADDITIONAL_COMPILE_FLAGS: -DNOMINMAX
-
 #include "filesystem_include.h"
 #include <chrono>
 #include <cstdio>
 #include <cstdlib>
 #include <ctime>
 #include <ratio>
 #include <type_traits>
 

libcxx/test/support/filesystem_test_helper.h

 #include "format_string.h"
 
 // For creating socket files
 #if !defined(__FreeBSD__) && !defined(__APPLE__) && !defined(_WIN32)
 # include <sys/socket.h>
 # include <sys/un.h>
 #endif
 
-_LIBCPP_PUSH_MACROS
-#include <__undef_macros>
-
 namespace utils {
 #ifdef _WIN32
     inline int mkdir(const char* path, int mode) { (void)mode; return ::_mkdir(path); }
     inline int ftruncate(int fd, off_t length) { return ::_chsize(fd, length); }
     inline int symlink(const char* oldname, const char* newname, bool is_dir) {
         DWORD flags = is_dir ? SYMBOLIC_LINK_FLAG_DIRECTORY : 0;
         if (CreateSymbolicLinkA(newname, oldname,
                                 flags | SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE))
           return 0;
for (const auto &ent : fs::directory_iterator(root, ec)) {
     }
     return ent;
   }
   fprintf(stderr, "No inaccessible directory \"%s\" found, skipping tests "
                   "regarding it\n", dir.string().c_str());
   return fs::path();
 }
 
-_LIBCPP_POP_MACROS
-
 #endif /* FILESYSTEM_TEST_HELPER_H */

libcxx/utils/libcxx/test/config.py

def configure_compile_flags(self):
         if self.target_info.is_windows():
             self.cxx.compile_flags += ['-D_CRT_SECURE_NO_WARNINGS']
             # Don't warn about using common but nonstandard unprefixed functions
             # like chdir, fileno.
             self.cxx.compile_flags += ['-D_CRT_NONSTDC_NO_WARNINGS']
             # Build the tests in the same configuration as libcxx itself,
             # to avoid mismatches if linked statically.
             self.cxx.compile_flags += ['-D_CRT_STDIO_ISO_WIDE_SPECIFIERS']
+            # 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']
         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):
         # Configure include paths
         self.configure_compile_flags_header_includes()
         self.target_info.add_cxx_compile_flags(self.cxx.compile_flags)