[vtk-developers] const correctness

[David Gobbi]

Hi Tom,

Const-correctness is good, but breaking backwards compatibility is
very, very bad.

All you need to do in order to push this patch through, is to ensure
that ParaView, Slicer, and other third-party packages will compile
with only minor modifications.

If you haven't at least tried to compile ParaView on top of your
patched VTK, though, you'll have a very hard time convincing people.
With my recent wrapper changes, for example, I was regularly testing
against ParaView, even though I'm not a regular ParaView user.

I'm sure that many of your changes will go through, but you will
probably find you'll have to back out some of them.

 - David


On Thu, Feb 10, 2011 at 11:25 AM, 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
>
>