Skip to content

Commit d4dd961

Browse files
mordantemordante
committed
Fixes complexity of map insert_or_assign with a hint.
Mitsuru Kariya reported the map operations insert_or_assign with a hint violates the complexity requirement. The function no longer uses a lower_bound, which caused the wrong complexity. Fixes PR38722: [C++17] std::map::insert_or_assign w/ hint violate complexity requirements Differential Revision: https://reviews.llvm.org/D62779
1 parent 2819cea commit d4dd961

File tree

3 files changed

+138
-36
lines changed

3 files changed

+138
-36
lines changed

‎libcxx/include/__tree

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1151,7 +1151,7 @@ public:
11511151
pair<iterator, bool>
11521152
__emplace_unique_key_args(_Key const&, _Args&&... __args);
11531153
template <class _Key, class ..._Args>
1154-
iterator
1154+
pair<iterator, bool>
11551155
__emplace_hint_unique_key_args(const_iterator, _Key const&, _Args&&...);
11561156

11571157
template <class... _Args>
@@ -1225,7 +1225,7 @@ public:
12251225
>::type __emplace_hint_unique(const_iterator __p, _First&& __f, _Second&& __s) {
12261226
return __emplace_hint_unique_key_args(__p, __f,
12271227
_VSTD::forward<_First>(__f),
1228-
_VSTD::forward<_Second>(__s));
1228+
_VSTD::forward<_Second>(__s)).first;
12291229
}
12301230

12311231
template <class... _Args>
@@ -1245,14 +1245,14 @@ public:
12451245
_LIBCPP_INLINE_VISIBILITY
12461246
iterator
12471247
__emplace_hint_unique_extract_key(const_iterator __p, _Pp&& __x, __extract_key_self_tag) {
1248-
return __emplace_hint_unique_key_args(__p, __x, _VSTD::forward<_Pp>(__x));
1248+
return __emplace_hint_unique_key_args(__p, __x, _VSTD::forward<_Pp>(__x)).first;
12491249
}
12501250

12511251
template <class _Pp>
12521252
_LIBCPP_INLINE_VISIBILITY
12531253
iterator
12541254
__emplace_hint_unique_extract_key(const_iterator __p, _Pp&& __x, __extract_key_first_tag) {
1255-
return __emplace_hint_unique_key_args(__p, __x.first, _VSTD::forward<_Pp>(__x));
1255+
return __emplace_hint_unique_key_args(__p, __x.first, _VSTD::forward<_Pp>(__x)).first;
12561256
}
12571257

12581258
#else
@@ -1261,7 +1261,7 @@ public:
12611261
pair<iterator, bool> __emplace_unique_key_args(_Key const&, _Args& __args);
12621262
template <class _Key, class _Args>
12631263
_LIBCPP_INLINE_VISIBILITY
1264-
iterator __emplace_hint_unique_key_args(const_iterator, _Key const&, _Args&);
1264+
pair<iterator, bool> __emplace_hint_unique_key_args(const_iterator, _Key const&, _Args&);
12651265
#endif
12661266

12671267
_LIBCPP_INLINE_VISIBILITY
@@ -1271,7 +1271,7 @@ public:
12711271

12721272
_LIBCPP_INLINE_VISIBILITY
12731273
iterator __insert_unique(const_iterator __p, const __container_value_type& __v) {
1274-
return __emplace_hint_unique_key_args(__p, _NodeTypes::__get_key(__v), __v);
1274+
return __emplace_hint_unique_key_args(__p, _NodeTypes::__get_key(__v), __v).first;
12751275
}
12761276

12771277
#ifdef _LIBCPP_CXX03_LANG
@@ -1287,7 +1287,7 @@ public:
12871287

12881288
_LIBCPP_INLINE_VISIBILITY
12891289
iterator __insert_unique(const_iterator __p, __container_value_type&& __v) {
1290-
return __emplace_hint_unique_key_args(__p, _NodeTypes::__get_key(__v), _VSTD::move(__v));
1290+
return __emplace_hint_unique_key_args(__p, _NodeTypes::__get_key(__v), _VSTD::move(__v)).first;
12911291
}
12921292

12931293
template <class _Vp, class = typename enable_if<
@@ -2147,17 +2147,16 @@ __tree<_Tp, _Compare, _Allocator>::__emplace_unique_key_args(_Key const& __k, _A
21472147
return pair<iterator, bool>(iterator(__r), __inserted);
21482148
}
21492149

2150-
21512150
#ifndef _LIBCPP_CXX03_LANG
21522151
template <class _Tp, class _Compare, class _Allocator>
21532152
template <class _Key, class... _Args>
2154-
typename __tree<_Tp, _Compare, _Allocator>::iterator
2153+
pair<typename __tree<_Tp, _Compare, _Allocator>::iterator, bool>
21552154
__tree<_Tp, _Compare, _Allocator>::__emplace_hint_unique_key_args(
21562155
const_iterator __p, _Key const& __k, _Args&&... __args)
21572156
#else
21582157
template <class _Tp, class _Compare, class _Allocator>
21592158
template <class _Key, class _Args>
2160-
typename __tree<_Tp, _Compare, _Allocator>::iterator
2159+
pair<typename __tree<_Tp, _Compare, _Allocator>::iterator, bool>
21612160
__tree<_Tp, _Compare, _Allocator>::__emplace_hint_unique_key_args(
21622161
const_iterator __p, _Key const& __k, _Args& __args)
21632162
#endif
@@ -2166,6 +2165,7 @@ __tree<_Tp, _Compare, _Allocator>::__emplace_hint_unique_key_args(
21662165
__node_base_pointer __dummy;
21672166
__node_base_pointer& __child = __find_equal(__p, __parent, __dummy, __k);
21682167
__node_pointer __r = static_cast<__node_pointer>(__child);
2168+
bool __inserted = false;
21692169
if (__child == nullptr)
21702170
{
21712171
#ifndef _LIBCPP_CXX03_LANG
@@ -2175,11 +2175,11 @@ __tree<_Tp, _Compare, _Allocator>::__emplace_hint_unique_key_args(
21752175
#endif
21762176
__insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__h.get()));
21772177
__r = __h.release();
2178+
__inserted = true;
21782179
}
2179-
return iterator(__r);
2180+
return pair<iterator, bool>(iterator(__r), __inserted);
21802181
}
21812182

2182-
21832183
#ifndef _LIBCPP_CXX03_LANG
21842184

21852185
template <class _Tp, class _Compare, class _Allocator>

‎libcxx/include/map

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1230,7 +1230,7 @@ public:
12301230
return __tree_.__emplace_hint_unique_key_args(__h.__i_, __k,
12311231
_VSTD::piecewise_construct,
12321232
_VSTD::forward_as_tuple(__k),
1233-
_VSTD::forward_as_tuple(_VSTD::forward<_Args>(__args)...));
1233+
_VSTD::forward_as_tuple(_VSTD::forward<_Args>(__args)...)).first;
12341234
}
12351235

12361236
template <class... _Args>
@@ -1240,7 +1240,7 @@ public:
12401240
return __tree_.__emplace_hint_unique_key_args(__h.__i_, __k,
12411241
_VSTD::piecewise_construct,
12421242
_VSTD::forward_as_tuple(_VSTD::move(__k)),
1243-
_VSTD::forward_as_tuple(_VSTD::forward<_Args>(__args)...));
1243+
_VSTD::forward_as_tuple(_VSTD::forward<_Args>(__args)...)).first;
12441244
}
12451245

12461246
template <class _Vp>
@@ -1270,30 +1270,30 @@ public:
12701270
}
12711271

12721272
template <class _Vp>
1273-
_LIBCPP_INLINE_VISIBILITY
1274-
iterator insert_or_assign(const_iterator __h, const key_type& __k, _Vp&& __v)
1275-
{
1276-
iterator __p = lower_bound(__k);
1277-
if ( __p != end() && !key_comp()(__k, __p->first))
1278-
{
1279-
__p->second = _VSTD::forward<_Vp>(__v);
1280-
return __p;
1281-
}
1282-
return emplace_hint(__h, __k, _VSTD::forward<_Vp>(__v));
1283-
}
1273+
_LIBCPP_INLINE_VISIBILITY iterator insert_or_assign(const_iterator __h,
1274+
const key_type& __k,
1275+
_Vp&& __v) {
1276+
auto [__r, __inserted] = __tree_.__emplace_hint_unique_key_args(
1277+
__h.__i_, __k, __k, _VSTD::forward<_Vp>(__v));
1278+
1279+
if (!__inserted)
1280+
__r->__get_value().second = _VSTD::forward<_Vp>(__v);
1281+
1282+
return __r;
1283+
}
12841284

12851285
template <class _Vp>
1286-
_LIBCPP_INLINE_VISIBILITY
1287-
iterator insert_or_assign(const_iterator __h, key_type&& __k, _Vp&& __v)
1288-
{
1289-
iterator __p = lower_bound(__k);
1290-
if ( __p != end() && !key_comp()(__k, __p->first))
1291-
{
1292-
__p->second = _VSTD::forward<_Vp>(__v);
1293-
return __p;
1294-
}
1295-
return emplace_hint(__h, _VSTD::move(__k), _VSTD::forward<_Vp>(__v));
1296-
}
1286+
_LIBCPP_INLINE_VISIBILITY iterator insert_or_assign(const_iterator __h,
1287+
key_type&& __k,
1288+
_Vp&& __v) {
1289+
auto [__r, __inserted] = __tree_.__emplace_hint_unique_key_args(
1290+
__h.__i_, __k, _VSTD::move(__k), _VSTD::forward<_Vp>(__v));
1291+
1292+
if (!__inserted)
1293+
__r->__get_value().second = _VSTD::forward<_Vp>(__v);
1294+
1295+
return __r;
1296+
}
12971297

12981298
#endif // _LIBCPP_STD_VER > 14
12991299

