On 11/05/2018 14:16, Jonathan Wakely wrote:
On 06/05/18 16:06 +0200, François Dumont wrote:
Here is the rework of this patch.
On 02/05/2018 13:49, Jonathan Wakely wrote:
There's no changelog entry with the patch, so to recap, the changes
are:
- noexcept specifications are automatically deduced instead of being
stated explicitly.
I simplified this part, it is not deduced anymore except for Debug
mode relying on Normal mode qualifications.
- The allocator-extended move constructors get correct exception
specifications in Debug Mode (consistent with normal mode). This
should be done anyway, independent of any other changes.
I kept this untouch.
- We avoid a `if (__x._M_root() != nullptr)` branch in the case where
the allocator-extended move constructor is used with an always-equal
allocator, right?
Kept too.
Compilation time for containers is already **much** slower since the
allocator-extended constructors were added for GCC 5.x, despite very
few people ever using those constructors. This patch seems to require
even more work from the compiler to parse constructors nobody uses.
I restore direct qualifications.
diff --git a/libstdc++-v3/include/bits/stl_map.h
b/libstdc++-v3/include/bits/stl_map.h
index a4a026e..2b8fd27 100644
--- a/libstdc++-v3/include/bits/stl_map.h
+++ b/libstdc++-v3/include/bits/stl_map.h
@@ -240,8 +240,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
/// Allocator-extended move constructor.
map(map&& __m, const allocator_type& __a)
- noexcept(is_nothrow_copy_constructible<_Compare>::value
- && _Alloc_traits::_S_always_equal())
+ noexcept( noexcept(
+ _Rep_type(std::move(__m._M_t), declval<_Pair_alloc_type>())) )
All these calls to declval need to be qualified to avoid ADL.
It seems strange to have a mix of real values and declval expressions,
rather than:
_Rep_type(std::declval<_Rep_type>(),
std::declval<_Pair_alloc_type>())) )
I add std:: qualitification and generalized usage.
- _Rb_tree(_Rb_tree&& __x, _Node_allocator&& __a);
+ private:
+ _Rb_tree(_Rb_tree&& __x, _Node_allocator&& __a, true_type)
+
noexcept(is_nothrow_move_constructible<_Rb_tree_impl<_Compare>>::value)
This is wrong.
As stated previously I removed this but based on this remark I also
change the _Rb_tree_impl() qualification so that it doesn't depend
anymore on allocator default constructor qualification.
I've added a test for that too.
This can use std::is_nothrow_constructible
Indeed.
+ "noexcept move constructor with allocator" );
+
+struct ExceptLess
Please name this "ThrowingLess" instead of "ExceptLess", I think
that's more descriptive.
I prefered "not_noexcept" prefix cause those operators are not
throwing anything neither.
Tested under Linux x86_64, ok to commit ?
* include/bits/stl_tree.h
(_Rb_tree_impl()):
Remove is_nothrow_default_constructible<_Node_allocator> from
noexcept
qualification.
(_Rb_tree_impl(_Rb_tree_impl&&, _Node_allocator&&)): New.
(_Rb_tree(_Rb_tree&&, _Node_allocator&&, std::true_type)): New,
use latter.
(_Rb_tree(_Rb_tree&&, _Node_allocator&&, std::false_type)): New.
(_Rb_tree(_Rb_tree&&, _Node_allocator&&)): Adapt, use latters.
* include/debug/map.h
(map(map&&, const_allocator_type&)): Add noexcept qualitication.
* include/debug/multimap.h
(multimap(multimap&&, const_allocator_type&)): Add noexcept
qualification.
* include/debug/set.h
(set(set&&, const_allocator_type&)): Add noexcept qualitication.
* include/debug/multiset.h
(multiset(multiset&&, const_allocator_type&)): Add noexcept
qualification.
* testsuite/23_containers/map/cons/noexcept_default_construct.cc:
Add checks.
* testsuite/23_containers/map/cons/noexcept_move_construct.cc:
Add checks.
*
testsuite/23_containers/multimap/cons/noexcept_default_construct.cc:
Add checks.
* testsuite/23_containers/multimap/cons/noexcept_move_construct.cc:
Add checks.
*
testsuite/23_containers/multiset/cons/noexcept_default_construct.cc:
Add checks.
* testsuite/23_containers/multiset/cons/noexcept_move_construct.cc:
Add checks.
* testsuite/23_containers/set/cons/noexcept_default_construct.cc:
Add checks.
* testsuite/23_containers/set/cons/noexcept_move_construct.cc:
Add checks.
François
diff --git a/libstdc++-v3/include/bits/stl_tree.h
b/libstdc++-v3/include/bits/stl_tree.h
index d0a8448..9e59259 100644
--- a/libstdc++-v3/include/bits/stl_tree.h
+++ b/libstdc++-v3/include/bits/stl_tree.h
@@ -698,8 +698,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_Rb_tree_impl()
_GLIBCXX_NOEXCEPT_IF(
- is_nothrow_default_constructible<_Node_allocator>::value
- && is_nothrow_default_constructible<_Base_key_compare>::value )
+ // is_nothrow_default_constructible<_Node_allocator>::value &&
+ is_nothrow_default_constructible<_Base_key_compare>::value )
: _Node_allocator()
{ }
This part wasn't in the previous patch and seems wrong:
https://cplusplus.github.io/LWG/issue2455
Yes, I must have been tired when I decided to do such a thing.
Here it is again even more simplified. Should I backport the Debug mode
fix to 8 branch ?
* include/bits/stl_tree.h
(_Rb_tree_impl(_Rb_tree_impl&&, _Node_allocator&&)): New.
(_Rb_tree(_Rb_tree&&, _Node_allocator&&, true_type)): New, use latter.
(_Rb_tree(_Rb_tree&&, _Node_allocator&&, false_type)): New.
(_Rb_tree(_Rb_tree&&, _Node_allocator&&)): Adapt, use latters.
* include/debug/map.h
(map(map&&, const_allocator_type&)): Add noexcept qualitication.
* include/debug/multimap.h
(multimap(multimap&&, const_allocator_type&)): Add noexcept
qualification.
* include/debug/set.h
(set(set&&, const_allocator_type&)): Add noexcept qualitication.
* include/debug/multiset.h
(multiset(multiset&&, const_allocator_type&)): Add noexcept
qualification.
* testsuite/23_containers/map/cons/noexcept_default_construct.cc:
Add checks.
* testsuite/23_containers/map/cons/noexcept_move_construct.cc:
Add checks.
* testsuite/23_containers/multimap/cons/noexcept_default_construct.cc:
Add checks.
* testsuite/23_containers/multimap/cons/noexcept_move_construct.cc:
Add checks.
* testsuite/23_containers/multiset/cons/noexcept_default_construct.cc:
Add checks.
* testsuite/23_containers/multiset/cons/noexcept_move_construct.cc:
Add checks.
* testsuite/23_containers/set/cons/noexcept_default_construct.cc:
Add checks.
* testsuite/23_containers/set/cons/noexcept_move_construct.cc:
Add checks.
Ok to commit ?
François
diff --git a/libstdc++-v3/include/bits/stl_tree.h b/libstdc++-v3/include/bits/stl_tree.h
index d0a8448..85f190a 100644
--- a/libstdc++-v3/include/bits/stl_tree.h
+++ b/libstdc++-v3/include/bits/stl_tree.h
@@ -715,6 +715,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
#else
_Rb_tree_impl(_Rb_tree_impl&&) = default;
+ _Rb_tree_impl(_Rb_tree_impl&& __x, _Node_allocator&& __a)
+ : _Node_allocator(std::move(__a)),
+ _Base_key_compare(std::move(__x)),
+ _Rb_tree_header(std::move(__x))
+ { }
+
_Rb_tree_impl(const _Key_compare& __comp, _Node_allocator&& __a)
: _Node_allocator(std::move(__a)), _Base_key_compare(__comp)
{ }
@@ -958,7 +964,27 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
: _Rb_tree(std::move(__x), _Node_allocator(__a))
{ }
- _Rb_tree(_Rb_tree&& __x, _Node_allocator&& __a);
+ private:
+ _Rb_tree(_Rb_tree&& __x, _Node_allocator&& __a, true_type)
+ noexcept(is_nothrow_default_constructible<_Compare>::value)
+ : _M_impl(std::move(__x._M_impl), std::move(__a))
+ { }
+
+ _Rb_tree(_Rb_tree&& __x, _Node_allocator&& __a, false_type)
+ : _M_impl(__x._M_impl._M_key_compare, std::move(__a))
+ {
+ if (__x._M_root() != nullptr)
+ _M_move_data(__x, false_type{});
+ }
+
+ public:
+ _Rb_tree(_Rb_tree&& __x, _Node_allocator&& __a)
+ noexcept( noexcept(
+ _Rb_tree(std::declval<_Rb_tree>(), std::declval<_Node_allocator>(),
+ std::declval<typename _Alloc_traits::is_always_equal>())) )
+ : _Rb_tree(std::move(__x), std::move(__a),
+ typename _Alloc_traits::is_always_equal{})
+ { }
#endif
~_Rb_tree() _GLIBCXX_NOEXCEPT
@@ -1358,22 +1384,22 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
private:
// Move elements from container with equal allocator.
void
- _M_move_data(_Rb_tree& __x, std::true_type)
+ _M_move_data(_Rb_tree& __x, true_type)
{ _M_impl._M_move_data(__x._M_impl); }
// Move elements from container with possibly non-equal allocator,
// which might result in a copy not a move.
void
- _M_move_data(_Rb_tree&, std::false_type);
+ _M_move_data(_Rb_tree&, false_type);
// Move assignment from container with equal allocator.
void
- _M_move_assign(_Rb_tree&, std::true_type);
+ _M_move_assign(_Rb_tree&, true_type);
// Move assignment from container with possibly non-equal allocator,
// which might result in a copy not a move.
void
- _M_move_assign(_Rb_tree&, std::false_type);
+ _M_move_assign(_Rb_tree&, false_type);
#endif
#if __cplusplus > 201402L
@@ -1601,23 +1627,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
#if __cplusplus >= 201103L
template<typename _Key, typename _Val, typename _KeyOfValue,
typename _Compare, typename _Alloc>
- _Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::
- _Rb_tree(_Rb_tree&& __x, _Node_allocator&& __a)
- : _M_impl(__x._M_impl._M_key_compare, std::move(__a))
- {
- using __eq = typename _Alloc_traits::is_always_equal;
- if (__x._M_root() != nullptr)
- _M_move_data(__x, __eq());
- }
-
- template<typename _Key, typename _Val, typename _KeyOfValue,
- typename _Compare, typename _Alloc>
void
_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::
- _M_move_data(_Rb_tree& __x, std::false_type)
+ _M_move_data(_Rb_tree& __x, false_type)
{
if (_M_get_Node_allocator() == __x._M_get_Node_allocator())
- _M_move_data(__x, std::true_type());
+ _M_move_data(__x, true_type());
else
{
_Alloc_node __an(*this);
@@ -1639,7 +1654,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{
clear();
if (__x._M_root() != nullptr)
- _M_move_data(__x, std::true_type());
+ _M_move_data(__x, true_type());
std::__alloc_on_move(_M_get_Node_allocator(),
__x._M_get_Node_allocator());
}
diff --git a/libstdc++-v3/include/debug/map.h b/libstdc++-v3/include/debug/map.h
index 3f0649a..23966ba 100644
--- a/libstdc++-v3/include/debug/map.h
+++ b/libstdc++-v3/include/debug/map.h
@@ -105,6 +105,7 @@ namespace __debug
: _Base(__m, __a) { }
map(map&& __m, const allocator_type& __a)
+ noexcept( noexcept(_Base(std::move(__m._M_base()), __a)) )
: _Safe(std::move(__m._M_safe()), __a),
_Base(std::move(__m._M_base()), __a) { }
diff --git a/libstdc++-v3/include/debug/multimap.h b/libstdc++-v3/include/debug/multimap.h
index e709eb7..8054984 100644
--- a/libstdc++-v3/include/debug/multimap.h
+++ b/libstdc++-v3/include/debug/multimap.h
@@ -105,6 +105,7 @@ namespace __debug
: _Base(__m, __a) { }
multimap(multimap&& __m, const allocator_type& __a)
+ noexcept( noexcept(_Base(std::move(__m._M_base()), __a)) )
: _Safe(std::move(__m._M_safe()), __a),
_Base(std::move(__m._M_base()), __a) { }
diff --git a/libstdc++-v3/include/debug/multiset.h b/libstdc++-v3/include/debug/multiset.h
index 461f4f6..6e4c1b0 100644
--- a/libstdc++-v3/include/debug/multiset.h
+++ b/libstdc++-v3/include/debug/multiset.h
@@ -104,6 +104,7 @@ namespace __debug
: _Base(__m, __a) { }
multiset(multiset&& __m, const allocator_type& __a)
+ noexcept( noexcept(_Base(std::move(__m._M_base()), __a)) )
: _Safe(std::move(__m._M_safe()), __a),
_Base(std::move(__m._M_base()), __a) { }
diff --git a/libstdc++-v3/include/debug/set.h b/libstdc++-v3/include/debug/set.h
index 2ac8f1c..571cc47 100644
--- a/libstdc++-v3/include/debug/set.h
+++ b/libstdc++-v3/include/debug/set.h
@@ -104,6 +104,7 @@ namespace __debug
: _Base(__x, __a) { }
set(set&& __x, const allocator_type& __a)
+ noexcept( noexcept(_Base(std::move(__x._M_base()), __a)) )
: _Safe(std::move(__x._M_safe()), __a),
_Base(std::move(__x._M_base()), __a) { }
diff --git a/libstdc++-v3/testsuite/23_containers/map/cons/noexcept_default_construct.cc b/libstdc++-v3/testsuite/23_containers/map/cons/noexcept_default_construct.cc
index 4b9e119..e5c924d 100644
--- a/libstdc++-v3/testsuite/23_containers/map/cons/noexcept_default_construct.cc
+++ b/libstdc++-v3/testsuite/23_containers/map/cons/noexcept_default_construct.cc
@@ -30,3 +30,19 @@ struct cmp
using mtype2 = std::map<int, int, cmp>;
static_assert( !std::is_nothrow_default_constructible<mtype2>::value, "Error");
+
+template<typename _Tp>
+ struct not_noexcept_cons_alloc : std::allocator<_Tp>
+ {
+ not_noexcept_cons_alloc() /* noexcept */
+ { }
+
+ template<typename _Tp1>
+ struct rebind
+ { typedef not_noexcept_cons_alloc<_Tp1> other; };
+ };
+
+using mtype3 = std::map<int, int, std::less<int>,
+ not_noexcept_cons_alloc<std::pair<const int, int>>>;
+
+static_assert(!std::is_nothrow_default_constructible<mtype3>::value, "Error");
diff --git a/libstdc++-v3/testsuite/23_containers/map/cons/noexcept_move_construct.cc b/libstdc++-v3/testsuite/23_containers/map/cons/noexcept_move_construct.cc
index 0041408..723a63f 100644
--- a/libstdc++-v3/testsuite/23_containers/map/cons/noexcept_move_construct.cc
+++ b/libstdc++-v3/testsuite/23_containers/map/cons/noexcept_move_construct.cc
@@ -23,4 +23,25 @@
typedef std::map<int, int> mtype;
-static_assert(std::is_nothrow_move_constructible<mtype>::value, "Error");
+static_assert( std::is_nothrow_move_constructible<mtype>::value,
+ "noexcept move constructor" );
+static_assert( std::is_nothrow_constructible<mtype,
+ mtype&&, const typename mtype::allocator_type&>::value,
+ "noexcept move constructor with allocator" );
+
+struct not_noexcept_less
+{
+ not_noexcept_less() = default;
+ not_noexcept_less(const not_noexcept_less&) /* noexcept */
+ { }
+
+ bool
+ operator()(int l, int r) const
+ { return l < r; }
+};
+
+typedef std::map<int, int, not_noexcept_less> emtype;
+
+static_assert( !std::is_nothrow_constructible<emtype, emtype&&,
+ const typename emtype::allocator_type&>::value,
+ "except move constructor with allocator" );
diff --git a/libstdc++-v3/testsuite/23_containers/multimap/cons/noexcept_default_construct.cc b/libstdc++-v3/testsuite/23_containers/multimap/cons/noexcept_default_construct.cc
index 9873770..62f5d60 100644
--- a/libstdc++-v3/testsuite/23_containers/multimap/cons/noexcept_default_construct.cc
+++ b/libstdc++-v3/testsuite/23_containers/multimap/cons/noexcept_default_construct.cc
@@ -30,3 +30,19 @@ struct cmp
using mtype2 = std::multimap<int, int, cmp>;
static_assert( !std::is_nothrow_default_constructible<mtype2>::value, "Error");
+
+template<typename _Tp>
+ struct not_noexcept_cons_alloc : std::allocator<_Tp>
+ {
+ not_noexcept_cons_alloc() /* noexcept */
+ { }
+
+ template<typename _Tp1>
+ struct rebind
+ { typedef not_noexcept_cons_alloc<_Tp1> other; };
+ };
+
+using mtype3 = std::multimap<int, int, std::less<int>,
+ not_noexcept_cons_alloc<std::pair<const int, int>>>;
+
+static_assert(!std::is_nothrow_default_constructible<mtype3>::value, "Error");
diff --git a/libstdc++-v3/testsuite/23_containers/multimap/cons/noexcept_move_construct.cc b/libstdc++-v3/testsuite/23_containers/multimap/cons/noexcept_move_construct.cc
index 0319a61..1612b0d 100644
--- a/libstdc++-v3/testsuite/23_containers/multimap/cons/noexcept_move_construct.cc
+++ b/libstdc++-v3/testsuite/23_containers/multimap/cons/noexcept_move_construct.cc
@@ -23,4 +23,25 @@
typedef std::multimap<int, int> mmtype;
-static_assert(std::is_nothrow_move_constructible<mmtype>::value, "Error");
+static_assert( std::is_nothrow_move_constructible<mmtype>::value,
+ "noexcept move constructor" );
+static_assert( std::is_nothrow_constructible<mmtype,
+ mmtype&&, const typename mmtype::allocator_type&>::value,
+ "noexcept move constructor with allocator" );
+
+struct not_noexcept_less
+{
+ not_noexcept_less() = default;
+ not_noexcept_less(const not_noexcept_less&) /* noexcept */
+ { }
+
+ bool
+ operator()(int l, int r) const
+ { return l < r; }
+};
+
+typedef std::multimap<int, int, not_noexcept_less> emmtype;
+
+static_assert( !std::is_nothrow_constructible<emmtype, emmtype&&,
+ const typename emmtype::allocator_type&>::value,
+ "except move constructor with allocator" );
diff --git a/libstdc++-v3/testsuite/23_containers/multiset/cons/noexcept_default_construct.cc b/libstdc++-v3/testsuite/23_containers/multiset/cons/noexcept_default_construct.cc
index 963e3d1..8e49168 100644
--- a/libstdc++-v3/testsuite/23_containers/multiset/cons/noexcept_default_construct.cc
+++ b/libstdc++-v3/testsuite/23_containers/multiset/cons/noexcept_default_construct.cc
@@ -30,3 +30,19 @@ struct cmp
using stype2 = std::multiset<int, cmp>;
static_assert( !std::is_nothrow_default_constructible<stype2>::value, "Error");
+
+template<typename _Tp>
+ struct not_noexcept_cons_alloc : std::allocator<_Tp>
+ {
+ not_noexcept_cons_alloc() /* noexcept */
+ { }
+
+ template<typename _Tp1>
+ struct rebind
+ { typedef not_noexcept_cons_alloc<_Tp1> other; };
+ };
+
+using stype3 = std::multiset<int, std::less<int>,
+ not_noexcept_cons_alloc<int>>;
+
+static_assert(!std::is_nothrow_default_constructible<stype3>::value, "Error");
diff --git a/libstdc++-v3/testsuite/23_containers/multiset/cons/noexcept_move_construct.cc b/libstdc++-v3/testsuite/23_containers/multiset/cons/noexcept_move_construct.cc
index b1c6dd5..4ef5127 100644
--- a/libstdc++-v3/testsuite/23_containers/multiset/cons/noexcept_move_construct.cc
+++ b/libstdc++-v3/testsuite/23_containers/multiset/cons/noexcept_move_construct.cc
@@ -23,4 +23,25 @@
typedef std::multiset<int> mstype;
-static_assert(std::is_nothrow_move_constructible<mstype>::value, "Error");
+static_assert( std::is_nothrow_move_constructible<mstype>::value,
+ "noexcept move constructor" );
+static_assert( std::is_nothrow_constructible<mstype,
+ mstype&&, const typename mstype::allocator_type&>::value,
+ "noexcept move constructor with allocator" );
+
+struct not_noexcept_less
+{
+ not_noexcept_less() = default;
+ not_noexcept_less(const not_noexcept_less&) /* noexcept */
+ { }
+
+ bool
+ operator()(int l, int r) const
+ { return l < r; }
+};
+
+typedef std::multiset<int, not_noexcept_less> emstype;
+
+static_assert( !std::is_nothrow_constructible<emstype, emstype&&,
+ const typename emstype::allocator_type&>::value,
+ "except move constructor with allocator" );
diff --git a/libstdc++-v3/testsuite/23_containers/set/cons/noexcept_default_construct.cc b/libstdc++-v3/testsuite/23_containers/set/cons/noexcept_default_construct.cc
index 882a12c..d3dc98d 100644
--- a/libstdc++-v3/testsuite/23_containers/set/cons/noexcept_default_construct.cc
+++ b/libstdc++-v3/testsuite/23_containers/set/cons/noexcept_default_construct.cc
@@ -30,3 +30,19 @@ struct cmp
using stype2 = std::set<int, cmp>;
static_assert( !std::is_nothrow_default_constructible<stype2>::value, "Error");
+
+template<typename _Tp>
+ struct not_noexcept_cons_alloc : std::allocator<_Tp>
+ {
+ not_noexcept_cons_alloc() /* noexcept */
+ { }
+
+ template<typename _Tp1>
+ struct rebind
+ { typedef not_noexcept_cons_alloc<_Tp1> other; };
+ };
+
+using stype3 = std::set<int, std::less<int>,
+ not_noexcept_cons_alloc<int>>;
+
+static_assert(!std::is_nothrow_default_constructible<stype3>::value, "Error");
diff --git a/libstdc++-v3/testsuite/23_containers/set/cons/noexcept_move_construct.cc b/libstdc++-v3/testsuite/23_containers/set/cons/noexcept_move_construct.cc
index a7de0ef..942400e 100644
--- a/libstdc++-v3/testsuite/23_containers/set/cons/noexcept_move_construct.cc
+++ b/libstdc++-v3/testsuite/23_containers/set/cons/noexcept_move_construct.cc
@@ -23,4 +23,25 @@
typedef std::set<int> stype;
-static_assert(std::is_nothrow_move_constructible<stype>::value, "Error");
+static_assert( std::is_nothrow_move_constructible<stype>::value,
+ "noexcept move constructor" );
+static_assert( std::is_nothrow_constructible<stype,
+ stype&&, const typename stype::allocator_type&>::value,
+ "noexcept move constructor with allocator" );
+
+struct not_noexcept_less
+{
+ not_noexcept_less() = default;
+ not_noexcept_less(const not_noexcept_less&) /* noexcept */
+ { }
+
+ bool
+ operator()(int l, int r) const
+ { return l < r; }
+};
+
+typedef std::set<int, not_noexcept_less> estype;
+
+static_assert( !std::is_nothrow_constructible<estype, estype&&,
+ const typename estype::allocator_type&>::value,
+ "except move constructor with allocator" );
