[vtk-developers] const correctness

[David Thompson]

> Isn't this too pervasive a change ? Its perfect if this change was  
> restricted to non-virtual methods, private class members, static  
> methods and private implementations in CXX files.
>
> I can think of several projects that depend on VTK choking at a  
> change like this :
>
>    http://review.source.kitware.com/#patch,sidebyside,881,1,Widgets/ 
> vtkWidgetRepresentation.h

Hi all,

I think this illustrates one of my problems with const-correctness.  
Changing
   virtual double *GetBounds() {return NULL;}
to
   virtual const double *GetBounds() {return NULL;}
isn't necessarily the right thing to do. Depending on how GetBounds is  
meant to be implemented by subclasses, it might be useful to instead  
have
   virtual double *GetBounds() {return NULL;}
   virtual const double *GetBounds() const {return NULL;}
but then subclasses must implement twice as many methods.

Also, const-correctness can be difficult in situations where some  
tasks appear to be const but in fact are not, leading to difficulties  
when you need to use them inside another const function. For example,  
vtkPointLocator::FindClosestPoint() seems like it should be const but  
in fact it may need to rebuild the bucket structure used to accelerate  
the search. If you need to find points in a mesh inside some other  
function that will never modify the mesh, you must cast things --  
which decreases legibility -- or propagate this non-const-ness  
upstream by making your method or its arguments non-const.

I'm not completely opposed to the idea of making some things const,  
but when taken to the extreme it can make code very hard to read and  
write. I would be happy if some of the GetXXX macros provided both a  
non-const and const implementation, for instance.

	David


>
> On Thu, Feb 10, 2011 at 11:55 PM, tom fogal <tfogal at sci.utah.edu>  
> wrote:
> Berk Geveci <berk.geveci at kitware.com> writes:
> > Hi Tom,
> >
> > Am I correct in assuming that almost every single subclass of VTK in
> > other projects will fail to compile after this change?
>
> Not every class, no.  Of course, things that were const and are no
> longer will complain about overloads being invalid.
>
> I do not think it will be as bad as the float->double conversion, but
> it will be up there.
>
> > Are there any other backwards compatibility implications of this?
>
> Not that I can think of.  Someone else brought up the Python issue,  
> see
> the other mails in this thread; I haven't had a chance to look into
> that yet.
>
> > Also, what are the advantages of this change?
>
> Those of us that are downstream run into a lot of problems because of
> VTK's lack of const.  A very well-behaved project could const_cast
> things all over the place, but I doubt anyone does this in practice;  
> in
> reality the lack of const just infects others' codebases.
>
> It's also an issue when one has multiple libraries and data
> from one needs to interop with VTK: try pulling out a list of
> constant-somethings from Qt and then populating a VTK object with the
> data, for example.
>
> As to const in general -- it's a very useful mechanism for keeping a
> mental image of large sets of code.  It is much easier to reason about
> code when you know a method call, for example, is const.  I can  
> already
> hear the "but I *know* GetCell is const!" arguments, so I'd like to
> shut them down with: sure you do, but 1) the people on this list are
> predispositioned to know a lot about how VTK works, 2) the existing
> setup provides no guarantees in the face of code evolution, and 3)
> infinitely mutable state makes a system more difficult to reason  
> about;
> the status quo scales poorly to larger bodies of code.
>
> The C++ FAQ has more information:
>
>  http://www.parashift.com/c++-faq-lite/const-correctness.html
>
> > Also it doesn't look like you attempted to mark const functions as
> > such. Was that because it was too hard?
>
> Well, "yes", but "hard" is pretty relative, see below.
>
> > This is probably a naive question because I admit that I don't  
> have a
> > good grasp of constant functions. For example, couldn't this  
> function
> > be const?
> >
> > in vtkDataSet:
> >   virtual void GetCell(vtkIdType cellId, vtkGenericCell *cell) = 0;
> >
> > This GetCell() does not change any data member of the dataset.
>
> Probably.  You would have to look in the implementation of every
> derived classes' GetCell method and ensure it does not modify the
> object.  My guess is that you are correct, and it would be good to  
> mark
> GetCell as const, but I have not verified.  Patch, please? :)
>
> The issue is that GetCell probably calls some LookupCell method
> or something like that (i'm making up method names).  That method
> might not be const, even though it doesn't modify anything.  In all
> likelihood, it's probably virtual too, and so you need to verify that
> all derived classes' LookupCell methods don't modify anything.  But
> then in, say, vtkUnstructuredGrid, LookupCell happens to call XYZ, a
> virtual method which isn't marked const, but is ...
>
> In short: adding these in is very, very easy.  I did a lot of this  
> with
> sed scripts.  But it's also incredibly tedious and eats up lots of
> time.
>
> To get full support, someone would need to read every single line of
> VTK and think about const.  That's not feasible for me to do (hence  
> the
> "Well, mostly" in my original email).  It would probably be feasible
> for a concerted effort, but emails like yours make me wonder if I  
> could
> drum up enough support from the developer base :)
>
> -tom
>
> > On Mon, Feb 7, 2011 at 5:56 PM, tom fogal <tfogal at sci.utah.edu>  
> wrote:
> >
> > > I have just pushed a new branch, `const-correctness', to my git  
> repo:
> > >
> > >  git://shigeru.sci.utah.edu/vtk.git
> > >
> > > As you might guess, this makes VTK const correct.  Well, mostly.
> > >
> > > I had to fix the wrapper-parsing code a bit.  I've tested the
> > > python wrappings thus far.  They all use the same parser, right?
> > > Anybody want to test / potentially fix up TCL and/or Java for me?
> > > Should be easy, given that the Python stuff already works....
> > >
> > > I'll start rebasing against VTK master after this email.  This  
> will
> > > get out of date fast (and thus be a pain to maintain), so I hope  
> we
> > > can push it through soon.
> > >
> > > -tom
> _______________________________________________
> 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
>
>
> <ATT00002..txt>