Skip to content

[libc++] Fix uses of non-empty transparent comparator in <map> #152624

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
merged 2 commits into from
Aug 8, 2025

Conversation

frederick-vs-ja
Copy link
Contributor

The __get_value() member function was removed in LLVM 21, but the calls in <map> weren't removed. This patch completes the removal and adds regression test cases.

Fixes #152543.

The `__get_value()` member function was removed in LLVM 21, but the
calls in `<map>` weren't removed. This patch completes the removal and
adds regression test cases.
@frederick-vs-ja frederick-vs-ja added this to the LLVM 21.x Release milestone Aug 8, 2025
@frederick-vs-ja frederick-vs-ja added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Aug 8, 2025
@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner August 8, 2025 02:19
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Aug 8, 2025
@frederick-vs-ja frederick-vs-ja changed the title [libc++] Fix uses of non-empty transparent comparator with in <map> [libc++] Fix uses of non-empty transparent comparator in <map> Aug 8, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 8, 2025

@llvm/pr-subscribers-libcxx

Author: A. Jiang (frederick-vs-ja)

Changes

The __get_value() member function was removed in LLVM 21, but the calls in &lt;map&gt; weren't removed. This patch completes the removal and adds regression test cases.

Fixes #152543.


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

12 Files Affected:

  • (modified) libcxx/include/map (+2-2)
  • (modified) libcxx/test/std/containers/associative/map/map.ops/count0.pass.cpp (+4)
  • (modified) libcxx/test/std/containers/associative/map/map.ops/equal_range0.pass.cpp (+7)
  • (modified) libcxx/test/std/containers/associative/map/map.ops/find0.pass.cpp (+5)
  • (modified) libcxx/test/std/containers/associative/map/map.ops/lower_bound0.pass.cpp (+5)
  • (modified) libcxx/test/std/containers/associative/map/map.ops/upper_bound0.pass.cpp (+5)
  • (modified) libcxx/test/std/containers/associative/multimap/multimap.ops/count0.pass.cpp (+4)
  • (modified) libcxx/test/std/containers/associative/multimap/multimap.ops/equal_range0.pass.cpp (+7)
  • (modified) libcxx/test/std/containers/associative/multimap/multimap.ops/find0.pass.cpp (+5)
  • (modified) libcxx/test/std/containers/associative/multimap/multimap.ops/lower_bound0.pass.cpp (+5)
  • (modified) libcxx/test/std/containers/associative/multimap/multimap.ops/upper_bound0.pass.cpp (+5)
  • (modified) libcxx/test/support/is_transparent.h (+13)
diff --git a/libcxx/include/map b/libcxx/include/map
index 6378218945ca0..9f98abef9afe0 100644
--- a/libcxx/include/map
+++ b/libcxx/include/map
@@ -691,12 +691,12 @@ public:
 #  if _LIBCPP_STD_VER >= 14
   template <typename _K2>
   _LIBCPP_HIDE_FROM_ABI bool operator()(const _K2& __x, const _CP& __y) const {
-    return __comp_(__x, __y.__get_value().first);
+    return __comp_(__x, __y.first);
   }
 
   template <typename _K2>
   _LIBCPP_HIDE_FROM_ABI bool operator()(const _CP& __x, const _K2& __y) const {
-    return __comp_(__x.__get_value().first, __y);
+    return __comp_(__x.first, __y);
   }
 #  endif
 };
diff --git a/libcxx/test/std/containers/associative/map/map.ops/count0.pass.cpp b/libcxx/test/std/containers/associative/map/map.ops/count0.pass.cpp
index c7ba765178969..62491e2bc4438 100644
--- a/libcxx/test/std/containers/associative/map/map.ops/count0.pass.cpp
+++ b/libcxx/test/std/containers/associative/map/map.ops/count0.pass.cpp
@@ -33,6 +33,10 @@ int main(int, char**) {
     typedef std::map<int, double, transparent_less_not_referenceable> M;
     assert(M().count(C2Int{5}) == 0);
   }
+  {
+    using M = std::map<int, double, transparent_less_nonempty>;
+    assert(M().count(C2Int{5}) == 0);
+  }
 
   return 0;
 }
diff --git a/libcxx/test/std/containers/associative/map/map.ops/equal_range0.pass.cpp b/libcxx/test/std/containers/associative/map/map.ops/equal_range0.pass.cpp
index 75724bdb387ce..57ce9339f6323 100644
--- a/libcxx/test/std/containers/associative/map/map.ops/equal_range0.pass.cpp
+++ b/libcxx/test/std/containers/associative/map/map.ops/equal_range0.pass.cpp
@@ -40,6 +40,13 @@ int main(int, char**) {
     P result = example.equal_range(C2Int{5});
     assert(result.first == result.second);
   }
