libstdc++: Diagnose broken allocator rebind members
authorJonathan Wakely <jwakely@redhat.com>
Wed, 14 Dec 2022 15:21:32 +0000 (15:21 +0000)
committerJonathan Wakely <jwakely@redhat.com>
Fri, 16 Dec 2022 20:58:09 +0000 (20:58 +0000)
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<rebind_alloc<value_type>, _Alloc>::value,
    "rebind_alloc<value_type> 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
libstdc++-v3/testsuite/20_util/allocator_traits/members/rebind_alloc.cc
libstdc++-v3/testsuite/20_util/allocator_traits/requirements/rebind_neg.cc [new file with mode: 0644]
libstdc++-v3/testsuite/20_util/scoped_allocator/outermost.cc
libstdc++-v3/testsuite/23_containers/forward_list/48101_neg.cc
libstdc++-v3/testsuite/23_containers/unordered_multiset/48101_neg.cc
libstdc++-v3/testsuite/23_containers/unordered_set/48101_neg.cc
libstdc++-v3/testsuite/23_containers/vector/52591.cc

index 203988a..6eae409 100644 (file)
@@ -51,12 +51,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   struct __allocator_traits_base
   {
     template<typename _Tp, typename _Up, typename = void>
-      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<A>::rebind_alloc<A::value_type> must be A");
+      };
 
     template<typename _Tp, typename _Up>
       struct __rebind<_Tp, _Up,
                      __void_t<typename _Tp::template rebind<_Up>::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<typename _Tp::value_type>::other,
+                       _Tp>::value,
+         "allocator_traits<A>::rebind_alloc<A::value_type> must be A");
+      };
 
   protected:
     template<typename _Tp>
index ca2a804..dc2b1af 100644 (file)
@@ -24,17 +24,16 @@ using std::is_same;
 template<typename T, typename U>
   using Rebind = typename std::allocator_traits<T>::template rebind_alloc<U>;
 
-#if __STDC_HOSTED__
-template<typename T>
+template<typename T, typename = T>
   struct HasRebind {
     using value_type = T;
-    template<typename U> struct rebind { using other = std::allocator<U>; };
+    template<typename U> struct rebind { using other = HasRebind<U>; };
   };
 
-static_assert(is_same<Rebind<HasRebind<int>, long>,
-                     std::allocator<long>>::value,
+// Would get HasRebind<long, int> here if the first template argument is
+// replaced instead of using the nested rebind.
+static_assert(is_same<Rebind<HasRebind<int>, long>, HasRebind<long>>::value,
              "nested alias template is used");
-#endif
 
 template<typename T>
   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 (file)
index 0000000..a446b59
--- /dev/null
@@ -0,0 +1,20 @@
+// { dg-do compile { target c++11 } }
+#include <vector>
+
+// Custom allocator defined with std::allocator, but doesn't provide rebind.
+template<typename T> struct Alloc : std::allocator<T> { };
+
+std::vector<int, Alloc<int>> v; // { dg-error "here" "" { target c++17_down } }
+
+// Custom allocator that does provide rebind, but incorrectly.
+template<typename T> struct Alloc2
+{
+  using value_type = T;
+  template<typename U> struct rebind { using other = Alloc<U>; }; // not Alloc2
+  T* allocate(std::size_t n) { return std::allocator<T>().allocate(n); }
+  void deallocate(T* p, std::size_t n) { std::allocator<T>().deallocate(p, n); }
+};
+
+std::vector<int, Alloc2<int>> v2; // { dg-error "here" }
+
+// { dg-error "static assertion failed: .*rebind_alloc" "" { target *-*-* } 0 }
index 7a5d41d..af4d294 100644 (file)
@@ -49,6 +49,14 @@ struct nested_alloc : A
   template<typename U>
     nested_alloc(nested_alloc<U>) { }
 
+  // Need to customize rebind, otherwise nested_alloc<alloc<T>> gets rebound
+  // to nested_alloc<U>.
+  template<typename U>
+    struct rebind
+    {
+      using other = typename std::allocator_traits<A>::template rebind_alloc<U>;
+    };
+
   A& outer_allocator() { return *this; }
 
   template<typename U, typename... Args>
index d15918a..195d329 100644 (file)
@@ -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" }
index 989bc4e..70babc6 100644 (file)
@@ -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" }
index d5b01db..30225f3 100644 (file)
@@ -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" }
index bd4050d..ea80bb2 100644 (file)
@@ -60,7 +60,7 @@ void test02()
 template<typename T>
 struct A2 : std::allocator<T>
 {
-  template<typename U> struct rebind { typedef A1<U> other; };
+  template<typename U> struct rebind { typedef A2<U> other; };
 
   A2() = default;
   template<typename U> A2(const A2<U>&) { }