[Coverage] Discard the last uncompleted deferred region in a decl

[vsk]

Summary

Discard the last uncompleted deferred region in a decl, if one exists.
This prevents lines at the end of a function containing only whitespace
or closing braces from being marked as uncovered, if they follow a
region terminator (return/break/etc).

The previous behavior was to heuristically complete deferred regions at
the end of a decl. In practice this ended up being too brittle for too
little gain. Users would complain that there was no way to reach full
code coverage because whitespace at the end of a function would be
marked uncovered.

rdar://40238228

[vsk]

• Add a regression test for switches.

[vsk]

Ping.

[arphaman]

LGTM

cfe/trunk/lib/CodeGen/CoverageMappingGen.cpp

void VisitStmt(const Stmt *S) {
     if (S->getLocStart().isValid())
       extendRegion(S);
     for (const Stmt *Child : S->children())
       if (Child)
         this->Visit(Child);
     handleFileExit(getEnd(S));
   }
 
-  /// Determine whether the final deferred region emitted in \p Body should be
-  /// discarded.
-  static bool discardFinalDeferredRegionInDecl(Stmt *Body) {
-    if (auto *CS = dyn_cast<CompoundStmt>(Body)) {
-      Stmt *LastStmt = CS->body_back();
-      if (auto *IfElse = dyn_cast<IfStmt>(LastStmt)) {
-        if (auto *Else = dyn_cast_or_null<CompoundStmt>(IfElse->getElse()))
-          LastStmt = Else->body_back();
-        else
-          LastStmt = IfElse->getElse();
-      }
-      return dyn_cast_or_null<ReturnStmt>(LastStmt);
-    }
-    return false;
-  }
-
   void VisitDecl(const Decl *D) {
     assert(!DeferredRegion && "Deferred region never completed");
 
     Stmt *Body = D->getBody();
 
     // Do not propagate region counts into system headers.
     if (Body && SM.isInSystemHeader(SM.getSpellingLoc(getStart(Body))))
       return;
 
-    Counter ExitCount = propagateCounts(getRegionCounter(Body), Body);
+    propagateCounts(getRegionCounter(Body), Body);
     assert(RegionStack.empty() && "Regions entered but never exited");
 
-    if (DeferredRegion) {
-      // Complete (or discard) any deferred regions introduced by the last
-      // statement.
-      if (discardFinalDeferredRegionInDecl(Body))
-        DeferredRegion = None;
-      else
-        popRegions(completeDeferred(ExitCount, getEnd(Body)));
-    }
+    // Discard the last uncompleted deferred region in a decl, if one exists.
+    // This prevents lines at the end of a function containing only whitespace
+    // or closing braces from being marked as uncovered.
   }
 
   void VisitReturnStmt(const ReturnStmt *S) {
     extendRegion(S);
     if (S->getRetValue())
       Visit(S->getRetValue());
     terminateRegion(S);
   }

cfe/trunk/test/CoverageMapping/deferred-region.cpp

 // RUN: %clang_cc1 -std=c++11 -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -fexceptions -fcxx-exceptions -emit-llvm-only -triple %itanium_abi_triple -main-file-name deferred-region.cpp -I %S/Inputs %s | FileCheck %s
 
 #define IF if
 #define STMT(S) S
 
 // CHECK-LABEL: _Z3fooi:
 void foo(int x) {
   if (x == 0) {
     return;
-  } // CHECK: Gap,File 0, [[@LINE]]:4 -> [[@LINE+2]]:2 = (#0 - #1)
-
+  } // CHECK-NOT: Gap,File 0, [[@LINE]]:4
+    //< Don't complete the last deferred region in a decl, even though it may
+    //< leave some whitespace marked with the same counter as the final return.
 }
 
-// CHECK-NEXT: _Z4foooi:
+// CHECK-LABEL: _Z4foooi:
 void fooo(int x) {
   if (x == 0) {
     return;
   } // CHECK: Gap,File 0, [[@LINE]]:4 -> [[@LINE+2]]:3 = (#0 - #1)
 
   if (x == 1) {
     return;
-  } // CHECK: Gap,File 0, [[@LINE]]:4 -> [[@LINE+2]]:2 = ((#0 - #1) - #2)
+  } // CHECK-NOT: Gap,File 0, [[@LINE]]:4
 
 }
 
 // CHECK-LABEL: _Z3bazv:
 void baz() { // CHECK: [[@LINE]]:12 -> [[@LINE+2]]:2
   return;    // CHECK-NOT: File
 }
 
