[vtkusers] Experimental enhanced wrappers for VTK

[David Gobbi]

Okay, I've tested removing the "i++;" as suggested below, and it is
the correct fix.  I was just a little paranoid.

    David


On Mon, Jun 7, 2010 at 8:38 AM, David Gobbi <david.gobbi at gmail.com> wrote:
> Please wait a bit before trying the fix that I just suggested... I'm
> pretty sure that I made a mistake on it.  Later this morning I'll be
> able to take a closer look.
>
>   David
>
>
> On Mon, Jun 7, 2010 at 8:35 AM, David Gobbi <david.gobbi at gmail.com> wrote:
>> Hi Keith,
>>
>> That code is supposed to pull out the text from function pointer
>> definitions like the following, in Graphics/vtkWarpScalar.h:88:
>>
>> double *(vtkWarpScalar::*PointNormal)(vtkIdType id, vtkDataArray *normals);
>>
>> So the loop should use i and j to mark the start and end of
>> "vtkWarpScalar::".  But I see the problem now: it fails in the case of
>> a simple "(*" because there is an extra "i++" that doesn't belong:
>>
>>
>> "("[\t\n\r ]*([a-zA-Z_][a-zA-Z0-9_]*::)*"*" {
>>
>>                int i = 1;
>>
>>                int j;
>>
>>                while (yytext[i]==' ' || yytext[i]=='\t' ||
>>
>>                       yytext[i]=='\r' || yytext[i]=='\n') { i++; }
>>
>>                j = i;    /* <-  HERE IS WHERE THE i++ WAS REMOVED */
>>
>>                while (yytext[j]!='*') { j++; }
>>
>>                yylval.str = (char *)malloc(j-i+1);
>>
>>                if (j > i) { strncpy(yylval.str, &yytext[i], j-i); }
>>
>>                yylval.str[j-i] = '\0';
>>
>>                return(LPAREN_POINTER);
>>
>>                }
>>
>> Try that out and let me know if valgrind likes it.
>>
>> To give a bit of background, I added LPAREN_POINTER because I was
>> having trouble parsing function pointer declarations in vtkParse.y.
>> My attempts to parse them would cause shift/reduce errors, so I
>> decided to do some of the parsing in vtkParse.l instead.
>>
>>   David
>>
>>
>> On Mon, Jun 7, 2010 at 7:48 AM, Keith Fieldhouse
>> <keith.fieldhouse at kitware.com> wrote:
>>> Hi David,
>>> As part of the testing to get your wrapping stuff merged, we built on
>>> Windows 64 bit and ran into some trouble.
>>> The first was a crash during vtkWrapPython that was just an uninitialized
>>> variable.  The fix can be seen at
>>> http://github.com/keith-fieldhouse/VTK/commit/c6d900
>>> If you're interested.
>>> The other is a little more troublesome and I'm hoping you can help me
>>> understand it.  I'm getting random failures during vtkWrapJava under Visual
>>> Studio.  The failures manifest themselves differently depending on how
>>> things are run (things proceed normally under nmake, but crash under Visual
>>> Studio for example) which leads to the suspicion of some sort of memory
>>> error.
>>> Valgrind under 64bit Linux provides a clue.  One of the files that tends to
>>> fail wrapping is Common/vtkMatrix4x4.h while Common/vtkMatrix3x3.h hasn't
>>> exhibited a failure.  Comparing the valgrind output for the two files
>>> suggests the following.
>>> In vtkParse.l after commit 047dc6ca  the LPAREN_POINTER stanza looks like
>>> this:
>>>
>>> "("[\t\n\r ]*([a-zA-Z_][a-zA-Z0-9_]*::)*"*" {
>>>                 int i = 1;
>>>                 int j;
>>>                 while (yytext[i]==' ' || yytext[i]=='\t' ||
>>>                        yytext[i]=='\r' || yytext[i]=='\n') { i++; }
>>>                 i++; j = i;
>>>                 while (yytext[j]!='*') { j++; }
>>>                 yylval.str = (char *)malloc(j-i+1);
>>>                 if (j > i) { strncpy(yylval.str, &yytext[i], j-i); }
>>>                 yylval.str[j-i] = '\0';
>>>                 return(LPAREN_POINTER);
>>>                 }
>>>
>>> Valgrind emits a number of errors that suggest that in the
>>> while(yytext[j]!='*'), A '*' is never found and j gets incremented beyond
>>> the bounds of yytext and hijinx ensue.    While I'm not certain that this is
>>> causing the crashes I'm seeing, it is suggestive.  My experience with Lex
>>> dates back more years than I care to think about so I'm not entirely sure
>>> why you're looking for another '*' here?  Or maybe I'm just misreading the
>>> code entirely, which is why I'm asking for your help. Does my analysis make
>>> sense and is there an approach you can see that would eliminate the problem?
>>> Thanks again for all of your help.
>>> Keith
>>>
>>> On Fri, May 28, 2010 at 12:48 PM, David Gobbi <david.gobbi at gmail.com> wrote:
>>>>
>>>> Hi Marcus,
>>>>
>>>> If you can get it merged by next week, that would be great.
>>>>
>>>> From my own tests, the only methods that will still need BTX/ETX after
>>>> the merge are methods that take pointers to special type, because the
>>>> wrappers will think these are vtkObjectBase-types.  I have a list of
>>>> these methods that I can send along after the merge.  It's easy to
>>>> find these methods, because the Java wrappers fail to build when it
>>>> encounters them.
>>>>
>>>> To solve this issue in the long term, I'm working on a tool that can
>>>> make a text file that describes the VTK class hierarchy (both for
>>>> vtkObjects and for special VTK types).  Through the use of this file,
>>>> when the wrappers encounter a parameter called "vtkSomething" they
>>>> will know at compile-time whether vtkSomething is a vtkObject.
>>>>
>>>>   David
>>>>
>>>>
>>>> On Fri, May 28, 2010 at 10:09 AM, Marcus D. Hanwell
>>>> <marcus.hanwell at kitware.com> wrote:
>>>> > Hi David,
>>>> > Thanks for all of the work you have put into this. David Cole is going
>>>> > to
>>>> > look over the topic branch, hopefully at some point today. If he doesn't
>>>> > spot any major issues I would like to merge this into master early next
>>>> > week.
>>>> > I am certainly excited about the improvements you are bringing to the
>>>> > wrapping in VTK.
>>>> > Thanks,
>>>> > Marcus
>>>> > On Thu, May 27, 2010 at 4:52 PM, David Gobbi <david.gobbi at gmail.com>
>>>> > wrote:
>>>> >>
>>>> >> I've pushed the last of the changes to my "wrapping" branch on github.
>>>> >>  It's ready for merge.
>>>> >>
>>>> >>   David
>>>> >>
>>>> >>
>>>> >> On Wed, May 26, 2010 at 4:49 PM, David Gobbi <david.gobbi at gmail.com>
>>>> >> wrote:
>>>> >> > In the new wrappers, there is already full support vtkStdString
>>>> >> > support Tcl and Java, and I've even added string regression test for
>>>> >> > Tcl.
>>>> >> >
>>>> >> > Unicode was easy to add to Python (a few hours work), and it should
>>>> >> > be
>>>> >> > similarly easy for someone to add it for Tcl and Java.  There are
>>>> >> > plenty of people who want it, I'm sure.
>>>> >> >
>>>> >> > Divergence is a concern, but at least all wrappers use the same
>>>> >> > front-end, so transferring enhancements from one set of wrappers to
>>>> >> > another is that much easier.  And Brad King mentioned a wrapper
>>>> >> > utility .cxx that all wrappers use... this would be great, because
>>>> >> > right now there is a huge amount of code duplication.
>>>> >> >
>>>> >> > I'll switch over to the branch immediately, and in the future I will
>>>> >> > make branches for any other projects that involve multiple classes.
>>>> >> >
>>>> >> >   David
>>>> >> >
>>>> >> >
>>>> >> > On Wed, May 26, 2010 at 3:58 PM, Marcus D. Hanwell
>>>> >> > <marcus.hanwell at kitware.com> wrote:
>>>> >> >> Hi David,
>>>> >> >> On Wed, May 26, 2010 at 5:46 PM, David Gobbi <david.gobbi at gmail.com>
>>>> >> >> wrote:
>>>> >> >>>
>>>> >> >>> Thanks for doing this!  I was not looking forward to doing the
>>>> >> >>> clean-up work myself... when I started it never occurred to me to
>>>> >> >>> create a branch, my plan was to eventually re-base my changes on
>>>> >> >>> the
>>>> >> >>> VTK master.
>>>> >> >>
>>>> >> >> Topic branches are much easier for us to work with, and if you avoid
>>>> >> >> merging
>>>> >> >> into them/rebasing then the eventual merge into master becomes
>>>> >> >> simpler.
>>>> >> >> Now
>>>> >> >> that we have it you could continue your work in that branch, or we
>>>> >> >> could
>>>> >> >> cherry pick the commits you make.
>>>> >> >>>
>>>> >> >>> There are just two minor changes that I can squeeze in over the
>>>> >> >>> next
>>>> >> >>> two days, and I will email you when they are done.  After that, the
>>>> >> >>> sooner that the branch can be merged into master, the better.  In
>>>> >> >>> order for the special-type wrapping to be fully realized, most (or
>>>> >> >>> all) of the BTX's will have to be removed from the code.  A change
>>>> >> >>> that big will probably be easier to do in the master than to merge
>>>> >> >>> in
>>>> >> >>> from a branch.
>>>> >> >>
>>>> >> >> A global change like that is definitely something that would be
>>>> >> >> easier
>>>> >> >> done
>>>> >> >> in master as you say. We can take care of that once it looks ready
>>>> >> >> to
>>>> >> >> go
>>>> >> >> in.
>>>> >> >>>
>>>> >> >>> I've already tested VTK (with python, tcl, and java) without
>>>> >> >>> BTX/ETX.
>>>> >> >>> The Tcl and Java wrappers compiled and tested cleanly, and so did
>>>> >> >>> Python after I did some small fixes to the way that it does special
>>>> >> >>> type wrapping.  It is these python fixes that I need to commit
>>>> >> >>> before
>>>> >> >>> the merge.
>>>> >> >>>
>>>> >> >> I think one of the remaining concerns is how much the wrapping might
>>>> >> >> diverge, i.e. if Python supports vtkVariant, unicode, vtkStdString
>>>> >> >> etc
>>>> >> >> but
>>>> >> >> other languages did not. I am certainly very pleased with the
>>>> >> >> changes
>>>> >> >> in
>>>> >> >> Python, and can see how much it will improve the Python wrapping in
>>>> >> >> charts
>>>> >> >> and infovis.
>>>> >> >>
>>>> >> >>>
>>>> >> >>> There are other changes that I'm planning for the Python wrappers
>>>> >> >>> (enums, etc) but for them I'd prefer to wait until after the merge.
>>>> >> >>> I'll put together a wiki page so that they can be reviewed.
>>>> >> >>>
>>>> >> >> You could publish them in a topic branch for us to review. The main
>>>> >> >> caveats
>>>> >> >> remain to avoid rebasing/merging from master. Once we are ready to
>>>> >> >> merge
>>>> >> >> into master we can take care of that. Thanks for putting this
>>>> >> >> together,
>>>> >> >> and
>>>> >> >> to Keith for preparing this topic branch.
>>>> >> >> I am very hopeful that we can merge these changes in soon. We have
>>>> >> >> been
>>>> >> >> working away behind the scenes to get this ready to be reviewed and
>>>> >> >> merged
>>>> >> >> in.
>>>> >> >> Thanks,
>>>> >> >> Marcus
>>>> >> >>>
>>>> >> >>> On Wed, May 26, 2010 at 2:39 PM, Keith Fieldhouse
>>>> >> >>> <keith.fieldhouse at kitware.com> wrote:
>>>> >> >>> > David
>>>> >> >>> > I've been spending some time looking at your Wrapping work.  It
>>>> >> >>> > appears
>>>> >> >>> > that
>>>> >> >>> > your git tree was built with a mixture of rebasing and merging
>>>> >> >>> > which
>>>> >> >>> > resulted in a somewhat wonky commit history (duplicated commits
>>>> >> >>> > interspersed
>>>> >> >>> > with commits coming from VTK master).
>>>> >> >>> > In order to get a clean topic branch that will be a little easier
>>>> >> >>> > for
>>>> >> >>> > folks
>>>> >> >>> > here to look at (and to eventually merge with master) I spent a
>>>> >> >>> >  bit
>>>> >> >>> > of
>>>> >> >>> > time
>>>> >> >>> > untangling your history (mostly by examining parentage and
>>>> >> >>> > looking
>>>> >> >>> > at
>>>> >> >>> > commit
>>>> >> >>> > subjects).  In the end, by cherry picking the right commits in
>>>> >> >>> > what
>>>> >> >>> > I
>>>> >> >>> > considered the "right" order I was able to build a clean topic
>>>> >> >>> > branch
>>>> >> >>> > with
>>>> >> >>> > no conflicts.  Once I had that, I rebased it against master (as
>>>> >> >>> > of
>>>> >> >>> > this
>>>> >> >>> > afternoon sometime when I did it).  The result can be found in
>>>> >> >>> > git://github.com/keith-fieldhouse/VTK.git
>>>> >> >>> > As the topic branch Gobbi/Wrapping.
>>>> >> >>> > The result builds and the tests run so I believe that I've built
>>>> >> >>> > a a
>>>> >> >>> > representative branch from your work.
>>>> >> >>> > If you plan on doing additional work on this, would it be
>>>> >> >>> > possible
>>>> >> >>> > for
>>>> >> >>> > you
>>>> >> >>> > to grab this topic branch and work on that (without rebasing or
>>>> >> >>> > merging
>>>> >> >>> > for
>>>> >> >>> > now)?    That way we'll have a clean topic branch  for an
>>>> >> >>> > eventual
>>>> >> >>> > merge
>>>> >> >>> > to
>>>> >> >>> > "Next" when we switch to branchy development for VTK.  Or if
>>>> >> >>> > branchy
>>>> >> >>> > development comes later, this clean topic branch will be easier
>>>> >> >>> > to
>>>> >> >>> > rebase as
>>>> >> >>> > well.
>>>> >> >>> > Thanks again for your work on this.
>>>> >> >>> > Keith
>>>> >> >>> > --
>>>> >> >>> > Keith Fieldhouse
>>>> >> >>> > R&D Engineer, Kitware Inc.
>>>> >> >>> > (518) 836-2190
>>>> >> >>> >
>>>> >> >>> >
>>>> >> >>> >
>>>> >> >>> >
>>>> >> >>
>>>> >> >>
>>>> >> >
>>>> >
>>>> >
>>>
>>>
>>
>