Add __gnu_test::NullablePointer utility to testsuite

Message ID 20190514112027.GA32384@redhat.com
State New
Headers show
Series
  • Add __gnu_test::NullablePointer utility to testsuite
Related show

Commit Message

Jonathan Wakely May 14, 2019, 11:20 a.m.
* testsuite/20_util/allocator_traits/members/allocate_hint_nonpod.cc:
	Use operator-> to access raw pointer member.
	* testsuite/23_containers/vector/59829.cc: Likewise.
	* testsuite/23_containers/vector/bool/80893.cc: Likewise.
	* testsuite/libstdc++-prettyprinters/cxx11.cc: Use NullablePointer.
	* testsuite/util/testsuite_allocator.h (NullablePointer): New utility
	for tests.
	(PointerBase, PointerBase_void): Derive from NullablePointer and use
	its constructors and equality operators. Change converting
	constructors to use operator-> to access private member of the other
	pointer type.
	(PointerBase_void::operator->()): Add, for access to private member.
	(operator-(PointerBase, PointerBase)): Change to hidden friend.
	(operator==(PointerBase, PointerBase)): Remove.
	(operator!=(PointerBase, PointerBase)): Remove.

There are probably more places in the testsuite that could use
NullablePointer as a minimally-conforming type that meets the
Cpp17NullablePointer requirements, instead of defining a new type each
time one is needed.

Tested powerpc64le-linux, committed to trunk.
commit d238e99b1eea20ba5bb52df6d4544fb4062fb075
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue May 14 11:02:41 2019 +0100

    Add __gnu_test::NullablePointer utility to testsuite
    
            * testsuite/20_util/allocator_traits/members/allocate_hint_nonpod.cc:
            Use operator-> to access raw pointer member.
            * testsuite/23_containers/vector/59829.cc: Likewise.
            * testsuite/23_containers/vector/bool/80893.cc: Likewise.
            * testsuite/libstdc++-prettyprinters/cxx11.cc: Use NullablePointer.
            * testsuite/util/testsuite_allocator.h (NullablePointer): New utility
            for tests.
            (PointerBase, PointerBase_void): Derive from NullablePointer and use
            its constructors and equality operators. Change converting
            constructors to use operator-> to access private member of the other
            pointer type.
            (PointerBase_void::operator->()): Add, for access to private member.
            (operator-(PointerBase, PointerBase)): Change to hidden friend.
            (operator==(PointerBase, PointerBase)): Remove.
            (operator!=(PointerBase, PointerBase)): Remove.

Comments

Daniel Krügler May 14, 2019, 5:09 p.m. | #1
Am Di., 14. Mai 2019 um 13:20 Uhr schrieb Jonathan Wakely <jwakely@redhat.com>:
>

>         * testsuite/20_util/allocator_traits/members/allocate_hint_nonpod.cc:

>         Use operator-> to access raw pointer member.

>         * testsuite/23_containers/vector/59829.cc: Likewise.

>         * testsuite/23_containers/vector/bool/80893.cc: Likewise.

>         * testsuite/libstdc++-prettyprinters/cxx11.cc: Use NullablePointer.

>         * testsuite/util/testsuite_allocator.h (NullablePointer): New utility

>         for tests.

>         (PointerBase, PointerBase_void): Derive from NullablePointer and use

>         its constructors and equality operators. Change converting

>         constructors to use operator-> to access private member of the other

>         pointer type.

>         (PointerBase_void::operator->()): Add, for access to private member.

>         (operator-(PointerBase, PointerBase)): Change to hidden friend.

>         (operator==(PointerBase, PointerBase)): Remove.

>         (operator!=(PointerBase, PointerBase)): Remove.

>

> There are probably more places in the testsuite that could use

> NullablePointer as a minimally-conforming type that meets the

> Cpp17NullablePointer requirements, instead of defining a new type each

> time one is needed.

>

> Tested powerpc64le-linux, committed to trunk.

>


In the primary template of NullablePointer we have:

explicit operator bool() const noexcept { return value == nullptr; }

This is incorrect according to the NullablePointer requirements, it needs to be:

explicit operator bool() const noexcept { return value != nullptr; }

- Daniel
Jonathan Wakely May 14, 2019, 8:01 p.m. | #2
On 14/05/19 19:09 +0200, Daniel Krügler wrote:
>Am Di., 14. Mai 2019 um 13:20 Uhr schrieb Jonathan Wakely <jwakely@redhat.com>:

>>

>>         * testsuite/20_util/allocator_traits/members/allocate_hint_nonpod.cc:

