On Wed, May 27, 2015 at 5:53 AM, Justin Bogner <[email protected]>
wrote:

> Alexander Kornienko <[email protected]> writes:
> > On Fri, May 22, 2015 at 10:23 PM, Justin Bogner <[email protected]>
> wrote:
> >> Sebastian Edman <[email protected]> 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.


>
> - 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.
>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to