[vtk-developers] vtk-developers Digest, Vol 158, Issue 21

[Andras Lasso]

It is great that you've brought this up! There is a very important, fundamental difference. Unlike std::shared_ptr or QSharedPointer, VTK smart pointer does NOT need clear distinction between the pointer object and the raw pointer it holds, because reference counting is implemented in the managed object itself.

See in std::shared_ptr documentation: "Constructing a new shared_ptr using the raw underlying pointer owned by another shared_ptr leads to undefined behavior." (http://en.cppreference.com/w/cpp/memory/shared_ptr)

In VTK, if you assign a vtkSmartPointer to another, internally the raw pointer is retrieved and used (Common/Core/vtkSmartPointer.h):

  vtkSmartPointer& operator=(const vtkSmartPointer<U>& r)
  {
    this->vtkSmartPointerBase::operator=(CheckType(r.GetPointer()));
    return *this;
  }

VTK smart pointers work differently from STL and Qt, so it is reasonable to use a different API that reflects this difference. As VTK allows safe use of a simpler API, we should take advantage of that and use it.

Andras

-----Original Message-----
From: Elvis Stansvik [mailto:elvis.stansvik at orexplore.com] 
Sent: Saturday, June 24, 2017 12:36 PM
To: Andras Lasso <lasso at queensu.ca>
Cc: David Lonie <david.lonie at kitware.com>; Shawn Waldon <shawn.waldon at kitware.com>; VTK Developers <vtk-developers at vtk.org>; Andrew Maclean <andrew.amaclean at gmail.com>
Subject: Re: [vtk-developers] vtk-developers Digest, Vol 158, Issue 21

2017-06-24 15:53 GMT+02:00 Andras Lasso <lasso at queensu.ca>:
> Based on the feedbacks and the poll results (11 out of 13 votes for 
> changing current behavior), it seems there is a strong consensus for adding implicit conversion.

I was away and missed the vote, but frankly I'm not sure what I would have voted.

Looking at what others do, I guess vtkNew is comparable to std::unique_ptr or Qt's QScopedPointer, none of which do implicit conversion to the underlying pointer (they have .get() and .data() respectively). So VTK would be rather unique in doing that for a scoped smart pointer I think..

Elvis

>
> Here is a pull request with the change:
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla
> b.kitware.com%2Fvtk%2Fvtk%2Fmerge_requests%2F2961&data=02%7C01%7Classo
> %40queensu.ca%7C14c5314b70e443f5be4908d4bb1f0bf4%7Cd61ecb3b38b142d582c
> 4efb2838b925c%7C1%7C0%7C636339189405346369&sdata=2Jv0QS1qlRUgSJvN%2BO8
> 0pXOt89kKC0NP7iXXRr2mg30%3D&reserved=0
>
> Andras
>
> -----Original Message-----
> From: David Lonie [mailto:david.lonie at kitware.com]
> Sent: Friday, June 23, 2017 1:51 PM
> To: Shawn Waldon <shawn.waldon at kitware.com>
> Cc: Andras Lasso <lasso at queensu.ca>; VTK Developers 
> <vtk-developers at vtk.org>; Andrew Maclean <andrew.amaclean at gmail.com>
> Subject: Re: [vtk-developers] vtk-developers Digest, Vol 158, Issue 21
>
> I'm so glad someone brought this up =)
>
> On Fri, Jun 23, 2017 at 10:52 AM, Shawn Waldon <shawn.waldon at kitware.com> wrote:
>>  But realistically what it does is
>> discourage use of vtkNew.  Most people I talk to say "I would use 
>> vtkNew but then I have to clutter up my code with GetPointer calls 
>> everywhere" and so they still use vtkSmartPointer.  Honestly I only 
>> used it to clean up declarations (the problem discussed earlier in 
>> this thread).  Now that we can use auto in VTK, I'll probably go back 
>> to vtkSmartPointer since it is easier to use.
>
> I agree with Shawn. It's an unnecessary hassle to require the Get() to be used.
>
> I suppose I'm just a fan of the design philosophy for C++ about "don't prevent developers from shooting themselves in the foot." If a dev doesn't understand VTK's reference counting well enough to understand that you can't simply return a vtkNew'd pointer without increfing it first, they're going to have far greater problems using the toolkit than this.
>
> Besides, you could easily (and safely) return a vtkNew'd pointer from a function:
>
> vtkObject* function()
> {
>   vtkNew<vtkObject> obj;
>   obj->Register();
>   return obj; // If we had implicit conversions, anyway }
>
> Making the class unwieldly simply so that people don't need to learn how reference counts work is a poor justification IMO.
>
> Just my 2c,
> Dave
> _______________________________________________
> Powered by 
> https://na01.safelinks.protection.outlook.com/?url=www.kitware.com&dat
> a=02%7C01%7Classo%40queensu.ca%7C14c5314b70e443f5be4908d4bb1f0bf4%7Cd6
> 1ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636339189405346369&sdata=EWly
> 8qDaOVuCYOrDhoQ7SyY3glwTdlm1J1nxY%2BBDO50%3D&reserved=0
>
> Visit other Kitware open-source projects at 
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.ki
> tware.com%2Fopensource%2Fopensource.html&data=02%7C01%7Classo%40queens
> u.ca%7C14c5314b70e443f5be4908d4bb1f0bf4%7Cd61ecb3b38b142d582c4efb2838b
> 925c%7C1%7C0%7C636339189405346369&sdata=v7U9kMfdmgpWZ6WYn1nyPTuUHWzn4s
> J6XvYEJdpw50I%3D&reserved=0
>
> Search the list archives at: 
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmarkma
> il.org%2Fsearch%2F%3Fq%3Dvtk-developers&data=02%7C01%7Classo%40queensu
> .ca%7C14c5314b70e443f5be4908d4bb1f0bf4%7Cd61ecb3b38b142d582c4efb2838b9
> 25c%7C1%7C0%7C636339189405346369&sdata=otfBLrSAA5d7a9u6mrb56%2ByrIVM%2
> Bl3f%2BZNQpwOvqgO0%3D&reserved=0
>
> Follow this link to subscribe/unsubscribe:
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpublic
> .kitware.com%2Fmailman%2Flistinfo%2Fvtk-developers&data=02%7C01%7Classo%40queensu.ca%7C14c5314b70e443f5be4908d4bb1f0bf4%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636339189405346369&sdata=whCFetn1LnPSRi0CZ%2BD1aGlMHqdv5vSgzkRdcuLw8qI%3D&reserved=0
>