[gcc/devel/omp/gcc-9] PR libstdc++/85965 move is_invocable assertions again

Tobias Burnus burnus@gcc.gnu.org
Thu Mar 5 13:52:00 GMT 2020


https://gcc.gnu.org/g:588a873a86fc5a60192da6e5cf3d3247f0c5dc98

commit 588a873a86fc5a60192da6e5cf3d3247f0c5dc98
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri May 24 17:09:28 2019 +0100

    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  <jwakely@redhat.com>
    
    	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

Diff:
---
 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 +++
 .../testsuite/23_containers/multimap/48101_neg.cc  |  4 +++
 .../testsuite/23_containers/multiset/48101_neg.cc  |  3 ++
 .../testsuite/23_containers/set/48101_neg.cc       |  3 ++
 libstdc++-v3/testsuite/23_containers/set/85965.cc  |  9 +++++
 .../23_containers/unordered_map/48101_neg.cc       |  2 ++
 .../23_containers/unordered_multimap/48101_neg.cc  |  2 ++
 .../23_containers/unordered_multiset/48101_neg.cc  |  2 ++
 .../23_containers/unordered_set/48101_neg.cc       |  2 ++
 .../testsuite/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 177491d..ca28ce4 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,3 +1,32 @@
+2019-05-24  Jonathan Wakely  <jwakely@redhat.com>
+
+	Backport from mainline
+	2019-05-17  Jonathan Wakely  <jwakely@redhat.com>
+
+	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  <jwakely@redhat.com>
 
 	Backport from mainline
diff --git a/libstdc++-v3/include/bits/hashtable.h b/libstdc++-v3/include/bits/hashtable.h
index da78c68..d7d9e99 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<const _H1&, const _Key&>{},
-	  "hash function must be invocable with an argument of key type");
-      static_assert(__is_invocable<const _Equal&, const _Key&, const _Key&>{},
-	  "key equality predicate must be invocable with two arguments of "
-	  "key type");
     }
 
   template<typename _Key, typename _Value,
diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h
index b38e8e0..b06c2fb 100644
--- a/libstdc++-v3/include/bits/hashtable_policy.h
+++ b/libstdc++-v3/include/bits/hashtable_policy.h
@@ -1286,7 +1286,11 @@ namespace __detail
 
       __hash_code
       _M_hash_code(const _Key& __k) const
-      { return _M_h1()(__k); }
+      {
+	static_assert(__is_invocable<const _H1&, const _Key&>{},
+	    "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<const _H1&, const _Key&>{},
+	    "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<const _Equal&, const _Key&, const _Key&>{},
+	  "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 7545ade..4dae1da 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<const _Compare&, const _Key&, const _Key&>,
+	      "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<const _Compare&, const _Key&, const _Key&>,
-		      "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 3552220..8a7429c 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<int, int, std::less<int*>> c;
+  c.find(1);  // { dg-error "here" }
   std::map<int, int, std::allocator<int>> c2;
+  c2.find(2); // { dg-error "here" }
 }
 
 // { dg-error "_Compare = std::less<int.>" "" { target *-*-* } 0 }
 // { dg-error "_Compare = std::allocator<int>" "" { 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 5f53cce..7bd56cc 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<int, int, std::less<int*>> c;
+  c.find(1);  // { dg-error "here" }
   std::multimap<int, int, std::allocator<int>> c2;
+  c2.find(2); // { dg-error "here" }
 }
 
 // { dg-error "_Compare = std::less<int.>" "" { target *-*-* } 0 }
 // { dg-error "_Compare = std::allocator<int>" "" { 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 71b90e8..f13aa09 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<const int> c;		   // { dg-error "here" }
   std::multiset<int, std::less<long*>> 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 e58c062..4ede042 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<const int> c;	      // { dg-error "here" }
   std::set<int, std::less<long*>> 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 54d501f..7d8f216 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<Derived*, std::less<Base*>> s;
 };
+
+std::size_t
+test01(std::set<Derived*, std::less<Base*>> 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 6c30925..8d823df 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<int, int, equal_to<int>, hash<int>> 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 f5de313..a81615b 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<int, int, equal_to<int>, hash<int>> 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 d4e479a..03ddb89 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<const int, hash<int>> c;		// { dg-error "here" }
   unordered_multiset<int, equal_to<int>, hash<int>> 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 c7e27c5..e79d376 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<const int, hash<int>> c;	    // { dg-error "here" }
   unordered_set<int, equal_to<int>, hash<int>> 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 8b90b36..8c48fa2 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<Derived*, std::equal_to<Base*>, std::hash<Base*>> u;
 };
+
+std::size_t
+test01(std::unordered_set<Derived*, std::equal_to<Base*>, std::hash<Base*>> s)
+{
+  // these operations should not require the comparison object
+  auto copy = s;
+  copy = s;
+  return s.size();
+}



More information about the Libstdc++-cvs mailing list