Overview
Features
Download
Documentation
Community
Add-Ons & Services

Const correctness and AutoPtr

General discussion regarding the development of POCO for contributors.

Const correctness and AutoPtr

Postby mandermo » 04 Mar 2011, 17:21

Hi,

There exist some problems with AutoPtr and SharedPtr, mostly related to constness.

First of all “const AutoPtr<TestObj> p” should mean that you can not reassign p (similar to “TestObj* const p”). “AutoPtr<const TestObj> p” should mean that you can’t modify the TestObj object (similar to “const TestObj* p”). That is not the current semantics and in my opinion AutoPtr should be changed to conform to that semantics. This is the semantics chosen by std::auto_ptr and std::shared_ptr. It makes sense because you can convert a “const AutoPtr<TestObj>” to an “AutoPtr<TestObj>” through a simple assignment. I personally do not regard an assignment semantically as a const_cast, but that is the current semantics of AutoPtr.

Secondly when you modify get(), operator* and operator-> to have the mainstream semantics explained above, then you will gets errors for the relation operators for AutoPtr<const TestObj>, because “bool operator < (C* ptr) const” and bool operator < (const C* ptr) const, will be the same because “C*” will be “const TestObj*” and “const C*” will also be regarded as “const TestObj*”.

When you only keep one of the the overloads of the conversion operators, then you do not need to define the relation operators (<, <=, etc), to get the code to compile. However, it is not always okay to compare pointers using < (it is okay if they are part of the same array, etc). Here is the excerpt from §5.9 from the Standard of C++03
“If two pointers p and q of the same type point to different objects that are not members of the same object or elements of the same array or to different functions, or if only one of them is null, the results of p<q, p>q, p<=q, and p>=q are unspecified.”
Therefore you should use std::less and the other predicates, because they lack that restriction. Here is an except form §20.3.3 in the Standard: “For templates greater, less, greater_equal, and less_equal, the specializations for any pointer type yield a total order, even if the built-in operators <, >, <=, >= do not.”

The change is backwards compatible, since all cases when you return a non-const pointer for the current AutoPtr, will continue to return a non-const pointer, the same goes for references.

When making those changes, I could remove some dangerous const_casts in the current implementation.

I have updated the test cases and the AutoPtr, here is the source:
AutoPtr.h: http://pastebin.com/b0wwHeS7
AutoPtr.cpp: http://pastebin.com/MB78mXkq

Here is the diff for AutoPtr.h:
Code: Select all
--- Foundation/include/Poco/AutoPtr.h.poco      2011-03-03 11:07:06.133501000 +0100                                                             
+++ Foundation/include/Poco/AutoPtr.h   2011-03-07 12:20:22.181319000 +0100                                                                     
@@ -43,7 +43,7 @@                                                                                                                               
 #include "Poco/Foundation.h"                                                                                                                   
 #include "Poco/Exception.h"                                                                                                                   
 #include <algorithm>                                                                                                                           
-                                                                                                                                               
+#include <functional> // for std::less, greater, etc                                                                                           
                                                                                                                                               
 namespace Poco {                                                                                                                               
                                                                                                                                               
@@ -102,7 +102,7 @@                                                                                                                             
        }                                                                                                                                       
                                                                                                                                               
        template <class Other>                                                                                                                 
-       AutoPtr(const AutoPtr<Other>& ptr): _ptr(const_cast<Other*>(ptr.get()))                                                                 
+       AutoPtr(const AutoPtr<Other>& ptr): _ptr(ptr.get())                                                                                     
        {                                                                                                                                       
                if (_ptr) _ptr->duplicate();                                                                                                   
        }                                                                                                                                       
@@ -150,7 +150,7 @@                                                                                                                             
                if (ptr.get() != _ptr)                                                                                                         
                {                                                                                                                               
                        if (_ptr) _ptr->release();                                                                                             
-                       _ptr = const_cast<Other*>(ptr.get());                                                                                   
+                       _ptr = ptr.get();                                                                                                       
                        if (_ptr) _ptr->duplicate();                                                                                           
                }                                                                                                                               
                return *this;                                                                                                                   
@@ -202,15 +202,7 @@                                                                                                                           
                return AutoPtr<Other>(pOther, true);                                                                                           
        }                                                                                                                                       
                                                                                                                                               
-       C* operator -> ()                                                                                                                       
-       {                                                                                                                                       
-               if (_ptr)                                                                                                                       
-                       return _ptr;                                                                                                           
-               else                                                                                                                           
-                       throw NullPointerException();                                                                                           
-       }                                                                                                                                       
-                                                                                                                                               
-       const C* operator -> () const                                                                                                           
+       C* operator -> () const                                                                                                                 
        {                                                                                                                                       
                if (_ptr)                                                                                                                       
                        return _ptr;                                                                                                           
@@ -218,15 +210,7 @@                                                                                                                           
                        throw NullPointerException();                                                                                           
        }                                                                                                                                       
                                                                                                                                               
