Skip to content

[libc++][test] Cleanup LIBCPP_ONLY(meow_assert(...)) to LIBCPP_MEOW_ASSERT(...) #74967

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

StephanTLavavej
Copy link
Member

This is a syntax cleanup, not needed for running libc++'s tests with MSVC's STL.

While changing libcxx/test/std/input.output/filesystems/fs.enum/enum.path.format.pass.cpp in #74965, I noticed that libc++'s tests almost always use the special-purpose macros for "this is a libc++-specific static_assert etc." defined by:

/* Macros for testing libc++ specific behavior and extensions */
#if defined(_LIBCPP_VERSION)
#define LIBCPP_ASSERT(...) assert(__VA_ARGS__)
#define LIBCPP_STATIC_ASSERT(...) static_assert(__VA_ARGS__)
#define LIBCPP_ASSERT_NOEXCEPT(...) ASSERT_NOEXCEPT(__VA_ARGS__)
#define LIBCPP_ASSERT_NOT_NOEXCEPT(...) ASSERT_NOT_NOEXCEPT(__VA_ARGS__)
#define LIBCPP_ONLY(...) __VA_ARGS__
#else
#define LIBCPP_ASSERT(...) static_assert(true, "")
#define LIBCPP_STATIC_ASSERT(...) static_assert(true, "")
#define LIBCPP_ASSERT_NOEXCEPT(...) static_assert(true, "")
#define LIBCPP_ASSERT_NOT_NOEXCEPT(...) static_assert(true, "")
#define LIBCPP_ONLY(...) static_assert(true, "")
#endif

However, there were a very small number of occurrences that were using the general-purpose LIBCPP_ONLY macro when they could have been using the special-purpose macros. I believe that they should be cleaned up, to make it easier to search for usage, and to make it clearer when the full power of LIBCPP_ONLY is necessary.

This is a pure regex replacement from LIBCPP_ONLY\((assert|static_assert|ASSERT_NOEXCEPT|ASSERT_NOT_NOEXCEPT)\((.*)\)\); to LIBCPP_\U$1($2); using the power of VSCode's case changing in regex replace.

To avoid merge conflicts, this isn't changing the line in libcxx/test/std/input.output/filesystems/fs.enum/enum.path.format.pass.cpp that #74965 is already changing to use LIBCPP_STATIC_ASSERT.

…T(...).

Pure regex replacement from:
LIBCPP_ONLY\((assert|static_assert|ASSERT_NOEXCEPT|ASSERT_NOT_NOEXCEPT)\((.*)\)\);
to:
LIBCPP_\U$1($2);
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner December 10, 2023 05:19
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Dec 10, 2023
@llvmbot
Copy link
Member

llvmbot commented Dec 10, 2023

@llvm/pr-subscribers-libcxx

Author: Stephan T. Lavavej (StephanTLavavej)

Changes

This is a syntax cleanup, not needed for running libc++'s tests with MSVC's STL.

While changing libcxx/test/std/input.output/filesystems/fs.enum/enum.path.format.pass.cpp in #74965, I noticed that libc++'s tests almost always use the special-purpose macros for "this is a libc++-specific static_assert etc." defined by:

/* Macros for testing libc++ specific behavior and extensions */
#if defined(_LIBCPP_VERSION)
#define LIBCPP_ASSERT(...) assert(__VA_ARGS__)
#define LIBCPP_STATIC_ASSERT(...) static_assert(__VA_ARGS__)
#define LIBCPP_ASSERT_NOEXCEPT(...) ASSERT_NOEXCEPT(__VA_ARGS__)
#define LIBCPP_ASSERT_NOT_NOEXCEPT(...) ASSERT_NOT_NOEXCEPT(__VA_ARGS__)
#define LIBCPP_ONLY(...) __VA_ARGS__
#else
#define LIBCPP_ASSERT(...) static_assert(true, "")
#define LIBCPP_STATIC_ASSERT(...) static_assert(true, "")
#define LIBCPP_ASSERT_NOEXCEPT(...) static_assert(true, "")
#define LIBCPP_ASSERT_NOT_NOEXCEPT(...) static_assert(true, "")
#define LIBCPP_ONLY(...) static_assert(true, "")
#endif

However, there were a very small number of occurrences that were using the general-purpose LIBCPP_ONLY macro when they could have been using the special-purpose macros. I believe that they should be cleaned up, to make it easier to search for usage, and to make it clearer when the full power of LIBCPP_ONLY is necessary.

This is a pure regex replacement from LIBCPP_ONLY\((assert|static_assert|ASSERT_NOEXCEPT|ASSERT_NOT_NOEXCEPT)\((.*)\)\); to LIBCPP_\U$1($2); using the power of VSCode's case changing in regex replace.

To avoid merge conflicts, this isn't changing the line in libcxx/test/std/input.output/filesystems/fs.enum/enum.path.format.pass.cpp that #74965 is already changing to use LIBCPP_STATIC_ASSERT.


Full diff: https://github.com/llvm/llvm-project/pull/74967.diff

10 Files Affected:

  • (modified) libcxx/test/std/input.output/filesystems/class.path/path.member/path.assign/source.pass.cpp (+1-1)
  • (modified) libcxx/test/std/input.output/filesystems/fs.enum/enum.copy_options.pass.cpp (+1-1)
  • (modified) libcxx/test/std/input.output/filesystems/fs.enum/enum.directory_options.pass.cpp (+1-1)
  • (modified) libcxx/test/std/input.output/filesystems/fs.enum/enum.file_type.pass.cpp (+1-1)
  • (modified) libcxx/test/std/input.output/filesystems/fs.enum/enum.perm_options.pass.cpp (+1-1)
  • (modified) libcxx/test/std/input.output/filesystems/fs.enum/enum.perms.pass.cpp (+1-1)
  • (modified) libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.absolute/absolute.pass.cpp (+1-1)
  • (modified) libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.canonical/canonical.pass.cpp (+2-2)
  • (modified) libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.permissions/permissions.pass.cpp (+1-1)
  • (modified) libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.temp_dir_path/temp_directory_path.pass.cpp (+2-2)
diff --git a/libcxx/test/std/input.output/filesystems/class.path/path.member/path.assign/source.pass.cpp b/libcxx/test/std/input.output/filesystems/class.path/path.member/path.assign/source.pass.cpp
index 893d09221fce2d..e969aa4e8d66aa 100644
--- a/libcxx/test/std/input.output/filesystems/class.path/path.member/path.assign/source.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/class.path/path.member/path.assign/source.pass.cpp
@@ -230,7 +230,7 @@ void RunStringMoveTest(const fs::path::value_type* Expect) {
   assert(p == Expect);
   {
     // Signature test
-    LIBCPP_ONLY(ASSERT_NOEXCEPT(p = std::move(ss)));
+    LIBCPP_ASSERT_NOEXCEPT(p = std::move(ss));
   }
 }
 
