[libcxx-commits] [libcxx] [libcxx] improves diagnostics for containers with bad value types (PR #106296)
Christopher Di Bella via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Aug 27 15:36:49 PDT 2024
================
@@ -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");
----------------
cjdb wrote:
Would it be possible to temporarily ship with this complexity, until we get Clang into a good state? Solving this in libc++ doesn't let the compiler off the hook, because it's going to be prevalent in other template libraries, but we can at least offer our users a bit of relief in the meantime. Here are the diagnostics issued when (a) we use what's proposed in this PR, (b) we delete my `__forward_list_base` specialisations, and (c) we put the static_asserts in `__forward_list_base`.
**Code under test**
```cpp
#include <forward_list>
std::forward_list<int&> fl;
```
**Checks go in `forward_list` + `__forward_list_base` specialisations**
```
/home/cjdb/projects/llvm-project/build/include/c++/v1/forward_list:770:17: error: static assertion failed due to requirement '!is_reference<int &>::value': 'std::forward_list' can only hold object types; references are not objects
770 | static_assert(!is_reference<_Tp>::value, "'std::forward_list' can only hold object types; references are not objects");
| ^~~~~~~~~~~~~~~~~~~~~~~~~
hello.cpp:3:25: note: in instantiation of template class 'std::forward_list<int &>' requested here
3 | std::forward_list<int&> x;
| ^
1 error generated.
```
**Checks go in `forward_list` only**
```
In file included from hello.cpp:1:
In file included from /home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:209:
/home/cjdb/projects/llvm2/build/include/c++/v1/__memory/allocator.h:87:17: error: static assertion failed due to requirement '!is_reference<int &>::value': 'std::allocator' can only allocate object types; references are not objects
87 | static_assert(!is_reference<_Tp>::value,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
/home/cjdb/projects/llvm2/build/include/c++/v1/__memory/allocator_traits.h:248:39: note: in instantiation of template class 'std::allocator<int &>' requested here
248 | using value_type = typename allocator_type::value_type;
| ^
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:488:20: note: in instantiation of template class 'std::allocator_traits<std::allocator<int &>>' requested here
488 | typedef typename allocator_traits<allocator_type>::void_pointer void_pointer;
| ^
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:644:51: note: in instantiation of template class 'std::__forward_list_base<int &, std::allocator<int &>>' requested here
644 | class _LIBCPP_TEMPLATE_VIS forward_list : private __forward_list_base<_Tp, _Alloc> {
| ^
hello.cpp:3:25: note: in instantiation of template class 'std::forward_list<int &>' requested here
3 | std::forward_list<int&> x;
| ^
In file included from hello.cpp:1:
In file included from /home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:199:
In file included from /home/cjdb/projects/llvm2/build/include/c++/v1/__algorithm/lexicographical_compare.h:14:
In file included from /home/cjdb/projects/llvm2/build/include/c++/v1/__algorithm/mismatch.h:16:
In file included from /home/cjdb/projects/llvm2/build/include/c++/v1/__algorithm/unwrap_iter.h:14:
/home/cjdb/projects/llvm2/build/include/c++/v1/__memory/pointer_traits.h:177:1: error: no member named 'rebind' in 'std::pointer_traits<int>'
177 | using __rebind_pointer_t = typename pointer_traits<_From>::template rebind<_To>;
| ^~~~~
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:315:1: note: in instantiation of template type alias '__rebind_pointer_t' requested here
315 | using __begin_node_of = __forward_begin_node<__rebind_pointer_t<_VoidPtr, __forward_list_node<_Tp, _VoidPtr> > >;
| ^
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:490:11: note: in instantiation of template type alias '__begin_node_of' requested here
490 | typedef __begin_node_of<value_type, void_pointer> __begin_node;
| ^
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:644:51: note: in instantiation of template class 'std::__forward_list_base<int &, std::allocator<int &>>' requested here
644 | class _LIBCPP_TEMPLATE_VIS forward_list : private __forward_list_base<_Tp, _Alloc> {
| ^
hello.cpp:3:25: note: in instantiation of template class 'std::forward_list<int &>' requested here
3 | std::forward_list<int&> x;
| ^
In file included from hello.cpp:1:
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:648:17: error: static assertion failed due to requirement '!is_reference<int &>::value': 'std::forward_list' can only hold object types; references are not objects2
648 | static_assert(!is_reference<_Tp>::value, "'std::forward_list' can only hold object types; references are not objects2");
| ^~~~~~~~~~~~~~~~~~~~~~~~~
hello.cpp:3:25: note: in instantiation of template class 'std::forward_list<int &>' requested here
3 | std::forward_list<int&> x;
| ^
3 errors generated.
```
**Checks go in `__forward_list_base`**
```
In file included from hello.cpp:1:
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:487:17: error: static assertion failed due to requirement '!is_reference<int &>::value': 'std::forward_list' can only hold object types; references are not objects2
487 | static_assert(!is_reference<_Tp>::value, "'std::forward_list' can only hold object types; references are not objects2");
| ^~~~~~~~~~~~~~~~~~~~~~~~~
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:650:51: note: in instantiation of template class 'std::__forward_list_base<int &, std::allocator<int &>>' requested here
650 | class _LIBCPP_TEMPLATE_VIS forward_list : private __forward_list_base<_Tp, _Alloc> {
| ^
hello.cpp:3:25: note: in instantiation of template class 'std::forward_list<int &>' requested here
3 | std::forward_list<int&> x;
| ^
In file included from hello.cpp:1:
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:652:26: error: no type named '__node_allocator' in 'std::__forward_list_base<int &, std::allocator<int &>>'
652 | typedef typename base::__node_allocator __node_allocator;
| ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
hello.cpp:3:25: note: in instantiation of template class 'std::forward_list<int &>' requested here
3 | std::forward_list<int&> x;
| ^
In file included from hello.cpp:1:
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:653:26: error: no type named '__node_type' in 'std::__forward_list_base<int &, std::allocator<int &>>'
653 | typedef typename base::__node_type __node_type;
| ~~~~~~~~~~~~~~~^~~~~~~~~~~
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:654:26: error: no type named '__node_traits' in 'std::__forward_list_base<int &, std::allocator<int &>>'
654 | typedef typename base::__node_traits __node_traits;
| ~~~~~~~~~~~~~~~^~~~~~~~~~~~~
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:655:26: error: no type named '__node_pointer' in 'std::__forward_list_base<int &, std::allocator<int &>>'
655 | typedef typename base::__node_pointer __node_pointer;
| ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:656:26: error: no type named '__begin_node_pointer' in 'std::__forward_list_base<int &, std::allocator<int &>>'
656 | typedef typename base::__begin_node_pointer __begin_node_pointer;
| ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
In file included from hello.cpp:1:
In file included from /home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:209:
/home/cjdb/projects/llvm2/build/include/c++/v1/__memory/allocator.h:87:17: error: static assertion failed due to requirement '!is_reference<int &>::value': 'std::allocator' can only allocate object types; references are not objects
87 | static_assert(!is_reference<_Tp>::value,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
/home/cjdb/projects/llvm2/build/include/c++/v1/__memory/allocator_traits.h:248:39: note: in instantiation of template class 'std::allocator<int &>' requested here
248 | using value_type = typename allocator_type::value_type;
| ^
/home/cjdb/projects/llvm2/build/include/c++/v1/__memory/allocator_traits.h:378:66: note: in instantiation of template class 'std::allocator_traits<std::allocator<int &>>' requested here
378 | static_assert(is_same<_Alloc, __rebind_alloc<_Traits, typename _Traits::value_type> >::value,
| ^
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:662:17: note: in instantiation of template class 'std::__check_valid_allocator<std::allocator<int &>>' requested here
662 | static_assert(__check_valid_allocator<allocator_type>::value, "");
| ^
hello.cpp:3:25: note: in instantiation of template class 'std::forward_list<int &>' requested here
3 | std::forward_list<int&> x;
| ^
In file included from hello.cpp:1:
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:664:62: error: no type named 'value_type' in 'std::allocator<int &>'
664 | static_assert(is_same<value_type, typename allocator_type::value_type>::value,
| ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~
hello.cpp:3:25: note: in instantiation of template class 'std::forward_list<int &>' requested here
3 | std::forward_list<int&> x;
| ^
In file included from hello.cpp:1:
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:677:26: error: no type named 'iterator' in 'std::__forward_list_base<int &, std::allocator<int &>>'
677 | typedef typename base::iterator iterator;
| ~~~~~~~~~~~~~~~^~~~~~~~
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:678:26: error: no type named 'const_iterator' in 'std::__forward_list_base<int &, std::allocator<int &>>'
678 | typedef typename base::const_iterator const_iterator;
| ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
In file included from hello.cpp:1:
In file included from /home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:199:
In file included from /home/cjdb/projects/llvm2/build/include/c++/v1/__algorithm/lexicographical_compare.h:13:
In file included from /home/cjdb/projects/llvm2/build/include/c++/v1/__algorithm/min.h:14:
In file included from /home/cjdb/projects/llvm2/build/include/c++/v1/__algorithm/min_element.h:16:
In file included from /home/cjdb/projects/llvm2/build/include/c++/v1/__functional/invoke.h:14:
In file included from /home/cjdb/projects/llvm2/build/include/c++/v1/__type_traits/invoke.h:15:
In file included from /home/cjdb/projects/llvm2/build/include/c++/v1/__type_traits/decay.h:15:
In file included from /home/cjdb/projects/llvm2/build/include/c++/v1/__type_traits/is_array.h:14:
In file included from /home/cjdb/projects/llvm2/build/include/c++/v1/cstddef:37:
/home/cjdb/projects/llvm2/build/include/c++/v1/__type_traits/enable_if.h:28:58: error: no type named 'type' in 'std::enable_if<false, int>'; 'enable_if' cannot be used to disable this declaration
28 | using __enable_if_t _LIBCPP_NODEBUG = typename enable_if<_Bp, _Tp>::type;
| ^~~
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:694:13: note: in instantiation of template type alias '__enable_if_t' requested here
694 | template <__enable_if_t<__is_allocator<_Alloc>::value, int> = 0>
| ^
hello.cpp:3:25: note: in instantiation of template class 'std::forward_list<int &>' requested here
3 | std::forward_list<int&> x;
| ^
In file included from hello.cpp:1:
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:789:30: error: multiple overloads of 'push_front' instantiate to the same signature 'void (int &)'
789 | _LIBCPP_HIDE_FROM_ABI void push_front(const value_type& __v);
| ^
hello.cpp:3:25: note: in instantiation of template class 'std::forward_list<int &>' requested here
3 | std::forward_list<int&> x;
| ^
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:787:30: note: previous declaration is here
787 | _LIBCPP_HIDE_FROM_ABI void push_front(value_type&& __v);
| ^
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:809:34: error: multiple overloads of 'insert_after' instantiate to the same signature 'iterator (const_iterator, int &)' (aka 'int (int, int &)')
809 | _LIBCPP_HIDE_FROM_ABI iterator insert_after(const_iterator __p, const value_type& __v);
| ^
/home/cjdb/projects/llvm2/build/include/c++/v1/forward_list:804:34: note: previous declaration is here
804 | _LIBCPP_HIDE_FROM_ABI iterator insert_after(const_iterator __p, value_type&& __v);
| ^
13 errors generated.
```
Each has its drawbacks:
* (a) has the clearest diagnostic, but adds a bit of source complexity for libc++ maintainers
* (b) is not particularly viable, because it talks about `allocator` rather than `forward_list`, and not everyone knows that `Container<T>` is a shorthand for `Container<T, allocator<T>>` (this is why I didn't update just `allocator`).
* (c) is the simplest for maintainers, but produces the most text.
@AaronBallman, do you know how difficult it would be to get Clang to achieve (a)'s diagnostic when the static_assert is embedded in a base class?
https://github.com/llvm/llvm-project/pull/106296
More information about the libcxx-commits
mailing list