>>         Use operator-> to access raw pointer member.

>>         * testsuite/23_containers/vector/59829.cc: Likewise.

>>         * testsuite/23_containers/vector/bool/80893.cc: Likewise.

>>         * testsuite/libstdc++-prettyprinters/cxx11.cc: Use NullablePointer.

>>         * testsuite/util/testsuite_allocator.h (NullablePointer): New utility

>>         for tests.

>>         (PointerBase, PointerBase_void): Derive from NullablePointer and use

>>         its constructors and equality operators. Change converting

>>         constructors to use operator-> to access private member of the other

>>         pointer type.

>>         (PointerBase_void::operator->()): Add, for access to private member.

>>         (operator-(PointerBase, PointerBase)): Change to hidden friend.

>>         (operator==(PointerBase, PointerBase)): Remove.

>>         (operator!=(PointerBase, PointerBase)): Remove.

>>

>> There are probably more places in the testsuite that could use

>> NullablePointer as a minimally-conforming type that meets the

>> Cpp17NullablePointer requirements, instead of defining a new type each

>> time one is needed.

>>

>> Tested powerpc64le-linux, committed to trunk.

>>

>

>In the primary template of NullablePointer we have:

>

>explicit operator bool() const noexcept { return value == nullptr; }

>

>This is incorrect according to the NullablePointer requirements, it needs to be:

>

>explicit operator bool() const noexcept { return value != nullptr; }


Argh! I changed that to test something and then didn't change it back
before committing.

It looks like we need a testsuite for our testsuite utilities.

Fixed with this patch, tested ppc64le-linux and committed to trunk.

Thanks for the diligent checks!
commit 5c5b3801911e4b47dd496ce024840d390b72023b
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue May 14 20:45:53 2019 +0100

    Fix NullablePointer test utility
    
            * testsuite/util/testsuite_allocator.h (NullablePointer::operator bool):
            Fix return value.

diff --git a/libstdc++-v3/testsuite/util/testsuite_allocator.h b/libstdc++-v3/testsuite/util/testsuite_allocator.h
index ac7dc8ee2c4..d817ac4e838 100644
--- a/libstdc++-v3/testsuite/util/testsuite_allocator.h
+++ b/libstdc++-v3/testsuite/util/testsuite_allocator.h
@@ -600,7 +600,7 @@ namespace __gnu_test
       NullablePointer() = default;
       NullablePointer(std::nullptr_t) noexcept : value() { }
 
-      explicit operator bool() const noexcept { return value == nullptr; }
+      explicit operator bool() const noexcept { return value != nullptr; }
 
       friend inline bool
       operator==(NullablePointer lhs, NullablePointer rhs) noexcept

Patch

diff --git a/libstdc++-v3/testsuite/20_util/allocator_traits/members/allocate_hint_nonpod.cc b/libstdc++-v3/testsuite/20_util/allocator_traits/members/allocate_hint_nonpod.cc
index a5e2a269a15..f9193e83e94 100644
--- a/libstdc++-v3/testsuite/20_util/allocator_traits/members/allocate_hint_nonpod.cc
+++ b/libstdc++-v3/testsuite/20_util/allocator_traits/members/allocate_hint_nonpod.cc
@@ -45,7 +45,7 @@  struct Alloc
   { return pointer(std::allocator<T>().allocate(n)); }
 
   void deallocate(pointer p, std::size_t n)
-  { std::allocator<T>().deallocate(p.value, n); }
+  { std::allocator<T>().deallocate(p.operator->(), n); }
 };
 
 template<typename T>
diff --git a/libstdc++-v3/testsuite/23_containers/vector/59829.cc b/libstdc++-v3/testsuite/23_containers/vector/59829.cc
index 0e053fa6627..892b9055eb4 100644
--- a/libstdc++-v3/testsuite/23_containers/vector/59829.cc
+++ b/libstdc++-v3/testsuite/23_containers/vector/59829.cc
@@ -51,7 +51,7 @@  struct Alloc
   { return pointer(std::allocator<T>().allocate(n)); }
 
   void deallocate(pointer p, std::size_t n)
-  { std::allocator<T>().deallocate(p.value, n); }
+  { std::allocator<T>().deallocate(p.operator->(), n); }
 };
 
 template<typename T>
