Update CowPtr and Clone to fix testClone2

ChangeLog.md

@@ -34,6 +34,9 @@
 - *Base*
   - Fixing wrong members in PredicateCombiner (David Coeurjolly,
     [#1321](https://github.com/DGtal-team/DGtal/pull/1321))
+  - Fix testClone2.cpp and efficiency issue in Clone/CountedPtr mechanism (Jacques-Olivier Lachaud,
+    [#1382](https://github.com/DGtal-team/DGtal/pull/1382)). Fix issue
+    [#1203](https://github.com/DGtal-team/DGtal/issues/1203))
 
 - *Shapes*
   - Add two new star shapes: Astroid and Lemniscate

src/DGtal/base/Clone.h

@@ -272,7 +272,30 @@ namespace DGtal
     enum Parameter { CONST_LEFT_VALUE_REF, LEFT_VALUE_REF, PTR, CONST_PTR,
 		     COW_PTR, COUNTED_PTR, RIGHT_VALUE_REF, COUNTED_PTR_OR_PTR,
 		     COUNTED_CONST_PTR_OR_CONST_PTR };
-
+    /**
+       Debug method for displaying what's happening when using Clone mechanism
+       @param method the name of the conversion method
+       @param p the type of parameter
+    */
+    void display( const std::string& method, Parameter p ) const
+    {
+      std::string sp;
+      switch (p) {
+      case CONST_LEFT_VALUE_REF: sp = "CONST_LEFT_VALUE_REF"; break;
+      case LEFT_VALUE_REF:       sp = "LEFT_VALUE_REF"; break;
+      case PTR:                  sp = "PTR"; break;
+      case CONST_PTR:            sp = "CONST_PTR"; break;
+      case COW_PTR:              sp = "COW_PTR"; break;
+      case COUNTED_PTR:          sp = "COUNTED_PTR"; break;
+      case RIGHT_VALUE_REF:      sp = "RIGHT_VALUE_REF"; break;
+      case COUNTED_PTR_OR_PTR:   sp = "COUNTED_PTR_OR_PTR"; break;
+      case COUNTED_CONST_PTR_OR_CONST_PTR: sp = "COUNTED_CONST_PTR_OR_CONST_PTR"; break;
+      default:                   sp = "UNKNOWN"; break;
+      }
+      trace.info() << "[Clone<T>::" << method << " param="
+                   << sp << "]" << std::endl;
+
+    }
     /// Internal class that is used for a late deletion of an acquired pointer.
     struct TempPtr {
       /**
@@ -360,6 +383,7 @@ namespace DGtal
     */
     inline operator T() const
     {
+      // display( "operator T() const", myParam );
       switch( myParam ) {
       case CONST_LEFT_VALUE_REF:
 	return T( * static_cast< const T* >( myPtr ) );
@@ -390,6 +414,7 @@ namespace DGtal
     */
     inline operator CowPtr<T>() const
     {
+      // display( "operator CowPtr<T>() const", myParam );
       switch( myParam ) {
       case CONST_LEFT_VALUE_REF:
 	return CowPtr<T>( new T( * static_cast< const T* >( myPtr ) ) );
@@ -398,14 +423,43 @@ namespace DGtal
       case COW_PTR:
 	return CowPtr<T>( * static_cast< const CowPtr<T>* >( myPtr ) );
       case COUNTED_PTR:
-	return CowPtr<T>( * static_cast< const CountedPtr<T>* >( myPtr ) );
+	return CowPtr<T>( * static_cast< const CountedPtr<T>* >( myPtr ), true );
       case RIGHT_VALUE_REF:
 	return CowPtr<T>( new T( std::move( * const_cast<T*>( static_cast< const T* >( myPtr ) ) ) ) );
       default: ASSERT( false && "[Clone::operator CowPtr<T>() const] Invalid cast for given type. " );
         return CowPtr<T>( 0 );
       }
     }
 
+    /**
+       Cast operator to a smart pointer on T instance. The object is duplicated or not
+       depending on the type of input parameter.
+
+      - const T & -> CountedPtr<T>     // immediate duplication 
+      - T* -> CountedPtr<T>            // acquired              
+      - CountedPtr<T> -> CountedPtr<T> // lazy duplication      
+      - CowPtr<T> -> CountedPtr<T>     // immediate duplication 
+      - T&& -> CountedPtr<T>           // move into member
+    */
+    inline operator CountedPtr<T>() const
+    {
+      // display( "operator CountedPtr<T>() const", myParam );
+      switch( myParam ) {
+      case CONST_LEFT_VALUE_REF:
+	return CountedPtr<T>( new T( * static_cast< const T* >( myPtr ) ) );
+      case PTR:
+	return CountedPtr<T>( const_cast<T*>( static_cast< const T* >( myPtr ) ) );
+      case COW_PTR:
+	return CountedPtr<T>( new T( * static_cast< const CowPtr<T>* >( myPtr )->get() ) );
+      case COUNTED_PTR:
+	return CountedPtr<T>( * static_cast< const CountedPtr<T>* >( myPtr ) );
+      case RIGHT_VALUE_REF:
+	return CountedPtr<T>( new T( std::move( * const_cast<T*>( static_cast< const T* >( myPtr ) ) ) ) );
+      default: ASSERT( false && "[Clone::operator CountedPtr<T>() const] Invalid cast for given type. " );
+        return CountedPtr<T>( 0 );
+      }
+    }
+
     /**
        Address operator that returns the address of the given T
        instance.  The object is duplicated or not depending on the
@@ -421,6 +475,7 @@ namespace DGtal
     */
     inline T* operator&() const
     {
+      // display( "T* operator &() const", myParam );
       switch( myParam ) {
       case CONST_LEFT_VALUE_REF:
 	return new T( * static_cast< const T* >( myPtr ) );

src/DGtal/base/CowPtr.h

@@ -75,9 +75,35 @@ namespace DGtal
     // no need for ~CowPtr - the CountedPtr takes care of everything.
     CowPtr(const CowPtr& r) noexcept             : myPtr(r.myPtr) {}
     /**
-     * @todo JOL: check this.
+       Builds a copy-on-write pointer from a counted pointer. Requires
+       an extra dummy parameter in order to solve ambiguities when
+       casting from Clone<T> to CowPtr<T>.
+
+       @param r any counted pointer
+
+       @note Since 1.0, there is no more the direct constructor
+       CowPtr<T>::CowPtr( CountedPtr<T> ). Indeed, it was creating an
+       ambiguity when using conversion operator in class
+       Clone<T>. More precisely it was impossible to have two
+       conversion operators: Clone<T>::operator CountedPtr<T> and
+       Clone<T>::operator CowPtr<T> since this was creating a
+       compilation ambiguity. This is why there is now a dummy bool
+       parameter in this constructor. And there are the conversion
+       operator Clone<T>::operator CountedPtr<T> and
+       Clone<T>::operator CowPtr<T>.
+
+       @code
+       struct B {};
+       struct A {
+         CowPtr<B> _bcow;
+         CountedPtr<B> _bcounted;
+         // both constructions below are valid and do lazy copy if possible.
+         A( Clone<B> b1, Clone<B> b2 ) : _bcow( b1 ), _bcounted( b2 ) {}
+       };
+       @endcode
+
      */
-    CowPtr(const CountedPtr<T> & r)    : myPtr( r ) {}
+    CowPtr(const CountedPtr<T> & r, bool /* unused */ )    : myPtr( r ) {}
     CowPtr& operator=(const CowPtr& r)
     {
         if (this != &r)

tests/base/testClone2.cpp

@@ -241,7 +241,7 @@ struct CloneToValueMember {
 
 struct CloneToCountedMember { // requires explicit duplication
   inline CloneToCountedMember( Clone<DummyTbl> a1 ) // : myDummyTbl( a1 ) {} does not compile
-    : myDummyTbl( new DummyTbl( a1 ) ) {}
+    : myDummyTbl( a1 ) {} // : myDummyTbl( new DummyTbl( a1 ) ) {}
   inline int value() const { return myDummyTbl->value(); }
   CountedPtr<DummyTbl> myDummyTbl;
 };