Skip to content

Conversation

@philnik777
Copy link
Contributor

@philnik777 philnik777 commented Jun 24, 2025

This patch adds a separate extensions directory, since there are quite a few extensions in libc++ that aren't necessarily libc++-specific. For example, the tests currently in libcxx/test/libcxx/extensions should also pass with libstdc++, since they originally added the extension. This also "documents" what users are allowed to rely on and what parts are just libc++ tests to make sure our implementation is behaving as we expect, which may be subject to change.

This patch also formats the tests and refactors .fail.cpp tests to .verify.cpp tests.

@github-actions
Copy link

github-actions bot commented Jun 24, 2025

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff HEAD~1 HEAD --extensions cpp -- libcxx/test/extensions/gnu/hash/specializations.pass.cpp libcxx/test/extensions/gnu/hash/specializations.verify.cpp libcxx/test/extensions/gnu/hash_map/const_iterator.verify.cpp
View the diff from clang-format here.
diff --git a/libcxx/test/extensions/gnu/hash_map/const_iterator.verify.cpp b/libcxx/test/extensions/gnu/hash_map/const_iterator.verify.cpp
index eacdacd03..362ae9190 100644
--- a/libcxx/test/extensions/gnu/hash_map/const_iterator.verify.cpp
+++ b/libcxx/test/extensions/gnu/hash_map/const_iterator.verify.cpp
@@ -14,7 +14,8 @@ int main(int, char**) {
   __gnu_cxx::hash_map<int, int> m;
   m[1]                                    = 1;
   const __gnu_cxx::hash_map<int, int>& cm = m;
-  cm.find(1)->second = 2; // expected-error {{cannot assign to return value because function 'operator->' returns a const value}}
+  cm.find(1)->second =
+      2; // expected-error {{cannot assign to return value because function 'operator->' returns a const value}}
 
   return 0;
 }

@philnik777 philnik777 force-pushed the move_extensions branch 2 times, most recently from decd20e to 800bcd6 Compare June 25, 2025 07:45
@philnik777 philnik777 marked this pull request as ready for review June 25, 2025 14:09
@philnik777 philnik777 requested a review from a team as a code owner June 25, 2025 14:09
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jun 25, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 25, 2025

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

This patch adds a separate extensions directory, since there are quite a few extensions in libc++ that aren't necessarily libc++-specific. For example, the tests currently in libcxx/test/libcxx/extensions should also pass with libstdc++, since they originally added the extension. This also "documents" what users are allowed to rely on and what parts are just libc++ tests to make sure our implementation is behaving as we expect, which may be subject to change.

This patch also formats the tests and refactors .fail.cpp tests to .verify.cpp tests.


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

4 Files Affected:

  • (added) libcxx/test/extensions/gnu/hash/specializations.pass.cpp (+31)
  • (renamed) libcxx/test/extensions/gnu/hash/specializations.verify.cpp (+4-3)
  • (renamed) libcxx/test/extensions/gnu/hash_map/const_iterator.compile.verify.cpp (+7-6)
  • (removed) libcxx/test/libcxx/extensions/hash/specializations.pass.cpp (-36)
diff --git a/libcxx/test/extensions/gnu/hash/specializations.pass.cpp b/libcxx/test/extensions/gnu/hash/specializations.pass.cpp
new file mode 100644
index 0000000000000..6661ed0da7dad
--- /dev/null
+++ b/libcxx/test/extensions/gnu/hash/specializations.pass.cpp
@@ -0,0 +1,31 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// Prevent <ext/hash_set> from generating deprecated warnings for this test.
+// ADDITIONAL_COMPILE_FLAGS: -Wno-deprecated
+
+#include <cassert>
+#include <ext/hash_map>
+#include <string>
+
+int main(int, char**) {
+  char str[] = "test";
+  assert(__gnu_cxx::hash<const char*>()("test") == std::hash<std::string>()("test"));
+  assert(__gnu_cxx::hash<char*>()(str) == std::hash<std::string>()("test"));
+  assert(__gnu_cxx::hash<char>()(42) == 42);
+  assert(__gnu_cxx::hash<signed char>()(42) == 42);
+  assert(__gnu_cxx::hash<unsigned char>()(42) == 42);
+  assert(__gnu_cxx::hash<short>()(42) == 42);
+  assert(__gnu_cxx::hash<unsigned short>()(42) == 42);
+  assert(__gnu_cxx::hash<int>()(42) == 42);
+  assert(__gnu_cxx::hash<unsigned int>()(42) == 42);
+  assert(__gnu_cxx::hash<long>()(42) == 42);
+  assert(__gnu_cxx::hash<unsigned long>()(42) == 42);
+
+  return 0;
+}
diff --git a/libcxx/test/libcxx/extensions/hash/specializations.compile.fail.cpp b/libcxx/test/extensions/gnu/hash/specializations.verify.cpp
similarity index 70%
rename from libcxx/test/libcxx/extensions/hash/specializations.compile.fail.cpp
rename to libcxx/test/extensions/gnu/hash/specializations.verify.cpp
index f81ec5dacb91e..5a74a52dd3956 100644
--- a/libcxx/test/libcxx/extensions/hash/specializations.compile.fail.cpp
+++ b/libcxx/test/extensions/gnu/hash/specializations.verify.cpp
@@ -6,13 +6,14 @@
 //
 //===----------------------------------------------------------------------===//
 