if (i + 100 > 0) { // CHECK: [[@LINE]]:20 -> [[@LINE+6]]:4 = #3
     if (false)       // CHECK: [[@LINE+1]]:7 -> [[@LINE+1]]:13 = #4
       return;        // CHECK: Gap,File 0, [[@LINE]]:13 -> [[@LINE+2]]:5 = (#3 - #4)
                      // CHECK: [[@LINE+1]]:5 -> [[@LINE+3]]:4 = (#3 - #4)
     return;          // CHECK: Gap,File 0, [[@LINE]]:5 -> [[@LINE+4]]:3 = ((#0 - #1) - #3)
 
   }
 
   if (false)
-    return; // CHECK: Gap,File 0, [[@LINE]]:11 -> [[@LINE+1]]:2
+    return; // CHECK-NOT: Gap,File 0, [[@LINE]]:11
 }
 
 // CHECK-LABEL: _Z8for_loopv:
 void for_loop() {
   if (false)
     return; // CHECK: Gap,File 0, [[@LINE]]:11 -> [[@LINE+2]]:3 = (#0 - #1)
 
   for (int i = 0; i < 10; ++i) {

 // CHECK-LABEL: _Z5gotosv:
 void gotos() {
   if (false)
     goto out; // CHECK: Gap,File 0, [[@LINE]]:13 -> [[@LINE+2]]:3 = (#0 - #1)
 
   return; // CHECK: [[@LINE]]:3 -> [[@LINE+4]]:2 = (#0 - #1)
 
 out:
-  return; // CHECK: Gap,File 0, [[@LINE]]:8 -> [[@LINE+1]]:2 = 0
+  return; // CHECK-NOT: Gap,File 0, [[@LINE]]:8
+}
+
+// CHECK-LABEL: _Z8switchesv:
+void switches() {
+  int x;
+  switch (x) {
+    case 0:
+      return;
+    default:
+      return; // CHECK-NOT: Gap,File 0, [[@LINE]]
+  }
 }
 
 #include "deferred-region-helper.h"
 // CHECK-LABEL: _Z13included_funcv:
 // CHECK:  Gap,File 0, 2:13 -> 3:5 = #1
 // CHECK:  Gap,File 0, 3:11 -> 4:3 = (#0 - #1)
 
 // CHECK-LABEL: _Z7includev:

cfe/trunk/test/CoverageMapping/label.cpp

if(i < 5) {              // CHECK: File 0, [[@LINE]]:15 -> [[@LINE+6]]:6 = #2
         x:                   // CHECK-NEXT: File 0, [[@LINE]]:9 -> [[@LINE+4]]:6 = #3
           int j = 1;
       }
       int m = 2;
     } else
       goto x;                // CHECK: File 0, [[@LINE]]:7 -> [[@LINE]]:13 = (#1 - #2)
     int k = 3;               // CHECK-NEXT: File 0, [[@LINE-1]]:13 -> [[@LINE]]:5 = #3
   }                          // CHECK-NEXT: File 0, [[@LINE-1]]:5 -> [[@LINE]]:4 = #3
-  static int j = 0;          // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+5]]:2 = ((#0 + #3) - #1)
+  static int j = 0;          // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+4]]:2 = ((#0 + #3) - #1)
   ++j;
   if(j == 1)                 // CHECK-NEXT: File 0, [[@LINE]]:6 -> [[@LINE]]:12 = ((#0 + #3) - #1)
     goto x;                  // CHECK: File 0, [[@LINE]]:5 -> [[@LINE]]:11 = #4
