Skip to content

Commit 3addd27

Browse files
huixie90huixie90
committed
[libc++] fix flat_set's transparent insert 
1 parent aacc4e9 commit 3addd27

File tree

3 files changed

+75
-111
lines changed

3 files changed

+75
-111
lines changed

‎libcxx/include/__flat_set/flat_set.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -382,10 +382,10 @@ class flat_set {
382382
template <class _Kp>
383383
requires(__is_transparent_v<_Compare> && is_constructible_v<value_type, _Kp>)
384384
_LIBCPP_HIDE_FROM_ABI pair<iterator, bool> insert(_Kp&& __x) {
385-
return emplace(std::forward<_Kp>(__x));
385+
return __emplace(std::forward<_Kp>(__x));
386386
}
387387
_LIBCPP_HIDE_FROM_ABI iterator insert(const_iterator __hint, const value_type& __x) {
388-
return emplace_hint(__hint, __x);
388+
return __emplace_hint(__hint, __x);
389389
}
390390

391391
_LIBCPP_HIDE_FROM_ABI iterator insert(const_iterator __hint, value_type&& __x) {
@@ -395,7 +395,7 @@ class flat_set {
395395
template <class _Kp>
396396
requires(__is_transparent_v<_Compare> && is_constructible_v<value_type, _Kp>)
397397
_LIBCPP_HIDE_FROM_ABI iterator insert(const_iterator __hint, _Kp&& __x) {
398-
return emplace_hint(__hint, std::forward<_Kp>(__x));
398+
return __emplace_hint(__hint, std::forward<_Kp>(__x));
399399
}
400400

401401
template <class _InputIterator>

‎libcxx/test/std/containers/container.adaptors/flat.set/flat.set.modifiers/insert_transparent.pass.cpp‎

Lines changed: 70 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010

1111
// <flat_set>
1212

13-
// template<class K> pair<iterator, bool> insert(P&& x);
14-
// template<class K> iterator insert(const_iterator hint, P&& x);
13+
// template<class K> pair<iterator, bool> insert(K&& x);
14+
// template<class K> iterator insert(const_iterator hint, K&& x);
1515

1616
#include <algorithm>
1717
#include <compare>
@@ -27,113 +27,89 @@
2727
#include "test_iterators.h"
2828
#include "min_allocator.h"
2929

30-
// Constraints: is_constructible_v<value_type, K> is true.
30+
// Constraints: The qualified-id Compare::is_transparent is valid and denotes a type. is_constructible_v<value_type, K> is true.
3131
template <class M, class... Args>
3232
concept CanInsert = requires(M m, Args&&... args) { m.insert(std::forward<Args>(args)...); };
3333

34-
using Set = std::flat_set<int>;
35-
using Iter = Set::const_iterator;
34+
using TransparentSet = std::flat_set<int, TransparentComparator>;
35+
using TransparentSetIter = typename TransparentSet::iterator;
36+
static_assert(CanInsert<TransparentSet, ExplicitlyConvertibleTransparent<int>>);
37+
static_assert(CanInsert<TransparentSet, TransparentSetIter, ExplicitlyConvertibleTransparent<int>>);
38+
static_assert(!CanInsert<TransparentSet, NonConvertibleTransparent<int>>);
39+
static_assert(!CanInsert<TransparentSet, TransparentSetIter, NonConvertibleTransparent<int>>);
3640

37-
static_assert(CanInsert<Set, short&&>);
38-
static_assert(CanInsert<Set, Iter, short&&>);
39-
static_assert(!CanInsert<Set, std::string>);
40-
static_assert(!CanInsert<Set, Iter, std::string>);
41-
42-
static int expensive_comparisons = 0;
43-
static int cheap_comparisons = 0;
44-
45-
struct CompareCounter {
46-
int i_ = 0;
47-
CompareCounter(int i) : i_(i) {}
48-
friend auto operator<=>(const CompareCounter& x, const CompareCounter& y) {
49-
expensive_comparisons += 1;
50-
return x.i_ <=> y.i_;
51-
}
52-
bool operator==(const CompareCounter&) const = default;
53-
friend auto operator<=>(const CompareCounter& x, int y) {
54-
cheap_comparisons += 1;
55-
return x.i_ <=> y;
56-
}
57-
};
41+
using NonTransparentSet = std::flat_set<int>;
42+
using NonTransparentSetIter = typename NonTransparentSet::iterator;
43+
static_assert(!CanInsert<NonTransparentSet, ExplicitlyConvertibleTransparent<int>>);
44+
static_assert(!CanInsert<NonTransparentSet, NonTransparentSetIter, ExplicitlyConvertibleTransparent<int>>);
45+
static_assert(!CanInsert<NonTransparentSet, NonConvertibleTransparent<int>>);
46+
static_assert(!CanInsert<NonTransparentSet, NonTransparentSetIter, NonConvertibleTransparent<int>>);
5847

5948
template <class KeyContainer>
6049
void test_one() {
6150
using Key = typename KeyContainer::value_type;
62-
using M = std::flat_set<Key, std::less<Key>, KeyContainer>;
51+
using M = std::flat_set<Key, TransparentComparator, KeyContainer>;
6352

64-
const int expected[] = {1, 2, 3, 4, 5};
6553
{
66-
// insert(P&&)
67-
// Unlike flat_set, here we can't use key_compare to compare value_type versus P,
68-
// so we must eagerly convert to value_type.
69-
M m = {1, 2, 4, 5};
70-
expensive_comparisons = 0;
71-
cheap_comparisons = 0;
72-
std::same_as<std::pair<typename M::iterator, bool>> auto p = m.insert(3); // conversion happens first
73-
assert(expensive_comparisons >= 2);
74-
assert(cheap_comparisons == 0);
75-
assert(p == std::make_pair(m.begin() + 2, true));
76-
assert(std::ranges::equal(m, expected));
77-
}
78-
{
79-
// insert(const_iterator, P&&)
80-
M m = {1, 2, 4, 5};
81-
expensive_comparisons = 0;
82-
cheap_comparisons = 0;
83-
std::same_as<typename M::iterator> auto it = m.insert(m.begin(), 3);
84-
assert(expensive_comparisons >= 2);
85-
assert(cheap_comparisons == 0);
86-
assert(it == m.begin() + 2);
87-
assert(std::ranges::equal(m, expected));
88-
}
89-
{
90-
// insert(value_type&&)
91-
M m = {1, 2, 4, 5};
92-
expensive_comparisons = 0;
93-
cheap_comparisons = 0;
94-
std::same_as<std::pair<typename M::iterator, bool>> auto p = m.insert(3); // conversion happens last
95-
assert(expensive_comparisons >= 2);
96-
assert(cheap_comparisons == 0);
97-
assert(p == std::make_pair(m.begin() + 2, true));
98-
assert(std::ranges::equal(m, expected));
99-
}
100-
{
101-
// insert(const_iterator, value_type&&)
102-
M m = {1, 2, 4, 5};
103-
expensive_comparisons = 0;
104-
cheap_comparisons = 0;
105-
std::same_as<typename M::iterator> auto it = m.insert(m.begin(), 3);
106-
assert(expensive_comparisons >= 2);
107-
assert(cheap_comparisons == 0);
108-
assert(it == m.begin() + 2);
109-
assert(std::ranges::equal(m, expected));
110-
}
111-
{
112-
// emplace(Args&&...)
113-
M m = {1, 2, 4, 5};
114-
expensive_comparisons = 0;
115-
cheap_comparisons = 0;
116-
std::same_as<std::pair<typename M::iterator, bool>> auto p = m.emplace(3); // conversion happens first
117-
assert(expensive_comparisons >= 2);
118-
assert(cheap_comparisons == 0);
119-
assert(p == std::make_pair(m.begin() + 2, true));
120-
assert(std::ranges::equal(m, expected));
54+
const int expected[] = {1, 2, 3, 4, 5};
55+
56+
{
57+
// insert(K&&)
58+
bool transparent_used = false;
59+
M m{{1, 2, 4, 5}, TransparentComparator{transparent_used}};
60+
assert(!transparent_used);
61+
std::same_as<std::pair<typename M::iterator, bool>> decltype(auto) r =
62+
m.insert(ExplicitlyConvertibleTransparent<Key>{3});
63+
assert(transparent_used);
64+
assert(r.first == m.begin() + 2);
65+
assert(r.second);
66+
assert(std::ranges::equal(m, expected));
67+
}
68+
{
69+
// insert(const_iterator, K&&)
70+
bool transparent_used = false;
71+
M m{{1, 2, 4, 5}, TransparentComparator{transparent_used}};
72+
assert(!transparent_used);
73+
std::same_as<typename M::iterator> auto it = m.insert(m.begin(), ExplicitlyConvertibleTransparent<Key>{3});
74+
assert(transparent_used);
75+
assert(it == m.begin() + 2);
76+
assert(std::ranges::equal(m, expected));
77+
}
12178
}
79+
12280
{
12381
// was empty
124-
M m;
125-
std::same_as<std::pair<typename M::iterator, bool>> auto p = m.insert(3);
126-
assert(p == std::make_pair(m.begin(), true));
127-
M expected2 = {3};
128-
assert(std::ranges::equal(m, expected2));
82+
const int expected[] = {3};
83+
{
84+
// insert(K&&)
85+
bool transparent_used = false;
86+
M m{{}, TransparentComparator{transparent_used}};
87+
assert(!transparent_used);
88+
std::same_as<std::pair<typename M::iterator, bool>> decltype(auto) r =
89+
m.insert(ExplicitlyConvertibleTransparent<Key>{3});
90+
assert(!transparent_used); // no elements to compare against
91+
assert(r.first == m.begin());
92+
assert(r.second);
93+
assert(std::ranges::equal(m, expected));
94+
}
95+
{
96+
// insert(const_iterator, K&&)
97+
bool transparent_used = false;
98+
M m{{}, TransparentComparator{transparent_used}};
99+
assert(!transparent_used);
100+
std::same_as<typename M::iterator> auto it = m.insert(m.begin(), ExplicitlyConvertibleTransparent<Key>{3});
101+
assert(!transparent_used); // no elements to compare against
102+
assert(it == m.begin());
103+
assert(std::ranges::equal(m, expected));
104+
}
129105
}
130106
}
131107

132108
void test() {
133-
test_one<std::vector<CompareCounter>>();
134-
test_one<std::deque<CompareCounter>>();
135-
test_one<MinSequenceContainer<CompareCounter>>();
136-
test_one<std::vector<CompareCounter, min_allocator<CompareCounter>>>();
109+
test_one<std::vector<int>>();
110+
test_one<std::deque<int>>();
111+
test_one<MinSequenceContainer<int>>();
112+
test_one<std::vector<int, min_allocator<int>>>();
137113

138114
{
139115
// no ambiguity between insert(pos, P&&) and insert(first, last)
@@ -153,26 +129,14 @@ void test_exception() {
153129
{
154130
auto insert_func = [](auto& m, auto key_arg) {
155131
using FlatSet = std::decay_t<decltype(m)>;
156-
struct T {
157-
typename FlatSet::key_type key_;
158-
T(typename FlatSet::key_type key) : key_(key) {}
159-
operator typename FlatSet::value_type() const { return key_; }
160-
};
161-
T t(key_arg);
162-
m.insert(t);
132+
m.insert(ExplicitlyConvertibleTransparent<typename FlatSet::key_type>{key_arg});
163133
};
164134
test_emplace_exception_guarantee(insert_func);
165135
}
166136
{
167137
auto insert_func_iter = [](auto& m, auto key_arg) {
168138
using FlatSet = std::decay_t<decltype(m)>;
169-
struct T {
170-
typename FlatSet::key_type key_;
171-
T(typename FlatSet::key_type key) : key_(key) {}
172-
operator typename FlatSet::value_type() const { return key_; }
173-
};
174-
T t(key_arg);
175-
m.insert(m.begin(), t);
139+
m.insert(m.begin(), ExplicitlyConvertibleTransparent<typename FlatSet::key_type>{key_arg});
176140
};
177141
test_emplace_exception_guarantee(insert_func_iter);
178142
}

‎libcxx/test/std/containers/container.adaptors/flat.set/helpers.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,15 @@ template <class T, bool ConvertibleToT = false>
6464
struct Transparent {
6565
T t;
6666

67-
operator T() const
67+
explicit operator T() const
6868
requires ConvertibleToT
6969
{
7070
return t;
7171
}
7272
};
7373

7474
template <class T>
75-
using ConvertibleTransparent = Transparent<T, true>;
75+
using ExplicitlyConvertibleTransparent = Transparent<T, true>;
7676

7777
template <class T>
7878
using NonConvertibleTransparent = Transparent<T, false>;

0 commit comments

Comments
 (0)