-       C& operator * ()                                                                                                                       
-       {                                                                                                                                       
-               if (_ptr)                                                                                                                       
-                       return *_ptr;                                                                                                           
-               else                                                                                                                           
-                       throw NullPointerException();                                                                                           
-       }                                                                                                                                       
-                                                                                                                                               
-       const C& operator * () const                                                                                                           
+       C& operator * () const                                                                                                                 
        {                                                                                                                                       
                if (_ptr)                                                                                                                       
                        return *_ptr;                                                                                                           
@@ -234,22 +218,13 @@                                                                                                                           
                        throw NullPointerException();                                                                                           
        }                                                                                                                                       
                                                                                                                                               
-       C* get()                                                                                                                               
-       {                                                                                                                                       
-               return _ptr;                                                                                                                   
-       }                                                                                                                                       
-                                                                                                                                               
-       const C* get() const                                                                                                                   
-       {                                                                                                                                       
-               return _ptr;                                                                                                                   
-       }                                                                                                                                       
-                                                                                                                                               
-       operator C* ()                                                                                                                         
+       C* get() const                                                                                                                         
        {                                                                                                                                       
                return _ptr;                                                                                                                   
        }                                                                                                                                       
                                                                                                                                               
-       operator const C* () const                                                                                                             
+       // This also serves as an safe implicit conversion to a boolean type                                                                   
+       operator C* () const                                                                                                                   
        {                                                                                                                                       
                return _ptr;                                                                                                                   
        }                                                                                                                                       
@@ -264,115 +239,199 @@                                                                                                                         
                return _ptr == 0;                                                                                                               
        }                                                                                                                                       
                                                                                                                                               
-       C* duplicate()                                                                                                                         
+       C* duplicate() const                                                                                                                   
        {                                                                                                                                       
                if (_ptr) _ptr->duplicate();                                                                                                   
                return _ptr;                                                                                                                   
        }                                                                                                                                       
+private:                                                                                                                                       
+       C* _ptr;                                                                                                                               
+};                                                                                                                                             
                                                                                                                                               
-       bool operator == (const AutoPtr& ptr) const                                                                                             
+namespace Detail {                                                                                                                             
+       // This class will generate a compilation error if the types T and U                                                                   
+       // can not be compared, i.e. they do not have a a composite                                                                             
+       // pointer type                                                                                                                         
+       // Intentionaly complex to reduce the likeliness of warnings                                                                           
+       template <class T, class U>                                                                                                             
+       struct EnsureHasCompositePointerType                                                                                                   
        {                                                                                                                                       
-               return _ptr == ptr._ptr;                                                                                                       
-       }                                                                                                                                       
+               static void constraints(T* a, U* b, bool& x)                                                                                   
+               {                                                                                                                               
+                       // If you get a compilation error here, then you have                                                                   
+                       // tried to compare two smart pointers with                                                                             
+                       // unrelated types.                                                                                                     
+                       x = a < b;                                                                                                             
+               }                                                                                                                               
+               typedef void f_type(T*, U*, bool&);                                                                                             
+               void no_op(f_type)                                                                                                             
+               {                                                                                                                               
+                       // empty on purpose                                                                                                     
+               }                                                                                                                               
+               EnsureHasCompositePointerType()                                                                                                 
+               {                                                                                                                               
+                       // This assignment should be optimized away by the compiler                                                             
+                       void (*p)(T*, U*, bool&) = constraints;                                                                                 
+                       // To get rid of the warning that we do not use the                                                                     
+                       // variable                                                                                                             
+                       no_op(p);                                                                                                               
+               }                                                                                                                               
+       };                                                                                                                                     
+                                                                                                                                               
+       // The use of EnsureHasCompositePointerType, is done to ensure that                                                                     
+       // the pointers can be compared using the normal C++ language rules.                                                                   
+       // The reason the operators are not used directly is because                                                                           
+       // they are not required to give a total ordering for objects of any                                                                   
+       // type, however the predicates, eg. std::less, are required to give                                                                   
+       // a total ordering.                                                                                                                   
                                                                                                                                               
-       bool operator == (const C* ptr) const                                                                                                   
+       template <class T, class U>                                                                                                             
+       bool pointerLess(T* a, U* b)                                                                                                           
        {                                                                                                                                       
-               return _ptr == ptr;                                                                                                             
+               EnsureHasCompositePointerType<T, U>();                                                                                         
+               return std::less<const void*>()(a, b);                                                                                         
        }                                                                                                                                       
