On Wednesday, May 27, 2015, Alexander Kornienko <<a href="mailto:alexfh@google.com">alexfh@google.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, May 27, 2015 at 5:53 AM, Justin Bogner <span dir="ltr"><<a href="javascript:_e(%7B%7D,'cvml','mail@justinbogner.com');" target="_blank">mail@justinbogner.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>Alexander Kornienko <<a href="javascript:_e(%7B%7D,'cvml','alexfh@google.com');" target="_blank">alexfh@google.com</a>> writes:<br>
> On Fri, May 22, 2015 at 10:23 PM, Justin Bogner <<a href="javascript:_e(%7B%7D,'cvml','mail@justinbogner.com');" target="_blank">mail@justinbogner.com</a>> wrote:<br>
>> Sebastian Edman <<a href="javascript:_e(%7B%7D,'cvml','sebastian.edman@evidente.se');" target="_blank">sebastian.edman@evidente.se</a>> writes:<br>
>>> Hi alexfh, danielmarjamaki,<br>
>>><br>
>>> Added possibilty to extract the arguments in a MacroInfo as a<br>
>>> container in order to use C++11 style for loops.<br>
>>><br>
>>> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D9934&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=fsTprKpktVDb-y7wZmvuuZjY9nuGeryCTEqWAYLUYXo&s=I-n9uwUfTC2afqGRvdaeZdWdirXOU2kDOemDm-M6BYM&e=" target="_blank">http://reviews.llvm.org/D9934</a><br>
>>><br>
>>> Files:<br>
>>>   include/clang/Lex/MacroInfo.h<br>
>>>   lib/Lex/PPMacroExpansion.cpp<br>
>>>   lib/Serialization/ASTWriter.cpp<br>
>>><br>
>>> Index: include/clang/Lex/MacroInfo.h<br>
>>> ===================================================================<br>
>>> --- include/clang/Lex/MacroInfo.h<br>
>>> +++ include/clang/Lex/MacroInfo.h<br>
>>> @@ -182,6 +182,9 @@<br>
>>>    bool arg_empty() const { return NumArguments == 0; }<br>
>>>    arg_iterator arg_begin() const { return ArgumentList; }<br>
>>>    arg_iterator arg_end() const { return ArgumentList+NumArguments; }<br>
>>> +  ArrayRef<IdentifierInfo*> args() const {<br>
>>> +       return ArrayRef<IdentifierInfo*>(ArgumentList, NumArguments);<br>
>>> +  }<br>
>><br>
>> It's probably better to use iterator_range from llvm/ADT/iterator_range.h.<br>
><br>
> Can you explain why you think it's better than ArrayRef here?<br>
<br>
</span>Primarily consistency, familiarity, and information hiding.<br></blockquote><div><br></div><div>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.</div><div><br></div><div>I'd clearly prefer ArrayRef here.</div></div></div></div></blockquote><div><br></div><div>It isn't actually all that important. I prefer the range, as stated, but either's fine.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
- As long as this is only used for range-for, it'll likely be identical<br>
  code generated and whoever's using this shouldn't know the difference.<br>
<br>
- Using iterator_range prevents uses such as random access, which limit<br>
  how the API can change without needing to change clients.<br>
<br>
- If we do want random access, llvm and clang would generally spell it<br>
  getArg(i) rather than args()[i], based on existing code.<br>
</blockquote></div><br>
</div></div>
</blockquote>