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

[Gregory Szorc]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: tu-refactor.patch
Type: application/octet-stream
Size: 24402 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120507/01e692e9/attachment.obj>