Skip to content

[libc++][pair] P2944R3: Constrain std::pair's equality operator #136672

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

H-G-Hristov
Copy link
Contributor

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

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.

Co-authored-by: Nikolas Klauser <nikolasklauser@berlin.de>
@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.

Co-authored-by: Nikolas Klauser <nikolasklauser@berlin.de>
@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
…vm#136672)

Implements https://wg21.link/P2944R3 (partially):
- [pairs.spec](https://eel.is/c++draft/pairs.spec)

Related issues:
- Related to llvm#105424
- Related to llvm#118135
  - PR llvm#135759
  - PR llvm#117664

Closes: [llvm#136763](llvm#136763)

# References
- https://eel.is/c++draft/concept.booleantestable
- https://eel.is/c++draft/concept.equalitycomparable

---------

Co-authored-by: Hristo Hristov <zingam@outlook.com>
Co-authored-by: Nikolas Klauser <nikolasklauser@berlin.de>
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…vm#136672)

Implements https://wg21.link/P2944R3 (partially):
- [pairs.spec](https://eel.is/c++draft/pairs.spec)

Related issues:
- Related to llvm#105424
- Related to llvm#118135
  - PR llvm#135759
  - PR llvm#117664

Closes: [llvm#136763](llvm#136763)

# References
- https://eel.is/c++draft/concept.booleantestable
- https://eel.is/c++draft/concept.equalitycomparable

---------

Co-authored-by: Hristo Hristov <zingam@outlook.com>
Co-authored-by: Nikolas Klauser <nikolasklauser@berlin.de>
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…vm#136672)

Implements https://wg21.link/P2944R3 (partially):
- [pairs.spec](https://eel.is/c++draft/pairs.spec)

Related issues:
- Related to llvm#105424
- Related to llvm#118135
  - PR llvm#135759
  - PR llvm#117664

Closes: [llvm#136763](llvm#136763)

# References
- https://eel.is/c++draft/concept.booleantestable
- https://eel.is/c++draft/concept.equalitycomparable

---------

Co-authored-by: Hristo Hristov <zingam@outlook.com>
Co-authored-by: Nikolas Klauser <nikolasklauser@berlin.de>
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
…vm#136672)

Implements https://wg21.link/P2944R3 (partially):
- [pairs.spec](https://eel.is/c++draft/pairs.spec)

Related issues:
- Related to llvm#105424
- Related to llvm#118135
  - PR llvm#135759
  - PR llvm#117664

Closes: [llvm#136763](llvm#136763)

# References
- https://eel.is/c++draft/concept.booleantestable
- https://eel.is/c++draft/concept.equalitycomparable

---------

Co-authored-by: Hristo Hristov <zingam@outlook.com>
Co-authored-by: Nikolas Klauser <nikolasklauser@berlin.de>
Ankur-0429 pushed a commit to Ankur-0429/llvm-project that referenced this pull request May 9, 2025
…vm#136672)

Implements https://wg21.link/P2944R3 (partially):
- [pairs.spec](https://eel.is/c++draft/pairs.spec)

Related issues:
- Related to llvm#105424
- Related to llvm#118135
  - PR llvm#135759
  - PR llvm#117664

Closes: [llvm#136763](llvm#136763)

# References
- https://eel.is/c++draft/concept.booleantestable
- https://eel.is/c++draft/concept.equalitycomparable

---------

Co-authored-by: Hristo Hristov <zingam@outlook.com>
Co-authored-by: Nikolas Klauser <nikolasklauser@berlin.de>
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