[cfe-commits] [PATCH 1/4] [clang.py] Store reference to TranslationUnit in Cursor and Type
klimek at google.com
Tue May 15 12:31:41 PDT 2012
On Mon, May 14, 2012 at 10:15 AM, Manuel Klimek <klimek at google.com> wrote:
> On Mon, May 14, 2012 at 5:46 AM, Gregory Szorc <gregory.szorc at gmail.com>
>> 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
> I'd prefer that - to me defensive programming means to:
> 1. never crash for invalid user input
> 2. crash hard if there's an actual bug in the library
> If the cursor should never be generated by a user (-> the contract of the
> library is to not break that), I think we should use _tu and have the
> AttributeError thrown if it's not there. Or I'm also fine with putting in an
> assert that checks that the attribute is there and delivers a nicer error in
> case it's missing.
>>> + # 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).
> The question is: don't we just want to assert that all of those have a TU
> (or are a TU)?
^ this question is still open I think...
More information about the cfe-commits