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

[Aaron Ballman via Phabricator via llvm-commits]

aaron.ballman 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)) {
----------------
asb wrote:
> 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.
Agreed that we use `auto` primarily when the type is explicitly spelled out in the initialization. I was mostly interested in clarification around range-based for loops. The above is an example where I usually recommend `auto` because I thought that was the direction we had picked back when the C++11 discussion happened. If that's an incorrect understanding, I think we really should clarify the coding standard so I have something more concrete to point to.


https://reviews.llvm.org/D37264