From 23ea498c081acebf383ddc3760c4adb8261134fa Mon Sep 17 00:00:00 2001 From: Mark Olesen Date: Thu, 30 Jul 2020 09:45:56 +0200 Subject: [PATCH] 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 be058bec7d639e59. 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 --- src/OpenFOAM/memory/refPtr/refPtr.H | 26 +++--- src/OpenFOAM/memory/refPtr/refPtrI.H | 114 ++++++++++++----------- src/OpenFOAM/memory/tmp/tmp.H | 28 +++--- src/OpenFOAM/memory/tmp/tmpI.H | 130 +++++++++++++++------------ 4 files changed, 161 insertions(+), 137 deletions(-) diff --git a/src/OpenFOAM/memory/refPtr/refPtr.H b/src/OpenFOAM/memory/refPtr/refPtr.H index 4a05d9af6e..905ba67f6c 100644 --- a/src/OpenFOAM/memory/refPtr/refPtr.H +++ b/src/OpenFOAM/memory/refPtr/refPtr.H @@ -123,13 +123,13 @@ public: inline refPtr(const T& obj) noexcept; //- Move construct, transferring ownership. - inline refPtr(refPtr&& t) noexcept; + inline refPtr(refPtr&& rhs) noexcept; //- Copy construct - inline refPtr(const refPtr& t); + inline refPtr(const refPtr& rhs); - //- Copy construct. Optionally reusing pointer. - inline refPtr(const refPtr& t, bool reuse); + //- Copy/move construct. Optionally reusing pointer. + inline refPtr(const refPtr& rhs, bool reuse); //- Destructor: deletes managed pointer @@ -138,6 +138,10 @@ public: // Member Functions + //- The type-name, constructed from type-name of T + inline static word typeName(); + + // Query //- Deprecated(2020-07) True if a null managed pointer @@ -152,15 +156,12 @@ public: //- True if this is a managed pointer (not a reference) 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; } //- True if this is a non-null managed pointer inline bool movable() const noexcept; - //- Return type-name of the tmp, constructed from type-name of T - inline word typeName() const; - // Access @@ -180,9 +181,8 @@ public: // Fatal for a null managed pointer or if the object is const. inline T& ref() const; - //- Non-const dereference, even if the object is const. - // This is similar to ref(), but applies a const_cast to access - // const objects. + //- Return non-const reference to the object or to the contents + //- of a (non-null) managed pointer, with an additional const_cast. // Fatal for a null managed pointer. inline T& constCast() const; @@ -245,14 +245,14 @@ public: //- Reset via assignment from literal nullptr 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(); }; // 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) template void Swap(refPtr& lhs, refPtr& rhs) diff --git a/src/OpenFOAM/memory/refPtr/refPtrI.H b/src/OpenFOAM/memory/refPtr/refPtrI.H index a16e0e6dc0..c20a13a276 100644 --- a/src/OpenFOAM/memory/refPtr/refPtrI.H +++ b/src/OpenFOAM/memory/refPtr/refPtrI.H @@ -47,6 +47,13 @@ inline Foam::refPtr Foam::refPtr::NewFrom(Args&&... args) } +template +inline Foam::word Foam::refPtr::typeName() +{ + return "refPtr<" + word(typeid(T).name()) + '>'; +} + + // * * * * * * * * * * * * * * * * Constructors * * * * * * * * * * * * * * // template @@ -82,32 +89,33 @@ inline Foam::refPtr::refPtr(const T& obj) noexcept template -inline Foam::refPtr::refPtr(refPtr&& t) noexcept +inline Foam::refPtr::refPtr(refPtr&& rhs) noexcept : - ptr_(t.ptr_), - type_(t.type_) + ptr_(rhs.ptr_), + type_(rhs.type_) { - t.ptr_ = nullptr; - t.type_ = PTR; + rhs.ptr_ = nullptr; + rhs.type_ = PTR; } template -inline Foam::refPtr::refPtr(const refPtr& t) +inline Foam::refPtr::refPtr(const refPtr& rhs) : - ptr_(t.ptr_), - type_(t.type_) + ptr_(rhs.ptr_), + type_(rhs.type_) { - if (isTmp()) + if (type_ == PTR) { if (ptr_) { - t.type_ = CREF; + rhs.type_ = REF; // (shallow copy) } else { FatalErrorInFunction - << "Attempted copy of a deallocated " << typeName() + << "Attempted copy of a deallocated " + << this->typeName() << abort(FatalError); } } @@ -115,28 +123,30 @@ inline Foam::refPtr::refPtr(const refPtr& t) template -inline Foam::refPtr::refPtr(const refPtr& t, bool reuse) +inline Foam::refPtr::refPtr(const refPtr& rhs, bool reuse) : - ptr_(t.ptr_), - type_(t.type_) + ptr_(rhs.ptr_), + type_(rhs.type_) { - if (isTmp()) + if (type_ == PTR) { if (ptr_) { if (reuse) { - t.ptr_ = nullptr; // t.type_ already set as PTR + rhs.ptr_ = nullptr; + // Note: rhs.type_ already set as PTR } else { - t.type_ = CREF; + rhs.type_ = REF; // (shallow copy) } } else { FatalErrorInFunction - << "Attempted copy of a deallocated " << typeName() + << "Attempted copy of a deallocated " + << this->typeName() << abort(FatalError); } } @@ -159,22 +169,15 @@ inline bool Foam::refPtr::movable() const noexcept } -template -inline Foam::word Foam::refPtr::typeName() const -{ - return "refPtr<" + word(typeid(T).name()) + '>'; -} - - template inline const T& Foam::refPtr::cref() const { - if (isTmp()) + if (type_ == PTR) { if (!ptr_) { FatalErrorInFunction - << typeName() << " deallocated" + << this->typeName() << " deallocated" << abort(FatalError); } } @@ -186,12 +189,12 @@ inline const T& Foam::refPtr::cref() const template inline T& Foam::refPtr::ref() const { - if (isTmp()) + if (type_ == PTR) { if (!ptr_) { FatalErrorInFunction - << typeName() << " deallocated" + << this->typeName() << " deallocated" << abort(FatalError); } } @@ -199,7 +202,7 @@ inline T& Foam::refPtr::ref() const { FatalErrorInFunction << "Attempted non-const reference to const object from a " - << typeName() + << this->typeName() << abort(FatalError); } @@ -220,12 +223,13 @@ inline T* Foam::refPtr::ptr() const if (!ptr_) { FatalErrorInFunction - << typeName() << " deallocated" + << this->typeName() << " deallocated" << abort(FatalError); } - if (isTmp()) + if (type_ == PTR) { + // Release pointer T* p = ptr_; ptr_ = nullptr; @@ -239,7 +243,7 @@ inline T* Foam::refPtr::ptr() const template inline void Foam::refPtr::clear() const noexcept { - if (isTmp() && ptr_) + if (type_ == PTR && ptr_) { delete ptr_; ptr_ = nullptr; @@ -299,7 +303,7 @@ inline void Foam::refPtr::swap(refPtr& other) noexcept 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_; ptr_ = other.ptr_; other.ptr_ = p; @@ -329,11 +333,14 @@ inline Foam::refPtr::operator const T&() const template inline const T* Foam::refPtr::operator->() const { - if (!ptr_ && isTmp()) + if (type_ == PTR) { - FatalErrorInFunction - << typeName() << " deallocated" - << abort(FatalError); + if (!ptr_) + { + FatalErrorInFunction + << this->typeName() << " deallocated" + << abort(FatalError); + } } return ptr_; @@ -343,19 +350,20 @@ inline const T* Foam::refPtr::operator->() const template inline T* Foam::refPtr::operator->() { - if (isTmp()) + if (type_ == PTR) { if (!ptr_) { FatalErrorInFunction - << typeName() << " deallocated" + << this->typeName() << " deallocated" << abort(FatalError); } } - else + else if (type_ == CREF) { 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); } @@ -364,33 +372,34 @@ inline T* Foam::refPtr::operator->() template -inline void Foam::refPtr::operator=(const refPtr& t) +inline void Foam::refPtr::operator=(const refPtr& other) { - if (&t == this) + if (&other == this) { return; // Self-assignment is a no-op } clear(); - if (t.isTmp()) + if (other.type_ == PTR) { - ptr_ = t.ptr_; + ptr_ = other.ptr_; type_ = PTR; - t.ptr_ = nullptr; + other.ptr_ = nullptr; if (!ptr_) { FatalErrorInFunction - << "Attempted assignment to a deallocated " << typeName() + << "Attempted assignment of a deallocated " + << this->typeName() << abort(FatalError); } } else { FatalErrorInFunction - << "Attempted assignment to a const reference to an object" - << " of type " << typeid(T).name() + << "Attempted assignment of an object reference of type " + << typeid(T).name() << abort(FatalError); } } @@ -419,7 +428,8 @@ inline void Foam::refPtr::operator=(T* p) if (!p) { FatalErrorInFunction - << "Attempted copy of a deallocated " << typeName() + << "Attempted copy of a deallocated " + << this->typeName() << abort(FatalError); } @@ -437,7 +447,7 @@ inline void Foam::refPtr::operator=(std::nullptr_t) noexcept template inline Foam::refPtr::operator tmp() { - if (isTmp()) + if (type_ == PTR) { return tmp(ptr()); } diff --git a/src/OpenFOAM/memory/tmp/tmp.H b/src/OpenFOAM/memory/tmp/tmp.H index aa6fc29f4d..1e33eb475d 100644 --- a/src/OpenFOAM/memory/tmp/tmp.H +++ b/src/OpenFOAM/memory/tmp/tmp.H @@ -139,19 +139,19 @@ public: //- Move construct, transferring ownership. // Does not affect ref-count - inline tmp(tmp&& t) noexcept; + inline tmp(tmp&& rhs) noexcept; //- Move construct, transferring ownership. // Does not affect ref-count // \note Non-standard definition - should be non-const - inline tmp(const tmp&& t) noexcept; + inline tmp(const tmp&& rhs) noexcept; //- Copy construct, incrementing ref-count of managed pointer. // \note Non-standard definition - should be non-const - inline tmp(const tmp& t); + inline tmp(const tmp& rhs); - //- Copy construct. Optionally reusing ref-counted pointer. - inline tmp(const tmp& t, bool reuse); + //- Copy/move construct. Optionally reusing ref-counted pointer. + inline tmp(const tmp& rhs, bool reuse); //- Destructor: deletes managed pointer when the ref-count is 0 @@ -160,6 +160,10 @@ public: // Member Functions + //- The type-name, constructed from type-name of T + inline static word typeName(); + + // Query //- Deprecated(2020-07) True if a null managed pointer @@ -174,15 +178,12 @@ public: //- True if this is a managed pointer (not a reference) 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; } //- True if this is a non-null managed pointer with a unique ref-count inline bool movable() const noexcept; - //- Return type-name of the tmp, constructed from type-name of T - inline word typeName() const; - // Access @@ -202,9 +203,8 @@ public: // Fatal for a null managed pointer or if the object is const. inline T& ref() const; - //- Non-const dereference, even if the object is const. - // This is similar to ref(), but applies a const_cast to access - // const objects. + //- Return non-const reference to the object or to the contents + //- of a (non-null) managed pointer, with an additional const_cast. // Fatal for a null pointer. inline T& constCast() const; @@ -255,7 +255,7 @@ public: //- Transfer ownership of the managed pointer. // Fatal for a null managed pointer or if the object is const. - inline void operator=(const tmp& t); + inline void operator=(const tmp& other); //- Clear existing and transfer ownership. inline void operator=(tmp&& other) noexcept; @@ -271,7 +271,7 @@ public: // 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) template void Swap(tmp& lhs, tmp& rhs) diff --git a/src/OpenFOAM/memory/tmp/tmpI.H b/src/OpenFOAM/memory/tmp/tmpI.H index 4f45557b36..99babc2f0c 100644 --- a/src/OpenFOAM/memory/tmp/tmpI.H +++ b/src/OpenFOAM/memory/tmp/tmpI.H @@ -40,7 +40,8 @@ inline void Foam::tmp::incrCount() { FatalErrorInFunction << "Attempt to create more than 2 tmp's referring to" - " the same object of type " << typeName() + " the same object of type " + << tmp::typeName() << abort(FatalError); } } @@ -64,6 +65,13 @@ inline Foam::tmp Foam::tmp::NewFrom(Args&&... args) } +template +inline Foam::word Foam::tmp::typeName() +{ + return "tmp<" + word(typeid(T).name()) + '>'; +} + + // * * * * * * * * * * * * * * * * Constructors * * * * * * * * * * * * * * // template @@ -91,7 +99,8 @@ inline Foam::tmp::tmp(T* p) if (p && !p->unique()) { FatalErrorInFunction - << "Attempted construction of a " << typeName() + << "Attempted construction of a " + << this->typeName() << " from non-unique pointer" << abort(FatalError); } @@ -107,34 +116,34 @@ inline Foam::tmp::tmp(const T& obj) noexcept template -inline Foam::tmp::tmp(tmp&& t) noexcept +inline Foam::tmp::tmp(tmp&& rhs) noexcept : - ptr_(t.ptr_), - type_(t.type_) + ptr_(rhs.ptr_), + type_(rhs.type_) { - t.ptr_ = nullptr; - t.type_ = PTR; + rhs.ptr_ = nullptr; + rhs.type_ = PTR; } template -inline Foam::tmp::tmp(const tmp&& t) noexcept +inline Foam::tmp::tmp(const tmp&& rhs) noexcept : - ptr_(t.ptr_), - type_(t.type_) + ptr_(rhs.ptr_), + type_(rhs.type_) { - t.ptr_ = nullptr; - t.type_ = PTR; + rhs.ptr_ = nullptr; + rhs.type_ = PTR; } template -inline Foam::tmp::tmp(const tmp& t) +inline Foam::tmp::tmp(const tmp& rhs) : - ptr_(t.ptr_), - type_(t.type_) + ptr_(rhs.ptr_), + type_(rhs.type_) { - if (isTmp()) + if (type_ == PTR) { if (ptr_) { @@ -143,7 +152,8 @@ inline Foam::tmp::tmp(const tmp& t) else { FatalErrorInFunction - << "Attempted copy of a deallocated " << typeName() + << "Attempted copy of a deallocated " + << this->typeName() << abort(FatalError); } } @@ -151,18 +161,19 @@ inline Foam::tmp::tmp(const tmp& t) template -inline Foam::tmp::tmp(const tmp& t, bool reuse) +inline Foam::tmp::tmp(const tmp& rhs, bool reuse) : - ptr_(t.ptr_), - type_(t.type_) + ptr_(rhs.ptr_), + type_(rhs.type_) { - if (isTmp()) + if (type_ == PTR) { if (ptr_) { if (reuse) { - t.ptr_ = nullptr; // t.type_ already set as PTR + rhs.ptr_ = nullptr; + // Note: rhs.type_ already set as PTR } else { @@ -172,7 +183,8 @@ inline Foam::tmp::tmp(const tmp& t, bool reuse) else { FatalErrorInFunction - << "Attempted copy of a deallocated " << typeName() + << "Attempted copy of a deallocated " + << this->typeName() << abort(FatalError); } } @@ -195,22 +207,15 @@ inline bool Foam::tmp::movable() const noexcept } -template -inline Foam::word Foam::tmp::typeName() const -{ - return "tmp<" + word(typeid(T).name()) + '>'; -} - - template inline const T& Foam::tmp::cref() const { - if (isTmp()) + if (type_ == PTR) { if (!ptr_) { FatalErrorInFunction - << typeName() << " deallocated" + << this->typeName() << " deallocated" << abort(FatalError); } } @@ -222,12 +227,12 @@ inline const T& Foam::tmp::cref() const template inline T& Foam::tmp::ref() const { - if (isTmp()) + if (type_ == PTR) { if (!ptr_) { FatalErrorInFunction - << typeName() << " deallocated" + << this->typeName() << " deallocated" << abort(FatalError); } } @@ -235,7 +240,7 @@ inline T& Foam::tmp::ref() const { FatalErrorInFunction << "Attempted non-const reference to const object from a " - << typeName() + << this->typeName() << abort(FatalError); } @@ -256,20 +261,22 @@ inline T* Foam::tmp::ptr() const if (!ptr_) { FatalErrorInFunction - << typeName() << " deallocated" + << this->typeName() << " deallocated" << abort(FatalError); } - if (isTmp()) + if (type_ == PTR) { if (!ptr_->unique()) { FatalErrorInFunction << "Attempt to acquire pointer to object referred to" - << " by multiple temporaries of type " << typeName() + << " by multiple temporaries of type " + << this->typeName() << abort(FatalError); } + // Release pointer T* p = ptr_; ptr_ = nullptr; @@ -283,7 +290,7 @@ inline T* Foam::tmp::ptr() const template inline void Foam::tmp::clear() const noexcept { - if (isTmp() && ptr_) + if (type_ == PTR && ptr_) { if (ptr_->unique()) { @@ -350,7 +357,7 @@ inline void Foam::tmp::swap(tmp& other) noexcept 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_; ptr_ = other.ptr_; other.ptr_ = p; @@ -380,11 +387,14 @@ inline Foam::tmp::operator const T&() const template inline const T* Foam::tmp::operator->() const { - if (!ptr_ && isTmp()) + if (type_ == PTR) { - FatalErrorInFunction - << typeName() << " deallocated" - << abort(FatalError); + if (!ptr_) + { + FatalErrorInFunction + << this->typeName() << " deallocated" + << abort(FatalError); + } } return ptr_; @@ -394,19 +404,20 @@ inline const T* Foam::tmp::operator->() const template inline T* Foam::tmp::operator->() { - if (isTmp()) + if (type_ == PTR) { if (!ptr_) { FatalErrorInFunction - << typeName() << " deallocated" + << this->typeName() << " deallocated" << abort(FatalError); } } - else + else if (type_ == CREF) { 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); } @@ -415,33 +426,34 @@ inline T* Foam::tmp::operator->() template -inline void Foam::tmp::operator=(const tmp& t) +inline void Foam::tmp::operator=(const tmp& other) { - if (&t == this) + if (&other == this) { return; // Self-assignment is a no-op } clear(); - if (t.isTmp()) + if (other.type_ == PTR) { - ptr_ = t.ptr_; + ptr_ = other.ptr_; type_ = PTR; - t.ptr_ = nullptr; + other.ptr_ = nullptr; if (!ptr_) { FatalErrorInFunction - << "Attempted assignment to a deallocated " << typeName() + << "Attempted assignment of a deallocated " + << this->typeName() << abort(FatalError); } } else { FatalErrorInFunction - << "Attempted assignment to a const reference to an object" - << " of type " << typeid(T).name() + << "Attempted assignment of an object reference of type " + << typeid(T).name() << abort(FatalError); } } @@ -470,13 +482,15 @@ inline void Foam::tmp::operator=(T* p) if (!p) { FatalErrorInFunction - << "Attempted copy of a deallocated " << typeName() + << "Attempted copy of a deallocated " + << this->typeName() << abort(FatalError); } else if (!p->unique()) { FatalErrorInFunction - << "Attempted assignment of a " << typeName() + << "Attempted assignment of a " + << this->typeName() << " to non-unique pointer" << abort(FatalError); } -- GitLab