[cfe-commits] [PATCH 1/5] [clang.py] Implement Type.is_function_variadic

Gregory Szorc gregory.szorc at gmail.com
Sat Feb 18 20:26:00 PST 2012


The attached patch updates existing tests to use the new get_tu function.

On Fri, Feb 17, 2012 at 10:17 AM, Gregory Szorc <gregory.szorc at gmail.com>wrote:

> On Fri, Feb 17, 2012 at 1:29 AM, Manuel Klimek <klimek at google.com> wrote:
>
>> Hi Gregory,
>>
>> in test_type.py: use the new get_tu function in the other tests around
>> the new one, too? (otherwise why pull out a function)
>>
>
> Sure, I can do that. (I wasn't sure if you would mind the scope creep or
> would prefer a follow-up).
>
>
>>
>> +    ok_(isinstance(foo.type.is_function_variadic(), bool))
>> This seems a non-pythonic test to me - why do we care about the
>> underlying type of the result as long as it evaluates to True / False
>> in bool context correctly?
>>
>
> Because it ensures the API contract is behaving properly. If the return
> type changes accidentally, there would be no check to catch that. I feel
> that breaking the return type should be a conscientious decision and should
> be caught via testing.
>
>
>>
>> Generally, why use ok_ when all the tests around use normal assert?
>>
>
> It saves a couple of characters. I also like the style of a function
> rather than magic of the assert statement. Neither are great reasons, I'll
> admit.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120218/a2b5ce40/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-clang.py-Implement-Type.is_function_variadic.patch
Type: application/octet-stream
Size: 6252 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120218/a2b5ce40/attachment.obj>


More information about the cfe-commits mailing list