[libcxx-commits] [libcxx] [libc++][test] Make filesystem_test_helper.h more portable to Windows (PR #74182)

Stephan T. Lavavej via libcxx-commits libcxx-commits at lists.llvm.org
Sun Dec 3 13:09:50 PST 2023


https://github.com/StephanTLavavej updated https://github.com/llvm/llvm-project/pull/74182

>From 49409dd213fbf859916137c7ce095a5992b7f0a1 Mon Sep 17 00:00:00 2001
From: "Stephan T. Lavavej" <stl at nuwen.net>
Date: Wed, 29 Nov 2023 21:25:57 -0800
Subject: [PATCH 1/4] Defend against windows.h macroizing max().

This header directly contains `#include <windows.h>`.

filesystem_test_helper.h(234,58): error: too few arguments provided to function-like macro invocation
---
 libcxx/test/support/filesystem_test_helper.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libcxx/test/support/filesystem_test_helper.h b/libcxx/test/support/filesystem_test_helper.h
index a049efe03d844..6d83a19612512 100644
--- a/libcxx/test/support/filesystem_test_helper.h
+++ b/libcxx/test/support/filesystem_test_helper.h
@@ -231,7 +231,7 @@ struct scoped_test_env
 
         if (size >
             static_cast<typename std::make_unsigned<utils::off64_t>::type>(
-                std::numeric_limits<utils::off64_t>::max())) {
+                (std::numeric_limits<utils::off64_t>::max)())) {
             fprintf(stderr, "create_file(%s, %ju) too large\n",
                     filename.c_str(), size);
             abort();

>From 2d2bb7c56178e7b454261aea92edfbaa9385baf0 Mon Sep 17 00:00:00 2001
From: "Stephan T. Lavavej" <stl at nuwen.net>
Date: Wed, 29 Nov 2023 21:34:53 -0800
Subject: [PATCH 2/4] Use _getcwd, _stat, _fileno, _chdir on Windows.

filesystem_test_helper.h(139,23): error: no member named 'getcwd' in the global namespace; did you mean '_getcwd'?
filesystem_test_helper.h(145,18): error: no struct named 'stat' in the global namespace
filesystem_test_helper.h(253,17): error: use of undeclared identifier 'fileno'; did you mean '_fileno'?
filesystem_test_helper.h(471,17): error: no member named 'chdir' in the global namespace; did you mean '_chdir'?
---
 libcxx/test/support/filesystem_test_helper.h | 22 ++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/libcxx/test/support/filesystem_test_helper.h b/libcxx/test/support/filesystem_test_helper.h
index 6d83a19612512..1dedbf5f22fdf 100644
--- a/libcxx/test/support/filesystem_test_helper.h
+++ b/libcxx/test/support/filesystem_test_helper.h
@@ -136,14 +136,23 @@ namespace utils {
         // Assume that path lengths are not greater than this.
         // This should be fine for testing purposes.
         char buf[4096];
+#ifdef _WIN32
+        char* ret = ::_getcwd(buf, sizeof(buf));
+#else
         char* ret = ::getcwd(buf, sizeof(buf));
+#endif
         assert(ret && "getcwd failed");
         return std::string(ret);
     }
 
     inline bool exists(std::string const& path) {
+#ifdef _WIN32
+        struct ::_stat tmp;
+        return ::_stat(path.c_str(), &tmp) == 0;
+#else
         struct ::stat tmp;
         return ::stat(path.c_str(), &tmp) == 0;
+#endif
     }
 } // end namespace utils
 
@@ -249,8 +258,13 @@ struct scoped_test_env
             abort();
         }
 
