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