[libcxx-commits] [libcxx] [llvm] [libcxx] improves diagnostics for containers with bad value types (PR #106296)

[Christopher Di Bella via libcxx-commits]

================
@@ -0,0 +1,94 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP___TYPE_TRAITS_DIAGNOSTIC_UTILITIES_H
+#define _LIBCPP___TYPE_TRAITS_DIAGNOSTIC_UTILITIES_H
+
+#include <__config>
+#include <__type_traits/decay.h>
+#include <__type_traits/integral_constant.h>
+#include <__type_traits/is_bounded_array.h>
+#include <__type_traits/is_const.h>
+#include <__type_traits/is_function.h>
+#include <__type_traits/is_reference.h>
+#include <__type_traits/is_same.h>
+#include <__type_traits/is_unbounded_array.h>
+#include <__type_traits/is_void.h>
+#include <__type_traits/is_volatile.h>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#  pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+// Many templates require their type parameters to be cv-unqualified objects.
+template <template <class...> class _Template, class _Tp, bool = is_same<__decay_t<_Tp>, _Tp>::value>
+struct __requires_cv_unqualified_object_type : true_type {};
+
+#define _LIBCPP_DEFINE__REQUIRES_CV_UNQUALIFIED_OBJECT_TYPE(_Template, _Verb)                                          \
+  template <class _Tp>                                                                                                 \
+  struct __requires_cv_unqualified_object_type<_Template, _Tp, false>                                                  \
----------------
cjdb wrote:

> Is there a reason why the patch now defines these `__requires_cv_unqualified_object_type` specializations for each container? Was that in response to the comment about too many instantiations?

Yes. I ended up benchmarking Chrome's build times and while we could probably tolerate this patch in isolation, stacking similar changes will eventually tank build times. 

> This adds complexity but I don't see the benefit (yet), I'd rather keep this patch as simple as can be. After all the goal here is to issue diagnostics -- that's a fairly simple problem and I'd like to keep the solution accordingly simple.

Issuing simple diagnostics is simple. The goal of this patch is to issue diagnostics that are informative to the reader, using language that they're likely to understand; achieving that without adversely impacting build times is apparently more challenging to simplify.

Could you help me understand what it is about the complexity that raises concern, please?

https://github.com/llvm/llvm-project/pull/106296