From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2181) id 2031B3850610; Fri, 16 Dec 2022 21:00:11 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2031B3850610 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1671224411; bh=7ArxRKrs7IDHo5RkBLd//ZxGDVFlVgLrALZD/0MZU9U=; h=From:To:Subject:Date:From; b=xGHv+Hgz+vlco84pldPkNKLwl3ry9de9SA3Q/8c96qNTVcOigKWXjbNXbCcwRnEKs yVVNmiVUPzB3JbQpyr2dw/i7523zWHD3KHVi7kG5pxfdDA6F0XEeaGKsXrQKc3//tV QWuHJnek5NeEXDnxSEQ1rpsXrJBfP8wMLB3ELy6E= MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Jonathan Wakely To: gcc-cvs@gcc.gnu.org, libstdc++-cvs@gcc.gnu.org Subject: [gcc r13-4752] libstdc++: Diagnose broken allocator rebind members X-Act-Checkin: gcc X-Git-Author: Jonathan Wakely X-Git-Refname: refs/heads/master X-Git-Oldrev: 0b1d66658ecdcc3d9251641a0b902b4c73ace303 X-Git-Newrev: 64c986b49558a7c356b85bda85195216936c29a3 Message-Id: <20221216210011.2031B3850610@sourceware.org> Date: Fri, 16 Dec 2022 21:00:11 +0000 (GMT) List-Id: https://gcc.gnu.org/g:64c986b49558a7c356b85bda85195216936c29a3 commit r13-4752-g64c986b49558a7c356b85bda85195216936c29a3 Author: Jonathan Wakely Date: Wed Dec 14 15:21:32 2022 +0000 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. Diff: --- 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(-) diff --git a/libstdc++-v3/include/bits/alloc_traits.h b/libstdc++-v3/include/bits/alloc_traits.h index 203988ab933..6eae409ab53 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 ca2a8044665..dc2b1afa2d5 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 00000000000..a446b592edf --- /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 7a5d41d2472..af4d29443f5 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 d15918a0e74..195d32931c1 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 989bc4e75c5..70babc682ae 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 d5b01dbfaea..30225f3d6e1 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 bd4050d51e6..ea80bb277c1 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&) { }