From 588a873a86fc5a60192da6e5cf3d3247f0c5dc98 Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Fri, 24 May 2019 17:09:28 +0100 Subject: [PATCH] PR libstdc++/85965 move is_invocable assertions again This is another attempt to reduce how often the assertions are evaluated, so that code which doesn't try to use the function objects doesn't need them to be invocable. For _Rb_tree we access the _M_key_compare object directly, so can't put the assertions in an accessor function for it. However, every invocation of _M_key_compare is accompanied by a use of _S_key, so the assertions can be put in there. For _Hashtable there are member functions that are consistently used to obtain a hash code or test for equality, so the assertions can go in those members. Backport from mainline 2019-05-17 Jonathan Wakely PR libstdc++/85965 * include/bits/hashtable.h (_Hashtable::~_Hashtable()): Remove static assertions from the destructor. * include/bits/hashtable_policy.h (_Hash_code_base::_M_hash_code): Move static_assert for hash function to here. (_Hash_table_base::_M_equals): Move static_assert for equality predicate to here. * include/bits/stl_tree.h (_Rb_tree::_S_key(_Const_Link_type)): Move assertions here. Access the value directly instead of calling _S_value. (_Rb_tree::_S_key(_Const_Base_ptr)): Do downcast and forward to _S_key(_Const_Link_type). * testsuite/23_containers/set/85965.cc: Check construction, destruction, assignment and size() do not trigger the assertions. * testsuite/23_containers/unordered_set/85965.cc: Likewise. * testsuite/23_containers/map/48101_neg.cc: Call find and adjust expected errors. * testsuite/23_containers/multimap/48101_neg.cc: Likewise. * testsuite/23_containers/multiset/48101_neg.cc: Likewise. * testsuite/23_containers/set/48101_neg.cc: Likewise. * testsuite/23_containers/unordered_map/48101_neg.cc: Likewise. * testsuite/23_containers/unordered_multimap/48101_neg.cc: Likewise. * testsuite/23_containers/unordered_multiset/48101_neg.cc: Likewise. * testsuite/23_containers/unordered_set/48101_neg.cc: Likewise. From-SVN: r271607 --- libstdc++-v3/ChangeLog | 29 ++++++++++++++ libstdc++-v3/include/bits/hashtable.h | 6 --- libstdc++-v3/include/bits/hashtable_policy.h | 15 +++++++- libstdc++-v3/include/bits/stl_tree.h | 38 ++++++++++--------- .../testsuite/23_containers/map/48101_neg.cc | 4 ++ .../23_containers/multimap/48101_neg.cc | 4 ++ .../23_containers/multiset/48101_neg.cc | 3 ++ .../testsuite/23_containers/set/48101_neg.cc | 3 ++ .../testsuite/23_containers/set/85965.cc | 9 +++++ .../23_containers/unordered_map/48101_neg.cc | 2 + .../unordered_multimap/48101_neg.cc | 2 + .../unordered_multiset/48101_neg.cc | 2 + .../23_containers/unordered_set/48101_neg.cc | 2 + .../23_containers/unordered_set/85965.cc | 9 +++++ 14 files changed, 103 insertions(+), 25 deletions(-) diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index 177491d68247..ca28ce46d6e9 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,3 +1,32 @@ +2019-05-24 Jonathan Wakely + + Backport from mainline + 2019-05-17 Jonathan Wakely + + PR libstdc++/85965 + * include/bits/hashtable.h (_Hashtable::~_Hashtable()): Remove static + assertions from the destructor. + * include/bits/hashtable_policy.h (_Hash_code_base::_M_hash_code): + Move static_assert for hash function to here. + (_Hash_table_base::_M_equals): Move static_assert for equality + predicate to here. + * include/bits/stl_tree.h (_Rb_tree::_S_key(_Const_Link_type)): Move + assertions here. Access the value directly instead of calling _S_value. + (_Rb_tree::_S_key(_Const_Base_ptr)): Do downcast and forward to + _S_key(_Const_Link_type). + * testsuite/23_containers/set/85965.cc: Check construction, + destruction, assignment and size() do not trigger the assertions. + * testsuite/23_containers/unordered_set/85965.cc: Likewise. + * testsuite/23_containers/map/48101_neg.cc: Call find and adjust + expected errors. + * testsuite/23_containers/multimap/48101_neg.cc: Likewise. + * testsuite/23_containers/multiset/48101_neg.cc: Likewise. + * testsuite/23_containers/set/48101_neg.cc: Likewise. + * testsuite/23_containers/unordered_map/48101_neg.cc: Likewise. + * testsuite/23_containers/unordered_multimap/48101_neg.cc: Likewise. + * testsuite/23_containers/unordered_multiset/48101_neg.cc: Likewise. + * testsuite/23_containers/unordered_set/48101_neg.cc: Likewise. + 2019-05-23 Jonathan Wakely Backport from mainline diff --git a/libstdc++-v3/include/bits/hashtable.h b/libstdc++-v3/include/bits/hashtable.h index da78c68434f7..d7d9e9940e39 100644 --- a/libstdc++-v3/include/bits/hashtable.h +++ b/libstdc++-v3/include/bits/hashtable.h @@ -1351,12 +1351,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { clear(); _M_deallocate_buckets(); - - static_assert(__is_invocable{}, - "hash function must be invocable with an argument of key type"); - static_assert(__is_invocable{}, - "key equality predicate must be invocable with two arguments of " - "key type"); } template{}, + "hash function must be invocable with an argument of key type"); + return _M_h1()(__k); + } std::size_t _M_bucket_index(const _Key&, __hash_code __c, std::size_t __n) const @@ -1374,7 +1378,11 @@ namespace __detail __hash_code _M_hash_code(const _Key& __k) const - { return _M_h1()(__k); } + { + static_assert(__is_invocable{}, + "hash function must be invocable with an argument of key type"); + return _M_h1()(__k); + } std::size_t _M_bucket_index(const _Key&, __hash_code __c, @@ -1820,6 +1828,9 @@ namespace __detail bool _M_equals(const _Key& __k, __hash_code __c, __node_type* __n) const { + static_assert(__is_invocable{}, + "key equality predicate must be invocable with two arguments of " + "key type"); return _EqualHelper::_S_equals(_M_eq(), this->_M_extract(), __k, __c, __n); } diff --git a/libstdc++-v3/include/bits/stl_tree.h b/libstdc++-v3/include/bits/stl_tree.h index 7545ade3f7bc..4dae1da6a0d5 100644 --- a/libstdc++-v3/include/bits/stl_tree.h +++ b/libstdc++-v3/include/bits/stl_tree.h @@ -765,7 +765,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION static const _Key& _S_key(_Const_Link_type __x) - { return _KeyOfValue()(_S_value(__x)); } + { +#if __cplusplus >= 201103L + // If we're asking for the key we're presumably using the comparison + // object, and so this is a good place to sanity check it. + static_assert(__is_invocable<_Compare&, const _Key&, const _Key&>{}, + "comparison object must be invocable " + "with two arguments of key type"); +# if __cplusplus >= 201703L + // _GLIBCXX_RESOLVE_LIB_DEFECTS + // 2542. Missing const requirements for associative containers + if constexpr (__is_invocable<_Compare&, const _Key&, const _Key&>{}) + static_assert( + is_invocable_v, + "comparison object must be invocable as const"); +# endif // C++17 +#endif // C++11 + + return _KeyOfValue()(*__x->_M_valptr()); + } static _Link_type _S_left(_Base_ptr __x) _GLIBCXX_NOEXCEPT @@ -789,7 +807,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION static const _Key& _S_key(_Const_Base_ptr __x) - { return _KeyOfValue()(_S_value(__x)); } + { return _S_key(static_cast<_Const_Link_type>(__x)); } static _Base_ptr _S_minimum(_Base_ptr __x) _GLIBCXX_NOEXCEPT @@ -974,21 +992,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #endif ~_Rb_tree() _GLIBCXX_NOEXCEPT - { - _M_erase(_M_begin()); - -#if __cplusplus >= 201103L - static_assert(__is_invocable<_Compare&, const _Key&, const _Key&>{}, - "comparison object must be invocable " - "with two arguments of key type"); -# if __cplusplus >= 201703L - // _GLIBCXX_RESOLVE_LIB_DEFECTS - // 2542. Missing const requirements for associative containers - static_assert(is_invocable_v, - "comparison object must be invocable as const"); -# endif // C++17 -#endif // C++11 - } + { _M_erase(_M_begin()); } _Rb_tree& operator=(const _Rb_tree& __x); diff --git a/libstdc++-v3/testsuite/23_containers/map/48101_neg.cc b/libstdc++-v3/testsuite/23_containers/map/48101_neg.cc index 355222058ffd..8a7429c85a8b 100644 --- a/libstdc++-v3/testsuite/23_containers/map/48101_neg.cc +++ b/libstdc++-v3/testsuite/23_containers/map/48101_neg.cc @@ -24,9 +24,13 @@ void test01() { std::map> c; + c.find(1); // { dg-error "here" } std::map> c2; + c2.find(2); // { dg-error "here" } } // { dg-error "_Compare = std::less" "" { target *-*-* } 0 } // { dg-error "_Compare = std::allocator" "" { target *-*-* } 0 } // { dg-error "comparison object must be invocable" "" { target *-*-* } 0 } +// { dg-prune-output "no match for call" } +// { dg-prune-output "invalid conversion" } diff --git a/libstdc++-v3/testsuite/23_containers/multimap/48101_neg.cc b/libstdc++-v3/testsuite/23_containers/multimap/48101_neg.cc index 5f53ccee168e..7bd56cc9c731 100644 --- a/libstdc++-v3/testsuite/23_containers/multimap/48101_neg.cc +++ b/libstdc++-v3/testsuite/23_containers/multimap/48101_neg.cc @@ -24,9 +24,13 @@ void test01() { std::multimap> c; + c.find(1); // { dg-error "here" } std::multimap> c2; + c2.find(2); // { dg-error "here" } } // { dg-error "_Compare = std::less" "" { target *-*-* } 0 } // { dg-error "_Compare = std::allocator" "" { target *-*-* } 0 } // { dg-error "comparison object must be invocable" "" { target *-*-* } 0 } +// { dg-prune-output "no match for call" } +// { dg-prune-output "invalid conversion" } diff --git a/libstdc++-v3/testsuite/23_containers/multiset/48101_neg.cc b/libstdc++-v3/testsuite/23_containers/multiset/48101_neg.cc index 71b90e80951c..f13aa098976f 100644 --- a/libstdc++-v3/testsuite/23_containers/multiset/48101_neg.cc +++ b/libstdc++-v3/testsuite/23_containers/multiset/48101_neg.cc @@ -24,9 +24,12 @@ test01() { std::multiset c; // { dg-error "here" } std::multiset> c2; + c2.find(2); // { dg-error "here" } } // { dg-error "non-const, non-volatile value_type" "" { target *-*-* } 0 } // { dg-error "comparison object must be invocable" "" { target *-*-* } 0 } // { dg-prune-output "std::allocator<.* has no member named " } // { dg-prune-output "must have the same value_type as its allocator" } +// { dg-prune-output "no match for call" } +// { dg-prune-output "invalid conversion" } diff --git a/libstdc++-v3/testsuite/23_containers/set/48101_neg.cc b/libstdc++-v3/testsuite/23_containers/set/48101_neg.cc index e58c0627eb89..4ede0421d903 100644 --- a/libstdc++-v3/testsuite/23_containers/set/48101_neg.cc +++ b/libstdc++-v3/testsuite/23_containers/set/48101_neg.cc @@ -24,9 +24,12 @@ test01() { std::set c; // { dg-error "here" } std::set> c2; + c2.find(2); // { dg-error "here" } } // { dg-error "non-const, non-volatile value_type" "" { target *-*-* } 0 } // { dg-error "comparison object must be invocable" "" { target *-*-* } 0 } // { dg-prune-output "std::allocator<.* has no member named " } // { dg-prune-output "must have the same value_type as its allocator" } +// { dg-prune-output "no match for call" } +// { dg-prune-output "invalid conversion" } diff --git a/libstdc++-v3/testsuite/23_containers/set/85965.cc b/libstdc++-v3/testsuite/23_containers/set/85965.cc index 54d501f6c4fe..7d8f2167519f 100644 --- a/libstdc++-v3/testsuite/23_containers/set/85965.cc +++ b/libstdc++-v3/testsuite/23_containers/set/85965.cc @@ -27,3 +27,12 @@ struct Foo // PR libstdc++/85965 std::set> s; }; + +std::size_t +test01(std::set> s) +{ + // these operations should not require the comparison object + auto copy = s; + copy = s; + return s.size(); +} diff --git a/libstdc++-v3/testsuite/23_containers/unordered_map/48101_neg.cc b/libstdc++-v3/testsuite/23_containers/unordered_map/48101_neg.cc index 6c3092554f3d..8d823dfa4764 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_map/48101_neg.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_map/48101_neg.cc @@ -24,8 +24,10 @@ test01() { using namespace std; unordered_map, hash> c2; + c2.find(2); // { dg-error "here" } } // { dg-error "hash function must be invocable" "" { target *-*-* } 0 } // { dg-error "key equality predicate must be invocable" "" { target *-*-* } 0 } // { dg-prune-output "use of deleted function" } +// { dg-prune-output "no match for call" } diff --git a/libstdc++-v3/testsuite/23_containers/unordered_multimap/48101_neg.cc b/libstdc++-v3/testsuite/23_containers/unordered_multimap/48101_neg.cc index f5de313a8f17..a81615b3607c 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_multimap/48101_neg.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_multimap/48101_neg.cc @@ -24,8 +24,10 @@ test01() { using namespace std; unordered_multimap, hash> c2; + c2.find(2); // { dg-error "here" } } // { dg-error "hash function must be invocable" "" { target *-*-* } 0 } // { dg-error "key equality predicate must be invocable" "" { target *-*-* } 0 } // { dg-prune-output "use of deleted function" } +// { dg-prune-output "no match for call" } diff --git a/libstdc++-v3/testsuite/23_containers/unordered_multiset/48101_neg.cc b/libstdc++-v3/testsuite/23_containers/unordered_multiset/48101_neg.cc index d4e479aaf974..03ddb898d6ce 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_multiset/48101_neg.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_multiset/48101_neg.cc @@ -25,6 +25,7 @@ test01() using namespace std; unordered_multiset> c; // { dg-error "here" } unordered_multiset, hash> c2; + c2.find(2); // { dg-error "here" } } // { dg-error "non-const, non-volatile value_type" "" { target *-*-* } 0 } @@ -32,3 +33,4 @@ test01() // { dg-error "key equality predicate must be invocable" "" { target *-*-* } 0 } // { dg-prune-output "use of deleted function" } // { dg-prune-output "must have the same value_type as its allocator" } +// { dg-prune-output "no match for call" } diff --git a/libstdc++-v3/testsuite/23_containers/unordered_set/48101_neg.cc b/libstdc++-v3/testsuite/23_containers/unordered_set/48101_neg.cc index c7e27c536905..e79d37692485 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_set/48101_neg.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_set/48101_neg.cc @@ -25,6 +25,7 @@ test01() using namespace std; unordered_set> c; // { dg-error "here" } unordered_set, hash> c2; + c2.find(2); // { dg-error "here" } } // { dg-error "non-const, non-volatile value_type" "" { target *-*-* } 0 } @@ -32,3 +33,4 @@ test01() // { dg-error "key equality predicate must be invocable" "" { target *-*-* } 0 } // { dg-prune-output "use of deleted function" } // { dg-prune-output "must have the same value_type as its allocator" } +// { dg-prune-output "no match for call" } diff --git a/libstdc++-v3/testsuite/23_containers/unordered_set/85965.cc b/libstdc++-v3/testsuite/23_containers/unordered_set/85965.cc index 8b90b3699013..8c48fa2a9781 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_set/85965.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_set/85965.cc @@ -27,3 +27,12 @@ struct Foo // PR libstdc++/85965 std::unordered_set, std::hash> u; }; + +std::size_t +test01(std::unordered_set, std::hash> s) +{ + // these operations should not require the comparison object + auto copy = s; + copy = s; + return s.size(); +} -- 2.47.2