[vtk-developers] RFC: vtkScopedPointer - a new hope?

[Marcus D. Hanwell]

On Mon, Jan 24, 2011 at 9:29 PM, David Cole <david.cole at kitware.com> wrote:
> On Mon, Jan 24, 2011 at 8:21 PM, Marcus D. Hanwell
> <marcus.hanwell at kitware.com> wrote:
>> On Mon, Jan 24, 2011 at 7:05 PM, David Gobbi <david.gobbi at gmail.com> wrote:
>>> On Mon, Jan 24, 2011 at 2:07 PM, David Cole <david.cole at kitware.com> wrote:
>>>> On Mon, Jan 24, 2011 at 4:06 PM, David Cole <david.cole at kitware.com> wrote:
>>>>> On Mon, Jan 24, 2011 at 3:48 PM, Marcus D. Hanwell
>>>>> <marcus.hanwell at kitware.com> wrote:
>>>>>> On Mon, Jan 24, 2011 at 3:41 PM, tom fogal <tfogal at sci.utah.edu> wrote:
>>>>>>> "Marcus D. Hanwell" <marcus.hanwell at kitware.com> writes:
>>>>>>>> Some of you may remember my dreams of a concise way of initializing
>>>>>>>> VTK objects, that would go out of scope and be destroyed at the
>>>>>>>> appropriate time. Inspired by the Qt scoped pointer, [. . .]
>>>>>>>>
>>>>>>>> http://review.source.kitware.com/759
>>>>>>>
>>>>>>> scoped (and other) smart pointers were introduced with tr1, back in
>>>>>>> 2003 IIRC.  Further boost has had this for quite some time:
>>>>>>>
>>>>>>>  http://www.boost.org/doc/libs/1_45_0/libs/smart_ptr/scoped_ptr.htm?sess=94fbb4fa5cfd2b2071c3be193acb355d
>>>>>>>
>>>>>>> I realize that vtk is a bit stuck because it has already shipped some
>>>>>>> smart pointers, and thus probably must continue to do so for some time
>>>>>>> due to backward compat requirements, but I'd discourage *extending*
>>>>>>> the trend.  This is a painful interop problem, especially with larger
>>>>>>> software systems -- there's probably some C++ platitude that states,
>>>>>>> "Every library eventually evolves a smart pointer implementation."
>>>>>>>
>>>>>>> Or there is now, at least.
>>>>>>>
>>>>>> Would any of these scoped pointers actually work with VTK derived
>>>>>> objects? We have private constructors, need to call the static New
>>>>>> method, and the Delete method on destruction. I also don't see VTK
>>>>>> depending on TR1 features, or Boost for quite some time. The class is
>>>>>> quite minimal, and I think it would complement the other two pointer
>>>>>> classes that help to manage VTK derived objects.
>>>>>>
>>>>>
>>>>> The boost discussion points out an important consideration here,
>>>>> though: the name "scoped pointer" is assumed to be non-copyable and
>>>>> non-transferrable out of the scoped context in the boost world. Here,
>>>>> with your implementation, and with a reference counted system like
>>>>> vtkObject, the ownership *can* be transferred outside the context of
>>>>> the scoped pointer variable itself.
>>>>>
>>>>> So.... by using the same name as boost for something that is slightly
>>>>> different, (but still extremely useful within VTK), we may be adding
>>>>> to the heap of confusion that is smart/auto/scoped pointer discussions
>>>>> in the C++ world.
>>>>>
>>>>> Might we be able to come up with a better name for it that does not
>>>>> already have a different shade of meaning in the context of a "very
>>>>> large" and "familiar to many" project like boost?
>>>>>
>>>>
>>>> It's really just a vtkSmartPointer with an "easy create" syntax... Isn't it?
>>>>
>>>> Maybe vtkEasyPointer would be a better name?
>>>
>>> I like ScopedPointer better than EasyPointer.  But I liked the old
>>> name vtkNew even more, because it made it obvious that an object was
>>> being created.  How about another suggestion:
>>>
>>> vtkAutoPointer<vtkSomething> a;
>>>
>>> The name "auto" gives a sense that an object is being allocated.  It
>>> implies "this creates an automatically garbage-collected object",
>>> while the name vtkScopedPointer makes it sound like a pointer is being
>>> created but not an object.
>>
>> I wonder if vtkScopedNew might satisfy more people? Brad was
>> explaining the VTK convention that anything that instantiates should
>> have New in its name, but he would not like to use vtkNew. It is a
>> scoped new object, and so I feel like it could be a reasonable
>> compromise clearing up any ambiguity.
>>
>> So in a test, one can simply use,
>>
>> vtkScopedNew<vtkClass> a;
>> a->DoSomething();
>>
>> When it goes out of scope, reference count is decremented. If you want
>> the raw pointer you must call GetPointer, and can use something like,
>>
>> vtkSmartPointer<vtkClass> b = a.GetPointer();
>>
>> I think this is a secondary use case of this class, and if you simply
>> want a smart pointer with an object, you should stick with the
>> conventional syntax. The primary use case was to replace the VTK
>> create macro used in many tests. It should also serve the use case of
>> ivars in classes where you cannot set that object (as you cannot
>> assign to a vtkScoped[TBD] object.
>>
>> I did like vtkNew, but the more I think about it vtkScopedNew seems to
>> encapsulate its name, and accomplishes one of the primary goals when
>> this thread was started all that time ago.
>>
>> Marcus
>>
>
>
> I like just "vtkNew" the best.
>
> Neither Auto nor Scoped mean quite the same thing here as in other contexts.
>
> Why don't we want to use vtkNew in this situation? What are we saving it for?
>
> Or, we could use "vtkNu" -- it's shorter, and it's sort of a Greek letter. ;-)
>
vtkNu sounds good, and saves a character ;-) Main reason was Brad
saying he wanted to reserve vtkNew for some future use, maybe he could
explain that better than I could. I was always happy with vtkNew.

I think the assignment operator interaction with vtkSmartPointer from
the original is not necessary, and adds more complication/confusion to
the API. I think this was our point of divergence when we were
discussing this offline.

Marcus