[libcxx] improves diagnostics for containers with bad value types

`std::vector<int&>` generates nearly 170 lines of diagnostic, most of
which are redundant, and it's only helpful if you are already familiar
with the rule that containers can't hold reference types. This commit
reduces it to only a handful of lines, all of which are dedicated to
clearly communicating the problem. It also applies this to all the other
containers, and for all non-cv-qualified and non-object types.

These static_asserts are placed at the very top of each container
because they short-circuit further instantiation errors, thereby
leading a smaller set of diagnostics for the programmer. Placing them in
`std::allocator` (which is the common denominator for all containers)
doesn't do the short circuiting, and we thus end up with several
unhelpful diagnostics after the helpful one.

This commit only cleans up things that are already ill-formed. In
particular, `std::map<int&&, int>` should be diagnosed per
[associative.reqmts.general]/p8. It is currently a valid production in
libc++ and libstdc++, but `std::map<int&, int>` is not. As such, only
lvalue references are diagnosed in this commit, and a future commit will
handle everything that is not already rejected by the compiler.

Author: cjdb

libcxx/include/__memory/allocator.h

@@ -16,9 +16,12 @@
 #include <__memory/allocator_traits.h>
 #include <__type_traits/is_const.h>
 #include <__type_traits/is_constant_evaluated.h>
+#include <__type_traits/is_function.h>
+#include <__type_traits/is_reference.h>
 #include <__type_traits/is_same.h>
 #include <__type_traits/is_void.h>
 #include <__type_traits/is_volatile.h>
+#include <__type_traits/remove_reference.h>
 #include <__utility/forward.h>
 #include <cstddef>
 #include <new>
@@ -76,8 +79,16 @@ struct __non_trivial_if<true, _Unique> {
 
 template <class _Tp>
 class _LIBCPP_TEMPLATE_VIS allocator : private __non_trivial_if<!is_void<_Tp>::value, allocator<_Tp> > {
-  static_assert(!is_const<_Tp>::value, "std::allocator does not support const types");
-  static_assert(!is_volatile<_Tp>::value, "std::allocator does not support volatile types");
+  static_assert(!is_const<_Tp>::value, "'std::allocator' can only allocate non-const object types");
+  static_assert(!is_volatile<_Tp>::value, "'std::allocator' can only allocate non-volatile object types");
+  static_assert(!is_reference<_Tp>::value || !is_function<typename remove_reference<_Tp>::type>::value,
+                "'std::allocator' can only allocate object types; function references are not objects (consider using "
+                "a function pointer)");
+  static_assert(!is_reference<_Tp>::value,
+                "'std::allocator' can only allocate object types; references are not objects");
+  static_assert(
+      !is_function<_Tp>::value,
+      "'std::allocator' can only allocate object types; functions are not objects (consider using a function pointer)");
 
 public:
   typedef size_t size_type;

libcxx/include/array

@@ -127,11 +127,15 @@ template <size_t I, class T, size_t N> const T&& get(const array<T, N>&&) noexce
 #include <__type_traits/is_array.h>
 #include <__type_traits/is_const.h>
 #include <__type_traits/is_constructible.h>
+#include <__type_traits/is_function.h>
 #include <__type_traits/is_nothrow_constructible.h>
+#include <__type_traits/is_reference.h>
 #include <__type_traits/is_same.h>
 #include <__type_traits/is_swappable.h>
 #include <__type_traits/is_trivially_relocatable.h>
+#include <__type_traits/is_void.h>
 #include <__type_traits/remove_cv.h>
+#include <__type_traits/remove_reference.h>
 #include <__utility/empty.h>
 #include <__utility/integer_sequence.h>
 #include <__utility/move.h>
@@ -167,6 +171,11 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 
 template <class _Tp, size_t _Size>
 struct _LIBCPP_TEMPLATE_VIS array {
+  static_assert(!is_reference<_Tp>::value || !is_function<typename remove_reference<_Tp>::type>::value, "'std::array' can only hold object types; function references are not objects (consider using a function pointer)");
+  static_assert(!is_reference<_Tp>::value, "'std::array' can only hold object types; references are not objects");
+  static_assert(!is_function<_Tp>::value, "'std::array' can only hold object types; functions are not objects (consider using a function pointer)");
+  static_assert(!is_void<_Tp>::value, "'std::array' can only hold object types; 'void' is not an object");
+
   using __trivially_relocatable = __conditional_t<__libcpp_is_trivially_relocatable<_Tp>::value, array, void>;
 
   // types:

libcxx/include/deque

@@ -212,9 +212,15 @@ template <class T, class Allocator, class Predicate>
 #include <__ranges/size.h>
 #include <__split_buffer>
 #include <__type_traits/is_allocator.h>
+#include <__type_traits/is_const.h>
 #include <__type_traits/is_convertible.h>
+#include <__type_traits/is_function.h>
+#include <__type_traits/is_reference.h>
 #include <__type_traits/is_same.h>
 #include <__type_traits/is_swappable.h>
+#include <__type_traits/is_void.h>
+#include <__type_traits/is_volatile.h>
+#include <__type_traits/remove_reference.h>
 #include <__type_traits/type_identity.h>
 #include <__utility/forward.h>
 #include <__utility/move.h>
@@ -468,6 +474,12 @@ const _DiffType __deque_iterator<_ValueType, _Pointer, _Reference, _MapPointer,
 
 template <class _Tp, class _Allocator /*= allocator<_Tp>*/>
 class _LIBCPP_TEMPLATE_VIS deque {
+  static_assert(!is_const<_Tp>::value, "'std::deque' can only hold non-const types");
+  static_assert(!is_volatile<_Tp>::value, "'std::deque' can only hold non-volatile types");
+  static_assert(!is_reference<_Tp>::value || !is_function<typename remove_reference<_Tp>::type>::value, "'std::deque' can only hold object types; function references are not objects (consider using a function pointer)");
+  static_assert(!is_reference<_Tp>::value, "'std::deque' can only hold object types; references are not objects");
+  static_assert(!is_function<_Tp>::value, "'std::deque' can only hold object types; functions are not objects (consider using a function pointer)");
+  static_assert(!is_void<_Tp>::value, "'std::deque' can only hold object types; 'void' is not an object");
 public:
   // types:
 

libcxx/include/forward_list

@@ -220,11 +220,16 @@ template <class T, class Allocator, class Predicate>
 #include <__type_traits/conditional.h>
 #include <__type_traits/is_allocator.h>
 #include <__type_traits/is_const.h>
+#include <__type_traits/is_function.h>
 #include <__type_traits/is_nothrow_assignable.h>
 #include <__type_traits/is_nothrow_constructible.h>
 #include <__type_traits/is_pointer.h>
+#include <__type_traits/is_reference.h>
 #include <__type_traits/is_same.h>
 #include <__type_traits/is_swappable.h>
+#include <__type_traits/is_void.h>
+#include <__type_traits/is_volatile.h>
+#include <__type_traits/remove_reference.h>
 #include <__type_traits/type_identity.h>
 #include <__utility/forward.h>
 #include <__utility/move.h>
@@ -635,8 +640,136 @@ void __forward_list_base<_Tp, _Alloc>::clear() _NOEXCEPT {
   __before_begin()->__next_ = nullptr;
 }
 
+// begin-diagnostic-helpers
+// std::forward_list can only have non-cv-qualified object types as its value type, which we diagnose.
+// In order to short-cirucit redundant (and cryptic) diagnostics, std::forward_list must be the one
+// to fire the static_assert. Since std::forward_list inherits from __forward_list_base, we also need
+// to create specialisations for the rejected types, so that everything appears "good" to std::forward_list
+// (otherwise we'll get lots of unhelpful diagnostics that suppress the one std::forward_list offers).
+template <class _Tp, class _Alloc>
+class _LIBCPP_TEMPLATE_VIS __forward_list_base<_Tp const, _Alloc> {
+public:
+  using __node_allocator = void*;
+  using __node_alloc_traits = void*;
+  using __node_pointer = void*;
+  using __node_type = void*;
+  using __node_base = void*;
+  using __node_base_pointer = void*;
+  using __link_pointer = void*;
+
+  using pointer = int*;
+  using const_pointer = int const*;
+  using size_type = unsigned int;
+  using difference_type = int;
+  using iterator = int*;
+  using const_iterator = int const*;
+};
+
+template <class _Tp, class _Alloc>
+class _LIBCPP_TEMPLATE_VIS __forward_list_base<_Tp volatile, _Alloc> {
+public:
+  using __node_allocator = void*;
+  using __node_alloc_traits = void*;
+  using __node_pointer = void*;
+  using __node_type = void*;
+  using __node_base = void*;
+  using __node_base_pointer = void*;
+  using __link_pointer = void*;
+
+  using pointer = int*;
+  using const_pointer = int const*;
+  using size_type = unsigned int;
+  using difference_type = int;
+  using iterator = int*;
+  using const_iterator = int const*;
+};
+
+template <class _Tp, class _Alloc>
+class _LIBCPP_TEMPLATE_VIS __forward_list_base<_Tp&, _Alloc> {
+public:
+  using __node_allocator = void*;
+  using __node_alloc_traits = void*;
+  using __node_pointer = void*;
+  using __node_type = void*;
+  using __node_base = void*;
+  using __node_base_pointer = void*;
+  using __link_pointer = void*;
+
+  using pointer = int*;
+  using const_pointer = int const*;
+  using size_type = unsigned int;
+  using difference_type = int;
+  using iterator = int*;
+  using const_iterator = int const*;
+};
+
+template <class _Tp, class _Alloc>
+class _LIBCPP_TEMPLATE_VIS __forward_list_base<_Tp&&, _Alloc> {
+public:
+  using __node_allocator = void*;
+  using __node_alloc_traits = void*;
+  using __node_pointer = void*;
+  using __node_type = void*;
+  using __node_base = void*;
+  using __node_base_pointer = void*;
+  using __link_pointer = void*;
+
+  using pointer = int*;
+  using const_pointer = int const*;
+  using size_type = unsigned int;
+  using difference_type = int;
+  using iterator = int*;
+  using const_iterator = int const*;
+};
+
+template <class _Tp, class... _Args, class _Alloc>
+class _LIBCPP_TEMPLATE_VIS __forward_list_base<_Tp(_Args...), _Alloc> {
+public:
+  using __node_allocator = void*;
+  using __node_alloc_traits = void*;
+  using __node_pointer = void*;
+  using __node_type = void*;
+  using __node_base = void*;
+  using __node_base_pointer = void*;
+  using __link_pointer = void*;
+
+  using pointer = int*;
+  using const_pointer = int const*;
+  using size_type = unsigned int;
+  using difference_type = int;
+  using iterator = int*;
+  using const_iterator = int const*;
+};
+
+template <class _Alloc>
+class _LIBCPP_TEMPLATE_VIS __forward_list_base<void, _Alloc> {
+public:
+  using __node_allocator = void*;
+  using __node_alloc_traits = void*;
+  using __node_pointer = void*;
+  using __node_type = void*;
+  using __node_base = void*;
+  using __node_base_pointer = void*;
+  using __link_pointer = void*;
+
+  using pointer = int*;
+  using const_pointer = int const*;
+  using size_type = unsigned int;
+  using difference_type = int;
+  using iterator = int*;
+  using const_iterator = int const*;
+};
+// end-diagnostic-helpers
+
 template <class _Tp, class _Alloc /*= allocator<_Tp>*/>
 class _LIBCPP_TEMPLATE_VIS forward_list : private __forward_list_base<_Tp, _Alloc> {
+  static_assert(!is_const<_Tp>::value, "'std::forward_list' can only hold non-const types");
+  static_assert(!is_volatile<_Tp>::value, "'std::forward_list' can only hold non-volatile types");
+  static_assert(!is_reference<_Tp>::value || !is_function<typename remove_reference<_Tp>::type>::value, "'std::forward_list' can only hold object types; function references are not objects (consider using a function pointer)");
+  static_assert(!is_reference<_Tp>::value, "'std::forward_list' can only hold object types; references are not objects");
+  static_assert(!is_function<_Tp>::value, "'std::forward_list' can only hold object types; functions are not objects (consider using a function pointer)");
+  static_assert(!is_void<_Tp>::value, "'std::forward_list' can only hold object types; 'void' is not an object");
+
   typedef __forward_list_base<_Tp, _Alloc> base;
   typedef typename base::__node_allocator __node_allocator;
   typedef typename base::__node_type __node_type;

libcxx/include/list

@@ -226,10 +226,16 @@ template <class T, class Allocator, class Predicate>
 #include <__ranges/from_range.h>
 #include <__type_traits/conditional.h>
 #include <__type_traits/is_allocator.h>
+#include <__type_traits/is_const.h>
+#include <__type_traits/is_function.h>
 #include <__type_traits/is_nothrow_assignable.h>
 #include <__type_traits/is_nothrow_constructible.h>
 #include <__type_traits/is_pointer.h>
+#include <__type_traits/is_reference.h>
 #include <__type_traits/is_same.h>
+#include <__type_traits/is_void.h>
+#include <__type_traits/is_volatile.h>
+#include <__type_traits/remove_reference.h>
 #include <__type_traits/type_identity.h>
 #include <__utility/forward.h>
 #include <__utility/move.h>
@@ -659,8 +665,136 @@ void __list_imp<_Tp, _Alloc>::swap(__list_imp& __c)
     __c.__end_.__prev_->__next_ = __c.__end_.__next_->__prev_ = __c.__end_as_link();
 }
 
+// begin-diagnostic-helpers
+// std::list can only have non-cv-qualified object types as its value type, which we diagnose. In order
+// to short-cirucit redundant (and cryptic) diagnostics, std::list must be the one to fire the static_assert.
+// Since std::list inherits from __list_imp, we also need to create specialisations for the rejected types,
+// so that everything appears "good" to std::list (otherwise we'll get lots of unhelpful diagnostics that
+// suppress the one std::list offers).
+template <class _Tp, class _Alloc>
+class _LIBCPP_TEMPLATE_VIS __list_imp<_Tp const, _Alloc> {
+public:
+  using __node_allocator = void*;
+  using __node_alloc_traits = void*;
+  using __node_pointer = void*;
+  using __node_type = void*;
+  using __node_base = void*;
+  using __node_base_pointer = void*;
+  using __link_pointer = void*;
+
+  using pointer = int*;
+  using const_pointer = int const*;
+  using size_type = unsigned int;
+  using difference_type = int;
+  using iterator = int*;
+  using const_iterator = int const*;
+};
+
+template <class _Tp, class _Alloc>
+class _LIBCPP_TEMPLATE_VIS __list_imp<_Tp volatile, _Alloc> {
+public:
+  using __node_allocator = void*;
+  using __node_alloc_traits = void*;
+  using __node_pointer = void*;
+  using __node_type = void*;
+  using __node_base = void*;
+  using __node_base_pointer = void*;
+  using __link_pointer = void*;
+
+  using pointer = int*;
+  using const_pointer = int const*;
+  using size_type = unsigned int;
+  using difference_type = int;
+  using iterator = int*;
+  using const_iterator = int const*;
+};
+
+template <class _Tp, class _Alloc>
+class _LIBCPP_TEMPLATE_VIS __list_imp<_Tp&, _Alloc> {
+public:
+  using __node_allocator = void*;
+  using __node_alloc_traits = void*;
+  using __node_pointer = void*;
+  using __node_type = void*;
+  using __node_base = void*;
+  using __node_base_pointer = void*;
+  using __link_pointer = void*;
+
+  using pointer = int*;
+  using const_pointer = int const*;
+  using size_type = unsigned int;
+  using difference_type = int;
+  using iterator = int*;
+  using const_iterator = int const*;
+};
+
+template <class _Tp, class _Alloc>
+class _LIBCPP_TEMPLATE_VIS __list_imp<_Tp&&, _Alloc> {
+public:
+  using __node_allocator = void*;
+  using __node_alloc_traits = void*;
+  using __node_pointer = void*;
+  using __node_type = void*;
+  using __node_base = void*;
+  using __node_base_pointer = void*;
+  using __link_pointer = void*;
+
+  using pointer = int*;
+  using const_pointer = int const*;
+  using size_type = unsigned int;
+  using difference_type = int;
+  using iterator = int*;
+  using const_iterator = int const*;
+};
+
+template <class _Tp, class... _Args, class _Alloc>
+class _LIBCPP_TEMPLATE_VIS __list_imp<_Tp(_Args...), _Alloc> {
+public:
+  using __node_allocator = void*;
+  using __node_alloc_traits = void*;
+  using __node_pointer = void*;
+  using __node_type = void*;
+  using __node_base = void*;
+  using __node_base_pointer = void*;
+  using __link_pointer = void*;
+
+  using pointer = int*;
+  using const_pointer = int const*;
+  using size_type = unsigned int;
+  using difference_type = int;
+  using iterator = int*;
+  using const_iterator = int const*;
+};
+
+template <class _Alloc>
+class _LIBCPP_TEMPLATE_VIS __list_imp<void, _Alloc> {
+public:
+  using __node_allocator = void*;
+  using __node_alloc_traits = void*;
+  using __node_pointer = void*;
+  using __node_type = void*;
+  using __node_base = void*;
+  using __node_base_pointer = void*;
+  using __link_pointer = void*;
+
+  using pointer = int*;
+  using const_pointer = int const*;
+  using size_type = unsigned int;
+  using difference_type = int;
+  using iterator = int*;
+  using const_iterator = int const*;
+};
+// end-diagnostic-helpers
+
 template <class _Tp, class _Alloc /*= allocator<_Tp>*/>
 class _LIBCPP_TEMPLATE_VIS list : private __list_imp<_Tp, _Alloc> {
+  static_assert(!is_const<_Tp>::value, "'std::list' can only hold non-const types");
+  static_assert(!is_volatile<_Tp>::value, "'std::list' can only hold non-volatile types");
+  static_assert(!is_reference<_Tp>::value || !is_function<typename remove_reference<_Tp>::type>::value, "'std::list' can only hold object types; function references are not objects (consider using a function pointer)");
+  static_assert(!is_reference<_Tp>::value, "'std::list' can only hold object types; references are not objects");
+  static_assert(!is_function<_Tp>::value, "'std::list' can only hold object types; functions are not objects (consider using a function pointer)");
+  static_assert(!is_void<_Tp>::value, "'std::list' can only hold object types; 'void' is not an object");
+
   typedef __list_imp<_Tp, _Alloc> base;
   typedef typename base::__node_type __node_type;
   typedef typename base::__node_allocator __node_allocator;