+// ADDITIONAL_COMPILE_FLAGS: -Wno-deprecated
+
 #include <assert.h>
 #include <ext/hash_map>
 #include <string>
 
-int main(int, char**)
-{
-    assert(__gnu_cxx::hash<std::string>()(std::string()) == 0);  // error
+int main(int, char**) {
+  assert(__gnu_cxx::hash<std::string>()(std::string()) == 0); // expected-error {{does not provide a call operator}}
 
   return 0;
 }
diff --git a/libcxx/test/libcxx/extensions/hash_map/const_iterator.compile.fail.cpp b/libcxx/test/extensions/gnu/hash_map/const_iterator.compile.verify.cpp
similarity index 53%
rename from libcxx/test/libcxx/extensions/hash_map/const_iterator.compile.fail.cpp
rename to libcxx/test/extensions/gnu/hash_map/const_iterator.compile.verify.cpp
index db09e40801a1f..cdb5fce16f44f 100644
--- a/libcxx/test/libcxx/extensions/hash_map/const_iterator.compile.fail.cpp
+++ b/libcxx/test/extensions/gnu/hash_map/const_iterator.compile.verify.cpp
@@ -6,14 +6,15 @@
 //
 //===----------------------------------------------------------------------===//
 
+// ADDITIONAL_COMPILE_FLAGS: -Wno-deprecated
+
 #include <ext/hash_map>
 
-int main(int, char**)
-{
-    __gnu_cxx::hash_map<int, int> m;
-    m[1] = 1;
-    const __gnu_cxx::hash_map<int, int> &cm = m;
-    cm.find(1)->second = 2;  // error
+int main(int, char**) {
+  __gnu_cxx::hash_map<int, int> m;
+  m[1]                                    = 1;
+  const __gnu_cxx::hash_map<int, int>& cm = m;
+  cm.find(1)->second                      = 2; // expected-error {{cannot assign to return value because function 'operator->' returns a const value}}
 
   return 0;
 }
diff --git a/libcxx/test/libcxx/extensions/hash/specializations.pass.cpp b/libcxx/test/libcxx/extensions/hash/specializations.pass.cpp
deleted file mode 100644
index 345a2721dc035..0000000000000
--- a/libcxx/test/libcxx/extensions/hash/specializations.pass.cpp
+++ /dev/null
@@ -1,36 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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: clang-modules-build
-
-// Prevent <ext/hash_set> from generating deprecated warnings for this test.
-// ADDITIONAL_COMPILE_FLAGS: -Wno-deprecated
-#include <assert.h>
-#include <ext/hash_map>
-#include <string>
-
-#include "test_macros.h"
-
-int main(int, char**)
-{
-    char str[] = "test";
-    assert(__gnu_cxx::hash<const char *>()("test") ==
-           std::hash<std::string>()("test"));
-    assert(__gnu_cxx::hash<char *>()(str) == std::hash<std::string>()("test"));
-    assert(__gnu_cxx::hash<char>()(42) == 42);
-    assert(__gnu_cxx::hash<signed char>()(42) == 42);
-    assert(__gnu_cxx::hash<unsigned char>()(42) == 42);
-    assert(__gnu_cxx::hash<short>()(42) == 42);
-    assert(__gnu_cxx::hash<unsigned short>()(42) == 42);
-    assert(__gnu_cxx::hash<int>()(42) == 42);
-    assert(__gnu_cxx::hash<unsigned int>()(42) == 42);
-    assert(__gnu_cxx::hash<long>()(42) == 42);
-    assert(__gnu_cxx::hash<unsigned long>()(42) == 42);
-
-  return 0;
-}

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but as discussed let's add a lit.local.cfg file that marks these tests as requiring libc++ or libstdc++.

As discussed just now, the underlying intent is that other implementations shouldn't have to know about the directory structure of our test suite in order to run our test suite.

@philnik777 philnik777 force-pushed the move_extensions branch 3 times, most recently from d35461e to 2aaa7ce Compare June 26, 2025 06:39
… and update the tests

This patch adds a separate `extensions` directory, since there are
quite a few extensions in libc++ that aren't necessarily
libc++-specific. For example, the tests currently in
`libcxx/test/libcxx/extensions` should also pass with libstdc++, since
they originally added the extension. This also "documents" what users
are allowed to rely on and what parts are just libc++ tests to make sure
our implementation is behaving as we expect, which may be subject to
change.
@philnik777 philnik777 merged commit e980523 into llvm:main Jun 27, 2025
69 of 79 checks passed
@philnik777 philnik777 deleted the move_extensions branch September 25, 2025 08:25
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