Commit 30605cd7 authored by Allison Vacanti's avatar Allison Vacanti
Browse files

Update smart pointer semantics for C++11.

This adds complete move and noexcept semantics to vtkNew, vtkWeakPointer,
and vtkSharedPointer, while doing more robust type-checking. In
particular, this patch may expose bugs of the form:

vtkWeakPointer<vtkRenderer> ren = ...;
vtkWeakPointer<vtkCamera> cam = ...;
cam = ren; // Shouldn't compile: types are unrelated.

since vtkWeakPointer can no longer be constructed from an arbitrary
vtkWeakPointerBase.

Similarly, the comparisons between these objects has been updated.
Comparing unrelated pointers is illegal in C++, and code such as:

vtkSmartPointer<vtkIntArray> array1 = ...;
vtkSmartPointer<vtkFloatArray> array2 = ...;
if (array1 == array2) { ... }

no longer compiles (similar to comparing float* and int*).

All move operations are made noexcept to allow optimizations when stored
in std containers (e.g. std::vector reallocs will now move instead of
copy, bypassing reference counting).

Note that some move operations (eg. move-constructing vtkWeakPointer from
vtkNew, or move-assigning to a vtkSmartPointer) are not implemented, as
they either don't makes sense, or don't provide any advantage over copying.

vtkWeakPointer has been updated to treat vtkNew<T> like T* when
constructing or assigning, similar to vtkSmartPointer.

