[libcxx-commits] [libcxx] r358189 - [libc++] Make sure we don't eagerly diagnose non-const comparators for containers of incomplete types
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Apr 11 09:14:56 PDT 2019
Author: ldionne
Date: Thu Apr 11 09:14:56 2019
New Revision: 358189
URL: http://llvm.org/viewvc/llvm-project?rev=358189&view=rev
Log:
[libc++] Make sure we don't eagerly diagnose non-const comparators for containers of incomplete types
Summary:
In r348529, I improved the library-defined diagnostic for using containers
with a non-const comparator/hasher. However, the check is now performed
too early, which leads to the diagnostic being emitted in cases where it
shouldn't. See PR41360 for details.
This patch moves the diagnostic to the destructor of the containers, which
means that the diagnostic will only be emitted when the container is instantiated
at a point where the comparator and the key/value are required to be complete.
We still retain better diagnostics than before r348529, because the diagnostics
are performed in the containers themselves instead of __tree and __hash_table.
As a drive-by fix, I improved the diagnostic to mention that we can't find
a _viable_ const call operator, as suggested by EricWF in PR41360.
Reviewers: EricWF, mclow.lists
Subscribers: christof, jkorous, dexonsmith, libcxx-commits, zoecarver
Tags: #libc
Differential Revision: https://reviews.llvm.org/D60540
Added:
libcxx/trunk/test/libcxx/containers/associative/non_const_comparator.pass.cpp
libcxx/trunk/test/libcxx/containers/unord/non_const_comparator.pass.cpp
Modified:
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/include/__hash_table
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__hash_table?rev=358189&r1=358188&r2=358189&view=diff
==============================================================================
--- libcxx/trunk/include/__hash_table (original)
+++ libcxx/trunk/include/__hash_table Thu Apr 11 09:14:56 2019
@@ -875,9 +875,9 @@ struct __enforce_unordered_container_req
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")
+ "the specified comparator type does not provide a viable const call operator")
_LIBCPP_DIAGNOSE_WARNING(!__invokable<_Hash const&, _Key const&>::value,
- "the specified hash functor does not provide a const call operator")
+ "the specified hash functor does not provide a viable const call operator")
#endif
typename __enforce_unordered_container_requirements<_Key, _Hash, _Equal>::type
__diagnose_unordered_container_requirements(int);
Modified: libcxx/trunk/include/__tree
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__tree?rev=358189&r1=358188&r2=358189&view=diff
==============================================================================
--- libcxx/trunk/include/__tree (original)
+++ libcxx/trunk/include/__tree Thu Apr 11 09:14:56 2019
@@ -971,7 +971,7 @@ private:
template<class _Tp, class _Compare>
#ifndef _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")
+ "the specified comparator type does not provide a viable const call operator")
#endif
int __diagnose_non_const_comparator();
Modified: libcxx/trunk/include/map
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/map?rev=358189&r1=358188&r2=358189&view=diff
==============================================================================
--- libcxx/trunk/include/map (original)
+++ libcxx/trunk/include/map Thu Apr 11 09:14:56 2019
@@ -907,7 +907,6 @@ 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");
@@ -1086,6 +1085,11 @@ public:
}
_LIBCPP_INLINE_VISIBILITY
+ ~map() {
+ static_assert(sizeof(__diagnose_non_const_comparator<_Key, _Compare>()), "");
+ }
+
+ _LIBCPP_INLINE_VISIBILITY
iterator begin() _NOEXCEPT {return __tree_.begin();}
_LIBCPP_INLINE_VISIBILITY
const_iterator begin() const _NOEXCEPT {return __tree_.begin();}
@@ -1638,7 +1642,6 @@ 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");
@@ -1818,6 +1821,11 @@ public:
}
_LIBCPP_INLINE_VISIBILITY
+ ~multimap() {
+ static_assert(sizeof(__diagnose_non_const_comparator<_Key, _Compare>()), "");
+ }
+
+ _LIBCPP_INLINE_VISIBILITY
iterator begin() _NOEXCEPT {return __tree_.begin();}
_LIBCPP_INLINE_VISIBILITY
const_iterator begin() const _NOEXCEPT {return __tree_.begin();}
Modified: libcxx/trunk/include/set
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/set?rev=358189&r1=358188&r2=358189&view=diff
==============================================================================
--- libcxx/trunk/include/set (original)
+++ libcxx/trunk/include/set Thu Apr 11 09:14:56 2019
@@ -450,7 +450,6 @@ 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");
@@ -597,6 +596,11 @@ public:
#endif // _LIBCPP_CXX03_LANG
_LIBCPP_INLINE_VISIBILITY
+ ~set() {
+ static_assert(sizeof(__diagnose_non_const_comparator<_Key, _Compare>()), "");
+ }
+
+ _LIBCPP_INLINE_VISIBILITY
iterator begin() _NOEXCEPT {return __tree_.begin();}
_LIBCPP_INLINE_VISIBILITY
const_iterator begin() const _NOEXCEPT {return __tree_.begin();}
@@ -938,7 +942,6 @@ 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");
@@ -1084,6 +1087,11 @@ public:
#endif // _LIBCPP_CXX03_LANG
_LIBCPP_INLINE_VISIBILITY
+ ~multiset() {
+ static_assert(sizeof(__diagnose_non_const_comparator<_Key, _Compare>()), "");
+ }
+
+ _LIBCPP_INLINE_VISIBILITY
iterator begin() _NOEXCEPT {return __tree_.begin();}
_LIBCPP_INLINE_VISIBILITY
const_iterator begin() const _NOEXCEPT {return __tree_.begin();}
Modified: libcxx/trunk/include/unordered_map
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/unordered_map?rev=358189&r1=358188&r2=358189&view=diff
==============================================================================
--- libcxx/trunk/include/unordered_map (original)
+++ libcxx/trunk/include/unordered_map Thu Apr 11 09:14:56 2019
@@ -852,7 +852,6 @@ 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;
@@ -963,7 +962,11 @@ public:
const allocator_type& __a)
: unordered_map(__il, __n, __hf, key_equal(), __a) {}
#endif
- // ~unordered_map() = default;
+ _LIBCPP_INLINE_VISIBILITY
+ ~unordered_map() {
+ static_assert(sizeof(__diagnose_unordered_container_requirements<_Key, _Hash, _Pred>(0)), "");
+ }
+
_LIBCPP_INLINE_VISIBILITY
unordered_map& operator=(const unordered_map& __u)
{
@@ -1678,7 +1681,6 @@ 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;
@@ -1787,7 +1789,11 @@ public:
const allocator_type& __a)
: unordered_multimap(__il, __n, __hf, key_equal(), __a) {}
#endif
- // ~unordered_multimap() = default;
+ _LIBCPP_INLINE_VISIBILITY
+ ~unordered_multimap() {
+ static_assert(sizeof(__diagnose_unordered_container_requirements<_Key, _Hash, _Pred>(0)), "");
+ }
+
_LIBCPP_INLINE_VISIBILITY
unordered_multimap& operator=(const unordered_multimap& __u)
{
Modified: libcxx/trunk/include/unordered_set
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/unordered_set?rev=358189&r1=358188&r2=358189&view=diff
==============================================================================
--- libcxx/trunk/include/unordered_set (original)
+++ libcxx/trunk/include/unordered_set Thu Apr 11 09:14:56 2019
@@ -390,7 +390,6 @@ 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;
@@ -486,7 +485,11 @@ public:
: unordered_set(__il, __n, __hf, key_equal(), __a) {}
#endif
#endif // _LIBCPP_CXX03_LANG
- // ~unordered_set() = default;
+ _LIBCPP_INLINE_VISIBILITY
+ ~unordered_set() {
+ static_assert(sizeof(__diagnose_unordered_container_requirements<_Value, _Hash, _Pred>(0)), "");
+ }
+
_LIBCPP_INLINE_VISIBILITY
unordered_set& operator=(const unordered_set& __u)
{
@@ -990,7 +993,6 @@ 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;
@@ -1084,7 +1086,11 @@ public:
: unordered_multiset(__il, __n, __hf, key_equal(), __a) {}
#endif
#endif // _LIBCPP_CXX03_LANG
- // ~unordered_multiset() = default;
+ _LIBCPP_INLINE_VISIBILITY
+ ~unordered_multiset() {
+ static_assert(sizeof(__diagnose_unordered_container_requirements<_Value, _Hash, _Pred>(0)), "");
+ }
+
_LIBCPP_INLINE_VISIBILITY
unordered_multiset& operator=(const unordered_multiset& __u)
{
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=358189&r1=358188&r2=358189&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 Apr 11 09:14:56 2019
@@ -26,8 +26,8 @@ int main(int, char**) {
static_assert(!std::__invokable<BadCompare const&, int const&, int const&>::value, "");
static_assert(std::__invokable<BadCompare&, int const&, int const&>::value, "");
- // 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}}
+ // expected-warning at set:* 2 {{the specified comparator type does not provide a viable const call operator}}
+ // expected-warning at map:* 2 {{the specified comparator type does not provide a viable const call operator}}
{
using C = std::set<int, BadCompare>;
C s;
Added: libcxx/trunk/test/libcxx/containers/associative/non_const_comparator.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/containers/associative/non_const_comparator.pass.cpp?rev=358189&view=auto
==============================================================================
--- libcxx/trunk/test/libcxx/containers/associative/non_const_comparator.pass.cpp (added)
+++ libcxx/trunk/test/libcxx/containers/associative/non_const_comparator.pass.cpp Thu Apr 11 09:14:56 2019
@@ -0,0 +1,58 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++98, c++03
+// REQUIRES: verify-support
+
+// Test that libc++ does not generate a warning diagnostic about the comparator
+// too early for containers of incomplete types.
+//
+// See PR41360.
+
+#include <set>
+#include <map>
+#include <functional>
+
+
+template <template <typename ...> class Container>
+void test_set() {
+ struct KeyBase { };
+ struct KeyDerived; // derives from KeyBase, but incomplete at this point
+
+ // Name the type but don't instantiate it.
+ using C = Container<KeyDerived*, std::less<KeyBase*>>;
+
+ // Instantiate it but don't ODR use any members.
+ typename C::value_type dummy; (void)dummy;
+
+ // Complete the types.
+ struct KeyDerived : KeyBase { };
+
+ C c; // ODR use it, which should be OK
+}
+
+template <template <typename ...> class Container>
+void test_map() {
+ struct Value { };
+ struct KeyBase { };
+ struct KeyDerived;
+ using C = Container<KeyDerived*, Value, std::less<KeyBase*>>;
+ typename C::value_type dummy; (void)dummy;
+ struct KeyDerived : KeyBase { };
+ C c;
+}
+
+int main(int, char**) {
+ // expected-no-diagnostics
+ test_set<std::set>();
+ test_set<std::multiset>();
+ test_map<std::map>();
+ test_map<std::multimap>();
+
+ return 0;
+}
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=358189&r1=358188&r2=358189&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 Apr 11 09:14:56 2019
@@ -33,10 +33,10 @@ int main(int, char**) {
static_assert(!std::__invokable<BadEqual const&, int const&, int const&>::value, "");
static_assert(std::__invokable<BadEqual&, int const&, int const&>::value, "");
- // 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}}
+ // expected-warning at unordered_set:* 2 {{the specified comparator type does not provide a viable const call operator}}
+ // expected-warning at unordered_map:* 2 {{the specified comparator type does not provide a viable const call operator}}
+ // expected-warning at unordered_set:* 2 {{the specified hash functor does not provide a viable const call operator}}
+ // expected-warning at unordered_map:* 2 {{the specified hash functor does not provide a viable const call operator}}
{
using C = std::unordered_set<int, BadHash, BadEqual>;
Added: libcxx/trunk/test/libcxx/containers/unord/non_const_comparator.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/containers/unord/non_const_comparator.pass.cpp?rev=358189&view=auto
==============================================================================
--- libcxx/trunk/test/libcxx/containers/unord/non_const_comparator.pass.cpp (added)
+++ libcxx/trunk/test/libcxx/containers/unord/non_const_comparator.pass.cpp Thu Apr 11 09:14:56 2019
@@ -0,0 +1,58 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++98, c++03
+// REQUIRES: verify-support
+
+// Test that libc++ does not generate a warning diagnostic about the comparator
+// or the hasher too early for containers of incomplete types.
+//
+// See PR41360.
+
+#include <unordered_set>
+#include <unordered_map>
+#include <functional>
+
+
+template <template <typename ...> class Container>
+void test_set() {
+ struct KeyBase { };
+ struct KeyDerived; // derives from KeyBase, but incomplete at this point
+
+ // Name the type but don't instantiate it.
+ using C = Container<KeyDerived*, std::hash<KeyBase*>, std::equal_to<KeyBase*>>;
+
+ // Instantiate it but don't ODR use any members.
+ typename C::value_type dummy; (void)dummy;
+
+ // Complete the types.
+ struct KeyDerived : KeyBase { };
+
+ C c; // ODR use it, which should be OK
+}
+
+template <template <typename ...> class Container>
+void test_map() {
+ struct Value { };
+ struct KeyBase { };
+ struct KeyDerived;
+ using C = Container<KeyDerived*, Value, std::hash<KeyBase*>, std::equal_to<KeyBase*>>;
+ typename C::value_type dummy; (void)dummy;
+ struct KeyDerived : KeyBase { };
+ C c;
+}
+
+int main(int, char**) {
+ // expected-no-disagnostics
+ test_set<std::unordered_set>();
+ test_set<std::unordered_multiset>();
+ test_map<std::unordered_map>();
+ test_map<std::unordered_multimap>();
+
+ return 0;
+}
More information about the libcxx-commits
mailing list