[Analysis] -Wunreachable-code shouldn't fire on the increment of a fo…

…reach loop

Summary:
The idea is that the code here isn't written, so doesn't indicate a bug.
Similar to code expanded from macros.

This means the warning no longer fires on this code:
  for (auto C : collection) {
    process(C);
    return;
  }
  handleEmptyCollection();
Unclear whether this is more often a bug or not in practice, I think it's a
reasonable idiom in some cases.
Either way, if we want to warn on "loop that doesn't loop", I think it should be
a separate warning, and catch `while(1) break;`

Reviewers: ilya-biryukov, ioeric

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D58134

llvm-svn: 354102

clang/lib/Analysis/ReachableCode.cpp

@@ -631,6 +631,10 @@ void DeadCodeScan::reportDeadCode(const CFGBlock *B,
     // a for/for-range loop.  This is the block that contains
     // the increment code.
     if (const Stmt *LoopTarget = B->getLoopTarget()) {
+      // The increment on a foreach statement is not written.
+      if (isa<CXXForRangeStmt>(LoopTarget))
+        return;
+
       SourceLocation Loc = LoopTarget->getBeginLoc();
       SourceRange R1(Loc, Loc), R2;
 

clang/test/SemaCXX/unreachable-code.cpp

@@ -52,6 +52,11 @@ void test3() {
   }
 }
 
+void test4() {
+  for (char c : "abc") // no-warning
+    break;
+}
+
 // PR 6130 - Don't warn about bogus unreachable code with throw's and
 // temporary objects.
 class PR6130 {