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

[Martin Storsjö via libcxx-commits]

Author: Martin Storsjö
Date: 2022-03-02T10:39:14+02:00
New Revision: ea4c198a0f56b3fe63600384a63c6ccaa0560b56

URL: https://github.com/llvm/llvm-project/commit/ea4c198a0f56b3fe63600384a63c6ccaa0560b56
DIFF: https://github.com/llvm/llvm-project/commit/ea4c198a0f56b3fe63600384a63c6ccaa0560b56.diff

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

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.

Therefore, reinstate `-DNOMINMAX` for clang-cl configurations
and remove the libc++ specific bits in filesystem_test_helper.h.

Differential Revision: https://reviews.llvm.org/D120478

Added: 
    

Modified: 
    libcxx/test/configs/llvm-libc++-shared-clangcl.cfg.in
    libcxx/test/configs/llvm-libc++-static-clangcl.cfg.in
    libcxx/test/libcxx/input.output/filesystems/convert_file_time.pass.cpp
    libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.obs/last_write_time.pass.cpp
    libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp
    libcxx/test/support/filesystem_test_helper.h
    libcxx/utils/libcxx/test/config.py

Removed: 
    


################################################################################
diff  --git a/libcxx/test/configs/llvm-libc++-shared-clangcl.cfg.in b/libcxx/test/configs/llvm-libc++-shared-clangcl.cfg.in
index b779dede8b303..dcb2b07ae724c 100644
--- a/libcxx/test/configs/llvm-libc++-shared-clangcl.cfg.in
+++ b/libcxx/test/configs/llvm-libc++-shared-clangcl.cfg.in
@@ -5,7 +5,7 @@ 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'

diff  --git a/libcxx/test/configs/llvm-libc++-static-clangcl.cfg.in b/libcxx/test/configs/llvm-libc++-static-clangcl.cfg.in
index 5a578f4785629..14812b2af9c13 100644
--- a/libcxx/test/configs/llvm-libc++-static-clangcl.cfg.in
+++ b/libcxx/test/configs/llvm-libc++-static-clangcl.cfg.in
@@ -5,7 +5,7 @@ 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'

diff  --git a/libcxx/test/libcxx/input.output/filesystems/convert_file_time.pass.cpp b/libcxx/test/libcxx/input.output/filesystems/convert_file_time.pass.cpp
index bbeaedd51f22e..88054f3e1e7d4 100644
--- a/libcxx/test/libcxx/input.output/filesystems/convert_file_time.pass.cpp
+++ b/libcxx/test/libcxx/input.output/filesystems/convert_file_time.pass.cpp
@@ -12,7 +12,7 @@
 
 // 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>

diff  --git a/libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.obs/last_write_time.pass.cpp b/libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.obs/last_write_time.pass.cpp
index 6038bfd140dee..928248b3c2b87 100644
--- a/libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.obs/last_write_time.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.obs/last_write_time.pass.cpp
@@ -19,9 +19,6 @@
 // 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>

diff  --git a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp
index 3f1a47515602d..4ecf6289c058a 100644
--- a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp
@@ -22,9 +22,6 @@
 // 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>

diff  --git a/libcxx/test/support/filesystem_test_helper.h b/libcxx/test/support/filesystem_test_helper.h
index e8c453c787e1c..f5cb137a053a1 100644
--- a/libcxx/test/support/filesystem_test_helper.h
+++ b/libcxx/test/support/filesystem_test_helper.h
@@ -31,9 +31,6 @@
 # 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); }
@@ -736,6 +733,4 @@ inline fs::path GetWindowsInaccessibleDir() {
   return fs::path();
 }
 
-_LIBCPP_POP_MACROS
-
 #endif /* FILESYSTEM_TEST_HELPER_H */

diff  --git a/libcxx/utils/libcxx/test/config.py b/libcxx/utils/libcxx/test/config.py
index b2f399bd26800..8dc50cd4d4575 100644
--- a/libcxx/utils/libcxx/test/config.py
+++ b/libcxx/utils/libcxx/test/config.py
@@ -245,6 +245,10 @@ def configure_compile_flags(self):
             # 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)