-                                                                                                                                               
-       bool operator == (C* ptr) const                                                                                                         
+       template <class T, class U>                                                                                                             
+       bool pointerLessEq(T* a, U* b)                                                                                                         
        {                                                                                                                                       
-               return _ptr == ptr;                                                                                                             
+               EnsureHasCompositePointerType<T, U>();                                                                                         
+               return std::less_equal<const void*>()(a, b);                                                                                   
        }                                                                                                                                       
-                                                                                                                                               
-       bool operator != (const AutoPtr& ptr) const                                                                                             
+       template <class T, class U>                                                                                                             
+       bool pointerGreater(T* a, U* b)                                                                                                         
        {                                                                                                                                       
-               return _ptr != ptr._ptr;                                                                                                       
+               EnsureHasCompositePointerType<T, U>();                                                                                         
+               return std::greater<const void*>()(a, b);                                                                                       
        }                                                                                                                                       
-                                                                                                                                               
-       bool operator != (const C* ptr) const                                                                                                   
+       template <class T, class U>                                                                                                             
+       bool pointerGreaterEq(T* a, U* b)                                                                                                       
        {                                                                                                                                       
-               return _ptr != ptr;                                                                                                             
+               EnsureHasCompositePointerType<T, U>();                                                                                         
+               return std::greater_equal<const void*>()(a, b);                                                                                 
        }                                                                                                                                       
+}                                                                                                                                             
                                                                                                                                               
-       bool operator != (C* ptr) const                                                                                                         
-       {                                                                                                                                       
-               return _ptr != ptr;                                                                                                             
-       }                                                                                                                                       
+template <class C>                                                                                                                             
+inline void swap(AutoPtr<C>& p1, AutoPtr<C>& p2)                                                                                               
+{                                                                                                                                             
+       p1.swap(p2);                                                                                                                           
+}                                                                                                                                             
                                                                                                                                               
-       bool operator < (const AutoPtr& ptr) const                                                                                             
-       {                                                                                                                                       
-               return _ptr < ptr._ptr;                                                                                                         
-       }                                                                                                                                       
+template <class T, class U>                                                                                                                   
+bool operator == (const AutoPtr<T>& a, const AutoPtr<U>& b)                                                                                   
+{                                                                                                                                             
+       return a.get() == b.get();                                                                                                             
+}                                                                                                                                             
                                                                                                                                               
-       bool operator < (const C* ptr) const                                                                                                   
-       {                                                                                                                                       
-               return _ptr < ptr;                                                                                                             
-       }                                                                                                                                       
+template <class T, class U>                                                                                                                   
+bool operator == (T* a, const AutoPtr<U>& b)                                                                                                   
+{                                                                                                                                             
+       return a == b.get();                                                                                                                   
+}                                                                                                                                             
                                                                                                                                               
-       bool operator < (C* ptr) const                                                                                                         
-       {                                                                                                                                       
-               return _ptr < ptr;                                                                                                             
-       }                                                                                                                                       
+template <class T, class U>                                                                                                                   
+bool operator == (const AutoPtr<T>& a, U* b)                                                                                                   
+{                                                                                                                                             
+       return a.get() == b;                                                                                                                   
+}                                                                                                                                             
                                                                                                                                               
-       bool operator <= (const AutoPtr& ptr) const                                                                                             
-       {                                                                                                                                       
-               return _ptr <= ptr._ptr;                                                                                                       
-       }                                                                                                                                       
                                                                                                                                               
-       bool operator <= (const C* ptr) const                                                                                                   
-       {                                                                                                                                       
-               return _ptr <= ptr;                                                                                                             
-       }                                                                                                                                       
+template <class T, class U>                                                                                                                   
+bool operator != (const AutoPtr<T>& a, const AutoPtr<U>& b)                                                                                   
+{                                                                                                                                             
+       return a.get() != b.get();                                                                                                             
+}                                                                                                                                             
                                                                                                                                               
-       bool operator <= (C* ptr) const                                                                                                         
-       {                                                                                                                                       
-               return _ptr <= ptr;                                                                                                             
-       }                                                                                                                                       
+template <class T, class U>                                                                                                                   
+bool operator != (T* a, const AutoPtr<U>& b)                                                                                                   
+{                                                                                                                                             
+       return a != b.get();                                                                                                                   
+}                                                                                                                                             
                                                                                                                                               
