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

[Justin Bogner]

Alexander Kornienko <alexfh at google.com> writes:
> On Fri, May 22, 2015 at 10:23 PM, Justin Bogner <mail at justinbogner.com> wrote:
>> Sebastian Edman <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.

- 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.