[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

git-svn-id: https://llvm.org/svn/llvm-project/libcxx/trunk@348529 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Louis Dionne 2018-12-06 21:46:17 +00:00
parent 21e47d9ff8
commit 5fe0a6a0bc
9 changed files with 52 additions and 72 deletions

View File

@ -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++

View File

@ -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_destructor<__hash_node<_Tp, _VoidPtr>, _Alloc>
};
#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_;

View File

@ -40,10 +40,6 @@ template <class _Tp, class _VoidPtr> class __tree_node;
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>::~__tree()
{
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());
}

View File

@ -486,7 +486,8 @@ swap(multimap<Key, T, Compare, Allocator>& x,
_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");

View File

@ -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");

View File

@ -414,7 +414,8 @@ template <class Key, class T, class Hash, class Pred, class Alloc>
_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, _Hash, __b>& __x,
__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;

View File

@ -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;

View File

@ -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@__tree:* 4 {{the specified comparator type does not provide a const call operator}}
// expected-warning@set:* 2 {{the specified comparator type does not provide a const call operator}}
// expected-warning@map:* 2 {{the specified comparator type does not provide a const call operator}}
{
using C = std::set<int, BadCompare>;
C s;

View File

@ -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@__hash_table:* 4 {{the specified comparator type does not provide a const call operator}}
// expected-warning@__hash_table:* 4 {{the specified hash functor does not provide a const call operator}}
// expected-warning@unordered_set:* 2 {{the specified comparator type does not provide a const call operator}}
// expected-warning@unordered_map:* 2 {{the specified comparator type does not provide a const call operator}}
// expected-warning@unordered_set:* 2 {{the specified hash functor does not provide a const call operator}}
// expected-warning@unordered_map:* 2 {{the specified hash functor does not provide a const call operator}}
{
using C = std::unordered_set<int, BadHash, BadEqual>;