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

[Gregory Szorc]

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!