diff --git a/libcxx/test/std/input.output/filesystems/fs.enum/enum.copy_options.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.enum/enum.copy_options.pass.cpp
index 56322575fef213..4ef28ee01d8d02 100644
--- a/libcxx/test/std/input.output/filesystems/fs.enum/enum.copy_options.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/fs.enum/enum.copy_options.pass.cpp
@@ -30,7 +30,7 @@ int main(int, char**) {
   typedef std::underlying_type<E>::type UT;
   static_assert(!std::is_convertible<E, UT>::value, "");
 
-  LIBCPP_ONLY(static_assert(std::is_same<UT, unsigned short>::value, "")); // Implementation detail
+  LIBCPP_STATIC_ASSERT(std::is_same<UT, unsigned short>::value, ""); // Implementation detail
 
   typedef check_bitmask_type<E, E::skip_existing, E::update_existing> BitmaskTester;
   assert(BitmaskTester::check());
diff --git a/libcxx/test/std/input.output/filesystems/fs.enum/enum.directory_options.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.enum/enum.directory_options.pass.cpp
index 2597457ed747e1..4480b1e4e3357c 100644
--- a/libcxx/test/std/input.output/filesystems/fs.enum/enum.directory_options.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/fs.enum/enum.directory_options.pass.cpp
@@ -29,7 +29,7 @@ int main(int, char**) {
   // Check that E is a scoped enum by checking for conversions.
   typedef std::underlying_type<E>::type UT;
   static_assert(!std::is_convertible<E, UT>::value, "");
-  LIBCPP_ONLY(static_assert(std::is_same<UT, unsigned char>::value, ""));
+  LIBCPP_STATIC_ASSERT(std::is_same<UT, unsigned char>::value, "");
 
   typedef check_bitmask_type<E, E::follow_directory_symlink, E::skip_permission_denied> BitmaskTester;
   assert(BitmaskTester::check());
diff --git a/libcxx/test/std/input.output/filesystems/fs.enum/enum.file_type.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.enum/enum.file_type.pass.cpp
index 63db8892b0928c..6062935126dd22 100644
--- a/libcxx/test/std/input.output/filesystems/fs.enum/enum.file_type.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/fs.enum/enum.file_type.pass.cpp
@@ -29,7 +29,7 @@ int main(int, char**) {
   typedef std::underlying_type<E>::type UT;
   static_assert(!std::is_convertible<E, UT>::value, "");
 
-  LIBCPP_ONLY(static_assert(std::is_same<UT, signed char>::value, "")); // Implementation detail
+  LIBCPP_STATIC_ASSERT(std::is_same<UT, signed char>::value, ""); // Implementation detail
 
   // The standard doesn't specify the numeric values of the enum.
   LIBCPP_STATIC_ASSERT(
diff --git a/libcxx/test/std/input.output/filesystems/fs.enum/enum.perm_options.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.enum/enum.perm_options.pass.cpp
index afc64f6f00b4da..0bdae487023854 100644
--- a/libcxx/test/std/input.output/filesystems/fs.enum/enum.perm_options.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/fs.enum/enum.perm_options.pass.cpp
@@ -32,7 +32,7 @@ int main(int, char**) {
   typedef std::underlying_type<E>::type UT;
   static_assert(!std::is_convertible<E, UT>::value, "");
 
-  LIBCPP_ONLY(static_assert(std::is_same<UT, unsigned char >::value, "")); // Implementation detail
+  LIBCPP_STATIC_ASSERT(std::is_same<UT, unsigned char >::value, ""); // Implementation detail
 
   typedef check_bitmask_type<E, E::replace, E::nofollow> BitmaskTester;
   assert(BitmaskTester::check());
diff --git a/libcxx/test/std/input.output/filesystems/fs.enum/enum.perms.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.enum/enum.perms.pass.cpp
index 27aa5e29de20c4..d1893847f01b43 100644
--- a/libcxx/test/std/input.output/filesystems/fs.enum/enum.perms.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/fs.enum/enum.perms.pass.cpp
@@ -30,7 +30,7 @@ int main(int, char**) {
   typedef std::underlying_type<E>::type UT;
   static_assert(!std::is_convertible<E, UT>::value, "");
 
-  LIBCPP_ONLY(static_assert(std::is_same<UT, unsigned >::value, "")); // Implementation detail
+  LIBCPP_STATIC_ASSERT(std::is_same<UT, unsigned >::value, ""); // Implementation detail
 
   typedef check_bitmask_type<E, E::group_all, E::owner_all> BitmaskTester;
   assert(BitmaskTester::check());
diff --git a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.absolute/absolute.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.absolute/absolute.pass.cpp
index 68fc09157c4b68..7a60d1ab29f4ad 100644
--- a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.absolute/absolute.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.absolute/absolute.pass.cpp
@@ -51,7 +51,7 @@ static void basic_test()
         assert(!ec);
         assert(ret.is_absolute());
         assert(PathEqIgnoreSep(ret, TC.expect));
-        LIBCPP_ONLY(assert(PathEq(ret, TC.expect)));
+        LIBCPP_ASSERT(PathEq(ret, TC.expect));
     }
 }
 
diff --git a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.canonical/canonical.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.canonical/canonical.pass.cpp
index 6e947a355a5413..0098fe8ee698ef 100644
--- a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.canonical/canonical.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.canonical/canonical.pass.cpp
@@ -104,7 +104,7 @@ static void test_exception_contains_paths()
     } catch (filesystem_error const& err) {
         assert(err.path1() == p);
         // libc++ provides the current path as the second path in the exception
-        LIBCPP_ONLY(assert(err.path2() == current_path()));
+        LIBCPP_ASSERT(err.path2() == current_path());
     }
     fs::current_path(static_env.Dir);
     try {
@@ -112,7 +112,7 @@ static void test_exception_contains_paths()
         assert(false);
     } catch (filesystem_error const& err) {
         assert(err.path1() == p);
-        LIBCPP_ONLY(assert(err.path2() == static_env.Dir));
+        LIBCPP_ASSERT(err.path2() == static_env.Dir);
     }
 #endif
 }
diff --git a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.permissions/permissions.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.permissions/permissions.pass.cpp
index 0f5a7692bb70d6..f009befa49c379 100644
--- a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.permissions/permissions.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.permissions/permissions.pass.cpp
@@ -40,7 +40,7 @@ static void test_signatures()
     ASSERT_NOT_NOEXCEPT(fs::permissions(p, pr));
     ASSERT_NOT_NOEXCEPT(fs::permissions(p, pr, opts));
     ASSERT_NOEXCEPT(fs::permissions(p, pr, ec));
-    LIBCPP_ONLY(ASSERT_NOT_NOEXCEPT(fs::permissions(p, pr, opts, ec)));
+    LIBCPP_ASSERT_NOT_NOEXCEPT(fs::permissions(p, pr, opts, ec));
 }
 
 static void test_error_reporting()
diff --git a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.temp_dir_path/temp_directory_path.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.temp_dir_path/temp_directory_path.pass.cpp
index c0a98a62e6f9ad..aed87b73121d06 100644
--- a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.temp_dir_path/temp_directory_path.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.temp_dir_path/temp_directory_path.pass.cpp
@@ -95,7 +95,7 @@ static void basic_tests()
         PutEnv(TC.name, dne);
         ec = GetTestEC();
         ret = temp_directory_path(ec);
-        LIBCPP_ONLY(assert(ErrorIs(ec, expect_errc)));
+        LIBCPP_ASSERT(ErrorIs(ec, expect_errc));
         assert(ec != GetTestEC());
         assert(ec);
         assert(ret == "");
@@ -104,7 +104,7 @@ static void basic_tests()
         PutEnv(TC.name, file);
         ec = GetTestEC();
         ret = temp_directory_path(ec);
-        LIBCPP_ONLY(assert(ErrorIs(ec, expect_errc)));
+        LIBCPP_ASSERT(ErrorIs(ec, expect_errc));
         assert(ec != GetTestEC());
         assert(ec);
         assert(ret == "");

@StephanTLavavej
Copy link
Member Author

Thanks! I'm going to go ahead and merge this one too - I looked at all of the failing stage3 checks and they're either the known failure I just fixed, or unrelated sporadic timeouts/failures. One check has also been queued for this PR for almost 8 hours (stage3 generic-msan, same as another PR); if this mechanical syntax cleanup was going to cause problems we would have seen it in the other checks.

@StephanTLavavej StephanTLavavej merged commit fde04e6 into llvm:main Dec 10, 2023
@StephanTLavavej StephanTLavavej deleted the stl-27-cleanup-LIBCPP_MEOW_ASSERT branch December 10, 2023 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants