[libcxx-commits] [libcxx] d4dd961 - Fixes complexity of map insert_or_assign with a hint.
Mark de Wever via libcxx-commits
libcxx-commits at lists.llvm.org
Sat Sep 19 07:29:26 PDT 2020
Author: Mark de Wever
Date: 2020-09-19T16:28:55+02:00
New Revision: d4dd961300588ce7e50625a1947befb45e3a0c42
URL: https://github.com/llvm/llvm-project/commit/d4dd961300588ce7e50625a1947befb45e3a0c42
DIFF: https://github.com/llvm/llvm-project/commit/d4dd961300588ce7e50625a1947befb45e3a0c42.diff
LOG: 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
Added:
Modified:
libcxx/include/__tree
libcxx/include/map
libcxx/test/std/containers/associative/map/map.modifiers/insert_or_assign.pass.cpp
Removed:
################################################################################
diff --git a/libcxx/include/__tree b/libcxx/include/__tree
index ed23d568751f..4c00ef83fad6 100644
--- a/libcxx/include/__tree
+++ b/libcxx/include/__tree
@@ -1151,7 +1151,7 @@ public:
pair<iterator, bool>
__emplace_unique_key_args(_Key const&, _Args&&... __args);
template <class _Key, class ..._Args>
- iterator
+ pair<iterator, bool>
__emplace_hint_unique_key_args(const_iterator, _Key const&, _Args&&...);
template <class... _Args>
@@ -1225,7 +1225,7 @@ public:
>::type __emplace_hint_unique(const_iterator __p, _First&& __f, _Second&& __s) {
return __emplace_hint_unique_key_args(__p, __f,
_VSTD::forward<_First>(__f),
- _VSTD::forward<_Second>(__s));
+ _VSTD::forward<_Second>(__s)).first;
}
template <class... _Args>
@@ -1245,14 +1245,14 @@ public:
_LIBCPP_INLINE_VISIBILITY
iterator
__emplace_hint_unique_extract_key(const_iterator __p, _Pp&& __x, __extract_key_self_tag) {
- return __emplace_hint_unique_key_args(__p, __x, _VSTD::forward<_Pp>(__x));
+ return __emplace_hint_unique_key_args(__p, __x, _VSTD::forward<_Pp>(__x)).first;
}
template <class _Pp>
_LIBCPP_INLINE_VISIBILITY
iterator
__emplace_hint_unique_extract_key(const_iterator __p, _Pp&& __x, __extract_key_first_tag) {
- return __emplace_hint_unique_key_args(__p, __x.first, _VSTD::forward<_Pp>(__x));
+ return __emplace_hint_unique_key_args(__p, __x.first, _VSTD::forward<_Pp>(__x)).first;
}
#else
@@ -1261,7 +1261,7 @@ public:
pair<iterator, bool> __emplace_unique_key_args(_Key const&, _Args& __args);
template <class _Key, class _Args>
_LIBCPP_INLINE_VISIBILITY
- iterator __emplace_hint_unique_key_args(const_iterator, _Key const&, _Args&);
+ pair<iterator, bool> __emplace_hint_unique_key_args(const_iterator, _Key const&, _Args&);
#endif
_LIBCPP_INLINE_VISIBILITY
@@ -1271,7 +1271,7 @@ public:
_LIBCPP_INLINE_VISIBILITY
iterator __insert_unique(const_iterator __p, const __container_value_type& __v) {
- return __emplace_hint_unique_key_args(__p, _NodeTypes::__get_key(__v), __v);
+ return __emplace_hint_unique_key_args(__p, _NodeTypes::__get_key(__v), __v).first;
}
#ifdef _LIBCPP_CXX03_LANG
@@ -1287,7 +1287,7 @@ public:
_LIBCPP_INLINE_VISIBILITY
iterator __insert_unique(const_iterator __p, __container_value_type&& __v) {
- return __emplace_hint_unique_key_args(__p, _NodeTypes::__get_key(__v), _VSTD::move(__v));
+ return __emplace_hint_unique_key_args(__p, _NodeTypes::__get_key(__v), _VSTD::move(__v)).first;
}
template <class _Vp, class = typename enable_if<
@@ -2147,17 +2147,16 @@ __tree<_Tp, _Compare, _Allocator>::__emplace_unique_key_args(_Key const& __k, _A
return pair<iterator, bool>(iterator(__r), __inserted);
}
-
#ifndef _LIBCPP_CXX03_LANG
template <class _Tp, class _Compare, class _Allocator>
template <class _Key, class... _Args>
-typename __tree<_Tp, _Compare, _Allocator>::iterator
+pair<typename __tree<_Tp, _Compare, _Allocator>::iterator, bool>
__tree<_Tp, _Compare, _Allocator>::__emplace_hint_unique_key_args(
const_iterator __p, _Key const& __k, _Args&&... __args)
#else
template <class _Tp, class _Compare, class _Allocator>
template <class _Key, class _Args>
-typename __tree<_Tp, _Compare, _Allocator>::iterator
+pair<typename __tree<_Tp, _Compare, _Allocator>::iterator, bool>
__tree<_Tp, _Compare, _Allocator>::__emplace_hint_unique_key_args(
const_iterator __p, _Key const& __k, _Args& __args)
#endif
@@ -2166,6 +2165,7 @@ __tree<_Tp, _Compare, _Allocator>::__emplace_hint_unique_key_args(
__node_base_pointer __dummy;
__node_base_pointer& __child = __find_equal(__p, __parent, __dummy, __k);
__node_pointer __r = static_cast<__node_pointer>(__child);
+ bool __inserted = false;
if (__child == nullptr)
{
#ifndef _LIBCPP_CXX03_LANG
@@ -2175,11 +2175,11 @@ __tree<_Tp, _Compare, _Allocator>::__emplace_hint_unique_key_args(
#endif
__insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__h.get()));
__r = __h.release();
+ __inserted = true;
}
- return iterator(__r);
+ return pair<iterator, bool>(iterator(__r), __inserted);
}
-
#ifndef _LIBCPP_CXX03_LANG
template <class _Tp, class _Compare, class _Allocator>
diff --git a/libcxx/include/map b/libcxx/include/map
index 96409f2389d9..a27002c279ec 100644
--- a/libcxx/include/map
+++ b/libcxx/include/map
@@ -1230,7 +1230,7 @@ public:
return __tree_.__emplace_hint_unique_key_args(__h.__i_, __k,
_VSTD::piecewise_construct,
_VSTD::forward_as_tuple(__k),
- _VSTD::forward_as_tuple(_VSTD::forward<_Args>(__args)...));
+ _VSTD::forward_as_tuple(_VSTD::forward<_Args>(__args)...)).first;
}
template <class... _Args>
@@ -1240,7 +1240,7 @@ public:
return __tree_.__emplace_hint_unique_key_args(__h.__i_, __k,
_VSTD::piecewise_construct,
_VSTD::forward_as_tuple(_VSTD::move(__k)),
- _VSTD::forward_as_tuple(_VSTD::forward<_Args>(__args)...));
+ _VSTD::forward_as_tuple(_VSTD::forward<_Args>(__args)...)).first;
}
template <class _Vp>
@@ -1270,30 +1270,30 @@ public:
}
template <class _Vp>
- _LIBCPP_INLINE_VISIBILITY
- iterator insert_or_assign(const_iterator __h, const key_type& __k, _Vp&& __v)
- {
- iterator __p = lower_bound(__k);
- if ( __p != end() && !key_comp()(__k, __p->first))
- {
- __p->second = _VSTD::forward<_Vp>(__v);
- return __p;
- }
- return emplace_hint(__h, __k, _VSTD::forward<_Vp>(__v));
- }
+ _LIBCPP_INLINE_VISIBILITY iterator insert_or_assign(const_iterator __h,
+ const key_type& __k,
+ _Vp&& __v) {
+ auto [__r, __inserted] = __tree_.__emplace_hint_unique_key_args(
+ __h.__i_, __k, __k, _VSTD::forward<_Vp>(__v));
+
+ if (!__inserted)
+ __r->__get_value().second = _VSTD::forward<_Vp>(__v);
+
+ return __r;
+ }
template <class _Vp>
- _LIBCPP_INLINE_VISIBILITY
- iterator insert_or_assign(const_iterator __h, key_type&& __k, _Vp&& __v)
- {
- iterator __p = lower_bound(__k);
- if ( __p != end() && !key_comp()(__k, __p->first))
- {
- __p->second = _VSTD::forward<_Vp>(__v);
- return __p;
- }
- return emplace_hint(__h, _VSTD::move(__k), _VSTD::forward<_Vp>(__v));
- }
+ _LIBCPP_INLINE_VISIBILITY iterator insert_or_assign(const_iterator __h,
+ key_type&& __k,
+ _Vp&& __v) {
+ auto [__r, __inserted] = __tree_.__emplace_hint_unique_key_args(
+ __h.__i_, __k, _VSTD::move(__k), _VSTD::forward<_Vp>(__v));
+
+ if (!__inserted)
+ __r->__get_value().second = _VSTD::forward<_Vp>(__v);
+
+ return __r;
+ }
#endif // _LIBCPP_STD_VER > 14
diff --git a/libcxx/test/std/containers/associative/map/map.modifiers/insert_or_assign.pass.cpp b/libcxx/test/std/containers/associative/map/map.modifiers/insert_or_assign.pass.cpp
index 0edfa2fd84fe..bdde35cff7c7 100644
--- a/libcxx/test/std/containers/associative/map/map.modifiers/insert_or_assign.pass.cpp
+++ b/libcxx/test/std/containers/associative/map/map.modifiers/insert_or_assign.pass.cpp
@@ -155,6 +155,51 @@ int main(int, char**)
assert(mv2.moved()); // was moved from
assert(r->first == 3); // key
assert(r->second.get() == 5); // value
+
+ // wrong hint: begin()
+ Moveable mv3(7, 7.0);
+ r = m.insert_or_assign(m.begin(), 4, std::move(mv3));
+ assert(m.size() == 11);
+ assert(mv3.moved()); // was moved from
+ assert(r->first == 4); // key
+ assert(r->second.get() == 7); // value
+
+ Moveable mv4(9, 9.0);
+ r = m.insert_or_assign(m.begin(), 5, std::move(mv4));
+ assert(m.size() == 12);
+ assert(mv4.moved()); // was moved from
+ assert(r->first == 5); // key
+ assert(r->second.get() == 9); // value
+
+ // wrong hint: end()
+ Moveable mv5(11, 11.0);
+ r = m.insert_or_assign(m.end(), 6, std::move(mv5));
+ assert(m.size() == 12);
+ assert(mv5.moved()); // was moved from
+ assert(r->first == 6); // key
+ assert(r->second.get() == 11); // value
+
+ Moveable mv6(13, 13.0);
+ r = m.insert_or_assign(m.end(), 7, std::move(mv6));
+ assert(m.size() == 13);
+ assert(mv6.moved()); // was moved from
+ assert(r->first == 7); // key
+ assert(r->second.get() == 13); // value
+
+ // wrong hint: third element
+ Moveable mv7(15, 15.0);
+ r = m.insert_or_assign(std::next(m.begin(), 2), 8, std::move(mv7));
+ assert(m.size() == 13);
+ assert(mv7.moved()); // was moved from
+ assert(r->first == 8); // key
+ assert(r->second.get() == 15); // value
+
+ Moveable mv8(17, 17.0);
+ r = m.insert_or_assign(std::next(m.begin(), 2), 9, std::move(mv8));
+ assert(m.size() == 14);
+ assert(mv8.moved()); // was moved from
+ assert(r->first == 9); // key
+ assert(r->second.get() == 17); // value
}
{ // iterator insert_or_assign(const_iterator hint, key_type&& k, M&& obj);
typedef std::map<Moveable, Moveable> M;
@@ -182,6 +227,63 @@ int main(int, char**)
assert(mvkey2.moved()); // was moved from
assert(r->first.get() == 3); // key
assert(r->second.get() == 5); // value
+
+ // wrong hint: begin()
+ Moveable mvkey3(6, 6.0);
+ Moveable mv3(8, 8.0);
+ r = m.insert_or_assign(m.begin(), std::move(mvkey3), std::move(mv3));
+ assert(m.size() == 11);
+ assert(mv3.moved()); // was moved from
+ assert(!mvkey3.moved()); // was not moved from
+ assert(r->first == mvkey3); // key
+ assert(r->second.get() == 8); // value
+
+ Moveable mvkey4(7, 7.0);
+ Moveable mv4(9, 9.0);
+ r = m.insert_or_assign(m.begin(), std::move(mvkey4), std::move(mv4));
+ assert(m.size() == 12);
+ assert(mv4.moved()); // was moved from
+ assert(mvkey4.moved()); // was moved from
+ assert(r->first.get() == 7); // key
+ assert(r->second.get() == 9); // value
+
+ // wrong hint: end()
+ Moveable mvkey5(8, 8.0);
+ Moveable mv5(10, 10.0);
+ r = m.insert_or_assign(m.end(), std::move(mvkey5), std::move(mv5));
+ assert(m.size() == 12);
+ assert(mv5.moved()); // was moved from
+ assert(!mvkey5.moved()); // was not moved from
+ assert(r->first == mvkey5); // key
+ assert(r->second.get() == 10); // value
+
+ Moveable mvkey6(9, 9.0);
+ Moveable mv6(11, 11.0);
+ r = m.insert_or_assign(m.end(), std::move(mvkey6), std::move(mv6));
+ assert(m.size() == 13);
+ assert(mv6.moved()); // was moved from
+ assert(mvkey6.moved()); // was moved from
+ assert(r->first.get() == 9); // key
+ assert(r->second.get() == 11); // value
+
+ // wrong hint: third element
+ Moveable mvkey7(10, 10.0);
+ Moveable mv7(12, 12.0);
+ r = m.insert_or_assign(std::next(m.begin(), 2), std::move(mvkey7), std::move(mv7));
+ assert(m.size() == 13);
+ assert(mv7.moved()); // was moved from
+ assert(!mvkey7.moved()); // was not moved from
+ assert(r->first == mvkey7); // key
+ assert(r->second.get() == 12); // value
+
+ Moveable mvkey8(11, 11.0);
+ Moveable mv8(13, 13.0);
+ r = m.insert_or_assign(std::next(m.begin(), 2), std::move(mvkey8), std::move(mv8));
+ assert(m.size() == 14);
+ assert(mv8.moved()); // was moved from
+ assert(mvkey8.moved()); // was moved from
+ assert(r->first.get() == 11); // key
+ assert(r->second.get() == 13); // value
}
return 0;
More information about the libcxx-commits
mailing list