-       bool operator > (const AutoPtr& ptr) const                                                                                             
-       {                                                                                                                                       
-               return _ptr > ptr._ptr;                                                                                                         
-       }                                                                                                                                       
+template <class T, class U>                                                                                                                   
+bool operator != (const AutoPtr<T>& a, U* b)                                                                                                   
+{                                                                                                                                             
+       return a.get() != b;                                                                                                                   
+}                                                                                                                                             
                                                                                                                                               
-       bool operator > (const C* ptr) const                                                                                                   
-       {                                                                                                                                       
-               return _ptr > ptr;                                                                                                             
-       }                                                                                                                                       
                                                                                                                                               
-       bool operator > (C* ptr) const                                                                                                         
-       {                                                                                                                                       
-               return _ptr > ptr;                                                                                                             
-       }                                                                                                                                       
+template <class T, class U>                                                                                                                   
+bool operator < (const AutoPtr<T>& a, const AutoPtr<U>& b)                                                                                     
+{                                                                                                                                             
+       return Detail::pointerLess(a.get(), b.get());                                                                                           
+}                                                                                                                                             
                                                                                                                                               
-       bool operator >= (const AutoPtr& ptr) const                                                                                             
-       {                                                                                                                                       
-               return _ptr >= ptr._ptr;                                                                                                       
-       }                                                                                                                                       
+template <class T, class U>                                                                                                                   
+bool operator < (T* a, const AutoPtr<U>& b)                                                                                                   
+{                                                                                                                                             
+       return Detail::pointerLess(a, b.get());                                                                                                 
+}                                                                                                                                             
                                                                                                                                               
-       bool operator >= (const C* ptr) const                                                                                                   
-       {                                                                                                                                       
-               return _ptr >= ptr;                                                                                                             
-       }                                                                                                                                       
+template <class T, class U>                                                                                                                   
+bool operator < (const AutoPtr<T>& a, U* b)                                                                                                   
+{                                                                                                                                             
+       return Detail::pointerLess(a.get(), b);                                                                                                 
+}                                                                                                                                             
                                                                                                                                               
-       bool operator >= (C* ptr) const                                                                                                         
-       {                                                                                                                                       
-               return _ptr >= ptr;                                                                                                             
-       }                                                                                                                                       
                                                                                                                                               
-private:                                                                                                                                       
-       C* _ptr;                                                                                                                               
-};                                                                                                                                             
+template <class T, class U>                                                                                                                   
+bool operator <= (const AutoPtr<T>& a, const AutoPtr<U>& b)                                                                                   
+{                                                                                                                                             
+       return Detail::pointerLessEq(a.get(), b.get());                                                                                         
+}                                                                                                                                             
                                                                                                                                               
+template <class T, class U>                                                                                                                   
+bool operator <= (T* a, const AutoPtr<U>& b)                                                                                                   
+{                                                                                                                                             
+       return Detail::pointerLessEq(a, b.get());                                                                                               
+}                                                                                                                                             
                                                                                                                                               
-template <class C>                                                                                                                             
-inline void swap(AutoPtr<C>& p1, AutoPtr<C>& p2)                                                                                               
+template <class T, class U>                                                                                                                   
+bool operator <= (const AutoPtr<T>& a, U* b)                                                                                                   
 {                                                                                                                                             
-       p1.swap(p2);                                                                                                                           
+       return Detail::pointerLessEq(a.get(), b);                                                                                               
 }                                                                                                                                             
                                                                                                                                               
                                                                                                                                               
+template <class T, class U>                                                                                                                   
+bool operator > (const AutoPtr<T>& a, const AutoPtr<U>& b)
+{
+       return Detail::pointerGreater(a.get(), b.get());
+}
+
+template <class T, class U>
+bool operator > (T* a, const AutoPtr<U>& b)
+{
+       return Detail::pointerGreater(a, b.get());
+}
+
+template <class T, class U>
+bool operator > (const AutoPtr<T>& a, U* b)
+{
+       return Detail::pointerGreater(a.get(), b);
+}
+
+
+template <class T, class U>
+bool operator >= (const AutoPtr<T>& a, const AutoPtr<U>& b)
+{
+       return Detail::pointerGreaterEq(a.get(), b.get());
+}
+
+template <class T, class U>
+bool operator >= (T* a, const AutoPtr<U>& b)
+{
+       return Detail::pointerGreaterEq(a, b.get());
+}
+
+template <class T, class U>
+bool operator >= (const AutoPtr<T>& a, U* b)
+{
+       return Detail::pointerGreaterEq(a.get(), b);
+}
+
 } // namespace Poco


 #endif // Foundation_AutoPtr_INCLUDED
+
mandermo
 
Posts: 1
Joined: 04 Mar 2011, 13:41

Return to Contributors

Who is online

Users browsing this forum: No registered users and 2 guests