[prev in list] [next in list] [prev in thread] [next in thread]
List: vtk-developers
Subject: Re: [vtk-developers] Implicit copy constructors in VTK
From: Jean-Christophe Fillion-Robin <jchris.fillionr () kitware ! com>
Date: 2013-07-24 18:29:23
Message-ID: CANLz8R4PeDkPjb3kYNs40SxntMMoX4zi73oXs0o7t7haQ7ouZA () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
@Robert, @Bill: My initial motivation was to make it easier for novice to
review/follow/understand the code. Not having user feedback to support
this, I will plan on doing a survey of our Slicer users/developers.
On Wed, Jul 24, 2013 at 12:51 PM, Bill Lorensen <bill.lorensen@gmail.com>wrote:
> I agree with Robert. I'd rather see the explicit lions rather than a
> macro. Makes code easier to follow for novices.
>
>
>
> On Wed, Jul 24, 2013 at 12:33 PM, Robert Maynard <
> robert.maynard@kitware.com> wrote:
>
>> Personally I don't see enough complaints about people forgetting to
>> follow the rule of three to add another macro helper to VTK.
>> We have a smoke test that does verify that all classes that derive
>> from vtkObject have private copy constructor and copy assignment
>> operator.
>>
>>
>> On Wed, Jul 24, 2013 at 11:55 AM, Jean-Christophe Fillion-Robin
>> <jchris.fillionr@kitware.com> wrote:
>> > Would it make sense to provide a VTK_DISABLE_COPY() macro ? (Similar to
>> > what is done within Qt with Q_DISABLE_COPY [1])
>> >
>> > [1] http://qt-project.org/doc/qt-5.0/qtcore/qobject.html#Q_DISABLE_COPY
>> >
>> >
>> > On Wed, Jul 24, 2013 at 11:48 AM, Marcus D. Hanwell
>> > <marcus.hanwell@kitware.com> wrote:
>> >>
>> >> I agree with Rob, and can take care of putting a patch together for
>> >> the classes you call out. The rules outlined only make sense for
>> >> vtkObject derived classes, we should provide clear guidelines for
>> >> those that are not part of that hierarchy too.
>> >>
>> >> On Wed, Jul 24, 2013 at 11:06 AM, Robert Maynard
>> >> <robert.maynard@kitware.com> wrote:
>> >> > The VTK coding standards should be amended to state that those rules
>> >> > are only valid for classes that derive from vtkObject.
>> >> >
>> >> > Classes like vtkBond, vtkTuple, vtkBoundingBox all which don't
>> inherit
>> >> > from vtkObject should follow the rule of three and implement the copy
>> >> > and assignment constructor if they implement a destructor.
>> >> >
>> >> > On Wed, Jul 24, 2013 at 10:50 AM, Sean McBride <
>> sean@rogue-research.com>
>> >> > wrote:
>> >> >> Hi all,
>> >> >>
>> >> >> So I'm looking at the clang warnings on Rogue7, but need C++ help
>> >> >> before I can proceed further. We have two similar warnings,
>> repeated for
>> >> >> several classes:
>> >> >>
>> >> >> VTK/Common/DataModel/vtkBond.h:30:3: warning: definition of implicit
>> >> >> copy constructor for 'vtkBond' is deprecated because it has a
>> user-declared
>> >> >> destructor [-Wdeprecated]
>> >> >> ~vtkBond();
>> >> >> ^
>> >> >>
>> >> >> VTK/Common/Math/vtkQuaternion.h:201:8: warning: definition of
>> implicit
>> >> >> copy constructor for 'vtkQuaternion<float>' is deprecated because
>> it has a
>> >> >> user-declared copy assignment operator [-Wdeprecated]
>> >> >> void operator=(const vtkQuaternion<T>& q);
>> >> >> ^
>> >> >>
>> >> >> From what I can tell, in C++11, the implicit generation of copy
>> >> >> constructors is now deprecated if 1) you provide a user-declared
>> destructor
>> >> >> or 2) you provide a user-declared copy assignment operator.
>> >> >> <http://stackoverflow.com/a/11255258>
>> >> >>
>> >> >> So it would seem a solution is to provide an explicit copy
>> constructor,
>> >> >> but VTK's coding standards gives me pause: "Classes should have
>> protected
>> >> >> constructors and destructors, and privately declared but
>> unimplemented copy
>> >> >> constructor and assignment operator. Rationale: VTK’s reference
>> counting
>> >> >> implementation depends on carefully controlling each object’s
>> reference
>> >> >> count".
>> >> >>
>> >> >> So now I'm not sure how to proceed...
>> >> >>
>> >> >> Thanks,
>> >> >>
>> >> >> --
>> >> >> ____________________________________________________________
>> >> >> Sean McBride, B. Eng sean@rogue-research.com
>> >> >> Rogue Research www.rogue-research.com
>> >> >> Mac Software Developer Montréal, Québec, Canada
>> >> >> _______________________________________________
>> >> >> Powered by www.kitware.com
>> >> >>
>> >> >> Visit other Kitware open-source projects at
>> >> >> http://www.kitware.com/opensource/opensource.html
>> >> >>
>> >> >> Follow this link to subscribe/unsubscribe:
>> >> >> http://www.vtk.org/mailman/listinfo/vtk-developers
>> >> >>
>> >> > _______________________________________________
>> >> > Powered by www.kitware.com
>> >> >
>> >> > Visit other Kitware open-source projects at
>> >> > http://www.kitware.com/opensource/opensource.html
>> >> >
>> >> > Follow this link to subscribe/unsubscribe:
>> >> > http://www.vtk.org/mailman/listinfo/vtk-developers
>> >> >
>> >> _______________________________________________
>> >> Powered by www.kitware.com
>> >>
>> >> Visit other Kitware open-source projects at
>> >> http://www.kitware.com/opensource/opensource.html
>> >>
>> >> Follow this link to subscribe/unsubscribe:
>> >> http://www.vtk.org/mailman/listinfo/vtk-developers
>> >>
>> >
>> >
>> >
>> > --
>> > +1 919 869 8849
>> _______________________________________________
>> Powered by www.kitware.com
>>
>> Visit other Kitware open-source projects at
>> http://www.kitware.com/opensource/opensource.html
>>
>> Follow this link to subscribe/unsubscribe:
>> http://www.vtk.org/mailman/listinfo/vtk-developers
>>
>>
>
>
> --
> Unpaid intern in BillsBasement at noware dot com
>
--
+1 919 869 8849
[Attachment #5 (text/html)]
<div dir="ltr">@Robert, @Bill: My initial motivation was to make it easier for \
novice to review/follow/understand the code. Not having user feedback to support \
this, I will plan on doing a survey of our Slicer users/developers.</div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Jul 24, 2013 at \
12:51 PM, Bill Lorensen <span dir="ltr"><<a href="mailto:bill.lorensen@gmail.com" \
target="_blank">bill.lorensen@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><div dir="ltr">I agree with Robert. I'd rather see the \
explicit lions rather than a macro. Makes code easier to follow for novices.<div>
<br></div></div><div class="gmail_extra"><div><div class="h5"><br><br><div \
class="gmail_quote">On Wed, Jul 24, 2013 at 12:33 PM, Robert Maynard <span \
dir="ltr"><<a href="mailto:robert.maynard@kitware.com" \
target="_blank">robert.maynard@kitware.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">Personally I don't see enough complaints about people \
forgetting to<br> follow the rule of three to add another macro helper to VTK.<br>
We have a smoke test that does verify that all classes that derive<br>
from vtkObject have private copy constructor and copy assignment<br>
operator.<br>
<div><div><br>
<br>
On Wed, Jul 24, 2013 at 11:55 AM, Jean-Christophe Fillion-Robin<br>
<<a href="mailto:jchris.fillionr@kitware.com" \
target="_blank">jchris.fillionr@kitware.com</a>> wrote:<br> > Would it make \
sense to provide a VTK_DISABLE_COPY() macro ? (Similar to<br> > what is done \
within Qt with Q_DISABLE_COPY [1])<br> ><br>
> [1] <a href="http://qt-project.org/doc/qt-5.0/qtcore/qobject.html#Q_DISABLE_COPY" \
target="_blank">http://qt-project.org/doc/qt-5.0/qtcore/qobject.html#Q_DISABLE_COPY</a><br>
><br>
><br>
> On Wed, Jul 24, 2013 at 11:48 AM, Marcus D. Hanwell<br>
> <<a href="mailto:marcus.hanwell@kitware.com" \
target="_blank">marcus.hanwell@kitware.com</a>> wrote:<br> >><br>
>> I agree with Rob, and can take care of putting a patch together for<br>
>> the classes you call out. The rules outlined only make sense for<br>
>> vtkObject derived classes, we should provide clear guidelines for<br>
>> those that are not part of that hierarchy too.<br>
>><br>
>> On Wed, Jul 24, 2013 at 11:06 AM, Robert Maynard<br>
>> <<a href="mailto:robert.maynard@kitware.com" \
target="_blank">robert.maynard@kitware.com</a>> wrote:<br> >> > The VTK \
coding standards should be amended to state that those rules<br> >> > are \
only valid for classes that derive from vtkObject.<br> >> ><br>
>> > Classes like vtkBond, vtkTuple, vtkBoundingBox all which don't \
inherit<br> >> > from vtkObject should follow the rule of three and \
implement the copy<br> >> > and assignment constructor if they implement a \
destructor.<br> >> ><br>
>> > On Wed, Jul 24, 2013 at 10:50 AM, Sean McBride <<a \
href="mailto:sean@rogue-research.com" \
target="_blank">sean@rogue-research.com</a>><br> >> > wrote:<br>
>> >> Hi all,<br>
>> >><br>
>> >> So I'm looking at the clang warnings on Rogue7, but need C++ \
help<br> >> >> before I can proceed further. We have two similar \
warnings, repeated for<br> >> >> several classes:<br>
>> >><br>
>> >> VTK/Common/DataModel/vtkBond.h:30:3: warning: definition of \
implicit<br> >> >> copy constructor for 'vtkBond' is deprecated \
because it has a user-declared<br> >> >> destructor [-Wdeprecated]<br>
>> >> ~vtkBond();<br>
>> >> ^<br>
>> >><br>
>> >> VTK/Common/Math/vtkQuaternion.h:201:8: warning: definition of \
implicit<br> >> >> copy constructor for \
'vtkQuaternion<float>' is deprecated because it has a<br> >> \
>> user-declared copy assignment operator [-Wdeprecated]<br> >> >> \
void operator=(const vtkQuaternion<T>& q);<br> >> >> \
^<br> >> >><br>
>> >> From what I can tell, in C++11, the implicit generation of copy<br>
>> >> constructors is now deprecated if 1) you provide a user-declared \
destructor<br> >> >> or 2) you provide a user-declared copy assignment \
operator.<br> >> >> <<a href="http://stackoverflow.com/a/11255258" \
target="_blank">http://stackoverflow.com/a/11255258</a>><br> >> >><br>
>> >> So it would seem a solution is to provide an explicit copy \
constructor,<br> >> >> but VTK's coding standards gives me pause: \
"Classes should have protected<br> >> >> constructors and \
destructors, and privately declared but unimplemented copy<br> >> >> \
constructor and assignment operator. Rationale: VTK’s reference counting<br> >> \
>> implementation depends on carefully controlling each object’s reference<br> \
>> >> count".<br> >> >><br>
>> >> So now I'm not sure how to proceed...<br>
>> >><br>
>> >> Thanks,<br>
>> >><br>
>> >> --<br>
>> >> ____________________________________________________________<br>
>> >> Sean McBride, B. Eng <a \
href="mailto:sean@rogue-research.com" target="_blank">sean@rogue-research.com</a><br> \
>> >> Rogue Research <a \
href="http://www.rogue-research.com" target="_blank">www.rogue-research.com</a><br> \
>> >> Mac Software Developer Montréal, Québec, Canada<br> \
>> >> _______________________________________________<br> >> \
>> Powered by <a href="http://www.kitware.com" \
target="_blank">www.kitware.com</a><br> >> >><br>
>> >> Visit other Kitware open-source projects at<br>
>> >> <a href="http://www.kitware.com/opensource/opensource.html" \
target="_blank">http://www.kitware.com/opensource/opensource.html</a><br> >> \
>><br> >> >> Follow this link to subscribe/unsubscribe:<br>
>> >> <a href="http://www.vtk.org/mailman/listinfo/vtk-developers" \
target="_blank">http://www.vtk.org/mailman/listinfo/vtk-developers</a><br> >> \
>><br> >> > _______________________________________________<br>
>> > Powered by <a href="http://www.kitware.com" \
target="_blank">www.kitware.com</a><br> >> ><br>
>> > Visit other Kitware open-source projects at<br>
>> > <a href="http://www.kitware.com/opensource/opensource.html" \
target="_blank">http://www.kitware.com/opensource/opensource.html</a><br> >> \
><br> >> > Follow this link to subscribe/unsubscribe:<br>
>> > <a href="http://www.vtk.org/mailman/listinfo/vtk-developers" \
target="_blank">http://www.vtk.org/mailman/listinfo/vtk-developers</a><br> >> \
><br> >> _______________________________________________<br>
>> Powered by <a href="http://www.kitware.com" \
target="_blank">www.kitware.com</a><br> >><br>
>> Visit other Kitware open-source projects at<br>
>> <a href="http://www.kitware.com/opensource/opensource.html" \
target="_blank">http://www.kitware.com/opensource/opensource.html</a><br> \
>><br> >> Follow this link to subscribe/unsubscribe:<br>
>> <a href="http://www.vtk.org/mailman/listinfo/vtk-developers" \
target="_blank">http://www.vtk.org/mailman/listinfo/vtk-developers</a><br> \
>><br> ><br>
><br>
><br>
> --<br>
> <a href="tel:%2B1%20919%20869%208849" value="+19198698849" target="_blank">+1 \
919 869 8849</a><br> _______________________________________________<br>
Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
<br>
Visit other Kitware open-source projects at <a \
href="http://www.kitware.com/opensource/opensource.html" \
target="_blank">http://www.kitware.com/opensource/opensource.html</a><br> <br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://www.vtk.org/mailman/listinfo/vtk-developers" \
target="_blank">http://www.vtk.org/mailman/listinfo/vtk-developers</a><br> <br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- \
<br></div></div><div class="im">Unpaid intern in BillsBasement at noware dot com<br> \
</div></div> </blockquote></div><br><br clear="all"><div><br></div>-- <br>+1 919 869 \
8849<br> </div>
_______________________________________________
Powered by www.kitware.com
Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://www.vtk.org/mailman/listinfo/vtk-developers
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic