[Coverage] Fix an issue where a coverage region might not be created for a macro containing for or while statements.

[ikudrin]

Summary

The situation happened when a macro contained a full loop statement,
which body ended at the end of the macro.

[vsk]

Thanks, lgtm with a nit.

lib/CodeGen/CoverageMappingGen.cpp

452

These asserts replicate checks inside of get{Start,End}Loc, so I think it's cleaner to drop them.

[davidxl]

lgtm

lib/CodeGen/CoverageMappingGen.cpp

468

Perhaps add a comment here that there is need to create 'outer most' code region for while/for stmt('s virtual file) with counter equal to the parent/enclosing scope's counter. The expansion region's counter will be copied from it during coverage reading. Also refer to handleFileExit where the region is created.

[ikudrin]

lib/CodeGen/CoverageMappingGen.cpp

468

We don't copy the outer code region's counter into an expansion region anymore. It was in the previous revision of D18831, but, with this patch, it's no longer needed. I'll refer to handleFileExit(), though.

lib/CodeGen/CoverageMappingGen.cpp

Counter propagateCounts(Counter TopCount, const Stmt *S) {
     // The statement may be spanned by an expansion. Make sure we handle a file
     // exit out of this expansion before moving to the next statement.
     if (SM.isBeforeInTranslationUnit(getStart(S), S->getLocStart()))
       MostRecentLocation = getEnd(S);
 
     return ExitCount;
   }
 
+  /// \brief Check whether a region with bounds \c StartLoc and \c EndLoc
+  /// is already added to \c SourceRegions.
+  bool isRegionAlreadyAdded(SourceLocation StartLoc, SourceLocation EndLoc) {
+    return SourceRegions.rend() !=
+           std::find_if(SourceRegions.rbegin(), SourceRegions.rend(),
+                        [=](const SourceMappingRegion &Region) {
+                          assert(Region.hasStartLoc() && "incomplete region");
+                          assert(Region.hasEndLoc() && "incomplete region");
+                          return Region.getStartLoc() == StartLoc &&
+                                 Region.getEndLoc() == EndLoc;
+                        });
+  }
+
   /// \brief Adjust the most recently visited location to \c EndLoc.
   ///
   /// This should be used after visiting any statements in non-source order.
   void adjustForOutOfOrderTraversal(SourceLocation EndLoc) {
     MostRecentLocation = EndLoc;
     // Avoid adding duplicate regions if we have a completed region on the top
-    // of the stack and are adjusting to the end of a virtual file.
+    // of the stack.
     if (getRegion().hasEndLoc() &&
-        MostRecentLocation == getEndOfFileOrMacro(MostRecentLocation))
+        MostRecentLocation == getEndOfFileOrMacro(MostRecentLocation) &&
+        isRegionAlreadyAdded(getStartOfFileOrMacro(MostRecentLocation),
+                             MostRecentLocation))
       MostRecentLocation = getIncludeOrExpansionLoc(MostRecentLocation);
   }
 
   /// \brief Check whether \c Loc is included or expanded from \c Parent.
   bool isNestedIn(SourceLocation Loc, FileID Parent) {
     do {
       Loc = getIncludeOrExpansionLoc(Loc);
       if (Loc.isInvalid())

test/CoverageMapping/macroscopes.cpp

if (true) {     \
     x = x;        \
   } else {        \
     x = x;        \
   }
 
 #define starts_a_while while (x < 5)
 #define simple_stmt ++x
 
+#define macro_with_for          \
+  x = 3;                        \
+  for (int i = 0; i < x; ++i) { \
+  }
+
+#define macro_with_while \
+  x = 4;                 \
+  while (x < 5) {        \
+    ++x;                 \
+  }
+
 // CHECK: main
 // CHECK-NEXT: File 0, [[@LINE+1]]:12 -> {{[0-9]+}}:2 = #0
 int main() {
   int x = 0;
   // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:3 -> [[@LINE+2]]:17 = #0
   // CHECK-NEXT: File 0, [[@LINE+1]]:17 -> [[@LINE+7]]:15 = #1
   starts_a_scope
     x = x;
int main() {
   // CHECK-NEXT: Expansion,File 0, [[@LINE+4]]:3 -> [[@LINE+4]]:17 = #0
   // CHECK-NEXT: File 0, [[@LINE+3]]:18 -> [[@LINE+5]]:15 = #9
   // CHECK-NEXT: Expansion,File 0, [[@LINE+3]]:5 -> [[@LINE+3]]:16 = #9
   // CHECK-NEXT: Expansion,File 0, [[@LINE+3]]:3 -> [[@LINE+3]]:15 = #9
   starts_a_while {
     simple_stmt;
   ends_a_scope
 
+  // CHECK-NEXT: Expansion,File 0, [[@LINE+1]]:3 -> [[@LINE+1]]:17 = #0
+  macro_with_for
+  // CHECK-NEXT: Expansion,File 0, [[@LINE+1]]:3 -> [[@LINE+1]]:19 = #0
+  macro_with_while
+
   return 0;
 }
 
 // CHECK-NEXT: File 1, 3:24 -> 3:53 = #0
 // CHECK-NEXT: File 1, 3:40 -> 3:45 = (#0 + #1)
 // CHECK-NEXT: File 1, 3:47 -> 3:50 = #1
 // CHECK-NEXT: File 1, 3:52 -> 3:53 = #1
 // CHECK-NEXT: File 2, 10:3 -> 20:4 = #1

 // CHECK-NEXT: File 8, 6:3 -> 7:4 = #7
 // CHECK-NEXT: File 9, 22:24 -> 22:37 = #0
 // CHECK-NEXT: File 9, 22:31 -> 22:36 = (#0 + #8)
 // CHECK-NEXT: File 10, 23:21 -> 23:24 = #8
 // CHECK-NEXT: File 11, 22:24 -> 22:37 = #0
 // CHECK-NEXT: File 11, 22:31 -> 22:36 = (#0 + #9)
 // CHECK-NEXT: File 12, 23:21 -> 23:24 = #9
 // CHECK-NEXT: File 13, 6:3 -> 7:4 = #9
+// CHECK-NEXT: File 14, 26:3 -> 28:4 = #0
+// CHECK-NEXT: File 14, 27:19 -> 27:24 = (#0 + #10)
+// CHECK-NEXT: File 14, 27:26 -> 27:29 = #10
+// CHECK-NEXT: File 14, 27:31 -> 28:4 = #10
+// CHECK-NEXT: File 15, 31:3 -> 34:4 = #0
+// CHECK-NEXT: File 15, 32:10 -> 32:15 = (#0 + #11)
+// CHECK-NEXT: File 15, 32:17 -> 34:4 = #11