[PATCH] Refactor MacroInfo for easier access in for-loops

[Justin Bogner]

On Wednesday, May 27, 2015, Alexander Kornienko <alexfh at google.com> wrote:

> On Wed, May 27, 2015 at 5:53 AM, Justin Bogner <mail at justinbogner.com
> <javascript:_e(%7B%7D,'cvml','mail at justinbogner.com');>> wrote:
>
>> Alexander Kornienko <alexfh at google.com
>> <javascript:_e(%7B%7D,'cvml','alexfh at google.com');>> writes:
>> > On Fri, May 22, 2015 at 10:23 PM, Justin Bogner <mail at justinbogner.com
>> <javascript:_e(%7B%7D,'cvml','mail at justinbogner.com');>> wrote:
>> >> Sebastian Edman <sebastian.edman at evidente.se
>> <javascript:_e(%7B%7D,'cvml','sebastian.edman at evidente.se');>> writes:
>> >>> Hi alexfh, danielmarjamaki,
>> >>>
>> >>> Added possibilty to extract the arguments in a MacroInfo as a
>> >>> container in order to use C++11 style for loops.
>> >>>
>> >>> http://reviews.llvm.org/D9934
>> >>>
>> >>> Files:
>> >>>   include/clang/Lex/MacroInfo.h
>> >>>   lib/Lex/PPMacroExpansion.cpp
>> >>>   lib/Serialization/ASTWriter.cpp
>> >>>
>> >>> Index: include/clang/Lex/MacroInfo.h
>> >>> ===================================================================
>> >>> --- include/clang/Lex/MacroInfo.h
>> >>> +++ include/clang/Lex/MacroInfo.h
>> >>> @@ -182,6 +182,9 @@
>> >>>    bool arg_empty() const { return NumArguments == 0; }
>> >>>    arg_iterator arg_begin() const { return ArgumentList; }
>> >>>    arg_iterator arg_end() const { return ArgumentList+NumArguments; }
>> >>> +  ArrayRef<IdentifierInfo*> args() const {
>> >>> +       return ArrayRef<IdentifierInfo*>(ArgumentList, NumArguments);
>> >>> +  }
>> >>
>> >> It's probably better to use iterator_range from
>> llvm/ADT/iterator_range.h.
>> >
>> > Can you explain why you think it's better than ArrayRef here?
>>
>> Primarily consistency, familiarity, and information hiding.
>>
>
> Well, the first two reasons are equally valid for ArrayRef: ArrayRef seems
> to be used more frequently in LLVM than iterator_range. The last one is
> arguable: it only matters in the unlikely situation when the underlying
> storage would need to be changed to something not allowing random access.
>
> I'd clearly prefer ArrayRef here.
>

It isn't actually all that important. I prefer the range, as stated, but
either's fine.


>
>> - As long as this is only used for range-for, it'll likely be identical
>>   code generated and whoever's using this shouldn't know the difference.
>>
>> - Using iterator_range prevents uses such as random access, which limit
>>   how the API can change without needing to change clients.
>>
>> - If we do want random access, llvm and clang would generally spell it
>>   getArg(i) rather than args()[i], based on existing code.
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150527/fe5775a8/attachment.html>