+  {
+    using M = std::map<int, double, transparent_less_nonempty>;
+    using P = std::pair<typename M::iterator, typename M::iterator>;
+    M example;
+    P result = example.equal_range(C2Int{5});
+    assert(result.first == result.second);
+  }
 
   return 0;
 }
diff --git a/libcxx/test/std/containers/associative/map/map.ops/find0.pass.cpp b/libcxx/test/std/containers/associative/map/map.ops/find0.pass.cpp
index 9825d6c5879b1..3f09d5608772d 100644
--- a/libcxx/test/std/containers/associative/map/map.ops/find0.pass.cpp
+++ b/libcxx/test/std/containers/associative/map/map.ops/find0.pass.cpp
@@ -36,6 +36,11 @@ int main(int, char**) {
     M example;
     assert(example.find(C2Int{5}) == example.end());
   }
+  {
+    using M = std::map<int, double, transparent_less_nonempty>;
+    M example;
+    assert(example.find(C2Int{5}) == example.end());
+  }
 
   return 0;
 }
diff --git a/libcxx/test/std/containers/associative/map/map.ops/lower_bound0.pass.cpp b/libcxx/test/std/containers/associative/map/map.ops/lower_bound0.pass.cpp
index fe7fe381a86eb..308a2ed1e3af0 100644
--- a/libcxx/test/std/containers/associative/map/map.ops/lower_bound0.pass.cpp
+++ b/libcxx/test/std/containers/associative/map/map.ops/lower_bound0.pass.cpp
@@ -36,6 +36,11 @@ int main(int, char**) {
     M example;
     assert(example.lower_bound(C2Int{5}) == example.end());
   }
+  {
+    using M = std::map<int, double, transparent_less_nonempty>;
+    M example;
+    assert(example.lower_bound(C2Int{5}) == example.end());
+  }
 
   return 0;
 }
diff --git a/libcxx/test/std/containers/associative/map/map.ops/upper_bound0.pass.cpp b/libcxx/test/std/containers/associative/map/map.ops/upper_bound0.pass.cpp
index 525aa673ea74b..332b71a84e9fb 100644
--- a/libcxx/test/std/containers/associative/map/map.ops/upper_bound0.pass.cpp
+++ b/libcxx/test/std/containers/associative/map/map.ops/upper_bound0.pass.cpp
@@ -36,6 +36,11 @@ int main(int, char**) {
     M example;
     assert(example.upper_bound(C2Int{5}) == example.end());
   }
+  {
+    using M = std::map<int, double, transparent_less_nonempty>;
+    M example;
+    assert(example.upper_bound(C2Int{5}) == example.end());
+  }
 
   return 0;
 }
diff --git a/libcxx/test/std/containers/associative/multimap/multimap.ops/count0.pass.cpp b/libcxx/test/std/containers/associative/multimap/multimap.ops/count0.pass.cpp
index 233d1a11e1d6c..36f0ac2647ba3 100644
--- a/libcxx/test/std/containers/associative/multimap/multimap.ops/count0.pass.cpp
+++ b/libcxx/test/std/containers/associative/multimap/multimap.ops/count0.pass.cpp
@@ -33,6 +33,10 @@ int main(int, char**) {
     typedef std::multimap<int, double, transparent_less_not_referenceable> M;
     assert(M().count(C2Int{5}) == 0);
   }
+  {
+    using M = std::multimap<int, double, transparent_less_nonempty>;
+    assert(M().count(C2Int{5}) == 0);
+  }
 
   return 0;
 }
diff --git a/libcxx/test/std/containers/associative/multimap/multimap.ops/equal_range0.pass.cpp b/libcxx/test/std/containers/associative/multimap/multimap.ops/equal_range0.pass.cpp
index 0bead6c7938db..a362c03e26385 100644
--- a/libcxx/test/std/containers/associative/multimap/multimap.ops/equal_range0.pass.cpp
+++ b/libcxx/test/std/containers/associative/multimap/multimap.ops/equal_range0.pass.cpp
@@ -40,6 +40,13 @@ int main(int, char**) {
     P result = example.equal_range(C2Int{5});
     assert(result.first == result.second);
   }
+  {
+    using M = std::multimap<int, double, transparent_less_nonempty>;
+    using P = std::pair<typename M::iterator, typename M::iterator>;
+    M example;
+    P result = example.equal_range(C2Int{5});
+    assert(result.first == result.second);
+  }
 
   return 0;
 }
diff --git a/libcxx/test/std/containers/associative/multimap/multimap.ops/find0.pass.cpp b/libcxx/test/std/containers/associative/multimap/multimap.ops/find0.pass.cpp
index 701d4e314e7a8..ccb0900e76835 100644
--- a/libcxx/test/std/containers/associative/multimap/multimap.ops/find0.pass.cpp
+++ b/libcxx/test/std/containers/associative/multimap/multimap.ops/find0.pass.cpp
@@ -36,6 +36,11 @@ int main(int, char**) {
     M example;
     assert(example.find(C2Int{5}) == example.end());
   }
