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

[Marcus D. Hanwell]

On Tue, Jan 25, 2011 at 9:23 AM, Marcus D. Hanwell
<marcus.hanwell at kitware.com> wrote:
> 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.
>
I have updated the proposed change, now called vtkNew. Not to sway any
of you, but it is my birthday tomorrow and I have wanted something
like this in VTK for so long now ;-)

http://review.source.kitware.com/#change,759

Marcus