The enable_if statements have been replaced with static_asserts, since
these will give nicer error messages ("Pointee types are incompatible",
rather than "no matching overload found").
parent acaf10d2
......@@ -123,5 +123,21 @@ int TestNew(int,char *[])
cerr << "Error, comparison of vtkNew object to it's raw pointer fails\n";
}
{
vtkNew<vtkIntArray> testArray1;
vtkNew<vtkIntArray> testArray2(std::move(testArray1));
if (testArray1 || !testArray2)
{
std::cerr << "Error, move construction of vtkNew failed.\n";
error = true;
}
vtkNew<vtkDataArray> testArray3(std::move(testArray2));
if (testArray2 || !testArray3)
{
std::cerr << "Error, move construction of vtkNew failed.\n";
error = true;
}
}
return error ? 1 : 0;
}
......@@ -17,8 +17,8 @@
// Tests instantiations of the vtkSmartPointer class template.
#include "vtkDebugLeaks.h"
#include "vtkFloatArray.h"
#include "vtkIntArray.h"
#include "vtkNew.h"
#include "vtkSmartPointer.h"
#include <vector>
......@@ -30,33 +30,17 @@ int TestSmartPointer(int,char *[])
// Coverage:
unsigned int testbits = 0;
unsigned int correctbits = 0x004d3953;
unsigned int correctbits = 0x00000953;
const char *tests[] = {
"da2 == da3", "da2 != da3", "da2 < da3", "da2 <= da3", "da2 > da3",
"da2 >= da3",
"ia == da3", "ia != da3", "ia < da3", "ia <= da3", "ia > da3", "ia >= da3",
"da2 == ia", "da2 != ia", "da2 < ia", "da2 <= ia", "da2 > ia", "da2 <= ia",
"da2 > ia", "da2 >= ia",
"da1 == 0", "da1 != 0", "da1 < 0", "da1 <= 0", "da1 > 0", "da1 >= 0",
nullptr };
vtkSmartPointer<vtkIntArray> da2(ia);
vtkSmartPointer<vtkFloatArray> da3;
vtkSmartPointer<vtkDataArray> da1(da2);
da1 = ia;
da1 = da2;
testbits = (testbits << 1) | ((da2 == da3) ? 1 : 0);
testbits = (testbits << 1) | ((da2 != da3) ? 1 : 0);
testbits = (testbits << 1) | ((da2 < da3) ? 1 : 0);
testbits = (testbits << 1) | ((da2 <= da3) ? 1 : 0);
testbits = (testbits << 1) | ((da2 > da3) ? 1 : 0);
testbits = (testbits << 1) | ((da2 >= da3) ? 1 : 0);
testbits = (testbits << 1) | ((ia == da3) ? 1 : 0);
testbits = (testbits << 1) | ((ia != da3) ? 1 : 0);
testbits = (testbits << 1) | ((ia < da3) ? 1 : 0);
testbits = (testbits << 1) | ((ia <= da3) ? 1 : 0);
testbits = (testbits << 1) | ((ia > da3) ? 1 : 0);
testbits = (testbits << 1) | ((ia >= da3) ? 1 : 0);
testbits = (testbits << 1) | ((da2 == ia) ? 1 : 0);
testbits = (testbits << 1) | ((da2 != ia) ? 1 : 0);
testbits = (testbits << 1) | ((da2 < ia) ? 1 : 0);
......@@ -97,7 +81,6 @@ int TestSmartPointer(int,char *[])
cerr << "da2 is nullptr!" << "\n";
rval = 1;
}
cout << "IntArray: " << da2 << "\n";
da1 = vtkSmartPointer<vtkDataArray>::NewInstance(ia);
da1.TakeReference(vtkIntArray::New());
vtkSmartPointer<vtkIntArray> da4 =
......@@ -117,5 +100,56 @@ int TestSmartPointer(int,char *[])
rval = 1;
}
// Test move constructors
{
vtkSmartPointer<vtkIntArray> intArray{vtkNew<vtkIntArray>{}};
if (intArray == nullptr ||
intArray->GetReferenceCount() != 1)
{
std::cerr << "Move constructing a vtkSmartPointer from a vtkNew "
"failed.\n";
rval = 1;
}
vtkSmartPointer<vtkIntArray> intArrayCopy(intArray);
if (intArrayCopy != intArray ||
intArray->GetReferenceCount() != 2 ||
intArrayCopy->GetReferenceCount() != 2)
{
std::cerr << "Copy constructing vtkSmartPointer yielded unexpected "
"result.\n";
rval = 1;
}
vtkSmartPointer<vtkIntArray> intArrayMoved(std::move(intArrayCopy));
if (intArrayCopy ||
!intArrayMoved ||
intArrayMoved->GetReferenceCount() != 2)
{
std::cerr << "Move constructing vtkSmartPointer yielded unexpected "
"result.\n";
rval = 1;
}
vtkSmartPointer<vtkDataArray> dataArrayCopy(intArray);
if (dataArrayCopy != intArray ||
intArray->GetReferenceCount() != 3 ||
dataArrayCopy->GetReferenceCount() != 3)
{
std::cerr << "Cast constructing vtkSmartPointer failed.\n";
rval = 1;
}
vtkSmartPointer<vtkDataArray> dataArrayMoved(std::move(intArrayMoved));
if (!dataArrayMoved ||
intArrayMoved ||
dataArrayMoved->GetReferenceCount() != 3)
{
std::cerr << "Cast move-constructing vtkSmartPointer failed.\n";
rval = 1;
}
}
return rval;
}
......@@ -17,7 +17,6 @@
// Tests instantiations of the vtkWeakPointer class template.
#include "vtkDebugLeaks.h"
#include "vtkFloatArray.h"
#include "vtkIntArray.h"
#include "vtkWeakPointer.h"
......@@ -28,33 +27,17 @@ int TestWeakPointer(int,char *[])
// Coverage:
unsigned int testbits = 0;
unsigned int correctbits = 0x004d3953;
unsigned int correctbits = 0x00000953;
const char *tests[] = {
"da2 == da3", "da2 != da3", "da2 < da3", "da2 <= da3", "da2 > da3",
"da2 >= da3",
"ia == da3", "ia != da3", "ia < da3", "ia <= da3", "ia > da3", "ia >= da3",
"da2 == ia", "da2 != ia", "da2 < ia", "da2 <= ia", "da2 > ia", "da2 <= ia",
"da2 > ia", "da2 >= ia",
"da1 == 0", "da1 != 0", "da1 < 0", "da1 <= 0", "da1 > 0", "da1 >= 0",
nullptr };
vtkWeakPointer<vtkIntArray> da2(ia);
vtkWeakPointer<vtkFloatArray> da3;
vtkWeakPointer<vtkDataArray> da1(da2);
da1 = ia;
da1 = da2;
testbits = (testbits << 1) | ((da2 == da3) ? 1 : 0);
testbits = (testbits << 1) | ((da2 != da3) ? 1 : 0);
testbits = (testbits << 1) | ((da2 < da3) ? 1 : 0);
testbits = (testbits << 1) | ((da2 <= da3) ? 1 : 0);
testbits = (testbits << 1) | ((da2 > da3) ? 1 : 0);
testbits = (testbits << 1) | ((da2 >= da3) ? 1 : 0);
testbits = (testbits << 1) | ((ia == da3) ? 1 : 0);
testbits = (testbits << 1) | ((ia != da3) ? 1 : 0);
testbits = (testbits << 1) | ((ia < da3) ? 1 : 0);
testbits = (testbits << 1) | ((ia <= da3) ? 1 : 0);
testbits = (testbits << 1) | ((ia > da3) ? 1 : 0);
testbits = (testbits << 1) | ((ia >= da3) ? 1 : 0);
testbits = (testbits << 1) | ((da2 == ia) ? 1 : 0);
testbits = (testbits << 1) | ((da2 != ia) ? 1 : 0);
testbits = (testbits << 1) | ((da2 < ia) ? 1 : 0);
......@@ -107,11 +90,6 @@ int TestWeakPointer(int,char *[])
cerr << "da2 is nullptr\n";
rval = 1;
}
if (da3 != nullptr)
{
cerr << "da3 is not nullptr\n";
rval = 1;
}
da2 = nullptr;
ia->Delete();
......@@ -122,5 +100,115 @@ int TestWeakPointer(int,char *[])
rval = 1;
}
{
vtkNew<vtkIntArray> array;
vtkWeakPointer<vtkIntArray> intArray(array);
if (array != intArray ||
array->GetReferenceCount() != 1)
{
std::cerr << "Constructing vtkWeakPointer from vtkNew failed.\n";
rval = 1;
}
array.Reset();
if (intArray)
{
std::cerr << "Weak pointer not nullptr\n";
rval = 1;
}
}
{
vtkNew<vtkIntArray> array;
vtkWeakPointer<vtkDataArray> dataArray(array);
if (array != dataArray ||
array->GetReferenceCount() != 1)
{
std::cerr << "Constructing vtkWeakPointer from vtkNew failed.\n";
rval = 1;
}
array.Reset();
if (dataArray)
{
std::cerr << "Weak pointer not nullptr\n";
rval = 1;
}
}
{
vtkNew<vtkIntArray> array;
vtkWeakPointer<vtkIntArray> intArray(array);
vtkWeakPointer<vtkIntArray> intArray2(intArray);
if (array != intArray ||
array != intArray2 ||
array->GetReferenceCount() != 1)
{
std::cerr << "Copy failed.\n";
rval = 1;
}
array.Reset();
if (intArray || intArray2)
{
std::cerr << "Weak pointer not nullptr\n";
rval = 1;
}
}
{
vtkNew<vtkIntArray> array;
vtkWeakPointer<vtkIntArray> intArray(array);
vtkWeakPointer<vtkIntArray> intArray2(std::move(intArray));
if (intArray ||
array != intArray2 ||
array->GetReferenceCount() != 1)
{
std::cerr << "Move failed.\n";
rval = 1;
}
array.Reset();
if (intArray || intArray2)
{
std::cerr << "Weak pointer not nullptr\n";
rval = 1;
}
}
{
vtkNew<vtkIntArray> array;
vtkWeakPointer<vtkIntArray> intArray(array);
vtkWeakPointer<vtkDataArray> dataArray(intArray);
if (array != intArray ||
array != dataArray ||
array->GetReferenceCount() != 1)
{
std::cerr << "Copy failed.\n";
rval = 1;
}
array.Reset();
if (dataArray || intArray)
{
std::cerr << "Weak pointer not nullptr\n";
rval = 1;
}
}
{
vtkNew<vtkIntArray> array;
vtkWeakPointer<vtkIntArray> intArray(array);
vtkWeakPointer<vtkDataArray> dataArray(std::move(intArray));
if (intArray ||
array != dataArray ||
array->GetReferenceCount() != 1)
{
std::cerr << "Move failed.\n";
rval = 1;
}
array.Reset();
if (dataArray)
{
std::cerr << "Weak pointer not nullptr\n";
rval = 1;
}
}
return rval;
}
......@@ -62,7 +62,7 @@ struct StripPointers<vtkSmartPointer<ArrayType>>
};
//------------------------------------------------------------------------------
// Test if a type is complete, or if a specialization exists.
// Test if a type is defined (true) or just forward declared (false).
template <typename T>
struct IsComplete
{
......@@ -71,9 +71,10 @@ private:
template <typename U, std::size_t = sizeof(U)>
static std::true_type impl(U*);
static std::false_type impl(...);
using bool_constant = decltype(impl(std::declval<T*>()));
public:
using value = decltype(impl(std::declval<T*>()));
static constexpr bool value = bool_constant::value;
};
}
......
......@@ -45,30 +45,79 @@
#define vtkNew_h
#include "vtkIOStream.h"
#include "vtkMeta.h" // for IsComplete
#include <type_traits> // for is_base_of
class vtkObjectBase;
template <class T>
class vtkNew
{
/**
* Compile time checking that the class is derived from vtkObjectBase.
*/
void CheckObjectBase(vtkObjectBase*) {}
// Allow other smart pointers friend access:
template <typename U> friend class vtkNew;
template <typename U> friend class vtkSmartPointer;
template <typename U> friend class vtkWeakPointer;
// These static asserts only fire when the function calling CheckTypes is
// used. Thus, this smart pointer class may still be used as a member variable
// with a forward declared T, so long as T is defined by the time the calling
// function is used.
template <typename U = T>
static void CheckTypes() noexcept
{
static_assert(vtk::detail::IsComplete<T>::value,
"vtkNew<T>'s T type has not been defined. Missing include?");
static_assert(vtk::detail::IsComplete<U>::value,
"Cannot store an object with undefined type in "
"vtkNew. Missing include?");
static_assert(std::is_base_of<T, U>::value,
"Argument type is not compatible with vtkNew<T>'s "
"T type.");
static_assert(std::is_base_of<vtkObjectBase, T>::value,
"vtkNew can only be used with subclasses of vtkObjectBase.");
}
public:
/**
* Create a new T on construction.
*/
vtkNew() : Object(T::New())
{
this->CheckObjectBase(this->Object);
vtkNew::CheckTypes();
}
/**
* Move the object into the constructed vtkNew wrapper, stealing its
* reference. The argument is reset to nullptr.
* @{
*/
vtkNew(vtkNew &&o) noexcept
: Object(o.Object)
{
o.Object = nullptr;
}
template <typename U>
vtkNew(vtkNew<U> &&o) noexcept
: Object(o.Object)
{
vtkNew::CheckTypes<U>();
o.Object = nullptr;
}
//@}
//@{
/**
* Deletes reference to instance of T on destruction.
* Deletes reference to instance of T.
*/
~vtkNew()
{
this->Reset();
}
void Reset()
{
T* obj = this->Object;
if (obj)
......@@ -83,7 +132,7 @@ public:
* Enable pointer-like dereference syntax. Returns a pointer to the contained
* object.
*/
T* operator->() const
T* operator->() const noexcept
{
return this->Object;
}
......@@ -95,15 +144,15 @@ public:
* returned. This will happen when the vtkNew object goes out of
* scope for example.
*/
T* GetPointer() const
T* GetPointer() const noexcept
{
return this->Object;
}
T* Get() const
T* Get() const noexcept
{
return this->Object;
}
operator T* () const
operator T* () const noexcept
{
return static_cast<T*>(this->Object);
}
......@@ -114,7 +163,7 @@ public:
* drop to 0 when using the pointer returned.
* This will happen when the vtkNew object goes out of scope for example.
*/
T& operator*() const
T& operator*() const noexcept
{
return *static_cast<T*>(this->Object);
}
......
......@@ -24,93 +24,159 @@
#define vtkSmartPointer_h
#include "vtkSmartPointerBase.h"
#include "vtkNew.h" // For converting New pointers to Smart pointers
#include <type_traits> // for enable_if
#include "vtkNew.h" // for vtkNew.h
#include "vtkMeta.h" // for IsComplete
#include <type_traits> // for is_base_of
#include <utility> // for std::move
template <class T>
class vtkSmartPointer: public vtkSmartPointerBase
{
// These static asserts only fire when the function calling CheckTypes is
// used. Thus, this smart pointer class may still be used as a member variable
// with a forward declared T, so long as T is defined by the time the calling
// function is used.
template <typename U = T>
static void CheckTypes() noexcept
{
static_assert(vtk::detail::IsComplete<T>::value,
"vtkSmartPointer<T>'s T type has not been defined. Missing "
"include?");
static_assert(vtk::detail::IsComplete<U>::value,
"Cannot store an object with undefined type in "
"vtkSmartPointer. Missing include?");
static_assert(std::is_base_of<T, U>::value,
"Argument type is not compatible with vtkSmartPointer<T>'s "
"T type.");
static_assert(std::is_base_of<vtkObjectBase, T>::value,
"vtkSmartPointer can only be used with subclasses of "
"vtkObjectBase.");
}
public:
/**
* Initialize smart pointer to nullptr.
*/
vtkSmartPointer() {}
/**
* Initialize smart pointer to given object.
*/
vtkSmartPointer(T* r): vtkSmartPointerBase(r) {}
/**
* Initialize smart pointer to given object.
*/
vtkSmartPointer(vtkNew<T>& r): vtkSmartPointerBase(r.GetPointer()) {}
vtkSmartPointer() noexcept
: vtkSmartPointerBase()
{
}
/**
* Initialize smart pointer with a new reference to the same object
* referenced by given smart pointer.
* @{
*/
template <class U, typename = typename std::enable_if<
std::is_convertible<U *, T *>::value>::type>
// Need both overloads because the copy-constructor must be non-templated:
vtkSmartPointer(const vtkSmartPointer &r)
: vtkSmartPointerBase(r)
{
}
template <class U>
vtkSmartPointer(const vtkSmartPointer<U> &r)
: vtkSmartPointerBase(r.GetPointer()) {}
: vtkSmartPointerBase(r)
{
vtkSmartPointer::CheckTypes<U>();
}
/* @} **/
#ifndef __VTK_WRAP__
/**
* Move the contents of @a r into @a this.
* @{
*/
template <class U,
typename = typename std::enable_if<std::is_convertible<U*, T*>::value>::type>
// Need both overloads because the move-constructor must be non-templated:
vtkSmartPointer(vtkSmartPointer &&r) noexcept
: vtkSmartPointerBase(std::move(r))
{
}
template <class U>
vtkSmartPointer(vtkSmartPointer<U>&& r) noexcept
: vtkSmartPointerBase(std::move(r)) {}
#endif
: vtkSmartPointerBase(std::move(r))
{
vtkSmartPointer::CheckTypes<U>();
}
/**@}*/
//@{
/**
* Assign object to reference. This removes any reference to an old
* object.
* Initialize smart pointer to given object.
* @{
*/
vtkSmartPointer& operator=(T* r)
vtkSmartPointer(T* r)
: vtkSmartPointerBase(r)
{
this->vtkSmartPointerBase::operator=(r);
return *this;
vtkSmartPointer::CheckTypes();
}
template <typename U>
vtkSmartPointer(const vtkNew<U> &r)
: vtkSmartPointerBase(r.Object)
{ // Create a new reference on copy
vtkSmartPointer::CheckTypes<U>();
}
//@}
/**
* Move the pointer from the vtkNew smart pointer to the new vtkSmartPointer,
* stealing its reference and resetting the vtkNew object to nullptr.
*/
template <typename U>
vtkSmartPointer(vtkNew<U> &&r) noexcept
: vtkSmartPointerBase(r.Object, vtkSmartPointerBase::NoReference{})
{ // Steal the reference on move
vtkSmartPointer::CheckTypes<U>();
r.Object = nullptr;
}
//@{
/**
* Assign object to reference. This removes a reference to an old
* Assign object to reference. This removes any reference to an old
* object.
*/
vtkSmartPointer& operator=(vtkNew<T>& r)
// Need this since the compiler won't recognize template functions as
// assignment operators.
vtkSmartPointer& operator=(const vtkSmartPointer &r)
{
this->vtkSmartPointerBase::operator=(r.GetPointer());
return *this;
}
template <class U>
vtkSmartPointer& operator=(const vtkSmartPointer<U> &r)
{
this->vtkSmartPointerBase::operator=(r);
vtkSmartPointer::CheckTypes<U>();
this->vtkSmartPointerBase::operator=(r.GetPointer());
return *this;
}