[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

llvm-svn: 358189

4 files changed, 122 insertions, 6 deletions

diff --git a/libcxx/test/libcxx/containers/associative/non_const_comparator.fail.cpp b/libcxx/test/libcxx/containers/associative/non_const_comparator.fail.cpp
index 432e7c08d14..9a9a1c6d6d1 100644
--- a/libcxx/test/libcxx/containers/associative/non_const_comparator.fail.cpp
+++ b/libcxx/test/libcxx/containers/associative/non_const_comparator.fail.cpp
@@ -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@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}}
+  // expected-warning@set:* 2 {{the specified comparator type does not provide a viable const call operator}}
+  // expected-warning@map:* 2 {{the specified comparator type does not provide a viable const call operator}}
   {
     using C = std::set<int, BadCompare>;
     C s;
diff --git a/libcxx/test/libcxx/containers/associative/non_const_comparator.pass.cpp b/libcxx/test/libcxx/containers/associative/non_const_comparator.pass.cpp
new file mode 100644
index 00000000000..83a1cee1127
--- /dev/null
+++ b/libcxx/test/libcxx/containers/associative/non_const_comparator.pass.cpp
@@ -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;
+}
diff --git a/libcxx/test/libcxx/containers/unord/non_const_comparator.fail.cpp b/libcxx/test/libcxx/containers/unord/non_const_comparator.fail.cpp
index 8fa500e45e4..c1027adbc8e 100644
--- a/libcxx/test/libcxx/containers/unord/non_const_comparator.fail.cpp
+++ b/libcxx/test/libcxx/containers/unord/non_const_comparator.fail.cpp
@@ -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@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}}
+  // expected-warning@unordered_set:* 2 {{the specified comparator type does not provide a viable const call operator}}
+  // expected-warning@unordered_map:* 2 {{the specified comparator type does not provide a viable const call operator}}
+  // expected-warning@unordered_set:* 2 {{the specified hash functor does not provide a viable const call operator}}
+  // expected-warning@unordered_map:* 2 {{the specified hash functor does not provide a viable const call operator}}
 
   {
     using C = std::unordered_set<int, BadHash, BadEqual>;
diff --git a/libcxx/test/libcxx/containers/unord/non_const_comparator.pass.cpp b/libcxx/test/libcxx/containers/unord/non_const_comparator.pass.cpp
new file mode 100644
index 00000000000..e2eeda53964
--- /dev/null
+++ b/libcxx/test/libcxx/containers/unord/non_const_comparator.pass.cpp
@@ -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;
+}