[PATCH] D83351: [llvm-reduce] Reducing attributes

Wed Jul 8 11:51:55 PDT 2020

dblaikie added a comment.

In D83351#2139599 <https://reviews.llvm.org/D83351#2139599>, @lebedev.ri wrote:

> In D83351#2139586 <https://reviews.llvm.org/D83351#2139586>, @nickdesaulniers wrote:
>
> > I'm not a fan of the inconsistent use of range-for and for-each; I would prefer range-for everywhere since it's more concise.
>
>
> My (consistently inconsistent) headcanon as to which to use when is that for_each should be used
>  when in principle we don't care in which order each item will be processed.

I don't think that's how the rest of LLVM is written, nor probably a great model. std::for_each is guaranteed to visit the elements in order, so it doesn't have a different contract to a range-based-for loop & adds some extra syntax (the lambda introducers, etc), complications to error messages, etc.

>> But I don't feel strongly enough to block the patch based on that.  Maybe LLVM's style guide should provide clarity and guidance on the difference of opinion?

I think this is one I'm willing to say LLVM convention's pretty clear - there's 30 calls to std::for_each across LLVM and subprojects - and about 25,000 range-based-for loops... - so please change these to range based for loops.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83351/new/

https://reviews.llvm.org/D83351