<div class="gmail_quote">On Mon, May 14, 2012 at 5:46 AM, Gregory Szorc <span dir="ltr"><<a href="mailto:gregory.szorc@gmail.com" target="_blank">gregory.szorc@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On 5/13/12 1:00 AM, Manuel Klimek wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+    @property<br>
+    def translation_unit(self):<br>
+        """Returns the TranslationUnit to which this Cursor belongs."""<br>
+        return getattr(self, '_tu', None)<br>
<br>
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?<br>
</blockquote></div>
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.</blockquote>
<div><br></div><div>I'd prefer that - to me defensive programming means to:<br>1. never crash for invalid user input</div><div>2. crash hard if there's an actual bug in the library</div><div><br></div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+        # Store a reference to the TU in the Python object so it won't get GC'd<br>
+        # before the Cursor.<br>
+        tu = None<br>
+        for arg in args:<br>
+            if isinstance(arg, TranslationUnit):<br>
+                tu = arg<br>
+                break<br>
+<br>
+            if hasattr(arg, 'translation_unit'):<br>
+                tu = arg.translation_unit<br>
+                break<br>
+<br>
+        assert tu is not None<br>
+<br>
+        res._tu = tu<br>
<br>
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?<br>
</blockquote></div>
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.<br>

<br>
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).</blockquote>
<div><br></div><div>The question is: don't we just want to assert that all of those have a TU (or are a TU)?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
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)?<br>
<br>
</blockquote></div>
Sure!<br>
<br>
</blockquote></div><br>