diff --git a/libstdc++-v3/testsuite/23_containers/vector/bool/80893.cc b/libstdc++-v3/testsuite/23_containers/vector/bool/80893.cc
index f44cdc4a75e..08b15c8d2da 100644
--- a/libstdc++-v3/testsuite/23_containers/vector/bool/80893.cc
+++ b/libstdc++-v3/testsuite/23_containers/vector/bool/80893.cc
@@ -59,7 +59,7 @@  struct Alloc
   void deallocate(pointer p, std::size_t n)
   {
     if (n)
-      std::allocator<T>().deallocate(p.value, n);
+      std::allocator<T>().deallocate(p.operator->(), n);
   }
 };
 
diff --git a/libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx11.cc b/libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx11.cc
index cc588125bdc..c87c8035c45 100644
--- a/libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx11.cc
+++ b/libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx11.cc
@@ -24,6 +24,7 @@ 
 #include <string>
 #include <memory>
 #include <iostream>
+#include "../util/testsuite_allocator.h" // NullablePointer
 
 typedef std::tuple<int, int> ExTuple;
 
@@ -59,21 +60,6 @@  struct datum
 
 std::unique_ptr<datum> global;
 
-struct Deleter
-{
-  // Deleter is not an empty class:
-  int deleter_member = -1;
-  // But pointer is an empty class:
-  struct pointer
-  {
-    pointer(const void* = nullptr) { }
-    explicit operator bool() const noexcept { return false; }
-    friend bool operator==(pointer, pointer) noexcept { return true; }
-    friend bool operator!=(pointer, pointer) noexcept { return false; }
-  };
-  void operator()(pointer) const noexcept { }
-};
-
 int
 main()
 {
@@ -151,6 +137,15 @@  main()
   std::unique_ptr<data>& rarrptr = arrptr;
 // { dg-final { regexp-test rarrptr {std::unique_ptr.datum \[\]. = {get\(\) = 0x.*}} } }
 
+  struct Deleter
+  {
+    int deleter_member = -1;
+    using pointer = __gnu_test::NullablePointer<void>;
+    void operator()(pointer) const noexcept { }
+  };
+  static_assert( !std::is_empty<Deleter>(), "Deleter is not empty" );
+  static_assert( std::is_empty<Deleter::pointer>(), "but pointer is empty" );
+
   std::unique_ptr<int, Deleter> empty_ptr;
 // { dg-final { note-test empty_ptr {std::unique_ptr<int> = {get() = {<No data fields>}}} } }
   std::unique_ptr<int, Deleter>& rempty_ptr = empty_ptr;
diff --git a/libstdc++-v3/testsuite/util/testsuite_allocator.h b/libstdc++-v3/testsuite/util/testsuite_allocator.h
index 0392421ca04..428c0823395 100644
--- a/libstdc++-v3/testsuite/util/testsuite_allocator.h
+++ b/libstdc++-v3/testsuite/util/testsuite_allocator.h
@@ -589,9 +589,54 @@  namespace __gnu_test
       { std::allocator<Tp>::deallocate(std::addressof(*p), n); }
     };
 
+  // A class type meeting *only* the Cpp17NullablePointer requirements.
+  // Can be used as a base class for fancy pointers (like PointerBase, below)
+  // or to wrap a built-in pointer type to remove operations not required
+  // by the Cpp17NullablePointer requirements (dereference, increment etc.)
+  template<typename Ptr>
+    struct NullablePointer
+    {
+      // N.B. default constructor does not initialize value
+      NullablePointer() = default;
+      NullablePointer(std::nullptr_t) noexcept : value() { }
+
+      explicit operator bool() const noexcept { return value == nullptr; }
+
+      friend inline bool
+      operator==(NullablePointer lhs, NullablePointer rhs) noexcept
+      { return lhs.value == rhs.value; }
+
+      friend inline bool
+      operator!=(NullablePointer lhs, NullablePointer rhs) noexcept
+      { return lhs.value != rhs.value; }
+
+    protected:
+      explicit NullablePointer(Ptr p) noexcept : value(p) { }
+      Ptr value;
+    };
+
+  // NullablePointer<void> is an empty type that models Cpp17NullablePointer.
+  template<>
+    struct NullablePointer<void>
+    {
+      NullablePointer() = default;
+      NullablePointer(std::nullptr_t) noexcept { }
+      explicit NullablePointer(const volatile void*) noexcept { }
+
+      explicit operator bool() const noexcept { return false; }
+
+      friend inline bool
+      operator==(NullablePointer, NullablePointer) noexcept
+      { return true; }
+
+      friend inline bool
+      operator!=(NullablePointer, NullablePointer) noexcept
+      { return false; }
+    };
+
   // Utility for use as CRTP base class of custom pointer types
   template<typename Derived, typename T>
