[cfe-commits] [PATCH 1/4] [clang.py] Store reference to TranslationUnit in Cursor and Type

[Gregory Szorc]

Maybe if I attached the patch...

On Tue, May 15, 2012 at 10:50 AM, Gregory Szorc <gregory.szorc at gmail.com> wrote:
> Let's see if the mail systems don't reject this one...
>
> On Sun, May 13, 2012 at 8:51 PM, Gregory Szorc <gregory.szorc at gmail.com> wrote:
>> Fresh patch attached.
>>
>>
>> On 5/13/12 8:46 PM, Gregory Szorc wrote:
>>>
>>> On 5/13/12 1:00 AM, Manuel Klimek wrote:
>>>>
>>>> +    @property
>>>> +    def translation_unit(self):
>>>> +        """Returns the TranslationUnit to which this Cursor belongs."""
>>>> +        return getattr(self, '_tu', None)
>>>>
>>>> What's the reason for the default value? Do we expect that people
>>>> create cursers via the lowlevel calls? Why would just return tu_ not
>>>> work?
>>>
>>> Just defensive programming. If you return self._tu, that may raise an
>>> AttributeError if _tu is not set. Theoretically, the API should ensure
>>> that a TU is defined on all Cursor instances, so maybe an AttributeError
>>> would be acceptable.
>>>
>>>> +        # Store a reference to the TU in the Python object so it
>>>> won't get GC'd
>>>> +        # before the Cursor.
>>>> +        tu = None
>>>> +        for arg in args:
>>>> +            if isinstance(arg, TranslationUnit):
>>>> +                tu = arg
>>>> +                break
>>>> +
>>>> +            if hasattr(arg, 'translation_unit'):
>>>> +                tu = arg.translation_unit
>>>> +                break
>>>> +
>>>> +        assert tu is not None
>>>> +
>>>> +        res._tu = tu
>>>>
>>>> That seems - odd. I can't find docs what from_result is supposed to
>>>> do, or what "args" are provided. But having to search through them for
>>>> a TU seems wrong - shouldn't they all have a TU?
>>>
>>> from_result is the errcheck function for the ctypes functions that
>>> return a Cursor. The 3rd argument to an errcheck function are the
>>> original arguments passed into the called function.  For many of the
>>> functions, the original argument is a Cursor. However,
>>> clang_getTypeDeclaration takes a Type and clang_getTranslationUnitCursor
>>> receives a TranslationUnit.
>>>
>>> It is true that all of the functions today receive a single argument, so
>>> the iteration isn't required. However, that may change with the addition
>>> of new APIs in the future (this is how I coded it in my nearly
>>> feature-complete branch, so I'm guessing it is actually required from a
>>> future patch).
>>>>
>>>>
>>>> Also, regarding the test - would it be possible to create a test that
>>>> drops the reference, triggers the gc and then makes sure the cursor is
>>>> still valid (as that is what we really care about)?
>>>>
>>> Sure!
>>>
>>
>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-clang.py-Store-reference-to-TranslationUnit-in-Curso.patch
Type: application/octet-stream
Size: 9850 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120515/2a7025c8/attachment.obj>