[vtk-developers] Safer item access in Python wrapped VTK arrays

Andras Lasso lasso at queensu.ca
Wed Aug 9 17:45:34 EDT 2017


For me, they look OK.

VTK_SIZEHINT is not a beautiful syntax, but I understand why it is like this and don't have a better suggestion. How do you specify size hints for multiple arguments?

Andras

From: David Gobbi<mailto:david.gobbi at gmail.com>
Sent: Wednesday, August 9, 2017 16:53
To: Andras Lasso<mailto:lasso at queensu.ca>; Berk Geveci<mailto:berk.geveci at kitware.com>
Cc: VTK Developers<mailto:vtk-developers at vtk.org>
Subject: Re: [vtk-developers] Safer item access in Python wrapped VTK arrays

Here's what I'm currently thinking about for wrapper hints, please let me know if the syntax is too obtrusive.

The basic precondition contract, similar to proposed C++ contracts:

    float GetValue(vtkIdType i)  VTK_EXPECTS(i >= 0 && i < GetNumberOfValues());

And here's the format that I'm currently thinking about for wrapper size hints (to replace our "hints" file):

    double *GetPoint(vtkIdType i)  VTK_SIZEHINT(return[3]);

When used in combination things start to look ugly, but the method signature is still readable:

    void SetTuple(vtkIdType i, float *a)
        VTK_EXPECTS(i >= 0 && i < GetNumberOfTuples())
        VTK_SIZEHINT(a[GetNumberOfComponents()]);

    float *GetTuple(vtkIdType i)
        VTK_EXPECTS(i >= 0 && i < GetNumberOfTuples())
        VTK_SIZEHINT(return[GetNumberOfComponents()]);

Any thoughts?  I'd really like to express the size hint as some kind of contract, but C++ doesn't provide a way to check the number of values pointed to by a pointer.

As an aside: for vtkDataArray, I don't actually intend to add VTK_SIZEHINT as shown above.  Currently the size hints for vtkDataArray are hard-coded into the wrappers, and I'll probably keep them that way in order to keep the C++ headers clean.

 - David


On Mon, Aug 7, 2017 at 6:56 AM, David Gobbi <david.gobbi at gmail.com<mailto:david.gobbi at gmail.com>> wrote:
Hi Andras,

I agree that these contracts are a better choice than inventing something of our own, I'm going to play around with this to see how easy it would be to add them to the wrappers.

The macros can be made a little more compact:
T& operator[](size_t i) VTK_PRECONDITION(i >= 0 && i < size());

Or we could use VTK_EXPECTS(i >= 0 && i < size());

 - David



On Fri, Aug 4, 2017 at 7:39 PM, Andras Lasso <lasso at queensu.ca<mailto:lasso at queensu.ca>> wrote:
Eventually, C++17 standard contracts may be used to specify these bounds checks. For now, we may use the same format but add them as comments or protected by macros; and when we switch to C++17 then these can be converted to actual contracts.

Proposal for contracts: http://www.open-std.org/JTC1/SC22/WG21/docs/papers/2015/n4415.pdf<https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.open-std.org%2FJTC1%2FSC22%2FWG21%2Fdocs%2Fpapers%2F2015%2Fn4415.pdf&data=02%7C01%7Classo%40queensu.ca%7C81f26f06e88740ecf91808d4df68c0b1%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C1%7C636379088380189665&sdata=9oNtyphRr76c6McZ6ML0IIMZbPYxbnUh1FzPFp3CgKA%3D&reserved=0>

Example contract:
T& operator[](size_t i) [[expects: i >= 0 && i < size()]];

For now, we could do this in VTK:
T& operator[](size_t i) VTK_CONTRACT([[expects: i >= 0 && i < size()]]);

Andras


From: David Gobbi [mailto:david.gobbi at gmail.com<mailto:david.gobbi at gmail.com>]
Sent: Friday, August 4, 2017 4:05 PM
To: Berk Geveci <berk.geveci at kitware.com<mailto:berk.geveci at kitware.com>>
Cc: Andras Lasso <lasso at queensu.ca<mailto:lasso at queensu.ca>>; VTK Developers <vtk-developers at vtk.org<mailto:vtk-developers at vtk.org>>
Subject: Re: [vtk-developers] Safer item access in Python wrapped VTK arrays

For a while, we kicked around the idea of having the hints in the comments.  This would be less intrusive than adding C++11 attributes the declarations (i.e. what I've been doing so far).

The C++11 attributes are definitely the cleanest way for the wrappers to store the hints, since the hints become part of the parse tree, so I want to keep them as the primary hinting mechanism.  Comment-based hints could be added as a secondary mechanism, i.e. after a declaration has been parsed we can check the docstring for extra hints, and then add the hints to the parse tree before the wrapper code is generated.

I'll have to search through the archives to remember what kind of syntax we were considering for comment-based hints.

 - David


On Fri, Aug 4, 2017 at 1:30 PM, Berk Geveci <berk.geveci at kitware.com<mailto:berk.geveci at kitware.com>> wrote:
As much as I support the concept of range checking in the wrappers, this hint will make things look very ugly and non-standard at a time where we are trying to move towards more standard looking C++. Isn't there a way of doing this without mangling the signatures?

On Fri, Aug 4, 2017 at 1:06 PM, David Gobbi <david.gobbi at gmail.com<mailto:david.gobbi at gmail.com>> wrote:
Hi Andras,

Yes, this would be possible.  It could be hard-coded into the wrappers, or even better, in the header file a generic hint could be added to the index parameter so that the index would be checked against the size of the array:

  float GetValue(vtkIdType idx VTK_RANGECHECK(0,GetNumberOfValues()));

I have a list of wrapper hints at http://www.vtk.org/Wiki/VTK/Wrapping_hints<https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.vtk.org%2FWiki%2FVTK%2FWrapping_hints&data=02%7C01%7Classo%40queensu.ca%7C37f3749bcbbf4422f0ba08d4db741f07%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636374739163412038&sdata=FtQM8JHWrl8ReFwC3EFD4j0COBmWqm1r6k9b%2BF5tZzU%3D&reserved=0>, and I can add this "RANGECHECK" hint to the "Proposed hints" section.

I'm hoping to find time to implement more of these wrapper hints over the next few weeks.

 - David


On Fri, Aug 4, 2017 at 9:53 AM, Andras Lasso <lasso at queensu.ca<mailto:lasso at queensu.ca>> wrote:
Hi David,

Users who mostly use VTK via Python wrapping are sometimes surprised how easily they can crash the application by accessing out-of-bounds elements in arrays. For example, this crashes an application:

>>> a=vtk.vtkStringArray()
>>> print(a.GetValue(0))

I first thought that it should be handled by documentation and training, but it would be nicer if the wrappers could handle this.

Would it be feasible to add bounds check to VTK array accessors in Python wrappers, so that in case of an out of bounds access, a Python exception would be raised instead of crashing the application?
Andras



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20170809/05fb1588/attachment.html>


More information about the vtk-developers mailing list