‎libcxx/test/std/containers/associative/map/map.modifiers/insert_or_assign.pass.cpp

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,51 @@ int main(int, char**)
155155
assert(mv2.moved()); // was moved from
156156
assert(r->first == 3); // key
157157
assert(r->second.get() == 5); // value
158+
159+
// wrong hint: begin()
160+
Moveable mv3(7, 7.0);
161+
r = m.insert_or_assign(m.begin(), 4, std::move(mv3));
162+
assert(m.size() == 11);
163+
assert(mv3.moved()); // was moved from
164+
assert(r->first == 4); // key
165+
assert(r->second.get() == 7); // value
166+
167+
Moveable mv4(9, 9.0);
168+
r = m.insert_or_assign(m.begin(), 5, std::move(mv4));
169+
assert(m.size() == 12);
170+
assert(mv4.moved()); // was moved from
171+
assert(r->first == 5); // key
172+
assert(r->second.get() == 9); // value
173+
174+
// wrong hint: end()
175+
Moveable mv5(11, 11.0);
176+
r = m.insert_or_assign(m.end(), 6, std::move(mv5));
177+
assert(m.size() == 12);
178+
assert(mv5.moved()); // was moved from
179+
assert(r->first == 6); // key
180+
assert(r->second.get() == 11); // value
181+
182+
Moveable mv6(13, 13.0);
183+
r = m.insert_or_assign(m.end(), 7, std::move(mv6));
184+
assert(m.size() == 13);
185+
assert(mv6.moved()); // was moved from
186+
assert(r->first == 7); // key
187+
assert(r->second.get() == 13); // value
188+
189+
// wrong hint: third element
190+
Moveable mv7(15, 15.0);
191+
r = m.insert_or_assign(std::next(m.begin(), 2), 8, std::move(mv7));
192+
assert(m.size() == 13);
193+
assert(mv7.moved()); // was moved from
194+
assert(r->first == 8); // key
195+
assert(r->second.get() == 15); // value
196+
197+
Moveable mv8(17, 17.0);
198+
r = m.insert_or_assign(std::next(m.begin(), 2), 9, std::move(mv8));
199+
assert(m.size() == 14);
200+
assert(mv8.moved()); // was moved from
201+
assert(r->first == 9); // key
202+
assert(r->second.get() == 17); // value
158203
}
159204
{ // iterator insert_or_assign(const_iterator hint, key_type&& k, M&& obj);
160205
typedef std::map<Moveable, Moveable> M;
@@ -182,6 +227,63 @@ int main(int, char**)
182227
assert(mvkey2.moved()); // was moved from
183228
assert(r->first.get() == 3); // key
184229
assert(r->second.get() == 5); // value
230+
231+
// wrong hint: begin()
232+
Moveable mvkey3(6, 6.0);
233+
Moveable mv3(8, 8.0);
234+
r = m.insert_or_assign(m.begin(), std::move(mvkey3), std::move(mv3));
235+
assert(m.size() == 11);
236+
assert(mv3.moved()); // was moved from
237+
assert(!mvkey3.moved()); // was not moved from
238+
assert(r->first == mvkey3); // key
239+
assert(r->second.get() == 8); // value
240+
241+
Moveable mvkey4(7, 7.0);
242+
Moveable mv4(9, 9.0);
243+
r = m.insert_or_assign(m.begin(), std::move(mvkey4), std::move(mv4));
244+
assert(m.size() == 12);
245+
assert(mv4.moved()); // was moved from
246+
assert(mvkey4.moved()); // was moved from
247+
assert(r->first.get() == 7); // key
248+
assert(r->second.get() == 9); // value
249+
250+
// wrong hint: end()
251+
Moveable mvkey5(8, 8.0);
252+
Moveable mv5(10, 10.0);
253+
r = m.insert_or_assign(m.end(), std::move(mvkey5), std::move(mv5));
254+
assert(m.size() == 12);
255+
assert(mv5.moved()); // was moved from
256+
assert(!mvkey5.moved()); // was not moved from
257+
assert(r->first == mvkey5); // key
258+
assert(r->second.get() == 10); // value
259+
260+
Moveable mvkey6(9, 9.0);
261+
Moveable mv6(11, 11.0);
262+
r = m.insert_or_assign(m.end(), std::move(mvkey6), std::move(mv6));
263+
assert(m.size() == 13);
264+
assert(mv6.moved()); // was moved from
265+
assert(mvkey6.moved()); // was moved from
266+
assert(r->first.get() == 9); // key
267+
assert(r->second.get() == 11); // value
268+
269+
// wrong hint: third element
270+
Moveable mvkey7(10, 10.0);
271+
Moveable mv7(12, 12.0);
272+
r = m.insert_or_assign(std::next(m.begin(), 2), std::move(mvkey7), std::move(mv7));
273+
assert(m.size() == 13);
274+
assert(mv7.moved()); // was moved from
275+
assert(!mvkey7.moved()); // was not moved from
276+
assert(r->first == mvkey7); // key
277+
assert(r->second.get() == 12); // value
278+
279+
Moveable mvkey8(11, 11.0);
280+
Moveable mv8(13, 13.0);
281+
r = m.insert_or_assign(std::next(m.begin(), 2), std::move(mvkey8), std::move(mv8));
282+
assert(m.size() == 14);
283+
assert(mv8.moved()); // was moved from
284+
assert(mvkey8.moved()); // was moved from
285+
assert(r->first.get() == 11); // key
286+
assert(r->second.get() == 13); // value
185287
}
186288

187289
return 0;

0 commit comments

Comments
 (0)