Commit 23ea498c authored by Mark Olesen's avatar Mark Olesen
Browse files

BUG: inconsistent check in non-const '->' dereference (tmp, refPtr)

- old code just checked for pointer vs non-pointer.
  Should actually treat CREF and REF types differently
  Overseen in commit be058bec. Only affects develop branch

ENH: improved naming consistency in tmp, refPtr

- also use long-form to check for pointer type instead of the isTmp()
  method. Makes differences between PTR, CREF, REF easier to spot.

STYLE: typeName() for tmp, refPtr is static
parent e56e195a
...@@ -123,13 +123,13 @@ public: ...@@ -123,13 +123,13 @@ public:
inline refPtr(const T& obj) noexcept; inline refPtr(const T& obj) noexcept;
//- Move construct, transferring ownership. //- Move construct, transferring ownership.
inline refPtr(refPtr<T>&& t) noexcept; inline refPtr(refPtr<T>&& rhs) noexcept;
//- Copy construct //- Copy construct
inline refPtr(const refPtr<T>& t); inline refPtr(const refPtr<T>& rhs);
//- Copy construct. Optionally reusing pointer. //- Copy/move construct. Optionally reusing pointer.
inline refPtr(const refPtr<T>& t, bool reuse); inline refPtr(const refPtr<T>& rhs, bool reuse);
//- Destructor: deletes managed pointer //- Destructor: deletes managed pointer
...@@ -138,6 +138,10 @@ public: ...@@ -138,6 +138,10 @@ public:
// Member Functions // Member Functions
//- The type-name, constructed from type-name of T
inline static word typeName();
// Query // Query
//- Deprecated(2020-07) True if a null managed pointer //- Deprecated(2020-07) True if a null managed pointer
...@@ -152,15 +156,12 @@ public: ...@@ -152,15 +156,12 @@ public:
//- True if this is a managed pointer (not a reference) //- True if this is a managed pointer (not a reference)
bool is_pointer() const noexcept { return type_ == PTR; } bool is_pointer() const noexcept { return type_ == PTR; }
//- True if this is a managed pointer (not a reference) //- Identical to is_pointer()
bool isTmp() const noexcept { return type_ == PTR; } bool isTmp() const noexcept { return type_ == PTR; }
//- True if this is a non-null managed pointer //- True if this is a non-null managed pointer
inline bool movable() const noexcept; inline bool movable() const noexcept;
//- Return type-name of the tmp, constructed from type-name of T
inline word typeName() const;
// Access // Access
...@@ -180,9 +181,8 @@ public: ...@@ -180,9 +181,8 @@ public:
// Fatal for a null managed pointer or if the object is const. // Fatal for a null managed pointer or if the object is const.
inline T& ref() const; inline T& ref() const;
//- Non-const dereference, even if the object is const. //- Return non-const reference to the object or to the contents
// This is similar to ref(), but applies a const_cast to access //- of a (non-null) managed pointer, with an additional const_cast.
// const objects.
// Fatal for a null managed pointer. // Fatal for a null managed pointer.
inline T& constCast() const; inline T& constCast() const;
...@@ -245,14 +245,14 @@ public: ...@@ -245,14 +245,14 @@ public:
//- Reset via assignment from literal nullptr //- Reset via assignment from literal nullptr
inline void operator=(std::nullptr_t) noexcept; inline void operator=(std::nullptr_t) noexcept;
//- Conversion to tmp - releases pointer or copies reference //- Conversion to tmp, releases pointer or shallow-copies reference
inline operator tmp<T>(); inline operator tmp<T>();
}; };
// Global Functions // Global Functions
//- Specializes the Swap algorithm for refPtr. //- Specialized Swap algorithm for refPtr.
// Swaps the pointers and types of lhs and rhs. Calls \c lhs.swap(rhs) // Swaps the pointers and types of lhs and rhs. Calls \c lhs.swap(rhs)
template<class T> template<class T>
void Swap(refPtr<T>& lhs, refPtr<T>& rhs) void Swap(refPtr<T>& lhs, refPtr<T>& rhs)
......
...@@ -47,6 +47,13 @@ inline Foam::refPtr<T> Foam::refPtr<T>::NewFrom(Args&&... args) ...@@ -47,6 +47,13 @@ inline Foam::refPtr<T> Foam::refPtr<T>::NewFrom(Args&&... args)
} }
template<class T>
inline Foam::word Foam::refPtr<T>::typeName()
{
return "refPtr<" + word(typeid(T).name()) + '>';
}
// * * * * * * * * * * * * * * * * Constructors * * * * * * * * * * * * * * // // * * * * * * * * * * * * * * * * Constructors * * * * * * * * * * * * * * //
template<class T> template<class T>
...@@ -82,32 +89,33 @@ inline Foam::refPtr<T>::refPtr(const T& obj) noexcept ...@@ -82,32 +89,33 @@ inline Foam::refPtr<T>::refPtr(const T& obj) noexcept
template<class T> template<class T>
inline Foam::refPtr<T>::refPtr(refPtr<T>&& t) noexcept inline Foam::refPtr<T>::refPtr(refPtr<T>&& rhs) noexcept
: :
ptr_(t.ptr_), ptr_(rhs.ptr_),
type_(t.type_) type_(rhs.type_)
{ {
t.ptr_ = nullptr; rhs.ptr_ = nullptr;
t.type_ = PTR; rhs.type_ = PTR;
} }
template<class T> template<class T>
inline Foam::refPtr<T>::refPtr(const refPtr<T>& t) inline Foam::refPtr<T>::refPtr(const refPtr<T>& rhs)
: :
ptr_(t.ptr_), ptr_(rhs.ptr_),
type_(t.type_) type_(rhs.type_)
{ {
if (isTmp()) if (type_ == PTR)
{ {
if (ptr_) if (ptr_)
{ {
t.type_ = CREF; rhs.type_ = REF; // (shallow copy)
} }
else else
{ {
FatalErrorInFunction FatalErrorInFunction
<< "Attempted copy of a deallocated " << typeName() << "Attempted copy of a deallocated "
<< this->typeName()
<< abort(FatalError); << abort(FatalError);
} }
} }
...@@ -115,28 +123,30 @@ inline Foam::refPtr<T>::refPtr(const refPtr<T>& t) ...@@ -115,28 +123,30 @@ inline Foam::refPtr<T>::refPtr(const refPtr<T>& t)
template<class T> template<class T>
inline Foam::refPtr<T>::refPtr(const refPtr<T>& t, bool reuse) inline Foam::refPtr<T>::refPtr(const refPtr<T>& rhs, bool reuse)
: :
ptr_(t.ptr_), ptr_(rhs.ptr_),
type_(t.type_) type_(rhs.type_)
{ {
if (isTmp()) if (type_ == PTR)
{ {
if (ptr_) if (ptr_)
{ {
if (reuse) if (reuse)
{ {
t.ptr_ = nullptr; // t.type_ already set as PTR rhs.ptr_ = nullptr;
// Note: rhs.type_ already set as PTR
} }
else else
{ {
t.type_ = CREF; rhs.type_ = REF; // (shallow copy)
} }
} }
else else
{ {
FatalErrorInFunction FatalErrorInFunction
<< "Attempted copy of a deallocated " << typeName() << "Attempted copy of a deallocated "
<< this->typeName()
<< abort(FatalError); << abort(FatalError);
} }
} }
...@@ -159,22 +169,15 @@ inline bool Foam::refPtr<T>::movable() const noexcept ...@@ -159,22 +169,15 @@ inline bool Foam::refPtr<T>::movable() const noexcept
} }
template<class T>
inline Foam::word Foam::refPtr<T>::typeName() const
{
return "refPtr<" + word(typeid(T).name()) + '>';
}
template<class T> template<class T>
inline const T& Foam::refPtr<T>::cref() const inline const T& Foam::refPtr<T>::cref() const
{ {
if (isTmp()) if (type_ == PTR)
{ {
if (!ptr_) if (!ptr_)
{ {
FatalErrorInFunction FatalErrorInFunction
<< typeName() << " deallocated" << this->typeName() << " deallocated"
<< abort(FatalError); << abort(FatalError);
} }
} }
...@@ -186,12 +189,12 @@ inline const T& Foam::refPtr<T>::cref() const ...@@ -186,12 +189,12 @@ inline const T& Foam::refPtr<T>::cref() const
template<class T> template<class T>
inline T& Foam::refPtr<T>::ref() const inline T& Foam::refPtr<T>::ref() const
{ {
if (isTmp()) if (type_ == PTR)
{ {
if (!ptr_) if (!ptr_)
{ {
FatalErrorInFunction FatalErrorInFunction
<< typeName() << " deallocated" << this->typeName() << " deallocated"
<< abort(FatalError); << abort(FatalError);
} }
} }
...@@ -199,7 +202,7 @@ inline T& Foam::refPtr<T>::ref() const ...@@ -199,7 +202,7 @@ inline T& Foam::refPtr<T>::ref() const
{ {
FatalErrorInFunction FatalErrorInFunction
<< "Attempted non-const reference to const object from a " << "Attempted non-const reference to const object from a "
<< typeName() << this->typeName()
<< abort(FatalError); << abort(FatalError);
} }
...@@ -220,12 +223,13 @@ inline T* Foam::refPtr<T>::ptr() const ...@@ -220,12 +223,13 @@ inline T* Foam::refPtr<T>::ptr() const
if (!ptr_) if (!ptr_)
{ {
FatalErrorInFunction FatalErrorInFunction
<< typeName() << " deallocated" << this->typeName() << " deallocated"
<< abort(FatalError); << abort(FatalError);
} }
if (isTmp()) if (type_ == PTR)
{ {
// Release pointer
T* p = ptr_; T* p = ptr_;
ptr_ = nullptr; ptr_ = nullptr;
...@@ -239,7 +243,7 @@ inline T* Foam::refPtr<T>::ptr() const ...@@ -239,7 +243,7 @@ inline T* Foam::refPtr<T>::ptr() const
template<class T> template<class T>
inline void Foam::refPtr<T>::clear() const noexcept inline void Foam::refPtr<T>::clear() const noexcept
{ {
if (isTmp() && ptr_) if (type_ == PTR && ptr_)
{ {
delete ptr_; delete ptr_;
ptr_ = nullptr; ptr_ = nullptr;
...@@ -299,7 +303,7 @@ inline void Foam::refPtr<T>::swap(refPtr<T>& other) noexcept ...@@ -299,7 +303,7 @@ inline void Foam::refPtr<T>::swap(refPtr<T>& other) noexcept
return; // Self-swap is a no-op return; // Self-swap is a no-op
} }
// Copy/assign for pointer types // Swap is just copy/assign for pointer and enum types
T* p = ptr_; T* p = ptr_;
ptr_ = other.ptr_; ptr_ = other.ptr_;
other.ptr_ = p; other.ptr_ = p;
...@@ -329,11 +333,14 @@ inline Foam::refPtr<T>::operator const T&() const ...@@ -329,11 +333,14 @@ inline Foam::refPtr<T>::operator const T&() const
template<class T> template<class T>
inline const T* Foam::refPtr<T>::operator->() const inline const T* Foam::refPtr<T>::operator->() const
{ {
if (!ptr_ && isTmp()) if (type_ == PTR)
{ {
FatalErrorInFunction if (!ptr_)
<< typeName() << " deallocated" {
<< abort(FatalError); FatalErrorInFunction
<< this->typeName() << " deallocated"
<< abort(FatalError);
}
} }
return ptr_; return ptr_;
...@@ -343,19 +350,20 @@ inline const T* Foam::refPtr<T>::operator->() const ...@@ -343,19 +350,20 @@ inline const T* Foam::refPtr<T>::operator->() const
template<class T> template<class T>
inline T* Foam::refPtr<T>::operator->() inline T* Foam::refPtr<T>::operator->()
{ {
if (isTmp()) if (type_ == PTR)
{ {
if (!ptr_) if (!ptr_)
{ {
FatalErrorInFunction FatalErrorInFunction
<< typeName() << " deallocated" << this->typeName() << " deallocated"
<< abort(FatalError); << abort(FatalError);
} }
} }
else else if (type_ == CREF)
{ {
FatalErrorInFunction FatalErrorInFunction
<< "Attempt to cast const object to non-const for a " << typeName() << "Attempt to cast const object to non-const for a "
<< this->typeName()
<< abort(FatalError); << abort(FatalError);
} }
...@@ -364,33 +372,34 @@ inline T* Foam::refPtr<T>::operator->() ...@@ -364,33 +372,34 @@ inline T* Foam::refPtr<T>::operator->()
template<class T> template<class T>
inline void Foam::refPtr<T>::operator=(const refPtr<T>& t) inline void Foam::refPtr<T>::operator=(const refPtr<T>& other)
{ {
if (&t == this) if (&other == this)
{ {
return; // Self-assignment is a no-op return; // Self-assignment is a no-op
} }
clear(); clear();
if (t.isTmp()) if (other.type_ == PTR)
{ {
ptr_ = t.ptr_; ptr_ = other.ptr_;
type_ = PTR; type_ = PTR;
t.ptr_ = nullptr; other.ptr_ = nullptr;
if (!ptr_) if (!ptr_)
{ {
FatalErrorInFunction FatalErrorInFunction
<< "Attempted assignment to a deallocated " << typeName() << "Attempted assignment of a deallocated "
<< this->typeName()
<< abort(FatalError); << abort(FatalError);
} }
} }
else else
{ {
FatalErrorInFunction FatalErrorInFunction
<< "Attempted assignment to a const reference to an object" << "Attempted assignment of an object reference of type "
<< " of type " << typeid(T).name() << typeid(T).name()
<< abort(FatalError); << abort(FatalError);
} }
} }
...@@ -419,7 +428,8 @@ inline void Foam::refPtr<T>::operator=(T* p) ...@@ -419,7 +428,8 @@ inline void Foam::refPtr<T>::operator=(T* p)
if (!p) if (!p)
{ {
FatalErrorInFunction FatalErrorInFunction
<< "Attempted copy of a deallocated " << typeName() << "Attempted copy of a deallocated "
<< this->typeName()
<< abort(FatalError); << abort(FatalError);
} }
...@@ -437,7 +447,7 @@ inline void Foam::refPtr<T>::operator=(std::nullptr_t) noexcept ...@@ -437,7 +447,7 @@ inline void Foam::refPtr<T>::operator=(std::nullptr_t) noexcept
template<class T> template<class T>
inline Foam::refPtr<T>::operator tmp<T>() inline Foam::refPtr<T>::operator tmp<T>()
{ {
if (isTmp()) if (type_ == PTR)
{ {
return tmp<T>(ptr()); return tmp<T>(ptr());
} }
......
...@@ -139,19 +139,19 @@ public: ...@@ -139,19 +139,19 @@ public:
//- Move construct, transferring ownership. //- Move construct, transferring ownership.
// Does not affect ref-count // Does not affect ref-count
inline tmp(tmp<T>&& t) noexcept; inline tmp(tmp<T>&& rhs) noexcept;
//- Move construct, transferring ownership. //- Move construct, transferring ownership.
// Does not affect ref-count // Does not affect ref-count
// \note Non-standard definition - should be non-const // \note Non-standard definition - should be non-const
inline tmp(const tmp<T>&& t) noexcept; inline tmp(const tmp<T>&& rhs) noexcept;
//- Copy construct, incrementing ref-count of managed pointer. //- Copy construct, incrementing ref-count of managed pointer.
// \note Non-standard definition - should be non-const // \note Non-standard definition - should be non-const
inline tmp(const tmp<T>& t); inline tmp(const tmp<T>& rhs);
//- Copy construct. Optionally reusing ref-counted pointer. //- Copy/move construct. Optionally reusing ref-counted pointer.
inline tmp(const tmp<T>& t, bool reuse); inline tmp(const tmp<T>& rhs, bool reuse);
//- Destructor: deletes managed pointer when the ref-count is 0 //- Destructor: deletes managed pointer when the ref-count is 0
...@@ -160,6 +160,10 @@ public: ...@@ -160,6 +160,10 @@ public:
// Member Functions // Member Functions
//- The type-name, constructed from type-name of T
inline static word typeName();
// Query // Query
//- Deprecated(2020-07) True if a null managed pointer //- Deprecated(2020-07) True if a null managed pointer
...@@ -174,15 +178,12 @@ public: ...@@ -174,15 +178,12 @@ public:
//- True if this is a managed pointer (not a reference) //- True if this is a managed pointer (not a reference)
bool is_pointer() const noexcept { return type_ == PTR; } bool is_pointer() const noexcept { return type_ == PTR; }
//- True if this is a managed pointer (not a reference) //- Identical to is_pointer()
bool isTmp() const noexcept { return type_ == PTR; } bool isTmp() const noexcept { return type_ == PTR; }
//- True if this is a non-null managed pointer with a unique ref-count //- True if this is a non-null managed pointer with a unique ref-count
inline bool movable() const noexcept; inline bool movable() const noexcept;
//- Return type-name of the tmp, constructed from type-name of T
inline word typeName() const;
// Access // Access
...@@ -202,9 +203,8 @@ public: ...@@ -202,9 +203,8 @@ public:
// Fatal for a null managed pointer or if the object is const. // Fatal for a null managed pointer or if the object is const.
inline T& ref() const; inline T& ref() const;
//- Non-const dereference, even if the object is const. //- Return non-const reference to the object or to the contents
// This is similar to ref(), but applies a const_cast to access //- of a (non-null) managed pointer, with an additional const_cast.
// const objects.
// Fatal for a null pointer. // Fatal for a null pointer.
inline T& constCast() const; inline T& constCast() const;
...@@ -255,7 +255,7 @@ public: ...@@ -255,7 +255,7 @@ public:
//- Transfer ownership of the managed pointer. //- Transfer ownership of the managed pointer.
// Fatal for a null managed pointer or if the object is const. // Fatal for a null managed pointer or if the object is const.
inline void operator=(const tmp<T>& t); inline void operator=(const tmp<T>& other);
//- Clear existing and transfer ownership. //- Clear existing and transfer ownership.
inline void operator=(tmp<T>&& other) noexcept; inline void operator=(tmp<T>&& other) noexcept;
...@@ -271,7 +271,7 @@ public: ...@@ -271,7 +271,7 @@ public:
// Global Functions // Global Functions
//- Specializes the Swap algorithm for tmp. //- Specialized Swap algorithm for tmp.
// Swaps the pointers and types of lhs and rhs. Calls \c lhs.swap(rhs) // Swaps the pointers and types of lhs and rhs. Calls \c lhs.swap(rhs)
template<class T> template<class T>
void Swap(tmp<T>& lhs, tmp<T>& rhs) void Swap(tmp<T>& lhs, tmp<T>& rhs)
......