[Insight-users] itk::SmartPointer - problem making code const-correct

[Stuart Golodetz]

Luis Ibanez wrote:
>
> Hi Stuart,
>
> <snip>
>
> When I wrote
>
> >    That is, if you use smart pointers, you can call that function
> >    later with an argument that is a derived class of that image
> >    type.
>
> I actually meant to write
>
> >    That is, if you use *raw* pointers, you can call that function
> >    later with an argument that is a derived class of that image
> >    type.
>
> Sorry about the mishap.
No probs - just wanted to double-check.
>  ----------------------------------------------------------------------------
>
>>         If you look at most ITK filters, you will find that raw pointers
>>         are used in the API, for passing and receiving images.
>>
>>         However, when we call those functions we pass a SmartPointer
>>         to them (since SmartPointers know how to cast themselves as
>>         raw pointers).
>>
>>
>>     2) The only case in which you may want to pass an image
>>         SmartPointer as argument to a function is when you are
>>         creating that image inside the function.
>>
>>         In that case, passing the SmartPointer by reference is
>>         a reasonable choice.
>     I think I've been trying to use itk::SmartPointer as a
>     boost::shared_ptr, which may not be the best plan :) The idea
>     being that the pointed-to image or whatever won't get destroyed as
>     long as I'm holding an itk::SmartPointer to it, so I can construct
>     it somewhere, pass the itk::SmartPointer into a function, store a
>     copy of the itk::SmartPointer somewhere else and not worry if the
>     original itk::SmartPointer to the image no longer exists.
>
>     What I've been missing is that itk::SmartPointer appears to use
>     *intrusive* reference-counting (i.e. the reference count is stored
>     in the image itself), so that if I pass in a raw image pointer to
>     a function I can then construct a separate itk::SmartPointer the
>     other end from it that will share the reference count with the
>     original itk::SmartPointer. Is that right?
>
>
>
> That's exactly right.
>
> The actual "reference count" is stored  in the image.
> More specifically in the member variable:
>
>                   m_ReferenceCount
>
> which is defined in itkLightObject.h : line 141.
>
> and the image inherits it through the chain:
>
> itk::LightObject
> itk::Object
> itk::DataObject
> itk::ImageBase
> itk::Image
Ah right, passing raw pointers around makes a bit more sense in that 
case :) I was in shared_ptr mindset...
>
>
>>     3) Please note that the construction
>>
>>               const    Image::ConstPointer &  ptr ....;
>>
>>         prevents the internal mechanisms of the SmartPointer
>>         from working, since the "const" keyword prevents the "ptr"
>>         variable from changing. (strictly speaking it prevents the
>>         smart pointer from calling the non-const method Register()
>>         on the object that it points to).
>     Offhand, I think that could be easily changed by making Register()
>     and UnRegister() const methods, since that would only have the
>     effect of making m_Pointer itself const, not the ObjectType it
>     points to:
>
>     /** The pointer to the object referrred to by this smart pointer. */
>     ObjectType* m_Pointer;
>
>     void Register() --> const <--
>     {
>         if(m_Pointer) { m_Pointer->Register(); }
>     }
>      
>     void UnRegister() --> const <--
>     {
>         if(m_Pointer) { m_Pointer->UnRegister(); }
>     }
>
>
>
> Yes,
> Ideally these methods should have been "const".
> Since the reference count of the object doesn't
> really imply a change in its state.

In hindsight, I'm not sure that in this particular case the const-ness 
or otherwise of those methods is actually the problem (see my other 
post). When you do:

const Image::ConstPointer& cptr = ptr;

that creates a temporary to which cptr is then bound. In the constructor 
of the temporary (the point at which you call Register()), *this isn't 
const, so there's no problem. AFAICS the problem is that there's no 
implicit conversion from a Pointer to a ConstPointer - one way of 
changing that is to add the templatised constructor I mentioned in my 
other message:

template <typename T>
   SmartPointer(const SmartPointer<T>& p)
   :    m_Pointer(p.GetPointer())
   { this->Register(); }

If you add that, the problem seems to go away (I tried it, and there 
didn't seem to be a problem). The question is whether that might break 
something elsewhere (especially since itk::SmartPointer is a pretty 
vital part of ITK which simply has to always work).
>  
>
>     I haven't really looked at the implications that would have
>     elsewhere though - any thoughts?
>
>
>
> The implications are huge...   :-/
>
> In 2002 we attempted to fix the lack of
> const-correctness in the pipeline, and
> the effects propagated all over the toolkit
> to the point where we have to give up.
I'm not surprised! Whenever I've had to retrofit const-correctness to 
one of my code bases, it's always a complete nightmare :)

That being said, I wonder whether adding the above constructor (as 
opposed to sorting out const-correctness generally) actually causes any 
problems. If not, is there anywhere I can submit it for proper 
consideration? I realise that suggesting even a minor change to 
something as fundamental as itk::SmartPointer is probably a big deal, 
but I thought I should ask just on the off-chance. I guess it partly 
depends on whether allowing the implicit conversion is considered 
desirable - what do you think?
> In order to fix the lack of const-correctness
> in the pipeline your will have to rewrite the
> pipeline from scratch. 
>
>
> Something that,
> may or may not happen as part of ITKv4....
I'll watch and wait :) I guess that probably won't happen till after 
I've finished my doctorate though.

Cheers,
Stu
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/pipermail/insight-users/attachments/20100529/102d2b75/attachment-0001.htm>