-    struct PointerBase
+    struct PointerBase : NullablePointer<T*>
     {
       typedef T element_type;
 
@@ -602,29 +647,38 @@  namespace __gnu_test
       typedef Derived pointer;
       typedef T& reference;
 
-      T* value;
+      using NullablePointer<T*>::NullablePointer;
 
-      explicit PointerBase(T* p = nullptr) : value(p) { }
-
-      PointerBase(std::nullptr_t) : value(nullptr) { }
+      // Public (but explicit) constructor from raw pointer:
+      explicit PointerBase(T* p) noexcept : NullablePointer<T*>(p) { }
 
       template<typename D, typename U,
 	       typename = decltype(static_cast<T*>(std::declval<U*>()))>
-	PointerBase(const PointerBase<D, U>& p) : value(p.value) { }
+	PointerBase(const PointerBase<D, U>& p)
+	: NullablePointer<T*>(p.operator->()) { }
 
-      T& operator*() const { return *value; }
-      T* operator->() const { return value; }
-      T& operator[](difference_type n) const { return value[n]; }
+      T& operator*() const { return *this->value; }
+      T* operator->() const { return this->value; }
+      T& operator[](difference_type n) const { return this->value[n]; }
 
-      Derived& operator++() { ++value; return derived(); }
-      Derived operator++(int) { Derived tmp(derived()); ++value; return tmp; }
-      Derived& operator--() { --value; return derived(); }
-      Derived operator--(int) { Derived tmp(derived()); --value; return tmp; }
+      Derived& operator++() { ++this->value; return derived(); }
+      Derived& operator--() { --this->value; return derived(); }
 
-      Derived& operator+=(difference_type n) { value += n; return derived(); }
-      Derived& operator-=(difference_type n) { value -= n; return derived(); }
+      Derived operator++(int) { return Derived(this->value++); }
 
-      explicit operator bool() const { return value != nullptr; }
+      Derived operator--(int) { return Derived(this->value--); }
+
+      Derived& operator+=(difference_type n)
+      {
+	this->value += n;
+	return derived();
+      }
+
+      Derived& operator-=(difference_type n)
+      {
+	this->value -= n;
+	return derived();
+      }
 
       Derived
       operator+(difference_type n) const
@@ -641,6 +695,9 @@  namespace __gnu_test
       }
 
     private:
+      friend std::ptrdiff_t operator-(PointerBase l, PointerBase r)
+      { return l.value - r.value; }
+
       Derived&
       derived() { return static_cast<Derived&>(*this); }
 
@@ -648,21 +705,9 @@  namespace __gnu_test
       derived() const { return static_cast<const Derived&>(*this); }
     };
 
-    template<typename D, typename T>
-    std::ptrdiff_t operator-(PointerBase<D, T> l, PointerBase<D, T> r)
-    { return l.value - r.value; }
-
-    template<typename D, typename T>
-    bool operator==(PointerBase<D, T> l, PointerBase<D, T> r)
-    { return l.value == r.value; }
-
-    template<typename D, typename T>
-    bool operator!=(PointerBase<D, T> l, PointerBase<D, T> r)
-    { return l.value != r.value; }
-
-    // implementation for void specializations
-    template<typename T>
-    struct PointerBase_void
+  // implementation for pointer-to-void specializations
+  template<typename T>
+    struct PointerBase_void : NullablePointer<T*>
     {
       typedef T element_type;
 
@@ -671,25 +716,24 @@  namespace __gnu_test
       typedef std::ptrdiff_t difference_type;
       typedef std::random_access_iterator_tag iterator_category;
 
-      T* value;
+      using NullablePointer<T*>::NullablePointer;
 
-      explicit PointerBase_void(T* p = nullptr) : value(p) { }
+      T* operator->() const { return this->value; }
 
       template<typename D, typename U,
 	       typename = decltype(static_cast<T*>(std::declval<U*>()))>
-	PointerBase_void(const PointerBase<D, U>& p) : value(p.value) { }
-
-      explicit operator bool() const { return value != nullptr; }
+	PointerBase_void(const PointerBase<D, U>& p)
+	: NullablePointer<T*>(p.operator->()) { }
     };
 
-    template<typename Derived>
+  template<typename Derived>
     struct PointerBase<Derived, void> : PointerBase_void<void>
     {
       using PointerBase_void::PointerBase_void;
       typedef Derived pointer;
     };
 
-    template<typename Derived>
+  template<typename Derived>
     struct PointerBase<Derived, const void> : PointerBase_void<const void>
     {
       using PointerBase_void::PointerBase_void;