[cfe-commits] [PATCH 3/5] [clang.py] Implement Type.arguments()

Gregory Szorc gregory.szorc at gmail.com
Tue Feb 21 10:06:02 PST 2012


Author: gps
Date: Mon Feb 20 11:45:30 2012
New Revision: 150970

URL: http://llvm.org/viewvc/llvm-project?rev=150970&view=rev
Log:
[clang.py] Implement Type.argument_types()

Modified:
     cfe/trunk/bindings/python/clang/cindex.py
     cfe/trunk/bindings/python/tests/cindex/test_type.py

On 2/20/12 9:06 AM, Manuel Klimek wrote:
> On Sun, Feb 19, 2012 at 10:55 AM, Gregory Szorc<gregory.szorc at gmail.com>  wrote:
>> On 2/19/2012 12:49 AM, Tobias Grosser wrote:
>>> On 02/19/2012 05:39 AM, Gregory Szorc wrote:
>>>> Does anyone else think that "arguments" may not be the best name?
>>>> Perhaps "argument_types" would be more suitable...
>>>
>>> Yes. I agree. Either name it argument_types, or return a class called
>>> 'argument' that than has a .type() function. Returning a class is
>>> probably overhead, as it would only have a single function. I would
>>> just name the iterator argument_types().
>>>
>>> Also, I just realized you define the __iter__(self) method. Why is
>>> that needed? Diagnostic.ranges() gets away without using it?
>>
>> argument_types() it is. It seems __iter__ has a default implementation
>> somewhere (I guess in object). In the current patch, I changed the class
>> to derive from collections.Sequence. This is more appropriate than
>> deriving from object and the default implementation of __iter__ there
>> works as expected (it calls __getitem__ from 0..n until __getitem__
>> throws IndexError).
>
> The one thing that strikes me as odd is that the method is on the
> "Type", but it looks like that's a design decision that was made
> earlier (just wandering what this will become in the future if *all*
> methods of all Type subclasses in the AST become accessible from that
> one class)
>
> Otherwise looks good.
>
> Cheers,
> /Manuel
>
>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>





More information about the cfe-commits mailing list