Index: docs/CodingStandards.rst =================================================================== --- docs/CodingStandards.rst +++ docs/CodingStandards.rst @@ -941,8 +941,8 @@ .. code-block:: c++ - for (BasicBlock::iterator II = BB->begin(), E = BB->end(); II != E; ++II) { - if (BinaryOperator *BO = dyn_cast(II)) { + for (auto &I : BB) { + if (auto *BO = dyn_cast(&I)) { Value *LHS = BO->getOperand(0); Value *RHS = BO->getOperand(1); if (LHS != RHS) { @@ -961,8 +961,8 @@ .. code-block:: c++ - for (BasicBlock::iterator II = BB->begin(), E = BB->end(); II != E; ++II) { - BinaryOperator *BO = dyn_cast(II); + for (auto &I : BB) { + auto *BO = dyn_cast(&I); if (!BO) continue; Value *LHS = BO->getOperand(0); @@ -1322,25 +1322,23 @@ individual enumerators. To suppress this warning, use ``llvm_unreachable`` after the switch. -Don't evaluate ``end()`` every time through a loop -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Use range-based ``for`` loops wherever possible +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -Because C++ doesn't have a standard "``foreach``" loop (though it can be -emulated with macros and may be coming in C++'0x) we end up writing a lot of -loops that manually iterate from begin to end on a variety of containers or -through other data structures. One common mistake is to write a loop in this -style: +The introduction of range-based ``for`` loops in C++11 means that explicit +manipulation of iterators is rarely necessary. We use range-based ``for`` +loops wherever possible for all newly added code. For example: .. code-block:: c++ BasicBlock *BB = ... - for (BasicBlock::iterator I = BB->begin(); I != BB->end(); ++I) + for (auto &I : *BB) ... use I ... -The problem with this construct is that it evaluates "``BB->end()``" every time -through the loop. Instead of writing the loop like this, we strongly prefer -loops to be written so that they evaluate it once before the loop starts. A -convenient way to do this is like so: +In cases where it is necessary to write an explicit iterator-based loop, +ensure that `end()` is evaluated only once as long as the container is not +being mutated. Use code like the following in preference to calling +``BB->end()`` every time through the loop: .. code-block:: c++ @@ -1348,32 +1346,6 @@ for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I != E; ++I) ... use I ... -The observant may quickly point out that these two loops may have different -semantics: if the container (a basic block in this case) is being mutated, then -"``BB->end()``" may change its value every time through the loop and the second -loop may not in fact be correct. If you actually do depend on this behavior, -please write the loop in the first form and add a comment indicating that you -did it intentionally. - -Why do we prefer the second form (when correct)? Writing the loop in the first -form has two problems. First it may be less efficient than evaluating it at the -start of the loop. In this case, the cost is probably minor --- a few extra -loads every time through the loop. However, if the base expression is more -complex, then the cost can rise quickly. I've seen loops where the end -expression was actually something like: "``SomeMap[X]->end()``" and map lookups -really aren't cheap. By writing it in the second form consistently, you -eliminate the issue entirely and don't even have to think about it. - -The second (even bigger) issue is that writing the loop in the first form hints -to the reader that the loop is mutating the container (a fact that a comment -would handily confirm!). If you write the loop in the second form, it is -immediately obvious without even looking at the body of the loop that the -container isn't being modified, which makes it easier to read the code and -understand what it does. - -While the second form of the loop is a few extra keystrokes, we do strongly -prefer it. - ``#include `` is Forbidden ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^