[PATCH] D37264: [Docs] Update CodingStandards to recommend range-based for loops

[Alex Bradbury via Phabricator via llvm-commits]

asb added inline comments.


================
Comment at: docs/CodingStandards.rst:944
 
-  for (BasicBlock::iterator II = BB->begin(), E = BB->end(); II != E; ++II) {
-    if (BinaryOperator *BO = dyn_cast<BinaryOperator>(II)) {
+  for (auto &I : BB) {
+    if (auto *BO = dyn_cast<BinaryOperator>(&I)) {
----------------
majnemer wrote:
> javed.absar wrote:
> > aaron.ballman wrote:
> > > majnemer wrote:
> > > > I'd replace the `auto` with `Instruction &`.
> > > We should clarify this in the coding standard, because I've always understood we generally *want* to use `auto` in range-based for loops, for brevity (and because the type is generally immediately apparent from the container).
> > In some complicated cases not using 'auto' improves code understanding. I have had cases where well established reviewers of llvm have recommended me not to  use 'auto' in their review comment.
> The most consistent rule I have seen is that we want to avoid repeating ourselves. `auto` makes a lot of sense if you just made a `std::vector<int>` a few lines up and are now iterating. If you have some `FancyContainerTy`, it is less awesome to use `auto` as it is not clear what it is you get from iterating it.
For `FancyContainerTy` sure, but my impression is it's quite common to use auto for simple container types where the types are well known to the average LLVM developer. e.g. Instructions in a BasicBlock, BasicBlocks in a Function.


https://reviews.llvm.org/D37264