]> granicus.if.org Git - clang/commitdiff
[coverage] Make smaller regions for the first case of a switch.
authorEli Friedman <efriedma@codeaurora.org>
Wed, 2 Aug 2017 23:22:50 +0000 (23:22 +0000)
committerEli Friedman <efriedma@codeaurora.org>
Wed, 2 Aug 2017 23:22:50 +0000 (23:22 +0000)
We never overwrite the end location of a region, so we would end up with
an overly large region when we reused the switch's region.

It's possible this code will be substantially rewritten in the near
future to deal with fallthrough more accurately, but this seems like
an improvement on its own for now.

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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@309901 91177308-0d34-0410-b5e6-96231b3b80d8

lib/CodeGen/CoverageMappingGen.cpp
test/CoverageMapping/switch.cpp

index 51f5902b91a4ff372dd65f9d61e197223b38533f..1484ec78b88638c88222ebfc187b2fd50f56c96d 100644 (file)
@@ -704,6 +704,8 @@ struct CounterCoverageMappingBuilder
     assert(!BreakContinueStack.empty() && "break not in a loop or switch!");
     BreakContinueStack.back().BreakCount = addCounters(
         BreakContinueStack.back().BreakCount, getRegion().getCounter());
+    // FIXME: a break in a switch should terminate regions for all preceding
+    // case statements, not just the most recent one.
     terminateRegion(S);
   }
 
@@ -845,15 +847,20 @@ struct CounterCoverageMappingBuilder
     extendRegion(Body);
     if (const auto *CS = dyn_cast<CompoundStmt>(Body)) {
       if (!CS->body_empty()) {
-        // The body of the switch needs a zero region so that fallthrough counts
-        // behave correctly, but it would be misleading to include the braces of
-        // the compound statement in the zeroed area, so we need to handle this
-        // specially.
+        // Make a region for the body of the switch.  If the body starts with
+        // a case, that case will reuse this region; otherwise, this covers
+        // the unreachable code at the beginning of the switch body.
         size_t Index =
-            pushRegion(Counter::getZero(), getStart(CS->body_front()),
-                       getEnd(CS->body_back()));
+            pushRegion(Counter::getZero(), getStart(CS->body_front()));
         for (const auto *Child : CS->children())
           Visit(Child);
+
+        // Set the end for the body of the switch, if it isn't already set.
+        for (size_t i = RegionStack.size(); i != Index; --i) {
+          if (!RegionStack[i - 1].hasEndLoc())
+            RegionStack[i - 1].setEndLoc(getEnd(CS->body_back()));
+        }
+
         popRegions(Index);
       }
     } else
index 312f26ca16e4fba589561d9b96ea7d19d705b546..17aa53bb48600231fb7cc570c7c1cf695d2d4237 100644 (file)
@@ -3,7 +3,7 @@
                     // CHECK: foo
 void foo(int i) {   // CHECK-NEXT: File 0, [[@LINE]]:17 -> [[@LINE+8]]:2 = #0
   switch(i) {
-  case 1:           // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+3]]:10 = #2
+  case 1:           // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:11 = #2
     return;
   case 2:           // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #3
     break;
@@ -48,7 +48,7 @@ void baz() {        // CHECK-NEXT: File 0, [[@LINE]]:12 -> [[@LINE+5]]:2 = #0
 int main() {        // CHECK-NEXT: File 0, [[@LINE]]:12 -> [[@LINE+35]]:2 = #0
   int i = 0;
   switch(i) {
-  case 0:           // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+7]]:10 = #2
+  case 0:           // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = #2
     i = 1;
     break;
   case 1:           // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = #3
@@ -58,7 +58,7 @@ int main() {        // CHECK-NEXT: File 0, [[@LINE]]:12 -> [[@LINE+35]]:2 = #0
     break;
   }
   switch(i) {       // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+23]]:2 = #1
-  case 0:           // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+6]]:10 = #6
+  case 0:           // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = #6
     i = 1;
     break;
   case 1:           // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+3]]:10 = #7
@@ -81,3 +81,19 @@ int main() {        // CHECK-NEXT: File 0, [[@LINE]]:12 -> [[@LINE+35]]:2 = #0
   baz();
   return 0;
 }
+
+// FIXME: End location for "case 1" shouldn't point at the end of the switch.
+                         // CHECK: fallthrough
+int fallthrough(int i) { // CHECK-NEXT: File 0, [[@LINE]]:24 -> [[@LINE+12]]:2 = #0
+  switch(i) {
+  case 1:           // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+8]]:10 = #2
+    i = 23;
+  case 2:           // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = (#2 + #3)
+    i = 11;
+    break;
+  case 3:           // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+3]]:10 = #4
+  case 4:           // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = (#4 + #5)
+    i = 99;
+    break;
+  }
+}