Skip to content

Conversation

@H-G-Hristov
Copy link
Contributor

@H-G-Hristov H-G-Hristov commented Apr 22, 2025

@github-actions
Copy link

github-actions bot commented Apr 22, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@H-G-Hristov H-G-Hristov changed the title [libc++][pair] P2944R3: Constrain pair equality operator [libc++][pair] P2944R3: Constrain std::pair's equality operator Apr 22, 2025
@Zingam Zingam marked this pull request as ready for review April 22, 2025 21:17
@Zingam Zingam requested a review from a team as a code owner April 22, 2025 21:17
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Apr 22, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 22, 2025

@llvm/pr-subscribers-libcxx

Author: Hristo Hristov (H-G-Hristov)

Changes

Implements https://wg21.link/P2944R3 (partially):

Related issues:

Closes: #136763

References


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

3 Files Affected:

  • (modified) libcxx/docs/Status/Cxx2cPapers.csv (+1-1)
  • (modified) libcxx/include/__utility/pair.h (+9-1)
  • (modified) libcxx/test/std/utilities/utility/pairs/pairs.spec/comparison.pass.cpp (+26)
diff --git a/libcxx/docs/Status/Cxx2cPapers.csv b/libcxx/docs/Status/Cxx2cPapers.csv
index 0cc41d2058dd5..0ed0e9f3ccb7c 100644
--- a/libcxx/docs/Status/Cxx2cPapers.csv
+++ b/libcxx/docs/Status/Cxx2cPapers.csv
@@ -59,7 +59,7 @@
 "`P2248R8 <https://wg21.link/P2248R8>`__","Enabling list-initialization for algorithms","2024-03 (Tokyo)","","",""
 "`P2810R4 <https://wg21.link/P2810R4>`__","``is_debugger_present`` ``is_replaceable``","2024-03 (Tokyo)","","",""
 "`P1068R11 <https://wg21.link/P1068R11>`__","Vector API for random number generation","2024-03 (Tokyo)","","",""
-"`P2944R3 <https://wg21.link/P2944R3>`__","Comparisons for ``reference_wrapper``","2024-03 (Tokyo)","|Partial|","19","Implemented comparisons for ``reference_wrapper`` only"
+"`P2944R3 <https://wg21.link/P2944R3>`__","Comparisons for ``reference_wrapper``","2024-03 (Tokyo)","|Partial|","21","Implemented changes to ``reference_wrapper``, ``pair``"
 "`P2642R6 <https://wg21.link/P2642R6>`__","Padded ``mdspan`` layouts","2024-03 (Tokyo)","","",""
 "`P3029R1 <https://wg21.link/P3029R1>`__","Better ``mdspan``'s CTAD","2024-03 (Tokyo)","|Complete|","19",""
 "","","","","",""
diff --git a/libcxx/include/__utility/pair.h b/libcxx/include/__utility/pair.h
index 1f596a87f7cc7..72363cac9ebec 100644
--- a/libcxx/include/__utility/pair.h
+++ b/libcxx/include/__utility/pair.h
@@ -11,6 +11,7 @@
 
 #include <__compare/common_comparison_category.h>
 #include <__compare/synth_three_way.h>
+#include <__concepts/boolean_testable.h>
 #include <__concepts/different_from.h>
 #include <__config>
 #include <__cstddef/size_t.h>
@@ -447,7 +448,14 @@ pair(_T1, _T2) -> pair<_T1, _T2>;
 
 template <class _T1, class _T2, class _U1, class _U2>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 bool
-operator==(const pair<_T1, _T2>& __x, const pair<_U1, _U2>& __y) {
+operator==(const pair<_T1, _T2>& __x, const pair<_U1, _U2>& __y)
+#if _LIBCPP_STD_VER >= 26
+  requires requires {
+    { __x.first == __y.first } -> __boolean_testable;
+    { __x.second == __y.second } -> __boolean_testable;
+  }
+#endif
+{
   return __x.first == __y.first && __x.second == __y.second;
 }
 
diff --git a/libcxx/test/std/utilities/utility/pairs/pairs.spec/comparison.pass.cpp b/libcxx/test/std/utilities/utility/pairs/pairs.spec/comparison.pass.cpp
index 76f9771f2b99b..c472906c5ed7f 100644
--- a/libcxx/test/std/utilities/utility/pairs/pairs.spec/comparison.pass.cpp
+++ b/libcxx/test/std/utilities/utility/pairs/pairs.spec/comparison.pass.cpp
@@ -19,9 +19,35 @@
 
 #include <utility>
 #include <cassert>
+#include <concepts>
 
 #include "test_macros.h"
 
+#if TEST_STD_VER >= 26
+
+// Test SFINAE.
+
+struct EqualityComparable {
+  constexpr EqualityComparable(int value) : value_{value} {};
+
+  friend constexpr bool operator==(const EqualityComparable&, const EqualityComparable&) noexcept = default;
+
+  int value_;
+};
+
+static_assert(std::equality_comparable<EqualityComparable>);
+
+static_assert(std::equality_comparable<std::pair<EqualityComparable, EqualityComparable>>);
+
+struct NonComparable {};
+
+static_assert(!std::equality_comparable<NonComparable>);
+
+static_assert(!std::equality_comparable<std::pair<EqualityComparable, NonComparable>>);
+static_assert(!std::equality_comparable<std::pair<NonComparable, EqualityComparable>>);
+
+#endif // TEST_STD_VER >= 26
+
 int main(int, char**)
 {
     {

Comment on lines +453 to +456
requires requires {
{ __x.first == __y.first } -> __boolean_testable;
{ __x.second == __y.second } -> __boolean_testable;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we have the same situation as in #135759? (that __boolean_testable might be too strict)

Copy link
Contributor

Choose a reason for hiding this comment

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

The standard requirements are inconsistent among these new constraints. boolean-testable is used for pair and tuple's operator==s (perhaps due to boolean-testable in old Preconditions added by P2167R3), while plain implicit convertibility is used elsewhere.

I guess the difference was because of that && (and possibly ||?) is expected to be used in pair and tuple's operator==s, but not in other mentioned operators.

Copy link
Contributor

Choose a reason for hiding this comment

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

As @frederick-vs-ja pointed out these are described as __boolean_testable in the standard so I prefer to keep them as such for consistency with the Standard but I'll update the reference_wrapper with the proposed concept in #135759 later.

@Zingam
Copy link
Contributor

Zingam commented Apr 29, 2025

@philnik777 Do you have any further comments?

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.

LGTM % nit.

@Zingam
Copy link
Contributor

Zingam commented Apr 29, 2025

Thank you!

@Zingam Zingam merged commit 4ed8f38 into llvm:main Apr 29, 2025
81 checks passed
@H-G-Hristov H-G-Hristov deleted the hgh/libcxx/P2944R3-constrained-equality-std_pair branch April 30, 2025 09:35
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
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.

P2944R3: Constrained equality - std::pair

5 participants