-        if (utils::ftruncate64(
-                fileno(file), static_cast<utils::off64_t>(size)) == -1) {
+#ifdef _WIN32
+        const int fd = _fileno(file);
+#else
+        const int fd = fileno(file);
+#endif
+
+        if (utils::ftruncate64(fd, static_cast<utils::off64_t>(size)) == -1) {
             fprintf(stderr, "ftruncate %s %ju failed: %s\n", filename.c_str(),
                     size, strerror(errno));
             fclose(file);
@@ -468,7 +482,11 @@ struct CWDGuard {
   std::string oldCwd_;
   CWDGuard() : oldCwd_(utils::getcwd()) { }
   ~CWDGuard() {
+#ifdef _WIN32
+    int ret = ::_chdir(oldCwd_.c_str());
+#else
     int ret = ::chdir(oldCwd_.c_str());
+#endif
     assert(ret == 0 && "chdir failed");
   }
 

>From 801535e25a0e9b6bc0d095532ad4f05e07b2a88c Mon Sep 17 00:00:00 2001
From: "Stephan T. Lavavej" <stl at nuwen.net>
Date: Fri, 1 Dec 2023 23:25:24 -0800
Subject: [PATCH 3/4] Apply clang-format from CI.

---
 libcxx/test/support/filesystem_test_helper.h | 25 +++++++++-----------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/libcxx/test/support/filesystem_test_helper.h b/libcxx/test/support/filesystem_test_helper.h
index 1dedbf5f22fdf..5f5641d4f2e72 100644
--- a/libcxx/test/support/filesystem_test_helper.h
+++ b/libcxx/test/support/filesystem_test_helper.h
@@ -147,11 +147,11 @@ namespace utils {
 
     inline bool exists(std::string const& path) {
 #ifdef _WIN32
-        struct ::_stat tmp;
-        return ::_stat(path.c_str(), &tmp) == 0;
+      struct ::_stat tmp;
+      return ::_stat(path.c_str(), &tmp) == 0;
 #else
-        struct ::stat tmp;
-        return ::stat(path.c_str(), &tmp) == 0;
+      struct ::stat tmp;
+      return ::stat(path.c_str(), &tmp) == 0;
 #endif
     }
 } // end namespace utils
@@ -238,12 +238,10 @@ struct scoped_test_env
     std::string create_file(fs::path filename_path, std::uintmax_t size = 0) {
         std::string filename = sanitize_path(filename_path.string());
 
-        if (size >
-            static_cast<typename std::make_unsigned<utils::off64_t>::type>(
-                (std::numeric_limits<utils::off64_t>::max)())) {
-            fprintf(stderr, "create_file(%s, %ju) too large\n",
-                    filename.c_str(), size);
-            abort();
+        if (size > static_cast<typename std::make_unsigned<utils::off64_t>::type>(
+                       (std::numeric_limits<utils::off64_t>::max)())) {
+          fprintf(stderr, "create_file(%s, %ju) too large\n", filename.c_str(), size);
+          abort();
         }
 
 #if defined(_WIN32) || defined(__MVS__)
@@ -265,10 +263,9 @@ struct scoped_test_env
 #endif
 
         if (utils::ftruncate64(fd, static_cast<utils::off64_t>(size)) == -1) {
-            fprintf(stderr, "ftruncate %s %ju failed: %s\n", filename.c_str(),
-                    size, strerror(errno));
-            fclose(file);
-            abort();
+          fprintf(stderr, "ftruncate %s %ju failed: %s\n", filename.c_str(), size, strerror(errno));
+          fclose(file);
+          abort();
         }
 
         fclose(file);

>From 9fe428b083f03356c30fc704794cbff757bc8b4a Mon Sep 17 00:00:00 2001
From: "Stephan T. Lavavej" <stl at nuwen.net>
Date: Sat, 2 Dec 2023 16:33:43 -0800
Subject: [PATCH 4/4] More min/max macro defenses in
 libcxx/test/std/input.output/filesystems.

---
 .../directory_entry.mods/refresh.pass.cpp     |  6 ++--
 .../last_write_time.pass.cpp                  | 10 +++---
 .../last_write_time.pass.cpp                  | 36 +++++++++----------
 3 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.mods/refresh.pass.cpp b/libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.mods/refresh.pass.cpp
index 22f94def8529e..49b0fd485eca5 100644
--- a/libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.mods/refresh.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.mods/refresh.pass.cpp
@@ -316,7 +316,7 @@ static void access_cache_after_refresh_fails() {
 
     CHECK_ACCESS(exists, false);
     CHECK_ACCESS(is_symlink, false);
-    CHECK_ACCESS(last_write_time, file_time_type::min());
+    CHECK_ACCESS(last_write_time, (file_time_type::min)());
     CHECK_ACCESS(hard_link_count, std::uintmax_t(-1));
   }
   permissions(dir, old_perms);
@@ -333,7 +333,7 @@ static void access_cache_after_refresh_fails() {
 
     CHECK_ACCESS(exists, false);
     CHECK_ACCESS(is_symlink, false);
-    CHECK_ACCESS(last_write_time, file_time_type::min());
+    CHECK_ACCESS(last_write_time, (file_time_type::min)());
     CHECK_ACCESS(hard_link_count, std::uintmax_t(-1));
   }
   permissions(dir, old_perms);
@@ -351,7 +351,7 @@ static void access_cache_after_refresh_fails() {
 
     CHECK_ACCESS(exists, false);
     CHECK_ACCESS(is_regular_file, false);
-    CHECK_ACCESS(last_write_time, file_time_type::min());
+    CHECK_ACCESS(last_write_time, (file_time_type::min)());
     CHECK_ACCESS(hard_link_count, std::uintmax_t(-1));
   }
 #undef CHECK_ACCESS
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 75877bc0a2752..e164cc31cd767 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
@@ -108,7 +108,7 @@ static void error_reporting() {
     assert(ErrorIs(ec, std::errc::no_such_file_or_directory));
 
     ec = GetTestEC();
-    assert(ent.last_write_time(ec) == file_time_type::min());
+    assert(ent.last_write_time(ec) == (file_time_type::min)());
     assert(ErrorIs(ec, std::errc::no_such_file_or_directory));
 
     ExceptionChecker Checker(static_env.DNE,
@@ -122,7 +122,7 @@ static void error_reporting() {
 
     std::error_code ec = GetTestEC();
     file_time_type expect_bad = last_write_time(static_env.BadSymlink, ec);
-    assert(expect_bad == file_time_type::min());
+    assert(expect_bad == (file_time_type::min)());
     assert(ErrorIs(ec, std::errc::no_such_file_or_directory));
 
     ec = GetTestEC();
@@ -154,7 +154,7 @@ static void error_reporting() {
     assert(ErrorIs(ec, std::errc::permission_denied));
 
     ec = GetTestEC();
-    assert(ent.last_write_time(ec) == file_time_type::min());
+    assert(ent.last_write_time(ec) == (file_time_type::min)());
     assert(ErrorIs(ec, std::errc::permission_denied));
 
     ExceptionChecker Checker(file, std::errc::permission_denied,
@@ -180,7 +180,7 @@ static void error_reporting() {
     assert(ErrorIs(ec, std::errc::permission_denied));
 
     ec = GetTestEC();
-    assert(ent.last_write_time(ec) == file_time_type::min());
+    assert(ent.last_write_time(ec) == (file_time_type::min)());
     assert(ErrorIs(ec, std::errc::permission_denied));
 
     ExceptionChecker Checker(sym_in_dir, std::errc::permission_denied,
@@ -206,7 +206,7 @@ static void error_reporting() {
     assert(!ec);
 
     ec = GetTestEC();
-    assert(ent.last_write_time(ec) == file_time_type::min());
+    assert(ent.last_write_time(ec) == (file_time_type::min)());
     assert(ErrorIs(ec, std::errc::permission_denied));
 
     ExceptionChecker Checker(sym_out_of_dir, std::errc::permission_denied,
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 0678b4d767d23..7b911f9f26aee 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
@@ -117,16 +117,16 @@ bool ConvertToTimeSpec(TimeSpec& ts, file_time_type ft) {
   auto secs = duration_cast<Sec>(ft.time_since_epoch());
   auto nsecs = duration_cast<NanoSec>(ft.time_since_epoch() - secs);
   if (nsecs.count() < 0) {
-    if (Sec::min().count() > SecLim::min()) {
+    if ((Sec::min)().count() > (SecLim::min)()) {
       secs += Sec(1);
       nsecs -= Sec(1);
     } else {
       nsecs = NanoSec(0);
     }
   }
-  if (SecLim::max() < secs.count() || SecLim::min() > secs.count())
+  if ((SecLim::max)() < secs.count() || (SecLim::min)() > secs.count())
     return false;
-  if (NSecLim::max() < nsecs.count() || NSecLim::min() > nsecs.count())
+  if ((NSecLim::max)() < nsecs.count() || (NSecLim::min)() > nsecs.count())
     return false;
   ts.tv_sec = secs.count();
   ts.tv_nsec = nsecs.count();
@@ -221,7 +221,7 @@ static const bool SupportsNegativeTimes = [] {
 static const bool SupportsMaxTime = [] {
   using namespace std::chrono;
   TimeSpec max_ts = {};
-  if (!ConvertToTimeSpec(max_ts, file_time_type::max()))
+  if (!ConvertToTimeSpec(max_ts, (file_time_type::max)()))
     return false;
 
   std::error_code ec;
@@ -230,7 +230,7 @@ static const bool SupportsMaxTime = [] {
     scoped_test_env env;
     const path file = env.create_file("file", 42);
     old_write_time = LastWriteTime(file);
-    file_time_type tp = file_time_type::max();
+    file_time_type tp = (file_time_type::max)();
     fs::last_write_time(file, tp, ec);
     new_write_time = LastWriteTime(file);
   }
@@ -240,7 +240,7 @@ static const bool SupportsMaxTime = [] {
 static const bool SupportsMinTime = [] {
   using namespace std::chrono;
   TimeSpec min_ts = {};
-  if (!ConvertToTimeSpec(min_ts, file_time_type::min()))
+  if (!ConvertToTimeSpec(min_ts, (file_time_type::min)()))
     return false;
   std::error_code ec;
   TimeSpec old_write_time, new_write_time;
@@ -248,7 +248,7 @@ static const bool SupportsMinTime = [] {
     scoped_test_env env;
     const path file = env.create_file("file", 42);
     old_write_time = LastWriteTime(file);
-    file_time_type tp = file_time_type::min();
+    file_time_type tp = (file_time_type::min)();
     fs::last_write_time(file, tp, ec);
     new_write_time = LastWriteTime(file);
   }
@@ -291,12 +291,12 @@ static const bool WorkaroundStatTruncatesToSeconds = [] {
 
 static const bool SupportsMinRoundTrip = [] {
   TimeSpec ts = {};
-  if (!ConvertToTimeSpec(ts, file_time_type::min()))
+  if (!ConvertToTimeSpec(ts, (file_time_type::min)()))
     return false;
   file_time_type min_val = {};
   if (!ConvertFromTimeSpec(min_val, ts))
     return false;
-  return min_val == file_time_type::min();
+  return min_val == (file_time_type::min)();
 }();
 
 } // end namespace
@@ -325,7 +325,7 @@ static bool CompareTime(TimeSpec t1, file_time_type t2) {
 }
 
 static bool CompareTime(file_time_type t1, file_time_type t2) {
-  auto min_secs = duration_cast<Sec>(file_time_type::min().time_since_epoch());
+  auto min_secs = duration_cast<Sec>((file_time_type::min)().time_since_epoch());
   bool IsMin =
       t1.time_since_epoch() < min_secs || t2.time_since_epoch() < min_secs;
 
@@ -358,9 +358,9 @@ inline bool TimeIsRepresentableByFilesystem(file_time_type tp) {
     return false;
   else if (tp.time_since_epoch().count() < 0 && !SupportsNegativeTimes)
     return false;
-  else if (tp == file_time_type::max() && !SupportsMaxTime)
+  else if (tp == (file_time_type::max)() && !SupportsMaxTime)
     return false;
-  else if (tp == file_time_type::min() && !SupportsMinTime)
+  else if (tp == (file_time_type::min)() && !SupportsMinTime)
     return false;
   return true;
 }
@@ -396,7 +396,7 @@ static void read_last_write_time_static_env_test()
 {
     static_test_env static_env;
     using C = file_time_type::clock;
-    file_time_type min = file_time_type::min();
+    file_time_type min = (file_time_type::min)();
     // Sleep a little to make sure that static_env.File created above is
     // strictly older than C::now() even with a coarser clock granularity
     // in C::now(). (GetSystemTimeAsFileTime on windows has a fairly coarse
@@ -567,7 +567,7 @@ static void test_write_min_time()
     scoped_test_env env;
     const path p = env.create_file("file", 42);
     const file_time_type old_time = last_write_time(p);
-    file_time_type new_time = file_time_type::min();
+    file_time_type new_time = (file_time_type::min)();
 
     std::error_code ec = GetTestEC();
     last_write_time(p, new_time, ec);
@@ -578,7 +578,7 @@ static void test_write_min_time()
         assert(CompareTime(tt, new_time));
 
         last_write_time(p, old_time);
-        new_time = file_time_type::min() + SubSec(1);
+        new_time = (file_time_type::min)() + SubSec(1);
 
         ec = GetTestEC();
         last_write_time(p, new_time, ec);
@@ -601,7 +601,7 @@ static void test_write_max_time() {
   scoped_test_env env;
   const path p = env.create_file("file", 42);
   const file_time_type old_time = last_write_time(p);
-  file_time_type new_time = file_time_type::max();
+  file_time_type new_time = (file_time_type::max)();
 
   std::error_code ec = GetTestEC();
   last_write_time(p, new_time, ec);
@@ -621,7 +621,7 @@ static void test_value_on_failure()
     static_test_env static_env;
     const path p = static_env.DNE;
     std::error_code ec = GetTestEC();
-    assert(last_write_time(p, ec) == file_time_type::min());
+    assert(last_write_time(p, ec) == (file_time_type::min)());
     assert(ErrorIs(ec, std::errc::no_such_file_or_directory));
 }
 
@@ -636,7 +636,7 @@ static void test_exists_fails()
     permissions(dir, perms::none);
 
     std::error_code ec = GetTestEC();
-    assert(last_write_time(file, ec) == file_time_type::min());
+    assert(last_write_time(file, ec) == (file_time_type::min)());
     assert(ErrorIs(ec, std::errc::permission_denied));
 
     ExceptionChecker Checker(file, std::errc::permission_denied,



More information about the libcxx-commits mailing list