From 64c986b49558a7c356b85bda85195216936c29a3 Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Wed, 14 Dec 2022 15:21:32 +0000 Subject: [PATCH] libstdc++: Diagnose broken allocator rebind members This adds a static assertion to std::allocator_traits::rebind_alloc to diagnose violations of the rule that rebinding an allocator to its own value type yields the same allocator type. This helps to catch the easy mistake of deriving from std::allocator but forgetting to override the rebind behaviour (no longer an issue in C++20 as std::allocator doesn't have a rebind member that can be inherited). It also catches bugs like in 23_containers/vector/52591.cc where a typo means the rebound allocator is a completely different type. I initially wanted to put this static assert into the body of allocator_traits: static_assert(is_same, _Alloc>::value, "rebind_alloc must be Alloc"); However, this causes a regression in the test for PR libstdc++/72792. It seems that instantiating std::allocator_traits should be allowed for invalid allocator types as long as you don't try to rebind them. To support that, only assert in the __allocator_traits_base::__rebind class template (in both the primary template and the partial specialization). As a result, the bug in 20_util/scoped_allocator/outermost.cc is not diagnosed, because nothing in that test rebinds the allocator. libstdc++-v3/ChangeLog: * include/bits/alloc_traits.h (__allocator_traits_base::__rebind): Add static assert for rebind requirement. * testsuite/20_util/allocator_traits/members/rebind_alloc.cc: Fix invalid rebind member in test allocator. * testsuite/20_util/allocator_traits/requirements/rebind_neg.cc: New test. * testsuite/20_util/scoped_allocator/outermost.cc: Add rebind to test allocator. * testsuite/23_containers/forward_list/48101_neg.cc: Prune new static assert error. * testsuite/23_containers/unordered_multiset/48101_neg.cc: Likewise. * testsuite/23_containers/unordered_set/48101_neg.cc: Likewise. * testsuite/23_containers/vector/52591.cc: Fix typo in rebind. --- libstdc++-v3/include/bits/alloc_traits.h | 17 +++++++++++++++-- .../20_util/allocator_traits/members/rebind_alloc.cc | 11 +++++------ .../allocator_traits/requirements/rebind_neg.cc | 20 ++++++++++++++++++++ .../testsuite/20_util/scoped_allocator/outermost.cc | 8 ++++++++ .../23_containers/forward_list/48101_neg.cc | 1 + .../23_containers/unordered_multiset/48101_neg.cc | 1 + .../23_containers/unordered_set/48101_neg.cc | 1 + libstdc++-v3/testsuite/23_containers/vector/52591.cc | 2 +- 8 files changed, 52 insertions(+), 9 deletions(-) create mode 100644 libstdc++-v3/testsuite/20_util/allocator_traits/requirements/rebind_neg.cc diff --git a/libstdc++-v3/include/bits/alloc_traits.h b/libstdc++-v3/include/bits/alloc_traits.h index 203988a..6eae409 100644 --- a/libstdc++-v3/include/bits/alloc_traits.h +++ b/libstdc++-v3/include/bits/alloc_traits.h @@ -51,12 +51,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION struct __allocator_traits_base { template - struct __rebind : __replace_first_arg<_Tp, _Up> { }; + struct __rebind : __replace_first_arg<_Tp, _Up> + { + static_assert(is_same< + typename __replace_first_arg<_Tp, typename _Tp::value_type>::type, + _Tp>::value, + "allocator_traits::rebind_alloc must be A"); + }; template struct __rebind<_Tp, _Up, __void_t::other>> - { using type = typename _Tp::template rebind<_Up>::other; }; + { + using type = typename _Tp::template rebind<_Up>::other; + + static_assert(is_same< + typename _Tp::template rebind::other, + _Tp>::value, + "allocator_traits::rebind_alloc must be A"); + }; protected: template diff --git a/libstdc++-v3/testsuite/20_util/allocator_traits/members/rebind_alloc.cc b/libstdc++-v3/testsuite/20_util/allocator_traits/members/rebind_alloc.cc index ca2a804..dc2b1af 100644 --- a/libstdc++-v3/testsuite/20_util/allocator_traits/members/rebind_alloc.cc +++ b/libstdc++-v3/testsuite/20_util/allocator_traits/members/rebind_alloc.cc @@ -24,17 +24,16 @@ using std::is_same; template using Rebind = typename std::allocator_traits::template rebind_alloc; -#if __STDC_HOSTED__ -template +template struct HasRebind { using value_type = T; - template struct rebind { using other = std::allocator; }; + template struct rebind { using other = HasRebind; }; }; -static_assert(is_same, long>, - std::allocator>::value, +// Would get HasRebind here if the first template argument is +// replaced instead of using the nested rebind. +static_assert(is_same, long>, HasRebind>::value, "nested alias template is used"); -#endif template struct NoRebind0 { diff --git a/libstdc++-v3/testsuite/20_util/allocator_traits/requirements/rebind_neg.cc b/libstdc++-v3/testsuite/20_util/allocator_traits/requirements/rebind_neg.cc new file mode 100644 index 0000000..a446b59 --- /dev/null +++ b/libstdc++-v3/testsuite/20_util/allocator_traits/requirements/rebind_neg.cc @@ -0,0 +1,20 @@ +// { dg-do compile { target c++11 } } +#include + +// Custom allocator defined with std::allocator, but doesn't provide rebind. +template struct Alloc : std::allocator { }; + +std::vector> v; // { dg-error "here" "" { target c++17_down } } + +// Custom allocator that does provide rebind, but incorrectly. +template struct Alloc2 +{ + using value_type = T; + template struct rebind { using other = Alloc; }; // not Alloc2 + T* allocate(std::size_t n) { return std::allocator().allocate(n); } + void deallocate(T* p, std::size_t n) { std::allocator().deallocate(p, n); } +}; + +std::vector> v2; // { dg-error "here" } + +// { dg-error "static assertion failed: .*rebind_alloc" "" { target *-*-* } 0 } diff --git a/libstdc++-v3/testsuite/20_util/scoped_allocator/outermost.cc b/libstdc++-v3/testsuite/20_util/scoped_allocator/outermost.cc index 7a5d41d..af4d294 100644 --- a/libstdc++-v3/testsuite/20_util/scoped_allocator/outermost.cc +++ b/libstdc++-v3/testsuite/20_util/scoped_allocator/outermost.cc @@ -49,6 +49,14 @@ struct nested_alloc : A template nested_alloc(nested_alloc) { } + // Need to customize rebind, otherwise nested_alloc> gets rebound + // to nested_alloc. + template + struct rebind + { + using other = typename std::allocator_traits::template rebind_alloc; + }; + A& outer_allocator() { return *this; } template diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/48101_neg.cc b/libstdc++-v3/testsuite/23_containers/forward_list/48101_neg.cc index d15918a..195d329 100644 --- a/libstdc++-v3/testsuite/23_containers/forward_list/48101_neg.cc +++ b/libstdc++-v3/testsuite/23_containers/forward_list/48101_neg.cc @@ -28,3 +28,4 @@ test01() // { dg-error "non-const, non-volatile value_type" "" { target *-*-* } 0 } // { dg-prune-output "std::allocator<.* has no member named " } // { dg-prune-output "must have the same value_type as its allocator" } +// { dg-prune-output "rebind_alloc" } diff --git a/libstdc++-v3/testsuite/23_containers/unordered_multiset/48101_neg.cc b/libstdc++-v3/testsuite/23_containers/unordered_multiset/48101_neg.cc index 989bc4e..70babc6 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_multiset/48101_neg.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_multiset/48101_neg.cc @@ -34,3 +34,4 @@ test01() // { dg-prune-output "use of deleted function" } // { dg-prune-output "must have the same value_type as its allocator" } // { dg-prune-output "no match for call" } +// { dg-prune-output "rebind_alloc" } diff --git a/libstdc++-v3/testsuite/23_containers/unordered_set/48101_neg.cc b/libstdc++-v3/testsuite/23_containers/unordered_set/48101_neg.cc index d5b01db..30225f3 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_set/48101_neg.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_set/48101_neg.cc @@ -34,3 +34,4 @@ test01() // { dg-prune-output "use of deleted function" } // { dg-prune-output "must have the same value_type as its allocator" } // { dg-prune-output "no match for call" } +// { dg-prune-output "rebind_alloc" } diff --git a/libstdc++-v3/testsuite/23_containers/vector/52591.cc b/libstdc++-v3/testsuite/23_containers/vector/52591.cc index bd4050d..ea80bb2 100644 --- a/libstdc++-v3/testsuite/23_containers/vector/52591.cc +++ b/libstdc++-v3/testsuite/23_containers/vector/52591.cc @@ -60,7 +60,7 @@ void test02() template struct A2 : std::allocator { - template struct rebind { typedef A1 other; }; + template struct rebind { typedef A2 other; }; A2() = default; template A2(const A2&) { } -- 2.7.4