[cfe-commits] [PATCH 4/4] [clang.py] TranslationUnit API improvements

Manuel Klimek klimek at google.com
Mon May 7 22:53:25 PDT 2012


Looks good.

On Tue, May 8, 2012 at 7:08 AM, Gregory Szorc <gregory.szorc at gmail.com> wrote:
> Everything is addressed.
>
> On Sun, May 6, 2012 at 10:39 PM, Manuel Klimek <klimek at google.com> wrote:
>> +    Unfortunately, the libclang library doesn't expose any additional error
>> +    information in this scenario.
>>
>> Change to: FIXME: Make libclang expose additional error information in
>> this scenario :)
>>
>> On Fri, May 4, 2012 at 7:31 PM, Gregory Szorc <gregory.szorc at gmail.com> wrote:
>>> On Mon, Apr 16, 2012 at 9:37 AM, Manuel Klimek <klimek at google.com> wrote:
>>>> I like the factory methods much better! Thanks :)
>>>>
>>>> ERROR_OK is still in there though?
>>>>
>>>> +        if isinstance(unsaved_files, dict):
>>>> +            for k, v in unsaved_files.iteritems():
>>>> +                unsaved_normalized.append((k, v))
>>>>
>>>> unsaved_normalized = unsaved_files.items()
>>>>
>>>> While I see how this is convenient, why do we not require a list and
>>>> let people outside call .items() if they have a dict? I kind of
>>>> dislike all this type specific code, but that's more a gut feeling
>>>> than being able to point my finger at problems.
>>>>
>>>> Also, what happens if we pass parameters of incorrect type to
>>>> TranslationUnit_parse? All that type checking in python code seems
>>>> somewhat strange...
>>>
>>> I agree with what you said. Looking at the code now, I don't know what
>>> I was thinking :)
>>
>> Hmm, there is still a lot of type checking in the code there... And
>> I'm still curious what would happen if we just assumed they all have
>> the right type - at least that's what most of the python code I know
>> does; would that crash when calling into the C-bindings?
>>
>>> Issues addressed with attached patch. Sorry it took so long to respond.
>>>
>>> Since the time I created this patch, the behavior of
>>> clang_saveTranslationUnit has changed. Previously, if you created a TU
>>> with critical errors (like bad syntax), that API would return an error
>>> code. Now, it seems to return success and write the file. I marked the
>>> test that exercises the exception raising bits as skipped as a result.
>>> I'm trying to think of the best way to test this now. Perhaps try
>>> writing to a file without write permissions?
>>
>> The easiest way I know to test file-open failures is write to a file
>> in a non-existent directory.
>>
>> Cheers,
>> /Manuel




More information about the cfe-commits mailing list