-                             // CHECK-NEXT: File 0, [[@LINE-1]]:11 -> [[@LINE+1]]:2 = (((#0 + #3) - #1) - #4)
 }
 
                              // CHECK-NEXT: test1
 void test1(int x) {          // CHECK-NEXT: File 0, [[@LINE]]:19 -> {{[0-9]+}}:2 = #0
   if(x == 0)                 // CHECK-NEXT: File 0, [[@LINE]]:6 -> [[@LINE]]:12 = #0
     goto a;                  // CHECK: File 0, [[@LINE]]:5 -> [[@LINE]]:11 = #1
                              // CHECK-NEXT: File 0, [[@LINE-1]]:11 -> [[@LINE+1]]:3 = (#0 - #1)
-  goto b;                    // CHECK: Gap,File 0, [[@LINE]]:3 -> [[@LINE+5]]:2 = #3
+  goto b;                    // CHECK: File 0, [[@LINE]]:3 -> [[@LINE+5]]:2 = (#0 - #1)
                              // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:9 -> [[@LINE+1]]:1 = #2
 a:                           // CHECK-NEXT: File 0, [[@LINE]]:1 -> [[@LINE+3]]:2 = #2
 b:                           // CHECK-NEXT: File 0, [[@LINE]]:1 -> [[@LINE+2]]:2 = #3
   x = x + 1;
 }
 
                              // CHECK-NEXT: test2
 void test2(int x) {          // CHECK-NEXT: File 0, [[@LINE]]:19 -> {{[0-9]+}}:2 = #0

cfe/trunk/test/CoverageMapping/moremacros.c

int main(int argc, const char *argv[]) {
   } // CHECK-NEXT: [[@LINE]]:4 -> [[@LINE+5]]:3 = ((#0 - #2) - #3)
 
   // CHECK-NEXT: File 0, [[@LINE+3]]:3 -> [[@LINE+7]]:2 = ((#0 - #2) - #3)
   // CHECK-NEXT: File 0, [[@LINE+2]]:7 -> [[@LINE+2]]:12 = ((#0 - #2) - #3)
   // CHECK: File 0, [[@LINE+1]]:14 -> [[@LINE+4]]:8 = #4
   if (!argc) {
     return 0;
   // CHECK-NEXT: Expansion,File 0, [[@LINE+1]]:3 -> [[@LINE+1]]:8 = #4
-  RBRAC // CHECK-NEXT: [[@LINE]]:8 -> [[@LINE+1]]:2 = (((#0 - #2) - #3) - #4)
+  RBRAC
 }
 
 // CHECK-NEXT: File 1, 3:15 -> 3:16 = #2
 // CHECK-NEXT: File 2, 4:15 -> 4:16 = #2
 // CHECK-NEXT: File 3, 3:15 -> 3:16 = #3
 // CHECK-NEXT: File 4, 4:15 -> 4:16 = #4

cfe/trunk/test/CoverageMapping/trycatch.cpp

 void func(int i) {                    // CHECK-NEXT: File 0, [[@LINE]]:18 -> {{[0-9]+}}:2 = #0
                                       // CHECK-NEXT: File 0, [[@LINE+1]]:6 -> [[@LINE+1]]:11 = #0
   if(i % 2) {                         // CHECK: File 0, [[@LINE]]:13 -> [[@LINE+4]]:4 = #1
     throw Error();
     int j = 0;                        // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE+2]]:4 = 0
                                       // CHECK: File 0, [[@LINE+1]]:10 -> [[@LINE+2]]:27 = (#0 - #1)
   } else if(i == 8)                   // CHECK-NEXT: File 0, [[@LINE]]:13 -> [[@LINE]]:19 = (#0 - #1)
     throw ImportantError();           // CHECK: File 0, [[@LINE]]:5 -> [[@LINE]]:27 = #2
-}                                     // CHECK-NEXT: File 0, [[@LINE-1]]:27 -> [[@LINE]]:2 = ((#0 - #1) - #2)
+}
 
                                       // CHECK-NEXT: main
 int main() {                          // CHECK-NEXT: File 0, [[@LINE]]:12 -> [[@LINE+13]]:2 = #0
   int j = 1;
   try {                               // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE+2]]:4 = #0
     func(j);
   } catch(const Error &e) {           // CHECK-NEXT: File 0, [[@LINE]]:27 -> [[@LINE+2]]:4 = #2
     j = 1;
   } catch(const ImportantError &e) {  // CHECK-NEXT: File 0, [[@LINE]]:36 -> [[@LINE+2]]:4 = #3
     j = 11;
   }
   catch(const Warning &w) {           // CHECK-NEXT: File 0, [[@LINE]]:27 -> [[@LINE+2]]:4 = #4
     j = 0;
   }
   return 0;                           // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE]]:11 = #1
 }