Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Motivating SO question: Is there a C++/CLI smart pointer project (e.g. scoped_ptr)?

I'm interested in any reviewer comments, and especially identified bugs or inconsistencies with the native scoped_ptr template after which this code is patterned.

#pragma once

/** @file clr_scoped_ptr.h
 ** @author R Benjamin Voigt (richardvoigt@gmail.com)
 ** 
 ** Rights reserved.  This code is not public domain.
 ** 
 ** Licensed under CC BY-SA 3.0             http://creativecommons.org/licenses/by-sa/3.0/
 **             or Lesser GPL 3 or later    http://www.gnu.org/copyleft/lesser.html
 ** with the following restrictions (per GPL section 7):
 **  - all warranties are disclaimed, and if this is prohibited by law, your sole remedy shall be recovery of the price you paid to receive the code
 **  - derived works must not remove this license notice or author attribution
 **  - modifications must not be represented as the work of the original author
 **  - attribution is required in the "about" or "--version" display of any work linked hereto, or wherever copyright notices are displayed by the composite work
 **/

struct safe_bool { private: safe_bool(); };

/** \brief C++/CLI analogue to boost::scoped_ptr, also similar to std::unique_ptr, for management of the lifetime of an unmanaged class instance by a managed object
 **/
template<typename T>
public ref class clr_scoped_ptr
{
    T* m_native_ptr;

    // declare copy-constructors and assignment operators to prevent double-free
    clr_scoped_ptr( clr_scoped_ptr<T>% ) /* = delete */ { throw gcnew System::InvalidOperationException("clr_scoped_ptr is non-copyable"); }

    template<typename U>
    clr_scoped_ptr( clr_scoped_ptr<U>% ) { throw gcnew System::InvalidOperationException("clr_scoped_ptr is non-copyable"); }

    clr_scoped_ptr% operator=( clr_scoped_ptr<T>% ) /* = delete */ { throw gcnew System::InvalidOperationException("clr_scoped_ptr is non-copyable"); }

    template<typename U>
    clr_scoped_ptr% operator=( clr_scoped_ptr<U>% ) { throw gcnew System::InvalidOperationException("clr_scoped_ptr is non-copyable"); }

public:
    clr_scoped_ptr( void ) : m_native_ptr(nullptr) {}
    explicit clr_scoped_ptr( T* ptr ) : m_native_ptr(ptr) {}
    !clr_scoped_ptr( void ) { reset(); }
    ~clr_scoped_ptr( void ) { clr_scoped_ptr::!clr_scoped_ptr(); }

    template<typename U>
    clr_scoped_ptr( U ptr ) : m_native_ptr(ptr) {}

    void reset( T* ptr ) { delete m_native_ptr; m_native_ptr = ptr; }
    void reset( void ) { reset(nullptr); }

    clr_scoped_ptr% operator=( T* ptr ) { reset(ptr); }

    template<typename U>
    clr_scoped_ptr% operator=( U ptr ) { reset(ptr); }

    operator struct safe_bool*() { return reinterpret_cast<struct safe_bool*>(m_native_ptr); }

    void swap( clr_scoped_ptr<T>% other )
    {
        using std::swap;
        swap(m_native_ptr, other.m_native_ptr);
    }

    T* release( void ) { T* retval = m_native_ptr; m_native_ptr = nullptr; return retval; }
    T* get( void ) { return m_native_ptr; }

    static T* operator->( clr_scoped_ptr<T>% sptr ) { return sptr.get(); }
    static T& operator*( clr_scoped_ptr<T>% sptr ) { return *sptr.get(); }
};

template<typename T>
inline void swap( clr_scoped_ptr<T>% left, clr_scoped_ptr<T>% right )
{
    left.swap(right);
}
share|improve this question
    
Well the code seems to fine. But on this page this syntax is used for copy ctor : ClassName(const ClassName^ & Name); What is the difference? –  ali_bahoo Apr 7 '11 at 9:37
    
@sad_man: That page is wrong in two ways. First, however useful passing in a ClassName^ to a constructor might be, it's not a copy constructor. And secondly, since a ClassName^ can live on the gc heap, the tracking reference % must be used instead of &. –  Ben Voigt Apr 7 '11 at 14:08
    
I see. So what does ClassName^ & Name mean? A native reference to a managed reference? What is the use of it? I thought it was a special syntax for copy constructor. –  ali_bahoo Apr 7 '11 at 15:00
    
@sad_man: Yes, it would be a native reference to a managed handle. Theoretically, since you can have a managed handle on the stack, you could have a native reference to it. But I don't think the compiler allows that. % is definitely better, since it works with both variables on the stack and in the heap. –  Ben Voigt Apr 7 '11 at 18:51

3 Answers 3

up vote 5 down vote accepted

I have only superficially tested the code so far, but it seems to make sense. However, there are some details where I'm either missing something (which wouldn't surprise me, I'm more at home with C#) or the code is more complex than it needs to be. Any comments are appreciated!

  • The private copy constructors and assignment operators throw exceptions although they can never be called. Wouldn't it suffice to leave them empty?
  • The constructor and assignment operator both accept not only arguments of type T*, but also exist in a templated version taking U*. I can't quite figure out why. My first thought was that this allows pointers to derived types to be passed; but then again, this is also possible using just the first form.
  • Similarly, I don't understand the role of the template versions of the private constructor and assignment operator. From my understanding, neither of these will be auto-generated if you don't provide private versions.
  • The safe bool pattern sound like a great idea (I first had to research it). However, VS2010 shows a compile error saying "cannot convert from 'clr_scoped_ptr<T>::operator safe_bool *::safe_bool *' to 'safe_bool *'". Apparently, it treats the two occurrences of struct safe_bool as two different types. Am I missing something here?
  • You end the private methods and operators with a semicolon and you write empty argument lists as (void) rather than (). Are these just stylistic choices, or are there advantages in doing this?
share|improve this answer
    
In standard C++, the "deleted" special functions would have no definition at all, resulting in a linker error if they were used. I think the semicolon got added then and simply stuck around. Throwing the exception is protection against accidentally calling those from inside the class, because private doesn't prevent that. The constructor taking U* is supposed to take simply U, accepting any smart pointer with an implicit conversion to T*, thanks for catching that. And the private template constructor is to exclude clr_scoped_ptr objects from using the public template. –  Ben Voigt Apr 10 '11 at 0:28
    
Also, not all C-like languages interpret an empty parameter list the same way, but void in the parameter list has the same meaning in every language, so that's what I use. The safe_bool idiom usually involves a private nested type, but a native type can't be nested inside a managed type (and a nested managed type would cause accessibility complaints from the compiler) so to make this work, it needs a struct defined in the same namespace, which I've just added. –  Ben Voigt Apr 10 '11 at 0:33
    
Ben, thanks for your explanations. They make perfect sense. –  Daniel Wolf Apr 10 '11 at 10:07

You declare the operator= as returning a clr_scoped_ptr%, but there's no return statement in their body. This gives me compilation errors. I suppose the implementation should be:

clr_scoped_ptr% operator=( T* ptr ) { reset(ptr); return (clr_scoped_ptr%)this; } 

template<typename U>
clr_scoped_ptr% operator=( U ptr ) { reset(ptr); return (clr_scoped_ptr%)this; }
share|improve this answer
    
Good catch, although the correct code is simply return *this; –  Ben Voigt Jul 4 at 22:41
    
Ah, that's right, thanks for the precision. –  Asik Jul 5 at 2:13

I would also add that the copy constructors and assignments operators should throw NotSupportedException rather than InvalidOperationException. The latter is supposed to be dependent on object state.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.