release/20.x: [libc++] Fixes (|multi)_set spaceship operator. (#127326) #127342
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@frederick-vs-ja What do you think about merging this PR to the release branch? |
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: None (llvmbot) ChangesBackport 248716f Requested by: @mordante Patch is 24.88 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/127342.diff 2 Files Affected:
diff --git a/libcxx/include/set b/libcxx/include/set
index 2784e82760d7e..3c6ea360bd06c 100644
--- a/libcxx/include/set
+++ b/libcxx/include/set
@@ -1003,9 +1003,9 @@ operator<=(const set<_Key, _Compare, _Allocator>& __x, const set<_Key, _Compare,
# else // _LIBCPP_STD_VER <= 17
-template <class _Key, class _Allocator>
+template <class _Key, class _Compare, class _Allocator>
_LIBCPP_HIDE_FROM_ABI __synth_three_way_result<_Key>
-operator<=>(const set<_Key, _Allocator>& __x, const set<_Key, _Allocator>& __y) {
+operator<=>(const set<_Key, _Compare, _Allocator>& __x, const set<_Key, _Compare, _Allocator>& __y) {
return std::lexicographical_compare_three_way(__x.begin(), __x.end(), __y.begin(), __y.end(), std::__synth_three_way);
}
@@ -1470,9 +1470,9 @@ operator<=(const multiset<_Key, _Compare, _Allocator>& __x, const multiset<_Key,
# else // _LIBCPP_STD_VER <= 17
-template <class _Key, class _Allocator>
+template <class _Key, class _Compare, class _Allocator>
_LIBCPP_HIDE_FROM_ABI __synth_three_way_result<_Key>
-operator<=>(const multiset<_Key, _Allocator>& __x, const multiset<_Key, _Allocator>& __y) {
+operator<=>(const multiset<_Key, _Compare, _Allocator>& __x, const multiset<_Key, _Compare, _Allocator>& __y) {
return std::lexicographical_compare_three_way(__x.begin(), __x.end(), __y.begin(), __y.end(), __synth_three_way);
}
diff --git a/libcxx/test/support/test_container_comparisons.h b/libcxx/test/support/test_container_comparisons.h
index 543c5899922d0..f7bf78e48a1f8 100644
--- a/libcxx/test/support/test_container_comparisons.h
+++ b/libcxx/test/support/test_container_comparisons.h
@@ -13,51 +13,52 @@
#include <functional>
#include <set>
+#include "test_allocator.h"
#include "test_comparisons.h"
// Implementation detail of `test_sequence_container_spaceship`
-template <template <typename...> typename Container, typename Elem, typename Order>
+template <template <typename...> typename Container, typename Elem, typename Allocator, typename Order>
constexpr void test_sequence_container_spaceship_with_type() {
// Empty containers
{
- Container<Elem> l1;
- Container<Elem> l2;
+ Container<Elem, Allocator> l1;
+ Container<Elem, Allocator> l2;
assert(testOrder(l1, l2, Order::equivalent));
}
// Identical contents
{
- Container<Elem> l1{1, 1};
- Container<Elem> l2{1, 1};
+ Container<Elem, Allocator> l1{1, 1};
+ Container<Elem, Allocator> l2{1, 1};
assert(testOrder(l1, l2, Order::equivalent));
}
// Less, due to contained values
{
- Container<Elem> l1{1, 1};
- Container<Elem> l2{1, 2};
+ Container<Elem, Allocator> l1{1, 1};
+ Container<Elem, Allocator> l2{1, 2};
assert(testOrder(l1, l2, Order::less));
}
// Greater, due to contained values
{
- Container<Elem> l1{1, 3};
- Container<Elem> l2{1, 2};
+ Container<Elem, Allocator> l1{1, 3};
+ Container<Elem, Allocator> l2{1, 2};
assert(testOrder(l1, l2, Order::greater));
}
// Shorter list
{
- Container<Elem> l1{1};
- Container<Elem> l2{1, 2};
+ Container<Elem, Allocator> l1{1};
+ Container<Elem, Allocator> l2{1, 2};
assert(testOrder(l1, l2, Order::less));
}
// Longer list
{
- Container<Elem> l1{1, 2};
- Container<Elem> l2{1};
+ Container<Elem, Allocator> l1{1, 2};
+ Container<Elem, Allocator> l2{1};
assert(testOrder(l1, l2, Order::greater));
}
// Unordered
if constexpr (std::is_same_v<Elem, PartialOrder>) {
- Container<Elem> l1{1, std::numeric_limits<int>::min()};
- Container<Elem> l2{1, 2};
+ Container<Elem, Allocator> l1{1, std::numeric_limits<int>::min()};
+ Container<Elem, Allocator> l2{1, 2};
assert(testOrder(l1, l2, Order::unordered));
}
}
@@ -69,13 +70,22 @@ constexpr bool test_sequence_container_spaceship() {
static_assert(std::three_way_comparable<Container<int>>);
// Test different comparison categories
- test_sequence_container_spaceship_with_type<Container, int, std::strong_ordering>();
- test_sequence_container_spaceship_with_type<Container, StrongOrder, std::strong_ordering>();
- test_sequence_container_spaceship_with_type<Container, WeakOrder, std::weak_ordering>();
- test_sequence_container_spaceship_with_type<Container, PartialOrder, std::partial_ordering>();
+ test_sequence_container_spaceship_with_type<Container, int, std::allocator<int>, std::strong_ordering>();
+ test_sequence_container_spaceship_with_type<Container,
+ StrongOrder,
+ test_allocator<StrongOrder>,
+ std::strong_ordering>();
+ test_sequence_container_spaceship_with_type<Container, WeakOrder, std::allocator<WeakOrder>, std::weak_ordering>();
+ test_sequence_container_spaceship_with_type<Container,
+ PartialOrder,
+ test_allocator<PartialOrder>,
+ std::partial_ordering>();
// `LessAndEqComp` does not have `operator<=>`. Ordering is synthesized based on `operator<`
- test_sequence_container_spaceship_with_type<Container, LessAndEqComp, std::weak_ordering>();
+ test_sequence_container_spaceship_with_type<Container,
+ LessAndEqComp,
+ std::allocator<LessAndEqComp>,
+ std::weak_ordering>();
// Thanks to SFINAE, the following is not a compiler error but returns `false`
struct NonComparable {};
@@ -175,109 +185,114 @@ constexpr bool test_sequence_container_adaptor_spaceship() {
}
// Implementation detail of `test_ordered_map_container_spaceship`
-template <template <typename...> typename Container, typename Key, typename Val, typename Order, typename Compare>
+template <template <typename...> typename Container,
+ typename Key,
+ typename Val,
+ typename Allocator,
+ typename Order,
+ typename Compare>
constexpr void test_ordered_map_container_spaceship_with_type(Compare comp) {
// Empty containers
{
- Container<Key, Val, Compare> l1{{}, comp};
- Container<Key, Val, Compare> l2{{}, comp};
+ Container<Key, Val, Compare, Allocator> l1{{}, comp};
+ Container<Key, Val, Compare, Allocator> l2{{}, comp};
assert(testOrder(l1, l2, Order::equivalent));
}
// Identical contents
{
- Container<Key, Val, Compare> l1{{{1, 1}, {2, 1}}, comp};
- Container<Key, Val, Compare> l2{{{1, 1}, {2, 1}}, comp};
+ Container<Key, Val, Compare, Allocator> l1{{{1, 1}, {2, 1}}, comp};
+ Container<Key, Val, Compare, Allocator> l2{{{1, 1}, {2, 1}}, comp};
assert(testOrder(l1, l2, Order::equivalent));
}
// Less, due to contained values
{
- Container<Key, Val, Compare> l1{{{1, 1}, {2, 1}}, comp};
- Container<Key, Val, Compare> l2{{{1, 1}, {2, 2}}, comp};
+ Container<Key, Val, Compare, Allocator> l1{{{1, 1}, {2, 1}}, comp};
+ Container<Key, Val, Compare, Allocator> l2{{{1, 1}, {2, 2}}, comp};
assert(testOrder(l1, l2, Order::less));
}
// Greater, due to contained values
{
- Container<Key, Val, Compare> l1{{{1, 1}, {2, 3}}, comp};
- Container<Key, Val, Compare> l2{{{1, 1}, {2, 2}}, comp};
+ Container<Key, Val, Compare, Allocator> l1{{{1, 1}, {2, 3}}, comp};
+ Container<Key, Val, Compare, Allocator> l2{{{1, 1}, {2, 2}}, comp};
assert(testOrder(l1, l2, Order::greater));
}
// Shorter list
{
- Container<Key, Val, Compare> l1{{{1, 1}}, comp};
- Container<Key, Val, Compare> l2{{{1, 1}, {2, 2}}, comp};
+ Container<Key, Val, Compare, Allocator> l1{{{1, 1}}, comp};
+ Container<Key, Val, Compare, Allocator> l2{{{1, 1}, {2, 2}}, comp};
assert(testOrder(l1, l2, Order::less));
}
// Longer list
{
- Container<Key, Val, Compare> l1{{{1, 2}, {2, 2}}, comp};
- Container<Key, Val, Compare> l2{{{1, 1}}, comp};
+ Container<Key, Val, Compare, Allocator> l1{{{1, 2}, {2, 2}}, comp};
+ Container<Key, Val, Compare, Allocator> l2{{{1, 1}}, comp};
assert(testOrder(l1, l2, Order::greater));
}
// Unordered
if constexpr (std::is_same_v<Val, PartialOrder>) {
- Container<Key, Val, Compare> l1{{{1, 1}, {2, std::numeric_limits<int>::min()}}, comp};
- Container<Key, Val, Compare> l2{{{1, 1}, {2, 2}}, comp};
+ Container<Key, Val, Compare, Allocator> l1{{{1, 1}, {2, std::numeric_limits<int>::min()}}, comp};
+ Container<Key, Val, Compare, Allocator> l2{{{1, 1}, {2, 2}}, comp};
assert(testOrder(l1, l2, Order::unordered));
}
// Identical contents
{
- Container<Key, Val, Compare> l1{{{1, 1}, {2, 1}, {2, 2}}, comp};
- Container<Key, Val, Compare> l2{{{1, 1}, {2, 1}, {2, 2}}, comp};
+ Container<Key, Val, Compare, Allocator> l1{{{1, 1}, {2, 1}, {2, 2}}, comp};
+ Container<Key, Val, Compare, Allocator> l2{{{1, 1}, {2, 1}, {2, 2}}, comp};
assert(testOrder(l1, l2, Order::equivalent));
- Container<Key, Val, Compare> l3{{{1, 1}, {2, 1}, {2, 2}}, comp};
- Container<Key, Val, Compare> l4{{{2, 1}, {2, 2}, {1, 1}}, comp};
+ Container<Key, Val, Compare, Allocator> l3{{{1, 1}, {2, 1}, {2, 2}}, comp};
+ Container<Key, Val, Compare, Allocator> l4{{{2, 1}, {2, 2}, {1, 1}}, comp};
assert(testOrder(l3, l4, Order::equivalent));
}
// Less, due to contained values
{
- Container<Key, Val, Compare> l1{{{1, 1}, {2, 1}, {2, 1}}, comp};
- Container<Key, Val, Compare> l2{{{1, 1}, {2, 2}, {2, 2}}, comp};
+ Container<Key, Val, Compare, Allocator> l1{{{1, 1}, {2, 1}, {2, 1}}, comp};
+ Container<Key, Val, Compare, Allocator> l2{{{1, 1}, {2, 2}, {2, 2}}, comp};
assert(testOrder(l1, l2, Order::less));
- Container<Key, Val, Compare> l3{{{1, 1}, {2, 1}, {2, 1}}, comp};
- Container<Key, Val, Compare> l4{{{2, 2}, {2, 2}, {1, 1}}, comp};
+ Container<Key, Val, Compare, Allocator> l3{{{1, 1}, {2, 1}, {2, 1}}, comp};
+ Container<Key, Val, Compare, Allocator> l4{{{2, 2}, {2, 2}, {1, 1}}, comp};
assert(testOrder(l3, l4, Order::less));
}
// Greater, due to contained values
{
- Container<Key, Val, Compare> l1{{{1, 1}, {2, 3}, {2, 3}}, comp};
- Container<Key, Val, Compare> l2{{{1, 1}, {2, 2}, {2, 2}}, comp};
+ Container<Key, Val, Compare, Allocator> l1{{{1, 1}, {2, 3}, {2, 3}}, comp};
+ Container<Key, Val, Compare, Allocator> l2{{{1, 1}, {2, 2}, {2, 2}}, comp};
assert(testOrder(l1, l2, Order::greater));
- Container<Key, Val, Compare> l3{{{1, 1}, {2, 3}, {2, 3}}, comp};
- Container<Key, Val, Compare> l4{{{2, 2}, {2, 2}, {1, 1}}, comp};
+ Container<Key, Val, Compare, Allocator> l3{{{1, 1}, {2, 3}, {2, 3}}, comp};
+ Container<Key, Val, Compare, Allocator> l4{{{2, 2}, {2, 2}, {1, 1}}, comp};
assert(testOrder(l3, l4, Order::greater));
}
// Shorter list
{
- Container<Key, Val, Compare> l1{{{1, 1}, {2, 2}}, comp};
- Container<Key, Val, Compare> l2{{{1, 1}, {2, 2}, {2, 2}, {3, 1}}, comp};
+ Container<Key, Val, Compare, Allocator> l1{{{1, 1}, {2, 2}}, comp};
+ Container<Key, Val, Compare, Allocator> l2{{{1, 1}, {2, 2}, {2, 2}, {3, 1}}, comp};
assert(testOrder(l1, l2, Order::less));
- Container<Key, Val, Compare> l3{{{1, 1}, {2, 2}}, comp};
- Container<Key, Val, Compare> l4{{{3, 1}, {2, 2}, {2, 2}, {1, 1}}, comp};
+ Container<Key, Val, Compare, Allocator> l3{{{1, 1}, {2, 2}}, comp};
+ Container<Key, Val, Compare, Allocator> l4{{{3, 1}, {2, 2}, {2, 2}, {1, 1}}, comp};
assert(testOrder(l3, l4, Order::less));
}
// Longer list
{
- Container<Key, Val, Compare> l1{{{1, 2}, {2, 2}, {2, 2}, {3, 1}}, comp};
- Container<Key, Val, Compare> l2{{{1, 1}, {2, 2}}, comp};
+ Container<Key, Val, Compare, Allocator> l1{{{1, 2}, {2, 2}, {2, 2}, {3, 1}}, comp};
+ Container<Key, Val, Compare, Allocator> l2{{{1, 1}, {2, 2}}, comp};
assert(testOrder(l1, l2, Order::greater));
- Container<Key, Val, Compare> l3{{{1, 2}, {2, 2}, {2, 2}, {3, 1}}, comp};
- Container<Key, Val, Compare> l4{{{2, 2}, {1, 1}}, comp};
+ Container<Key, Val, Compare, Allocator> l3{{{1, 2}, {2, 2}, {2, 2}, {3, 1}}, comp};
+ Container<Key, Val, Compare, Allocator> l4{{{2, 2}, {1, 1}}, comp};
assert(testOrder(l3, l4, Order::greater));
}
// Unordered
if constexpr (std::is_same_v<Val, PartialOrder>) {
- Container<Key, Val, Compare> l1{{{1, 1}, {2, std::numeric_limits<int>::min()}, {2, 3}}, comp};
- Container<Key, Val, Compare> l2{{{1, 1}, {2, 2}, {2, 3}}, comp};
+ Container<Key, Val, Compare, Allocator> l1{{{1, 1}, {2, std::numeric_limits<int>::min()}, {2, 3}}, comp};
+ Container<Key, Val, Compare, Allocator> l2{{{1, 1}, {2, 2}, {2, 3}}, comp};
assert(testOrder(l1, l2, Order::unordered));
- Container<Key, Val, Compare> l3{{{1, 1}, {2, std::numeric_limits<int>::min()}, {2, 3}}, comp};
- Container<Key, Val, Compare> l4{{{2, 3}, {2, 2}, {1, 1}}, comp};
+ Container<Key, Val, Compare, Allocator> l3{{{1, 1}, {2, std::numeric_limits<int>::min()}, {2, 3}}, comp};
+ Container<Key, Val, Compare, Allocator> l4{{{2, 3}, {2, 2}, {1, 1}}, comp};
assert(testOrder(l3, l4, Order::unordered));
}
}
@@ -293,94 +308,134 @@ constexpr bool test_ordered_map_container_spaceship() {
static_assert(std::three_way_comparable<Container<int, int>>);
// Test different comparison categories
- test_ordered_map_container_spaceship_with_type<Container, int, int, std::strong_ordering>(std::less{});
- test_ordered_map_container_spaceship_with_type<Container, int, int, std::strong_ordering>(std::greater{});
- test_ordered_map_container_spaceship_with_type<Container, int, StrongOrder, std::strong_ordering>(std::less{});
- test_ordered_map_container_spaceship_with_type<Container, int, StrongOrder, std::strong_ordering>(std::greater{});
- test_ordered_map_container_spaceship_with_type<Container, int, WeakOrder, std::weak_ordering>(std::less{});
- test_ordered_map_container_spaceship_with_type<Container, int, WeakOrder, std::weak_ordering>(std::greater{});
- test_ordered_map_container_spaceship_with_type<Container, int, PartialOrder, std::partial_ordering>(std ::less{});
- test_ordered_map_container_spaceship_with_type<Container, int, PartialOrder, std::partial_ordering>(std ::greater{});
+ test_ordered_map_container_spaceship_with_type<Container,
+ int,
+ int,
+ std::allocator<std::pair<const int, int>>,
+ std::strong_ordering>(std::less{});
+ test_ordered_map_container_spaceship_with_type<Container,
+ int,
+ int,
+ test_allocator<std::pair<const int, int>>,
+ std::strong_ordering>(std::greater{});
+ test_ordered_map_container_spaceship_with_type<Container,
+ int,
+ StrongOrder,
+ std::allocator<std::pair<const int, StrongOrder>>,
+ std::strong_ordering>(std::less{});
+ test_ordered_map_container_spaceship_with_type<Container,
+ int,
+ StrongOrder,
+ test_allocator<std::pair<const int, StrongOrder>>,
+ std::strong_ordering>(std::greater{});
+ test_ordered_map_container_spaceship_with_type<Container,
+ int,
+ WeakOrder,
+ std::allocator<std::pair<const int, WeakOrder>>,
+ std::weak_ordering>(std::less{});
+ test_ordered_map_container_spaceship_with_type<Container,
+ int,
+ WeakOrder,
+ test_allocator<std::pair<const int, WeakOrder>>,
+ std::weak_ordering>(std::greater{});
+ test_ordered_map_container_spaceship_with_type<Container,
+ int,
+ PartialOrder,
+ std::allocator<std::pair<const int, PartialOrder>>,
+ std::partial_ordering>(std ::less{});
+ test_ordered_map_container_spaceship_with_type<Container,
+ int,
+ PartialOrder,
+ test_allocator<std::pair<const int, PartialOrder>>,
+ std::partial_ordering>(std ::greater{});
// `LessAndEqComp` does not have `operator<=>`. Ordering is synthesized based on `operator<`
- test_ordered_map_container_spaceship_with_type<Container, int, LessAndEqComp, std::weak_ordering>(std::less{});
+ test_ordered_map_container_spaceship_with_type<Container,
+ int,
+ LessAndEqComp,
+ std::allocator<std::pair<const int, LessAndEqComp>>,
+ std::weak_ordering>(std::less{});
return true;
}
// Implementation detail of `test_ordered_set_container_spaceship`
-template <template <typename...> typename Container, typename Elem, typename Order, typename Compare>
+template <template <typename...> typename Container,
+ typename Elem,
+ typename Allocator,
+ typename Order,
+ typename Compare>
constexpr void test_ordered_set_spaceship_with_type(Compare comp) {
// Empty containers
{
- Container<Elem, Compare> l1{{}, comp};
- Container<Elem, Compare> l2{{}, comp};
+ Container<Elem, Compare, Allocator> l1{{}, comp};
+ Container<Elem, Compare, Allocator> l2{{}, comp};
assert(testOrder(l1, l2, Order::equivalent));
}
// Identical contents
{
- Container<Elem, Compare> l1{{1, 1, 2}, comp};
- Container<Elem, Compare> l2{{1, 1, 2}, comp};
+ Container<Elem, Compare, Allocator> l1{{1, 1, 2}, comp};
+ Container<Elem, Compare, Allocator> l2{{1, 1, 2}, comp};
assert(testOrder(l1, l2, Order::equivalent));
}
// Less, due to contained values
{
- Container<Elem, Compare> l1{{1, 1, 2, 3}, comp};
- Container<Elem, Compare> l2{{1, 2, 2, 4}, comp};
+ Container<Elem, Compare, Allocator> l1{{1, 1, 2, 3}, comp};
+ Container<Elem, Compare, Allocator> l2{{1, 2, 2, 4}, comp};
assert(testOrder(l1, l2, Order::less));
}
// Greater, due to contained values
{
- Container<Elem, Compare> l1{{1, 2, 2, 4}, comp};
- Container<Elem, Compare> l2{{1, 1, 2, 3}, comp};
+ Container<Elem, Compare, Allocator> l1{{1, 2, 2, 4}, comp};
+ Container<Elem, Compare, Allocator> l2{{1, 1, 2, 3}, comp};
assert(testOrder(l1, l2, Order::greater));
}
// Shorter list
{
- Container<Elem, Compare> l1{{1, 1, 2, 2}, comp};
- Container<Elem, Compare> l2{{1, 1, 2, 2, 3}, comp};
+ Container<Elem, Compare, Allocator> l1{{1, 1, 2, 2}, comp};
+ Container<Elem, Compare, Allocator> l2{{1, 1, 2, 2, 3}, comp};
assert(testOrder(l1, l2, Order::less));
}
// Longer list
{
- Container<Elem, Compare> l1{{1, 1, 2, 2, 3}, comp};
- Container<Elem, Compare> l2{{1, 1, 2, 2}, comp};
+ Container<Elem, Compare, Allocator> l1{{1, 1, 2, 2, 3}, comp};
+ Container<Elem, Compare, Allocator> l2{{1, 1, 2, 2}, comp};
assert(testOrder(l1, l2, Order::greater));
}
// Unordered
if constexpr (std::is_same_v< Container<Elem>, std::multiset<PartialOrder>>) {
if constexpr (std::is_same_v<Elem, PartialOrder> && std::is_same_v<Compare, decltype(std::less{})>) {
- Container<Elem, Compare> l1{{1, std::numeric_limits<int>::min()},...
[truncated]
|
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
frederick-vs-ja
approved these changes
Feb 16, 2025
ldionne
approved these changes
Feb 17, 2025
The operators did not have a _Compare template arguement. The fix updates the generic container test to use allocators for all types used. No other issues were found. Fixes: llvm#127095 (cherry picked from commit 248716f)
|
@mordante (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport 248716f
Requested by: @mordante