[libcxx] r348529 - [libc++] Improve diagnostics for non-const comparators and hashers in associative containers
Louis Dionne
ldionne at apple.com
Thu Dec 6 13:46:17 PST 2018
Author: ldionne
Date: Thu Dec 6 13:46:17 2018
New Revision: 348529
URL: http://llvm.org/viewvc/llvm-project?rev=348529&view=rev
Log:
[libc++] Improve diagnostics for non-const comparators and hashers in associative containers
Summary:
When providing a non-const-callable comparator in a map or set, the
warning diagnostic does not include the point of instantiation of
the container that triggered the warning, which makes it difficult
to track down the problem. This commit improves the diagnostic by
placing it directly in the body of the associative container.
The same change is applied to unordered associative containers, which
had a similar problem.
Finally, this commit cleans up the forward declarations of several
map and unordered_map helpers, which are not needed anymore.
<rdar://problem/41370747>
Reviewers: EricWF, mclow.lists
Subscribers: christof, dexonsmith, llvm-commits
Differential Revision: https://reviews.llvm.org/D48955
Modified:
libcxx/trunk/docs/UsingLibcxx.rst
libcxx/trunk/include/__hash_table
libcxx/trunk/include/__tree
libcxx/trunk/include/map
libcxx/trunk/include/set
libcxx/trunk/include/unordered_map
libcxx/trunk/include/unordered_set
libcxx/trunk/test/libcxx/containers/associative/non_const_comparator.fail.cpp
libcxx/trunk/test/libcxx/containers/unord/non_const_comparator.fail.cpp
Modified: libcxx/trunk/docs/UsingLibcxx.rst
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/UsingLibcxx.rst?rev=348529&r1=348528&r2=348529&view=diff
==============================================================================
--- libcxx/trunk/docs/UsingLibcxx.rst (original)
+++ libcxx/trunk/docs/UsingLibcxx.rst Thu Dec 6 13:46:17 2018
@@ -203,8 +203,10 @@ thread safety annotations.
This macro disables the additional diagnostics generated by libc++ using the
`diagnose_if` attribute. These additional diagnostics include checks for:
- * Giving `set`, `map`, `multiset`, `multimap` a comparator which is not
- const callable.
+ * Giving `set`, `map`, `multiset`, `multimap` and their `unordered_`
+ counterparts a comparator which is not const callable.
+ * Giving an unordered associative container a hasher that is not const
+ callable.
**_LIBCPP_NO_VCRUNTIME**:
Microsoft's C and C++ headers are fairly entangled, and some of their C++
Modified: libcxx/trunk/include/__hash_table
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__hash_table?rev=348529&r1=348528&r2=348529&view=diff
==============================================================================
--- libcxx/trunk/include/__hash_table (original)
+++ libcxx/trunk/include/__hash_table Thu Dec 6 13:46:17 2018
@@ -35,15 +35,6 @@ _LIBCPP_BEGIN_NAMESPACE_STD
template <class _Key, class _Tp>
struct __hash_value_type;
-template <class _Key, class _Cp, class _Hash,
- bool = is_empty<_Hash>::value && !__libcpp_is_final<_Hash>::value>
-class __unordered_map_hasher;
-
-template <class _Key, class _Cp, class _Pred,
- bool = is_empty<_Pred>::value && !__libcpp_is_final<_Pred>::value
- >
-class __unordered_map_equal;
-
#ifndef _LIBCPP_CXX03_LANG
template <class _Tp>
struct __is_hash_value_type_imp : false_type {};
@@ -418,7 +409,7 @@ public:
_LIBCPP_DEBUG_MODE(__get_db()->__insert_i(this));
}
- _LIBCPP_INLINE_VISIBILITY
+ _LIBCPP_INLINE_VISIBILITY
__hash_const_iterator(const __non_const_iterator& __x) _NOEXCEPT
: __node_(__x.__node_)
{
@@ -871,35 +862,32 @@ struct __generic_container_node_destruct
};
#endif
+template <class _Key, class _Hash, class _Equal>
+struct __enforce_unordered_container_requirements {
#ifndef _LIBCPP_CXX03_LANG
-template <class _Key, class _Hash, class _Equal, class _Alloc>
-struct __diagnose_hash_table_helper {
- static constexpr bool __trigger_diagnostics()
- _LIBCPP_DIAGNOSE_WARNING(__check_hash_requirements<_Key, _Hash>::value
- && !__invokable<_Hash const&, _Key const&>::value,
- "the specified hash functor does not provide a const call operator")
- _LIBCPP_DIAGNOSE_WARNING(is_copy_constructible<_Equal>::value
- && !__invokable<_Equal const&, _Key const&, _Key const&>::value,
- "the specified comparator type does not provide a const call operator")
- {
static_assert(__check_hash_requirements<_Key, _Hash>::value,
- "the specified hash does not meet the Hash requirements");
+ "the specified hash does not meet the Hash requirements");
static_assert(is_copy_constructible<_Equal>::value,
- "the specified comparator is required to be copy constructible");
- return true;
- }
+ "the specified comparator is required to be copy constructible");
+#endif
+ typedef int type;
};
-template <class _Key, class _Value, class _Hash, class _Equal, class _Alloc>
-struct __diagnose_hash_table_helper<
- __hash_value_type<_Key, _Value>,
- __unordered_map_hasher<_Key, __hash_value_type<_Key, _Value>, _Hash>,
- __unordered_map_equal<_Key, __hash_value_type<_Key, _Value>, _Equal>,
- _Alloc>
-: __diagnose_hash_table_helper<_Key, _Hash, _Equal, _Alloc>
-{
-};
-#endif // _LIBCPP_CXX03_LANG
+template <class _Key, class _Hash, class _Equal>
+#ifndef _LIBCPP_CXX03_LANG
+ _LIBCPP_DIAGNOSE_WARNING(!__invokable<_Equal const&, _Key const&, _Key const&>::value,
+ "the specified comparator type does not provide a const call operator")
+ _LIBCPP_DIAGNOSE_WARNING(!__invokable<_Hash const&, _Key const&>::value,
+ "the specified hash functor does not provide a const call operator")
+#endif
+typename __enforce_unordered_container_requirements<_Key, _Hash, _Equal>::type
+__diagnose_unordered_container_requirements(int);
+
+// This dummy overload is used so that the compiler won't emit a spurious
+// "no matching function for call to __diagnose_unordered_xxx" diagnostic
+// when the overload above causes a hard error.
+template <class _Key, class _Hash, class _Equal>
+int __diagnose_unordered_container_requirements(void*);
template <class _Tp, class _Hash, class _Equal, class _Alloc>
class __hash_table
@@ -963,10 +951,6 @@ private:
typedef allocator_traits<__pointer_allocator> __pointer_alloc_traits;
typedef typename __bucket_list_deleter::pointer __node_pointer_pointer;
-#ifndef _LIBCPP_CXX03_LANG
- static_assert(__diagnose_hash_table_helper<_Tp, _Hash, _Equal, _Alloc>::__trigger_diagnostics(), "");
-#endif
-
// --- Member data begin ---
__bucket_list __bucket_list_;
__compressed_pair<__first_node, __node_allocator> __p1_;
Modified: libcxx/trunk/include/__tree
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__tree?rev=348529&r1=348528&r2=348529&view=diff
==============================================================================
--- libcxx/trunk/include/__tree (original)
+++ libcxx/trunk/include/__tree Thu Dec 6 13:46:17 2018
@@ -40,10 +40,6 @@ template <class _Tp, class _VoidPtr> cla
template <class _Key, class _Value>
struct __value_type;
-template <class _Key, class _CP, class _Compare,
- bool = is_empty<_Compare>::value && !__libcpp_is_final<_Compare>::value>
-class __map_value_compare;
-
template <class _Allocator> class __map_node_destructor;
template <class _TreeIterator> class _LIBCPP_TEMPLATE_VIS __map_iterator;
template <class _TreeIterator> class _LIBCPP_TEMPLATE_VIS __map_const_iterator;
@@ -966,24 +962,12 @@ private:
};
+template<class _Tp, class _Compare>
#ifndef _LIBCPP_CXX03_LANG
-template <class _Tp, class _Compare, class _Allocator>
-struct __diagnose_tree_helper {
- static constexpr bool __trigger_diagnostics()
- _LIBCPP_DIAGNOSE_WARNING(!__invokable<_Compare const&, _Tp const&, _Tp const&>::value,
- "the specified comparator type does not provide a const call operator")
- { return true; }
-};
-
-template <class _Key, class _Value, class _KeyComp, class _Alloc>
-struct __diagnose_tree_helper<
- __value_type<_Key, _Value>,
- __map_value_compare<_Key, __value_type<_Key, _Value>, _KeyComp>,
- _Alloc
-> : __diagnose_tree_helper<_Key, _KeyComp, _Alloc>
-{
-};
-#endif // !_LIBCPP_CXX03_LANG
+ _LIBCPP_DIAGNOSE_WARNING(!std::__invokable<_Compare const&, _Tp const&, _Tp const&>::value,
+ "the specified comparator type does not provide a const call operator")
+#endif
+int __diagnose_non_const_comparator();
template <class _Tp, class _Compare, class _Allocator>
class __tree
@@ -1855,10 +1839,6 @@ __tree<_Tp, _Compare, _Allocator>::~__tr
{
static_assert((is_copy_constructible<value_compare>::value),
"Comparator must be copy-constructible.");
-#ifndef _LIBCPP_CXX03_LANG
- static_assert((__diagnose_tree_helper<_Tp, _Compare, _Allocator>::
- __trigger_diagnostics()), "");
-#endif
destroy(__root());
}
Modified: libcxx/trunk/include/map
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/map?rev=348529&r1=348528&r2=348529&view=diff
==============================================================================
--- libcxx/trunk/include/map (original)
+++ libcxx/trunk/include/map Thu Dec 6 13:46:17 2018
@@ -486,7 +486,8 @@ swap(multimap<Key, T, Compare, Allocator
_LIBCPP_BEGIN_NAMESPACE_STD
-template <class _Key, class _CP, class _Compare, bool _IsSmall>
+template <class _Key, class _CP, class _Compare,
+ bool = is_empty<_Compare>::value && !__libcpp_is_final<_Compare>::value>
class __map_value_compare
: private _Compare
{
@@ -900,6 +901,7 @@ public:
typedef value_type& reference;
typedef const value_type& const_reference;
+ static_assert(sizeof(__diagnose_non_const_comparator<_Key, _Compare>()), "");
static_assert((is_same<typename allocator_type::value_type, value_type>::value),
"Allocator::value_type must be same type as value_type");
@@ -1626,6 +1628,7 @@ public:
typedef value_type& reference;
typedef const value_type& const_reference;
+ static_assert(sizeof(__diagnose_non_const_comparator<_Key, _Compare>()), "");
static_assert((is_same<typename allocator_type::value_type, value_type>::value),
"Allocator::value_type must be same type as value_type");
Modified: libcxx/trunk/include/set
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/set?rev=348529&r1=348528&r2=348529&view=diff
==============================================================================
--- libcxx/trunk/include/set (original)
+++ libcxx/trunk/include/set Thu Dec 6 13:46:17 2018
@@ -445,6 +445,7 @@ public:
typedef value_type& reference;
typedef const value_type& const_reference;
+ static_assert(sizeof(__diagnose_non_const_comparator<_Key, _Compare>()), "");
static_assert((is_same<typename allocator_type::value_type, value_type>::value),
"Allocator::value_type must be same type as value_type");
@@ -925,6 +926,7 @@ public:
typedef value_type& reference;
typedef const value_type& const_reference;
+ static_assert(sizeof(__diagnose_non_const_comparator<_Key, _Compare>()), "");
static_assert((is_same<typename allocator_type::value_type, value_type>::value),
"Allocator::value_type must be same type as value_type");
Modified: libcxx/trunk/include/unordered_map
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/unordered_map?rev=348529&r1=348528&r2=348529&view=diff
==============================================================================
--- libcxx/trunk/include/unordered_map (original)
+++ libcxx/trunk/include/unordered_map Thu Dec 6 13:46:17 2018
@@ -414,7 +414,8 @@ template <class Key, class T, class Hash
_LIBCPP_BEGIN_NAMESPACE_STD
-template <class _Key, class _Cp, class _Hash, bool _IsEmpty>
+template <class _Key, class _Cp, class _Hash,
+ bool = is_empty<_Hash>::value && !__libcpp_is_final<_Hash>::value>
class __unordered_map_hasher
: private _Hash
{
@@ -482,7 +483,8 @@ swap(__unordered_map_hasher<_Key, _Cp, _
__x.swap(__y);
}
-template <class _Key, class _Cp, class _Pred, bool _IsEmpty>
+template <class _Key, class _Cp, class _Pred,
+ bool = is_empty<_Pred>::value && !__libcpp_is_final<_Pred>::value>
class __unordered_map_equal
: private _Pred
{
@@ -845,6 +847,7 @@ public:
typedef const value_type& const_reference;
static_assert((is_same<value_type, typename allocator_type::value_type>::value),
"Invalid allocator::value_type");
+ static_assert(sizeof(__diagnose_unordered_container_requirements<_Key, _Hash, _Pred>(0)), "");
private:
typedef __hash_value_type<key_type, mapped_type> __value_type;
@@ -1667,6 +1670,7 @@ public:
typedef const value_type& const_reference;
static_assert((is_same<value_type, typename allocator_type::value_type>::value),
"Invalid allocator::value_type");
+ static_assert(sizeof(__diagnose_unordered_container_requirements<_Key, _Hash, _Pred>(0)), "");
private:
typedef __hash_value_type<key_type, mapped_type> __value_type;
Modified: libcxx/trunk/include/unordered_set
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/unordered_set?rev=348529&r1=348528&r2=348529&view=diff
==============================================================================
--- libcxx/trunk/include/unordered_set (original)
+++ libcxx/trunk/include/unordered_set Thu Dec 6 13:46:17 2018
@@ -384,6 +384,7 @@ public:
typedef const value_type& const_reference;
static_assert((is_same<value_type, typename allocator_type::value_type>::value),
"Invalid allocator::value_type");
+ static_assert(sizeof(__diagnose_unordered_container_requirements<_Value, _Hash, _Pred>(0)), "");
private:
typedef __hash_table<value_type, hasher, key_equal, allocator_type> __table;
@@ -976,6 +977,7 @@ public:
typedef const value_type& const_reference;
static_assert((is_same<value_type, typename allocator_type::value_type>::value),
"Invalid allocator::value_type");
+ static_assert(sizeof(__diagnose_unordered_container_requirements<_Value, _Hash, _Pred>(0)), "");
private:
typedef __hash_table<value_type, hasher, key_equal, allocator_type> __table;
Modified: libcxx/trunk/test/libcxx/containers/associative/non_const_comparator.fail.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/containers/associative/non_const_comparator.fail.cpp?rev=348529&r1=348528&r2=348529&view=diff
==============================================================================
--- libcxx/trunk/test/libcxx/containers/associative/non_const_comparator.fail.cpp (original)
+++ libcxx/trunk/test/libcxx/containers/associative/non_const_comparator.fail.cpp Thu Dec 6 13:46:17 2018
@@ -27,7 +27,8 @@ int main() {
static_assert(!std::__invokable<BadCompare const&, int const&, int const&>::value, "");
static_assert(std::__invokable<BadCompare&, int const&, int const&>::value, "");
- // expected-warning at __tree:* 4 {{the specified comparator type does not provide a const call operator}}
+ // expected-warning at set:* 2 {{the specified comparator type does not provide a const call operator}}
+ // expected-warning at map:* 2 {{the specified comparator type does not provide a const call operator}}
{
using C = std::set<int, BadCompare>;
C s;
Modified: libcxx/trunk/test/libcxx/containers/unord/non_const_comparator.fail.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/containers/unord/non_const_comparator.fail.cpp?rev=348529&r1=348528&r2=348529&view=diff
==============================================================================
--- libcxx/trunk/test/libcxx/containers/unord/non_const_comparator.fail.cpp (original)
+++ libcxx/trunk/test/libcxx/containers/unord/non_const_comparator.fail.cpp Thu Dec 6 13:46:17 2018
@@ -11,7 +11,7 @@
// REQUIRES: diagnose-if-support, verify-support
// Test that libc++ generates a warning diagnostic when the container is
-// provided a non-const callable comparator.
+// provided a non-const callable comparator or a non-const hasher.
#include <unordered_set>
#include <unordered_map>
@@ -34,8 +34,10 @@ int main() {
static_assert(!std::__invokable<BadEqual const&, int const&, int const&>::value, "");
static_assert(std::__invokable<BadEqual&, int const&, int const&>::value, "");
- // expected-warning at __hash_table:* 4 {{the specified comparator type does not provide a const call operator}}
- // expected-warning at __hash_table:* 4 {{the specified hash functor does not provide a const call operator}}
+ // expected-warning at unordered_set:* 2 {{the specified comparator type does not provide a const call operator}}
+ // expected-warning at unordered_map:* 2 {{the specified comparator type does not provide a const call operator}}
+ // expected-warning at unordered_set:* 2 {{the specified hash functor does not provide a const call operator}}
+ // expected-warning at unordered_map:* 2 {{the specified hash functor does not provide a const call operator}}
{
using C = std::unordered_set<int, BadHash, BadEqual>;
More information about the libcxx-commits
mailing list