Re: do not allow implicit conversion in constructor
On 23.07.14 00.04, Christopher Pisz wrote:
I am working on some code where yet another developer in the past
decided to roll their own reference counting pointer. Like every time
I've run across this in the past, it is broken, sure enough.
:-)
The problem arises when a comparison statement like
if (theDevilsRefCountedHandle != 0)
is made.
In debugging, I find that rather than do the intended comparison of
underlying raw pointers, the compiler wants to create another object by
calling
TheDevilsRefCountedHandle(RefCountedObjectBody * pBody)
:
mRefCountedObjectBodyPointer(pBody)
Is there a handy dandy way, pre C++'11 to say, don't do that conversion
from integral value to pointer and call a copy instructor, look for
another operator instead?
You can use the explicit keyword on the constructor. However, there may
be drawbacks.
However, it should not make any serious semantic difference.
and what operator can I implement to handle the intended comparison?
I wonder, since I never had this kind of problem.
Does the smart pointer implement operator T*()? Conversion operators
almost always cause errors.
You could try to implement
struct unspecifiedType;
friend bool operator==(const mRefCountedObjectBodyPointer<T>&,
const unspecifiedType*);
and so on. But I am unsure whether this could become ambiguous if the
first argument needs an implicit conversion to match. E.g. a const
conversion.
operator bool maybe to convert the lhs to a boolean before the compare
is done?
Another conversion operator is likely to make things even worse. Since
bool converts to int, you might pass pointers almost everywhere.
I am using the following signature. It has been quite reliable for years.
I added some helpers to disambiguate calls. AFAIR the are only needed to
prefer const over volatile.
OK, it is an intrusive counter, but this should not make any difference
here.
/** @details This is a simple and highly efficient reference counted
smart pointer
* implementation for objects of type T.
* The class is similar to boost::intrusive_ptr but works on very old
C++ compilers.
* The implementation is strongly thread-safe for volatile instances
and wait-free.
* Non-volatile instances are binary compatible to T* const.
* All objects of type T must implement a function called
access_counter() that
* provides access to the reference counter. The easiest way to do so
is to derive
* from Iref_count.
* @note Note that all objects of type T MUST be aligned to
INT_PTR_ALIGNMENT in memory
* to be used in volatile instances! This is normally sizeof(int). */
template <class T>
class int_ptr
{private:
T* Data;
private:
/// Strongly thread safe read
T* acquire() volatile const;
/// Destructor core
static void release(T* data);
/// Transfer hold count to the main counter and return the data with
hold count 0.
static T* transfer(T* data);
/// Raw initialization
struct uninitialized_tag {};
explicit int_ptr(T* data, uninitialized_tag) : Data(data) {
P0ASSERT(data); }
public:
/// Initialize a NULL pointer.
int_ptr() : Data(NULL) {}
/// Store a new or an existing object under reference count control.
int_ptr(T* ptr) : Data(ptr) { if (Data)
InterlockedAdd(&Data->access_counter(), INT_PTR_ALIGNMENT); }
/// Helper to disambiguate calls.
int_ptr(int_ptr<T>& r) : Data(r.Data) { if (Data)
InterlockedAdd(&Data->access_counter(), INT_PTR_ALIGNMENT); }
/// Copy constructor
int_ptr(const int_ptr<T>& r) : Data(r.Data) { if (Data)
InterlockedAdd(&Data->access_counter(), INT_PTR_ALIGNMENT); }
/// Copy constructor, strongly thread-safe.
int_ptr(volatile const int_ptr<T>& r) : Data(r.acquire()) {}
/// Destructor, frees the stored object if this is the last reference.
~int_ptr() { release(Data); }
/// swap instances (not thread safe)
void swap(int_ptr<T>& r) { T* temp = r.Data; r.Data =
Data; Data = temp; }
/// Strongly thread safe swap
void swap(volatile int_ptr<T>& r) { Data =
transfer(InterlockedXch(&r.Data, Data)); }
/// Strongly thread safe swap
void swap(int_ptr<T>& r) volatile { r.swap(*this); }
/// reset the current instance to NULL
void reset() { T* d = Data; Data = 0;
release(d); }
/// reset the current instance to NULL, strongly thread-safe.
void reset() volatile { release(InterlockedXch(&Data,
0)); }
/// @brief Atomic compare and swap.
/// @details replaces the current value of this instance by \a newval
/// if and only if the current value equals \a oldval.
/// @param oldval Old object to be replaced.
/// @param newval New object to be assigned.
/// @return Returns true if the swap has been done, i.e. the value
changed from
/// \a oldval to \a newval. Otherwise if the current value is no
longer \a oldval
/// nothing is changed and the return value is false.
/// @remarks The pointers \a oldval and \a newval must be valid
before and after the call,
/// regardless of the result. I.e. you must hold strong references to
both values.
bool cmpassign(T* oldval, T* newval) volatile;
// Basic operators
T* get() const { return Data; }
operator T*() const { return Data; }
bool operator!() const { return !Data; }
bool operator!() const volatile { return !((unsigned)Data &
INT_PTR_POINTER_MASK); }
T& operator*() const { ASSERT(Data); return *Data; }
T* operator->() const { ASSERT(Data); return Data; }
// assignment
int_ptr<T>& operator=(T* ptr) { int_ptr<T>(ptr).swap(*this);
return *this; }
int_ptr<T>& operator=(int_ptr<T>& r) { int_ptr<T>(r).swap(*this);
return *this; } // Helper to disambiguate calls.
int_ptr<T>& operator=(const int_ptr<T>& r) {
int_ptr<T>(r).swap(*this); return *this; }
int_ptr<T>& operator=(volatile const int_ptr<T>& r) {
int_ptr<T>(r).swap(*this); return *this; }
void operator=(T* ptr) volatile { int_ptr<T>(ptr).swap(*this);
return *this; }
void operator=(int_ptr<T>& r) volatile {
int_ptr<T>(r).swap(*this); return *this; } // Helper to disambiguate calls.
void operator=(const int_ptr<T>& r) volatile {
int_ptr<T>(r).swap(*this); return *this; }
void operator=(volatile const int_ptr<T>& r) volatile {
int_ptr<T>(r).swap(*this); return *this; }
// manual resource management for adaption of C libraries.
T* toCptr() { T* ret = Data; Data = NULL;
return ret; }
T* toCptr() volatile { return
transfer(InterlockedXch(&Data, 0)); }
int_ptr<T>& fromCptr(T* ptr) { int_ptr<T>(ptr,
uninitialized_tag()).swap(*this); return *this; }
void fromCptr(T* ptr) volatile { int_ptr<T>(ptr,
uninitialized_tag()).swap(*this); }
};
Marcel