+  {
+    using M = std::multimap<int, double, transparent_less_nonempty>;
+    M example;
+    assert(example.find(C2Int{5}) == example.end());
+  }
 
   return 0;
 }
diff --git a/libcxx/test/std/containers/associative/multimap/multimap.ops/lower_bound0.pass.cpp b/libcxx/test/std/containers/associative/multimap/multimap.ops/lower_bound0.pass.cpp
index 79f994847f131..4b4853062001f 100644
--- a/libcxx/test/std/containers/associative/multimap/multimap.ops/lower_bound0.pass.cpp
+++ b/libcxx/test/std/containers/associative/multimap/multimap.ops/lower_bound0.pass.cpp
@@ -36,6 +36,11 @@ int main(int, char**) {
     M example;
     assert(example.lower_bound(C2Int{5}) == example.end());
   }
+  {
+    using M = std::multimap<int, double, transparent_less_nonempty>;
+    M example;
+    assert(example.lower_bound(C2Int{5}) == example.end());
+  }
 
   return 0;
 }
diff --git a/libcxx/test/std/containers/associative/multimap/multimap.ops/upper_bound0.pass.cpp b/libcxx/test/std/containers/associative/multimap/multimap.ops/upper_bound0.pass.cpp
index 62f52416b9156..f2ae94577b6c1 100644
--- a/libcxx/test/std/containers/associative/multimap/multimap.ops/upper_bound0.pass.cpp
+++ b/libcxx/test/std/containers/associative/multimap/multimap.ops/upper_bound0.pass.cpp
@@ -36,6 +36,11 @@ int main(int, char**) {
     M example;
     assert(example.upper_bound(C2Int{5}) == example.end());
   }
+  {
+    using M = std::multimap<int, double, transparent_less_nonempty>;
+    M example;
+    assert(example.upper_bound(C2Int{5}) == example.end());
+  }
 
   return 0;
 }
diff --git a/libcxx/test/support/is_transparent.h b/libcxx/test/support/is_transparent.h
index 700c894a8b60f..350ef97f31877 100644
--- a/libcxx/test/support/is_transparent.h
+++ b/libcxx/test/support/is_transparent.h
@@ -36,6 +36,19 @@ struct transparent_less_not_referenceable
     using is_transparent = void () const &;  // it's a type; a weird one, but a type
 };
 
+// Prevent regression when empty base class optimization is not suitable.
+// See https://github.com/llvm/llvm-project/issues/152543.
+struct transparent_less_nonempty {
+  template <class T, class U>
+  constexpr auto operator()(T&& t, U&& u) const                   //
+      noexcept(noexcept(std::forward<T>(t) < std::forward<U>(u))) //
+      -> decltype /**/ (std::forward<T>(t) < std::forward<U>(u)) {
+    return /*--------*/ std::forward<T>(t) < std::forward<U>(u);
+  }
+  struct is_transparent {
+  } pad_; // making this comparator non-empty
+};
+
 struct transparent_less_no_type
 {
     template <class T, class U>

Copy link
Contributor

@philnik777 philnik777 left a comment

Choose a reason for hiding this comment

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

Except for the IMO overcomplicated comparator LGTM.

@github-project-automation github-project-automation bot moved this from Needs Triage to Needs Merge in LLVM Release Status Aug 8, 2025
@tru tru moved this from Needs Merge to Needs Backport PR in LLVM Release Status Aug 8, 2025
@frederick-vs-ja
Copy link
Contributor Author

Merging as CI failures are probably unrelated.

@frederick-vs-ja frederick-vs-ja merged commit 7ae1424 into llvm:main Aug 8, 2025
71 of 76 checks passed
@github-project-automation github-project-automation bot moved this from Needs Backport PR to Done in LLVM Release Status Aug 8, 2025
@frederick-vs-ja
Copy link
Contributor Author

/cherry-pick 7ae1424

@llvmbot
Copy link
Member

llvmbot commented Aug 8, 2025

/pull-request #152776

@frederick-vs-ja frederick-vs-ja deleted the map-non-empty-comparator branch August 8, 2025 18:48
tru pushed a commit to llvmbot/llvm-project that referenced this pull request Aug 11, 2025
…m#152624)

The `__get_value()` member function was removed in LLVM 21, but the
calls in `<map>` weren't removed. This patch completes the removal and
adds regression test cases.

Fixes llvm#152543.

(cherry picked from commit 7ae1424)
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
Development

Successfully merging this pull request may close these issues.

[libcxx] calling find() on a map with a non-empty transparent comparator causes a compilation failure (found in PoDoFo)
3 participants