From c031f7d00a898f9cae03def4e3ce616530c5f6fc Mon Sep 17 00:00:00 2001 From: Mark Olesen <Mark.Olesen@esi-group.com> Date: Mon, 5 Sep 2022 13:39:01 +0200 Subject: [PATCH] ENH: improve autoPtr/refPtr/tmp consistency (#2571) - disallow inadvertant casting and hidden copy constructions etc --- applications/test/autoPtr/Test-autoPtr.C | 21 ++- applications/test/refPtr/Test-refPtr.C | 118 +++++++++++- applications/test/tmp/Test-tmp.C | 25 +++ .../reconstructParMesh/reconstructParMesh.C | 2 +- src/OpenFOAM/memory/autoPtr/autoPtr.H | 126 ++++++++----- src/OpenFOAM/memory/autoPtr/autoPtrI.H | 107 +---------- src/OpenFOAM/memory/refPtr/refPtr.H | 130 +++++++++---- src/OpenFOAM/memory/refPtr/refPtrI.H | 176 ++++++++++-------- src/OpenFOAM/memory/tmp/tmp.H | 94 +++++++--- src/OpenFOAM/memory/tmp/tmpI.H | 83 ++++----- src/OpenFOAM/memory/tmp/tmpNrc.H | 26 +-- 11 files changed, 530 insertions(+), 378 deletions(-) diff --git a/applications/test/autoPtr/Test-autoPtr.C b/applications/test/autoPtr/Test-autoPtr.C index e084ffb7eeb..888d47b320e 100644 --- a/applications/test/autoPtr/Test-autoPtr.C +++ b/applications/test/autoPtr/Test-autoPtr.C @@ -5,7 +5,7 @@ \\ / A nd | www.openfoam.com \\/ M anipulation | ------------------------------------------------------------------------------- - Copyright (C) 2018-2020 OpenCFD Ltd. + Copyright (C) 2018-2022 OpenCFD Ltd. ------------------------------------------------------------------------------- License This file is part of OpenFOAM. @@ -27,7 +27,6 @@ License // #define Foam_autoPtr_deprecate_setMethod -#include <memory> #include "autoPtr.H" #include "labelList.H" #include "ListOps.H" @@ -77,6 +76,14 @@ autoPtr<T> testNullReturn2() } +template<class T> +struct DerivedList : public List<T> +{ + // Inherit constructors + using List<T>::List; +}; + + // * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * // // Main program: @@ -254,6 +261,16 @@ int main(int argc, char *argv[]) // autoPtr<labelList> ptr2 = testNullReturn2<labelList>(); } + { + auto input1 = autoPtr<DerivedList<label>>::New(label(10), 1); + auto input2 = autoPtr<DerivedList<scalar>>::New(label(10), 1.0); + + autoPtr<labelList> ptr1(std::move(input1)); + + // Does not compile: ptr1 = std::move(input2); + // Does not compile: ptr1 = autoPtr<List<scalar>>::New(label(10), 2); + } + return 0; } diff --git a/applications/test/refPtr/Test-refPtr.C b/applications/test/refPtr/Test-refPtr.C index 823fdb71680..9edfcf6da7d 100644 --- a/applications/test/refPtr/Test-refPtr.C +++ b/applications/test/refPtr/Test-refPtr.C @@ -5,7 +5,7 @@ \\ / A nd | www.openfoam.com \\/ M anipulation | ------------------------------------------------------------------------------- - Copyright (C) 2020-2021 OpenCFD Ltd. + Copyright (C) 2020-2022 OpenCFD Ltd. ------------------------------------------------------------------------------- License This file is part of OpenFOAM, distributed under GPL-3.0-or-later. @@ -19,6 +19,9 @@ Description \*---------------------------------------------------------------------------*/ #include "primitiveFields.H" +#include "autoPtr.H" +#include "refPtr.H" +#include "tmp.H" #include "Switch.H" using namespace Foam; @@ -29,6 +32,36 @@ struct myScalarField : public scalarField }; +template<class T> +void constructInfo() +{ + Info<< " move-constructible:" + << std::is_move_constructible<T>::value + << " move-assignable:" + << std::is_move_assignable<T>::value + << " nothrow:" + << std::is_nothrow_move_assignable<T>::value + << " trivially:" + << std::is_trivially_move_assignable<T>::value + << nl; +} + + +template<class T> +void printInfo(const autoPtr<T>& item, const bool verbose = false) +{ + Info<< "autoPtr good:" << Switch::name(item.good()) + << " addr: " << Foam::name(item.get()); + + constructInfo<autoPtr<T>>(); + + if (verbose && item) + { + Info<< "content: " << item() << nl; + } +} + + template<class T> void printInfo(const refPtr<T>& item, const bool verbose = false) { @@ -37,15 +70,24 @@ void printInfo(const refPtr<T>& item, const bool verbose = false) << " addr: " << Foam::name(item.get()) << " movable:" << Switch(item.movable()); - Info<< " move-constructible:" - << std::is_move_constructible<refPtr<T>>::value - << " move-assignable:" - << std::is_move_assignable<refPtr<T>>::value - << " nothrow:" - << std::is_nothrow_move_assignable<refPtr<T>>::value - << " trivially:" - << std::is_trivially_move_assignable<refPtr<T>>::value - << nl; + constructInfo<refPtr<T>>(); + + if (verbose && item) + { + Info<< "content: " << item() << nl; + } +} + + +template<class T> +void printInfo(const tmp<T>& item, const bool verbose = false) +{ + Info<< "tmp good:" << Switch::name(item.good()) + << " pointer:" << Switch::name(item.is_pointer()) + << " addr: " << Foam::name(item.get()) + << " movable:" << Switch(item.movable()); + + constructInfo<tmp<T>>(); if (verbose && item) { @@ -101,6 +143,62 @@ int main() printInfo(tfld3, true); } + { + refPtr<scalarField> tfld1; + auto aptr = autoPtr<scalarField>::New(2, scalar(2)); + + tmp<scalarField> tfld2; + printInfo(tfld2, true); + + tfld2 = new scalarField(10, Zero); + + /* + tfld2 = aptr.get(); + + // tfld1.reset(aptr); + // tfld1 = std::move(aptr); + // tfld1 = aptr; + + Info<< nl << "From autoPtr" << nl; + printInfo(aptr, true); + //& Info<< nl << "Construct from autoPtr" << nl; + //& // refPtr<scalarField> tfld2(autoPtr<scalarField>::New(10, scalar(2))); + //& printInfo(tfld2, true); +*/ + } + + { + auto aptr1 = autoPtr<labelField>::New(2, Zero); + //auto aptr1 = autoPtr<scalarField>::New(2, scalar(2)); + auto aptr2 = autoPtr<scalarField>::New(2, scalar(2)); + + refPtr<scalarField> tfld2(std::move(aptr2)); + + // aptr2 = std::move(aptr1); + } + + { + auto tptr1 = tmp<labelField>::New(2, Zero); + auto aptr1 = autoPtr<labelField>::New(2, Zero); + auto tfld2 = refPtr<labelField>::New(2, Zero); + + // Deleted: refPtr<labelField> tfld1(aptr1); + refPtr<labelField> tfld1; + + // refPtr<labelField> tfld1(std::move(tptr1)); + // refPtr<labelField> tfld1(tptr1); + + tfld1 = std::move(aptr1); + + // tfld1.reset(aptr1); + // tfld1.reset(tfld2); + + // tfld1 = std::move(tptr1); + // Does not compile: tfld1.ref(tptr1); + // Deleted: tfld1.cref(tptr1); + // Deleted: tfld1.ref(aptr1); + } + Info<< "\nEnd" << endl; return 0; diff --git a/applications/test/tmp/Test-tmp.C b/applications/test/tmp/Test-tmp.C index 52f1fdac2f1..cd9e7083440 100644 --- a/applications/test/tmp/Test-tmp.C +++ b/applications/test/tmp/Test-tmp.C @@ -19,6 +19,9 @@ Description \*---------------------------------------------------------------------------*/ #include "primitiveFields.H" +#include "autoPtr.H" +#include "refPtr.H" +#include "tmp.H" #include "Switch.H" using namespace Foam; @@ -126,6 +129,28 @@ int main() printInfo(tfld2); } + { + auto tptr1 = refPtr<labelField>::New(2, Zero); + auto aptr1 = autoPtr<labelField>::New(2, Zero); + + // Deleted: tmp<labelField> tfld1(aptr1); + // tmp<labelField> tfld1(std::move(aptr1)); + // tmp<labelField> tfld1(std::move(tptr1)); + tmp<labelField> tfld1; + //tfld1.cref(tptr1); + //tfld1.cref(aptr1); + + // refPtr<labelField> tfld1(std::move(tptr1)); + // refPtr<labelField> tfld1(tptr1); + + // tfld1 = std::move(aptr1); + + // tfld1 = std::move(tptr1); + // Does not compile: tfld1.ref(tptr1); + // Deleted: tfld1.cref(tptr1); + // Deleted: tfld1.ref(aptr1); + } + Info<< "\nEnd" << endl; return 0; diff --git a/applications/utilities/parallelProcessing/reconstructParMesh/reconstructParMesh.C b/applications/utilities/parallelProcessing/reconstructParMesh/reconstructParMesh.C index e285bd12a31..05f55248a73 100644 --- a/applications/utilities/parallelProcessing/reconstructParMesh/reconstructParMesh.C +++ b/applications/utilities/parallelProcessing/reconstructParMesh/reconstructParMesh.C @@ -1184,7 +1184,7 @@ int main(int argc, char *argv[]) false ); - masterMeshPtr = fvMeshes[0]; + masterMeshPtr.cref(fvMeshes[0]); } diff --git a/src/OpenFOAM/memory/autoPtr/autoPtr.H b/src/OpenFOAM/memory/autoPtr/autoPtr.H index a7d7ff21499..b879e85c0e6 100644 --- a/src/OpenFOAM/memory/autoPtr/autoPtr.H +++ b/src/OpenFOAM/memory/autoPtr/autoPtr.H @@ -41,6 +41,9 @@ Note SourceFiles autoPtrI.H +See also + Foam::refPtr + \*---------------------------------------------------------------------------*/ #ifndef Foam_autoPtr_H @@ -80,62 +83,92 @@ public: typedef T* pointer; - // Factory Methods - - //- Construct autoPtr of T with forwarding arguments - // \param args list of arguments with which an instance of T - // will be constructed. - // - // \note Similar to std::make_unique, but the overload for - // array types is not disabled. - template<class... Args> - inline static autoPtr<T> New(Args&&... args); - - //- Construct autoPtr from derived type with forwarding arguments - // \param args list of arguments with which an instance of U - // will be constructed. - // - // \note Similar to New but for derived types. - // Future check for std::is_base_of ? - template<class U, class... Args> - inline static autoPtr<T> NewFrom(Args&&... args); - - // Constructors //- Construct with no managed pointer. - inline constexpr autoPtr() noexcept; + constexpr autoPtr() noexcept + : + ptr_(nullptr) + {} - //- Construct with no managed pointer (literal nullptr). - inline constexpr autoPtr(std::nullptr_t) noexcept; + //- Implicit construct from literal nullptr: no managed pointer + constexpr autoPtr(std::nullptr_t) noexcept + : + ptr_(nullptr) + {} //- Construct, taking ownership of the pointer. - inline explicit autoPtr(T* p) noexcept; + explicit autoPtr(T* p) noexcept + : + ptr_(p) + {} //- Move construct, transferring ownership. - inline autoPtr(autoPtr<T>&& ap) noexcept; + autoPtr(autoPtr<T>&& rhs) noexcept + : + ptr_(rhs.release()) + {} //- Move construct, transferring ownership from derived type. // U must be derivable from T - // \note Future check for std::is_base_of ? + // \note Future check std::enable_if + std::is_convertible ? template<class U> - inline explicit autoPtr(autoPtr<U>&& ap); + explicit autoPtr(autoPtr<U>&& rhs) + : + ptr_(rhs.release()) + {} + + //- Move construct from unique_ptr, transferring ownership. + explicit autoPtr(std::unique_ptr<T>&& rhs) noexcept + : + ptr_(rhs.release()) + {} //- A move construct disguised as a copy construct (transfers ownership) // \remark This should ideally be deleted - pending cleanup of code // currently relying on this behaviour. #ifdef Foam_autoPtr_copyConstruct - autoPtr(const autoPtr<T>& ap) noexcept + autoPtr(const autoPtr<T>& rhs) noexcept : - ptr_(const_cast<autoPtr<T>&>(ap).release()) + ptr_(const_cast<autoPtr<T>&>(rhs).release()) {} #else - autoPtr(const autoPtr<T>& ap) = delete; + autoPtr(const autoPtr<T>&) = delete; #endif - //- Deletes the managed pointer - inline ~autoPtr() noexcept; + //- Destructor: deletes managed pointer + ~autoPtr() noexcept + { + delete ptr_; + } + + + // Factory Methods + + //- Construct autoPtr with forwarding arguments + // \param args list of arguments with which an instance of T + // will be constructed. + // + // \note Similar to std::make_unique, but the overload for + // array types is not disabled. + template<class... Args> + static autoPtr<T> New(Args&&... args) + { + return autoPtr<T>(new T(std::forward<Args>(args)...)); + } + + //- Construct autoPtr from derived type with forwarding arguments + // \param args list of arguments with which an instance of U + // will be constructed. + // + // \note Similar to New but for derived types. + // Future check std::enable_if + std::is_convertible ? + template<class U, class... Args> + static autoPtr<T> NewFrom(Args&&... args) + { + return autoPtr<T>(new U(std::forward<Args>(args)...)); + } // Member Functions @@ -176,14 +209,13 @@ public: // \remark Method naming consistent with Foam::tmp void clear() noexcept { reset(nullptr); } + //- Delete managed object and set to new given pointer + inline void reset(T* p = nullptr) noexcept; //- Delete managed object and set to new given pointer // \remark Same as move assign, but better for code documentation inline void reset(autoPtr<T>&& other) noexcept; - //- Delete managed object and set to new given pointer - inline void reset(T* p = nullptr) noexcept; - //- Swaps the managed object with other autoPtr. inline void swap(autoPtr<T>& other) noexcept; @@ -252,19 +284,26 @@ public: //- No copy assignment from plain pointer (uncontrolled access) void operator=(T* p) = delete; + //- No move assignment disguised as a copy assignment + // \deprecated(2018-02) can have unintended behaviour + void operator=(const autoPtr<T>&) = delete; + //- Transfer object ownership from parameter - inline void operator=(autoPtr<T>&& ap) noexcept; + void operator=(autoPtr<T>&& other) noexcept { reset(std::move(other)); } //- Transfer object ownership from parameter + // \note Future check std::enable_if + std::is_convertible ? template<class U> - inline void operator=(autoPtr<U>&& ap) noexcept; + void operator=(autoPtr<U>&& other) noexcept + { + reset(other.release()); + } - //- No move assignment disguised as a copy assignment - // \deprecated(2018-02) can have unintended behaviour - void operator=(const autoPtr<T>& ap) = delete; + //- Transfer ownership by move assignment from unique_ptr + void operator=(std::unique_ptr<T>&& other) { reset(other.release()); } //- Reset via assignment from literal nullptr - inline void operator=(std::nullptr_t) noexcept; + void operator=(std::nullptr_t) noexcept { reset(nullptr); } // Housekeeping @@ -294,8 +333,7 @@ public: // Global Functions -//- Specializes the Swap algorithm for autoPtr. -// Swaps the pointers of lhs and rhs. Calls \c lhs.swap(rhs) +//- Specializes the Swap algorithm for autoPtr (swaps pointers). template<class T> void Swap(autoPtr<T>& lhs, autoPtr<T>& rhs) { diff --git a/src/OpenFOAM/memory/autoPtr/autoPtrI.H b/src/OpenFOAM/memory/autoPtr/autoPtrI.H index 12411277421..3374b8d2da0 100644 --- a/src/OpenFOAM/memory/autoPtr/autoPtrI.H +++ b/src/OpenFOAM/memory/autoPtr/autoPtrI.H @@ -6,7 +6,7 @@ \\/ M anipulation | ------------------------------------------------------------------------------- Copyright (C) 2011-2017 OpenFOAM Foundation - Copyright (C) 2016-2020 OpenCFD Ltd. + Copyright (C) 2016-2022 OpenCFD Ltd. ------------------------------------------------------------------------------- License This file is part of OpenFOAM. @@ -29,71 +29,6 @@ License #include "error.H" #include <typeinfo> -// * * * * * * * * * * * * * Static Member Functions * * * * * * * * * * * * // - -template<class T> -template<class... Args> -inline Foam::autoPtr<T> Foam::autoPtr<T>::New(Args&&... args) -{ - return autoPtr<T>(new T(std::forward<Args>(args)...)); -} - - -template<class T> -template<class U, class... Args> -inline Foam::autoPtr<T> Foam::autoPtr<T>::NewFrom(Args&&... args) -{ - return autoPtr<T>(new U(std::forward<Args>(args)...)); -} - - -// * * * * * * * * * * * * * * * * Constructors * * * * * * * * * * * * * * // - -template<class T> -inline constexpr Foam::autoPtr<T>::autoPtr() noexcept -: - ptr_(nullptr) -{} - - -template<class T> -inline constexpr Foam::autoPtr<T>::autoPtr(std::nullptr_t) noexcept -: - ptr_(nullptr) -{} - - -template<class T> -inline Foam::autoPtr<T>::autoPtr(T* p) noexcept -: - ptr_(p) -{} - - -template<class T> -inline Foam::autoPtr<T>::autoPtr(autoPtr<T>&& ap) noexcept -: - ptr_(ap.release()) -{} - - -template<class T> -template<class U> -inline Foam::autoPtr<T>::autoPtr(autoPtr<U>&& ap) -: - ptr_(ap.release()) -{} - - -// * * * * * * * * * * * * * * * * Destructor * * * * * * * * * * * * * * * // - -template<class T> -inline Foam::autoPtr<T>::~autoPtr() noexcept -{ - reset(nullptr); -} - - // * * * * * * * * * * * * * * * Member Functions * * * * * * * * * * * * * // template<class T> @@ -108,7 +43,7 @@ inline T* Foam::autoPtr<T>::release() noexcept template<class T> inline void Foam::autoPtr<T>::reset(T* p) noexcept { - if (ptr_) delete ptr_; + delete ptr_; ptr_ = p; } @@ -116,11 +51,13 @@ inline void Foam::autoPtr<T>::reset(T* p) noexcept template<class T> inline void Foam::autoPtr<T>::reset(autoPtr<T>&& other) noexcept { - if (&other != this) + // Could also make Fatal with FULLDEBUG + if (&other == this) { - // Ignore self-assignment - reset(other.release()); + return; // No self-assignment } + + reset(other.release()); } @@ -204,34 +141,4 @@ inline const T& Foam::autoPtr<T>::operator()() const } -template<class T> -inline void Foam::autoPtr<T>::operator=(autoPtr<T>&& ap) noexcept -{ - if (this != &ap) - { - // Ignore self-assignment - reset(ap.release()); - } -} - - -template<class T> -template<class U> -inline void Foam::autoPtr<T>::operator=(autoPtr<U>&& ap) noexcept -{ - if (this != &ap) - { - // Ignore self-assignment - reset(ap.release()); - } -} - - -template<class T> -inline void Foam::autoPtr<T>::operator=(std::nullptr_t) noexcept -{ - reset(nullptr); -} - - // ************************************************************************* // diff --git a/src/OpenFOAM/memory/refPtr/refPtr.H b/src/OpenFOAM/memory/refPtr/refPtr.H index 90cb3b5691a..c110ae30229 100644 --- a/src/OpenFOAM/memory/refPtr/refPtr.H +++ b/src/OpenFOAM/memory/refPtr/refPtr.H @@ -6,7 +6,7 @@ \\/ M anipulation | ------------------------------------------------------------------------------- Copyright (C) 2016 OpenFOAM Foundation - Copyright (C) 2018-2021 OpenCFD Ltd. + Copyright (C) 2018-2022 OpenCFD Ltd. ------------------------------------------------------------------------------- License This file is part of OpenFOAM. @@ -89,44 +89,18 @@ public: typedef Foam::refCount::zero refCount; - // Factory Methods - - //- Construct refPtr of T with forwarding arguments - // \param args list of arguments with which an instance of T - // will be constructed. - // - // \note Similar to std::make_shared, but the overload for - // array types is not disabled. - template<class... Args> - inline static refPtr<T> New(Args&&... args); - - //- Construct refPtr from derived type with forwarding arguments - // \param args list of arguments with which an instance of U - // will be constructed. - // - // \note Similar to New but for derived types - template<class U, class... Args> - inline static refPtr<T> NewFrom(Args&&... args); - - // Constructors //- Construct with no managed pointer. inline constexpr refPtr() noexcept; - //- Construct with no managed pointer (literal nullptr). + //- Implicit construct from literal nullptr: no managed pointer inline constexpr refPtr(std::nullptr_t) noexcept; //- Construct, taking ownership of the pointer. inline explicit constexpr refPtr(T* p) noexcept; - //- Move construct from autoPtr, transferring ownership. - inline explicit refPtr(autoPtr<T>&& ptr) noexcept; - - //- Move construct from unique_ptr, transferring ownership. - inline explicit refPtr(std::unique_ptr<T>&& ptr) noexcept; - - //- Construct for a const reference to an object. + //- Implicit construct for a const reference to an object. inline constexpr refPtr(const T& obj) noexcept; //- Move construct, transferring ownership. @@ -138,12 +112,58 @@ public: //- Copy/move construct. Optionally reusing pointer. inline refPtr(const refPtr<T>& rhs, bool reuse); + //- Move construct from unique_ptr, transferring ownership. + inline explicit refPtr(std::unique_ptr<T>&& rhs) noexcept; + + //- No copy construct from autoPtr, + //- also avoids implicit cast to object or pointer + refPtr(const autoPtr<T>&) = delete; + + //- Move construct from autoPtr, transferring ownership. + inline explicit refPtr(autoPtr<T>&& rhs) noexcept; + + //- Reference tmp contents or transfer ownership if requested/possible + inline refPtr(const tmp<T>& rhs, bool reuse); + + //- Reference the tmp contents + inline explicit refPtr(const tmp<T>& rhs); + + //- Move construct from tmp, transfer ownership if possible. + inline explicit refPtr(tmp<T>&& rhs); + //- Destructor: deletes managed pointer - inline ~refPtr(); + inline ~refPtr() noexcept; + + // Factory Methods - // Member Functions + //- Construct refPtr with forwarding arguments + // \param args list of arguments with which an instance of T + // will be constructed. + // + // \note Similar to std::make_shared, but the overload for + // array types is not disabled. + template<class... Args> + static refPtr<T> New(Args&&... args) + { + return refPtr<T>(new T(std::forward<Args>(args)...)); + } + + //- Construct refPtr from derived type with forwarding arguments + // \param args list of arguments with which an instance of U + // will be constructed. + // + // \note Similar to New but for derived types. + // Future check std::enable_if + std::is_convertible ? + template<class U, class... Args> + static refPtr<T> NewFrom(Args&&... args) + { + return refPtr<T>(new U(std::forward<Args>(args)...)); + } + + + // Static Member Functions //- The type-name, constructed from type-name of T inline static word typeName(); @@ -207,12 +227,14 @@ public: //- delete object and set pointer to nullptr inline void clear() const noexcept; + //- Delete managed pointer and set to new given pointer + inline void reset(T* p = nullptr) noexcept; //- Clear existing and transfer ownership. inline void reset(refPtr<T>&& other) noexcept; - //- Delete managed pointer and set to new given pointer - inline void reset(T* p = nullptr) noexcept; + //- No reset from autoPtr reference (potentially confusing) + void reset(const autoPtr<T>&) = delete; //- Clear existing and transfer ownership from autoPtr. void reset(autoPtr<T>&& other) noexcept { reset(other.release()); } @@ -220,6 +242,8 @@ public: //- Clear existing and transfer ownership from unique_ptr void reset(std::unique_ptr<T>&& other) { reset(other.release()); } + //- Reference tmp contents or transfer pointer ownership if possible + inline void reset(tmp<T>& rhs, bool reuse); //- Clear existing and set (const) reference from other inline void cref(const refPtr<T>& other) noexcept; @@ -231,6 +255,11 @@ public: // The pointer can be null, which is handled like a clear(). inline void cref(const T* p) noexcept; + //- Avoid inadvertent casting (to object or pointer) + void cref(const autoPtr<T>&) = delete; + + //- Avoid inadvertent casting (to object) + void cref(const tmp<T>&) = delete; //- Clear existing and set (non-const) reference inline void ref(T& obj) noexcept; @@ -239,6 +268,11 @@ public: // The pointer can be null, which is handled like a clear(). inline void ref(T* p) noexcept; + //- Avoid inadvertent casting (to object or pointer) + void ref(const autoPtr<T>&) = delete; + + //- Avoid inadvertent casting (object) + void ref(const tmp<T>&) = delete; //- Swaps the managed object with other. inline void swap(refPtr<T>& other) noexcept; @@ -277,19 +311,34 @@ public: // Assignment - //- Transfer ownership of the managed pointer. + //- Transfer ownership of managed pointer. // Fatal for a null managed pointer or if the object is const. inline void operator=(const refPtr<T>& other); //- Clear existing and transfer ownership. - inline void operator=(refPtr<T>&& other) noexcept; + void operator=(refPtr<T>&& other) noexcept { reset(std::move(other)); } - //- Take ownership of the pointer. - // Fatal for a null pointer - inline void operator=(T* p); + //- No copy assignment from plain pointer (uncontrolled access) + void operator=(T* p) = delete; //- Reset via assignment from literal nullptr - inline void operator=(std::nullptr_t) noexcept; + void operator=(std::nullptr_t) noexcept { reset(nullptr); } + + //- No copy assignment from autoPtr (can have unintended behaviour) + void operator=(const autoPtr<T>&) = delete; + + //- Transfer ownership by move assignment from autoPtr. + void operator=(autoPtr<T>&& other) noexcept { reset(other.release()); } + + //- Transfer ownership by move assignment from unique_ptr + void operator=(std::unique_ptr<T>&& other) { reset(other.release()); } + + //- No copy assignment from tmp + void operator=(const tmp<T>&) = delete; + + //- Move construct, transferring pointer ownership if possible. + inline void operator=(tmp<T>&& other); + //- Conversion to tmp, releases pointer or shallow-copies reference inline operator tmp<T>(); @@ -312,8 +361,7 @@ public: // Global Functions -//- Specialized Swap algorithm for refPtr. -// Swaps the pointers and types of lhs and rhs. Calls \c lhs.swap(rhs) +//- Specializes the Swap algorithm for refPtr (swaps pointers and types). template<class T> void Swap(refPtr<T>& lhs, refPtr<T>& rhs) { diff --git a/src/OpenFOAM/memory/refPtr/refPtrI.H b/src/OpenFOAM/memory/refPtr/refPtrI.H index c5d87ab160f..34511f3ffa6 100644 --- a/src/OpenFOAM/memory/refPtr/refPtrI.H +++ b/src/OpenFOAM/memory/refPtr/refPtrI.H @@ -6,7 +6,7 @@ \\/ M anipulation | ------------------------------------------------------------------------------- Copyright (C) 2016-2017 OpenFOAM Foundation - Copyright (C) 2018-2021 OpenCFD Ltd. + Copyright (C) 2018-2022 OpenCFD Ltd. ------------------------------------------------------------------------------- License This file is part of OpenFOAM. @@ -34,23 +34,7 @@ License template<class T> inline Foam::word Foam::refPtr<T>::typeName() { - return "refPtr<" + word(typeid(T).name()) + '>'; -} - - -template<class T> -template<class... Args> -inline Foam::refPtr<T> Foam::refPtr<T>::New(Args&&... args) -{ - return refPtr<T>(new T(std::forward<Args>(args)...)); -} - - -template<class T> -template<class U, class... Args> -inline Foam::refPtr<T> Foam::refPtr<T>::NewFrom(Args&&... args) -{ - return refPtr<T>(new U(std::forward<Args>(args)...)); + return Foam::word("refPtr<" + std::string(typeid(T).name()) + '>', false); } @@ -80,19 +64,6 @@ inline constexpr Foam::refPtr<T>::refPtr(T* p) noexcept {} -template<class T> -inline Foam::refPtr<T>::refPtr(autoPtr<T>&& rhs) noexcept -: - refPtr<T>(rhs.release()) -{} - - -template<class T> -inline Foam::refPtr<T>::refPtr(std::unique_ptr<T>&& rhs) noexcept -: - refPtr<T>(rhs.release()) -{} - template<class T> inline constexpr Foam::refPtr<T>::refPtr(const T& obj) noexcept @@ -119,7 +90,7 @@ inline Foam::refPtr<T>::refPtr(const refPtr<T>& rhs) ptr_(rhs.ptr_), type_(rhs.type_) { - if (type_ == PTR) + if (is_pointer()) { if (ptr_) { @@ -128,7 +99,7 @@ inline Foam::refPtr<T>::refPtr(const refPtr<T>& rhs) else { FatalErrorInFunction - << "Attempted copy of a deallocated " + << "Attempted copy/move of a deallocated " << this->typeName() << abort(FatalError); } @@ -142,14 +113,14 @@ inline Foam::refPtr<T>::refPtr(const refPtr<T>& rhs, bool reuse) ptr_(rhs.ptr_), type_(rhs.type_) { - if (type_ == PTR) + if (is_pointer()) { if (ptr_) { if (reuse) { rhs.ptr_ = nullptr; - // Note: rhs.type_ already set as PTR + rhs.type_ = PTR; } else { @@ -159,7 +130,7 @@ inline Foam::refPtr<T>::refPtr(const refPtr<T>& rhs, bool reuse) else { FatalErrorInFunction - << "Attempted copy of a deallocated " + << "Attempted copy/move of a deallocated " << this->typeName() << abort(FatalError); } @@ -167,10 +138,51 @@ inline Foam::refPtr<T>::refPtr(const refPtr<T>& rhs, bool reuse) } +template<class T> +inline Foam::refPtr<T>::refPtr(std::unique_ptr<T>&& rhs) noexcept +: + refPtr<T>(rhs.release()) +{} + + +template<class T> +inline Foam::refPtr<T>::refPtr(autoPtr<T>&& rhs) noexcept +: + refPtr<T>(rhs.release()) +{} + + +template<class T> +inline Foam::refPtr<T>::refPtr(const tmp<T>& rhs, bool reuse) +: + refPtr<T>() +{ + reset(const_cast<tmp<T>&>(rhs), reuse); +} + + +template<class T> +inline Foam::refPtr<T>::refPtr(const tmp<T>& rhs) +: + refPtr<T>() +{ + reset(const_cast<tmp<T>&>(rhs), false); +} + + +template<class T> +inline Foam::refPtr<T>::refPtr(tmp<T>&& rhs) +: + refPtr<T>() +{ + reset(const_cast<tmp<T>&>(rhs), true); +} + + // * * * * * * * * * * * * * * * * Destructor * * * * * * * * * * * * * * * // template<class T> -inline Foam::refPtr<T>::~refPtr() +inline Foam::refPtr<T>::~refPtr() noexcept { clear(); } @@ -181,14 +193,14 @@ inline Foam::refPtr<T>::~refPtr() template<class T> inline bool Foam::refPtr<T>::movable() const noexcept { - return (type_ == PTR && ptr_); + return (is_pointer() && ptr_); } template<class T> inline const T& Foam::refPtr<T>::cref() const { - if (type_ == PTR && !ptr_) + if (is_pointer() && !ptr_) { FatalErrorInFunction << this->typeName() << " deallocated" @@ -209,7 +221,7 @@ inline T& Foam::refPtr<T>::ref() const << this->typeName() << abort(FatalError); } - else if (type_ == PTR && !ptr_) + else if (is_pointer() && !ptr_) { FatalErrorInFunction << this->typeName() << " deallocated" @@ -245,7 +257,7 @@ inline Foam::refPtr<T> Foam::refPtr<T>::shallowClone() const noexcept template<class T> inline T* Foam::refPtr<T>::release() noexcept { - if (type_ == PTR) + if (is_pointer()) { T* p = ptr_; ptr_ = nullptr; @@ -266,7 +278,7 @@ inline T* Foam::refPtr<T>::ptr() const << abort(FatalError); } - if (type_ == PTR) + if (is_pointer()) { // Release pointer T* p = ptr_; @@ -282,7 +294,7 @@ inline T* Foam::refPtr<T>::ptr() const template<class T> inline void Foam::refPtr<T>::clear() const noexcept { - if (type_ == PTR && ptr_) + if (is_pointer()) { delete ptr_; ptr_ = nullptr; @@ -302,9 +314,10 @@ inline void Foam::refPtr<T>::reset(T* p) noexcept template<class T> inline void Foam::refPtr<T>::reset(refPtr<T>&& other) noexcept { + // Could also make Fatal with FULLDEBUG if (&other == this) { - return; // Self-assignment is a no-op + return; // No self-assignment } clear(); @@ -316,12 +329,40 @@ inline void Foam::refPtr<T>::reset(refPtr<T>&& other) noexcept } +template<class T> +inline void Foam::refPtr<T>::reset(tmp<T>& other, bool reuse) +{ + if (other.get()) + { + if (reuse && other.is_pointer()) + { + // Acquire pointer. + // Fatal if pointer is not unique (avoids potential leaks) + reset(other.ptr()); + } + else if (other.is_const()) + { + cref(other.get()); + } + else + { + ref(other.get()); + } + } + else + { + clear(); + } +} + + template<class T> inline void Foam::refPtr<T>::cref(const refPtr<T>& other) noexcept { + // Could also make Fatal with FULLDEBUG if (&other == this) { - return; // Self-assignment is a no-op + return; // No self-assignment } clear(); @@ -455,18 +496,21 @@ inline T* Foam::refPtr<T>::operator->() template<class T> inline void Foam::refPtr<T>::operator=(const refPtr<T>& other) { + // Could also make Fatal with FULLDEBUG if (&other == this) { - return; // Self-assignment is a no-op + return; // No self-assignment } clear(); - if (other.type_ == PTR) + if (other.is_pointer()) { ptr_ = other.ptr_; type_ = PTR; + other.ptr_ = nullptr; + other.type_ = PTR; if (!ptr_) { @@ -487,48 +531,16 @@ inline void Foam::refPtr<T>::operator=(const refPtr<T>& other) template<class T> -inline void Foam::refPtr<T>::operator=(refPtr<T>&& other) noexcept -{ - if (&other == this) - { - return; // Self-assignment is a no-op - } - - clear(); - ptr_ = other.ptr_; - type_ = other.type_; - - other.ptr_ = nullptr; - other.type_ = PTR; -} - - -template<class T> -inline void Foam::refPtr<T>::operator=(T* p) -{ - if (!p) - { - FatalErrorInFunction - << "Attempted copy of a deallocated " - << this->typeName() - << abort(FatalError); - } - - reset(p); -} - - -template<class T> -inline void Foam::refPtr<T>::operator=(std::nullptr_t) noexcept +inline void Foam::refPtr<T>::operator=(tmp<T>&& other) { - reset(nullptr); + reset(other, true); // reuse } template<class T> inline Foam::refPtr<T>::operator tmp<T>() { - if (type_ == PTR) + if (is_pointer()) { return tmp<T>(ptr()); } diff --git a/src/OpenFOAM/memory/tmp/tmp.H b/src/OpenFOAM/memory/tmp/tmp.H index c76ae912a50..c0157eb7cbd 100644 --- a/src/OpenFOAM/memory/tmp/tmp.H +++ b/src/OpenFOAM/memory/tmp/tmp.H @@ -6,7 +6,7 @@ \\/ M anipulation | ------------------------------------------------------------------------------- Copyright (C) 2011-2016 OpenFOAM Foundation - Copyright (C) 2018-2021 OpenCFD Ltd. + Copyright (C) 2018-2022 OpenCFD Ltd. ------------------------------------------------------------------------------- License This file is part of OpenFOAM. @@ -48,15 +48,18 @@ See also #ifndef Foam_tmp_H #define Foam_tmp_H +#include "autoPtr.H" #include "refCount.H" #include "word.H" -#include "stdFoam.H" // * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * // namespace Foam { +// Forward Declarations +template<class T> class refPtr; + /*---------------------------------------------------------------------------*\ Class tmp Declaration \*---------------------------------------------------------------------------*/ @@ -103,38 +106,18 @@ public: typedef Foam::refCount refCount; - // Factory Methods - - //- Construct tmp of T with forwarding arguments - // \param args list of arguments with which an instance of T - // will be constructed. - // - // \note Similar to std::make_shared, but the overload for - // array types is not disabled. - template<class... Args> - inline static tmp<T> New(Args&&... args); - - //- Construct tmp from derived type with forwarding arguments - // \param args list of arguments with which an instance of U - // will be constructed. - // - // \note Similar to New but for derived types - template<class U, class... Args> - inline static tmp<T> NewFrom(Args&&... args); - - // Constructors //- Construct with no managed pointer. inline constexpr tmp() noexcept; - //- Construct with no managed pointer (literal nullptr). + //- Implicit construct from literal nullptr: no managed pointer inline constexpr tmp(std::nullptr_t) noexcept; //- Construct, taking ownership of the pointer. inline explicit tmp(T* p); - //- Construct for a const reference to an object. + //- Implicit construct for a const reference to an object. inline constexpr tmp(const T& obj) noexcept; //- Move construct, transferring ownership. @@ -153,12 +136,50 @@ public: //- Copy/move construct. Optionally reusing ref-counted pointer. inline tmp(const tmp<T>& rhs, bool reuse); + //- No copy construct from autoPtr, + //- also avoids implicit cast to object or pointer + tmp(const autoPtr<T>&) = delete; + + //- No copy construct from refPtr, + //- also avoids implicit cast to object + tmp(const refPtr<T>&) = delete; + + //- Move construct from autoPtr, transferring ownership. + inline explicit tmp(autoPtr<T>&& rhs) noexcept; + //- Destructor: deletes managed pointer when the ref-count is 0 - inline ~tmp(); + inline ~tmp() noexcept; + + // Factory Methods - // Member Functions + //- Construct tmp with forwarding arguments + // \param args list of arguments with which an instance of T + // will be constructed. + // + // \note Similar to std::make_shared, but the overload for + // array types is not disabled. + template<class... Args> + static tmp<T> New(Args&&... args) + { + return tmp<T>(new T(std::forward<Args>(args)...)); + } + + //- Construct tmp from derived type with forwarding arguments + // \param args list of arguments with which an instance of U + // will be constructed. + // + // \note Similar to New but for derived types. + // Future check std::enable_if + std::is_convertible ? + template<class U, class... Args> + static tmp<T> NewFrom(Args&&... args) + { + return tmp<T>(new U(std::forward<Args>(args)...)); + } + + + // Static Member Functions //- The type-name, constructed from type-name of T inline static word typeName(); @@ -212,13 +233,17 @@ public: //- delete object and set pointer to nullptr inline void clear() const noexcept; - //- Clear existing and transfer ownership. inline void reset(tmp<T>&& other) noexcept; //- Delete managed temporary object and set to new given pointer inline void reset(T* p = nullptr) noexcept; + //- Avoid inadvertent casting (to object or pointer) + void reset(const autoPtr<T>&) = delete; + + //- Avoid inadvertent casting (to object) + void reset(const refPtr<T>&) = delete; //- Clear existing and set (const) reference from other inline void cref(const tmp<T>& other) noexcept; @@ -230,6 +255,11 @@ public: // The pointer can be null, which is handled like a clear(). inline void cref(const T* p) noexcept; + //- Avoid inadvertent casting (to object or pointer) + void cref(const autoPtr<T>&) = delete; + + //- Avoid inadvertent casting (to object) + void cref(const refPtr<T>&) = delete; //- Clear existing and set to (non-const) reference inline void ref(T& obj) noexcept; @@ -238,6 +268,11 @@ public: // The pointer can be null, which is handled like a clear(). inline void ref(T* p) noexcept; + //- Avoid inadvertent casting (to object or pointer) + void ref(const autoPtr<T>&) = delete; + + //- Avoid inadvertent casting (to object) + void ref(const refPtr<T>&) = delete; //- Swaps the managed object with other. inline void swap(tmp<T>& other) noexcept; @@ -284,7 +319,7 @@ public: inline void operator=(T* p); //- Reset via assignment from literal nullptr - inline void operator=(std::nullptr_t) noexcept; + void operator=(std::nullptr_t) noexcept { reset(nullptr); } // Housekeeping @@ -307,8 +342,7 @@ public: // Global Functions -//- Specialized Swap algorithm for tmp. -// Swaps the pointers and types of lhs and rhs. Calls \c lhs.swap(rhs) +//- Specializes the Swap algorithm for tmp (swaps pointers and types). template<class T> void Swap(tmp<T>& lhs, tmp<T>& rhs) { diff --git a/src/OpenFOAM/memory/tmp/tmpI.H b/src/OpenFOAM/memory/tmp/tmpI.H index ab21fef60f3..9065831d0e5 100644 --- a/src/OpenFOAM/memory/tmp/tmpI.H +++ b/src/OpenFOAM/memory/tmp/tmpI.H @@ -6,7 +6,7 @@ \\/ M anipulation | ------------------------------------------------------------------------------- Copyright (C) 2011-2017 OpenFOAM Foundation - Copyright (C) 2018-2021 OpenCFD Ltd. + Copyright (C) 2018-2022 OpenCFD Ltd. ------------------------------------------------------------------------------- License This file is part of OpenFOAM. @@ -39,9 +39,8 @@ inline void Foam::tmp<T>::incrCount() if (ptr_->count() > 1) { FatalErrorInFunction - << "Attempt to create more than 2 tmp's referring to" - " the same object of type " - << tmp<T>::typeName() + << "Attempt to create more than 2 tmp's referring to the same" + " object of type tmp<" << typeid(T).name() << '>' << abort(FatalError); } } @@ -52,23 +51,7 @@ inline void Foam::tmp<T>::incrCount() template<class T> inline Foam::word Foam::tmp<T>::typeName() { - return "tmp<" + word(typeid(T).name()) + '>'; -} - - -template<class T> -template<class... Args> -inline Foam::tmp<T> Foam::tmp<T>::New(Args&&... args) -{ - return tmp<T>(new T(std::forward<Args>(args)...)); -} - - -template<class T> -template<class U, class... Args> -inline Foam::tmp<T> Foam::tmp<T>::NewFrom(Args&&... args) -{ - return tmp<T>(new U(std::forward<Args>(args)...)); + return Foam::word("tmp<" + std::string(typeid(T).name()) + '>', false); } @@ -96,7 +79,7 @@ inline Foam::tmp<T>::tmp(T* p) ptr_(p), type_(PTR) { - if (p && !p->unique()) + if (ptr_ && !ptr_->unique()) { FatalErrorInFunction << "Attempted construction of a " @@ -143,7 +126,7 @@ inline Foam::tmp<T>::tmp(const tmp<T>& rhs) ptr_(rhs.ptr_), type_(rhs.type_) { - if (type_ == PTR) + if (is_pointer()) { if (ptr_) { @@ -152,7 +135,7 @@ inline Foam::tmp<T>::tmp(const tmp<T>& rhs) else { FatalErrorInFunction - << "Attempted copy of a deallocated " + << "Attempted copy/move of a deallocated " << this->typeName() << abort(FatalError); } @@ -166,14 +149,14 @@ inline Foam::tmp<T>::tmp(const tmp<T>& rhs, bool reuse) ptr_(rhs.ptr_), type_(rhs.type_) { - if (type_ == PTR) + if (is_pointer()) { if (ptr_) { if (reuse) { rhs.ptr_ = nullptr; - // Note: rhs.type_ already set as PTR + rhs.type_ = PTR; } else { @@ -183,7 +166,7 @@ inline Foam::tmp<T>::tmp(const tmp<T>& rhs, bool reuse) else { FatalErrorInFunction - << "Attempted copy of a deallocated " + << "Attempted copy/move of a deallocated " << this->typeName() << abort(FatalError); } @@ -191,10 +174,17 @@ inline Foam::tmp<T>::tmp(const tmp<T>& rhs, bool reuse) } +template<class T> +inline Foam::tmp<T>::tmp(autoPtr<T>&& rhs) noexcept +: + tmp<T>(rhs.release()) +{} + + // * * * * * * * * * * * * * * * * Destructor * * * * * * * * * * * * * * * // template<class T> -inline Foam::tmp<T>::~tmp() +inline Foam::tmp<T>::~tmp() noexcept { clear(); } @@ -205,14 +195,14 @@ inline Foam::tmp<T>::~tmp() template<class T> inline bool Foam::tmp<T>::movable() const noexcept { - return (type_ == PTR && ptr_ && ptr_->unique()); + return (is_pointer() && ptr_ && ptr_->unique()); } template<class T> inline const T& Foam::tmp<T>::cref() const { - if (type_ == PTR && !ptr_) + if (is_pointer() && !ptr_) { FatalErrorInFunction << this->typeName() << " deallocated" @@ -233,7 +223,7 @@ inline T& Foam::tmp<T>::ref() const << this->typeName() << abort(FatalError); } - else if (type_ == PTR && !ptr_) + else if (is_pointer() && !ptr_) { FatalErrorInFunction << this->typeName() << " deallocated" @@ -261,7 +251,7 @@ inline T* Foam::tmp<T>::ptr() const << abort(FatalError); } - if (type_ == PTR) + if (is_pointer()) { if (!ptr_->unique()) { @@ -286,7 +276,7 @@ inline T* Foam::tmp<T>::ptr() const template<class T> inline void Foam::tmp<T>::clear() const noexcept { - if (type_ == PTR && ptr_) + if (is_pointer() && ptr_) { if (ptr_->unique()) { @@ -313,9 +303,10 @@ inline void Foam::tmp<T>::reset(T* p) noexcept template<class T> inline void Foam::tmp<T>::reset(tmp<T>&& other) noexcept { + // Could also make Fatal with FULLDEBUG if (&other == this) { - return; // Self-assignment is a no-op + return; // No self-assignment } clear(); @@ -330,9 +321,10 @@ inline void Foam::tmp<T>::reset(tmp<T>&& other) noexcept template<class T> inline void Foam::tmp<T>::cref(const tmp<T>& other) noexcept { + // Could also make Fatal with FULLDEBUG if (&other == this) { - return; // Self-assignment is a no-op + return; // No self-assignment } clear(); @@ -397,7 +389,7 @@ inline void Foam::tmp<T>::swap(tmp<T>& other) noexcept template<class T> inline const T* Foam::tmp<T>::operator->() const { - if (type_ == PTR && !ptr_) + if (is_pointer() && !ptr_) { FatalErrorInFunction << this->typeName() << " deallocated" @@ -418,7 +410,7 @@ inline T* Foam::tmp<T>::operator->() << this->typeName() << abort(FatalError); } - else if (type_ == PTR && !ptr_) + else if (is_pointer() && !ptr_) { FatalErrorInFunction << this->typeName() << " deallocated" @@ -432,18 +424,21 @@ inline T* Foam::tmp<T>::operator->() template<class T> inline void Foam::tmp<T>::operator=(const tmp<T>& other) { + // Could also make Fatal with FULLDEBUG if (&other == this) { - return; // Self-assignment is a no-op + return; // No self-assignment } clear(); - if (other.type_ == PTR) + if (other.is_pointer()) { ptr_ = other.ptr_; type_ = PTR; + other.ptr_ = nullptr; + other.type_ = PTR; if (!ptr_) { @@ -466,9 +461,10 @@ inline void Foam::tmp<T>::operator=(const tmp<T>& other) template<class T> inline void Foam::tmp<T>::operator=(tmp<T>&& other) noexcept { + // Could also make Fatal with FULLDEBUG if (&other == this) { - return; // Self-assignment is a no-op + return; // No self-assignment } clear(); @@ -503,11 +499,4 @@ inline void Foam::tmp<T>::operator=(T* p) } -template<class T> -inline void Foam::tmp<T>::operator=(std::nullptr_t) noexcept -{ - reset(nullptr); -} - - // ************************************************************************* // diff --git a/src/OpenFOAM/memory/tmp/tmpNrc.H b/src/OpenFOAM/memory/tmp/tmpNrc.H index 0f58697e70b..17b09e63da1 100644 --- a/src/OpenFOAM/memory/tmp/tmpNrc.H +++ b/src/OpenFOAM/memory/tmp/tmpNrc.H @@ -1,25 +1,7 @@ -/*---------------------------------------------------------------------------*\ - ========= | - \\ / F ield | OpenFOAM: The Open Source CFD Toolbox - \\ / O peration | - \\ / A nd | www.openfoam.com - \\/ M anipulation | -------------------------------------------------------------------------------- - Copyright (C) 2020 OpenCFD Ltd. -------------------------------------------------------------------------------- -License - This file is part of OpenFOAM, distributed under GPL-3.0-or-later. +// Compatibility include. For v2006 and earlier -Typedef - Foam::tmpNrc - -Description - Compatibility name. Superseded (JUL-2020) by Foam::refPtr - -\*---------------------------------------------------------------------------*/ - -#ifndef Foam_tmpNrc_H -#define Foam_tmpNrc_H +#ifndef FoamCompat_tmpNrc_H +#define FoamCompat_tmpNrc_H #include "refPtr.H" @@ -27,6 +9,8 @@ Description namespace Foam { + //- Deprecated(2020-07) - use refPtr instead + // \deprecated(2020-07) - use refPtr instead typedef refPtr tmpNrc; } -- GitLab