Here is a revamped patch. I think we'll all be much happier with it. Strangely, the unit test for TranslationUnit.save() on an invalid TU is now failing because clang_saveTranslationUnit() is no longer returning a non-0 error. I'm not sure if it is an error in the Python bindings, the test, or a change in behavior in libclang (possibly r153560 or r152192). Whatever it is should be resolved before this lands.
On Sun, Apr 15, 2012 at 1:27 PM, Gregory Szorc <[email protected]> wrote: > On Fri, Apr 13, 2012 at 3:38 AM, Manuel Klimek <[email protected]> wrote: >> What's ERROR_OK needed for? It looks like it's not currently used; it >> actually looks like it's impossible to ever raise an exception with >> it... > > That's true. I added it for parity with the C API. It can safely be removed. > >> + def __init__(self, ptr=None, filename=None, index=None): >> + """Create a TranslationUnit instance. >> + >> + Instances can be created in the following ways: >> + >> + * By passing a pointer to a c_object_p instance via ptr. This is >> + an internal mechanism and should not be used outside of this >> + module. >> >> This interface seems strange - why have the mixture of 2 constructors in one? > > Why not? > > Unfortunately, I can't find any reputable style guidelines to defend > either perspective. The closest I have is > http://stackoverflow.com/questions/682504/what-is-a-clean-pythonic-way-to-have-multiple-constructors-in-python. > And, that seems to indicate a mix of an "overloaded" __init__ with > @classmethod is preferred. But, that's just one SO question. > > Is this particular case, a TranslationUnit is ultimately instantiated > from a c_object_p "ptr." If we limited __init__ to a single > instantiation mode, we'd have to pass a c_object_p and since these are > internal to the module, __init__ wouldn't be an external API. In other > words, we'd be throwing __init__ away. Since Python programmers look > to __init__ first, I think this would be inconvenient. From an > external perspective, TranslationUnit still only has 1 instantiation > mode. If it had more, I'd definitely favor adding @classmethods to > cover each. I'm not against it today: I just see no reason for it. > > Anyway, as I typed this, I realized that we need an additional > constructor mode: from source file (e.g. Index.parse). Let me code up > a new patch and we'll see what you think.
0001-clang.py-TranslationUnit-API-improvements.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
