[PATCH] D86116: [Coverage] Adjust skipped regions only if {Prev,Next}TokLoc is in the same file as regions' {start, end}Loc

[Zequan Wu via Phabricator via cfe-commits]

zequanwu updated this revision to Diff 286167.
zequanwu added a comment.

Update test.
The bug is when start or end location skipped regions has the same spelling line number as PrevTokLoc or NextTokLoc but in different files.
In the test case, end location of skipped regions is in line 6 and PreTokLoc is also in line 6, but since they are in different files, it shouldn't be shrinked.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86116/new/

https://reviews.llvm.org/D86116

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CoverageMapping/Inputs/comment.h
  clang/test/CoverageMapping/comment.cpp


Index: clang/test/CoverageMapping/comment.cpp
===================================================================
--- /dev/null
+++ clang/test/CoverageMapping/comment.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s | FileCheck %s
+
+int f() {
+  int x = 0;
+#include "Inputs/comment.h" /*
+    */
+  return x;
+}
+
+// CHECK: File 0, 3:9 -> 8:2 = #0
+// CHECK-NEXT: Expansion,File 0, 5:14 -> 5:32 = #0
+// CHECK-NEXT: Skipped,File 0, 6:1 -> 6:7 = 0
+// CHECK-NEXT: File 1, 1:1 -> 6:7 = #0
Index: clang/test/CoverageMapping/Inputs/comment.h
===================================================================
--- /dev/null
+++ clang/test/CoverageMapping/Inputs/comment.h
@@ -0,0 +1,6 @@
+
+
+
+
+
+x = 0;
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===================================================================
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -44,7 +44,8 @@
   PP.setTokenWatcher([CoverageInfo](clang::Token Tok) {
     // Update previous token location.
     CoverageInfo->PrevTokLoc = Tok.getLocation();
-    CoverageInfo->updateNextTokLoc(Tok.getLocation());
+    if (Tok.getKind() != clang::tok::eod)
+      CoverageInfo->updateNextTokLoc(Tok.getLocation());
   });
   return CoverageInfo;
 }
@@ -305,20 +306,24 @@
   /// non-comment token. If shrinking the skipped range would make it empty,
   /// this returns None.
   Optional<SpellingRegion> adjustSkippedRange(SourceManager &SM,
-                                              SpellingRegion SR,
+                                              SourceLocation LocStart,
+                                              SourceLocation LocEnd,
                                               SourceLocation PrevTokLoc,
                                               SourceLocation NextTokLoc) {
+    SpellingRegion SR{SM, LocStart, LocEnd};
     // If Range begin location is invalid, it's not a comment region.
     if (PrevTokLoc.isInvalid())
       return SR;
     unsigned PrevTokLine = SM.getSpellingLineNumber(PrevTokLoc);
     unsigned NextTokLine = SM.getSpellingLineNumber(NextTokLoc);
     SpellingRegion newSR(SR);
-    if (SR.LineStart == PrevTokLine) {
+    if (SM.isWrittenInSameFile(LocStart, PrevTokLoc) &&
+        SR.LineStart == PrevTokLine) {
       newSR.LineStart = SR.LineStart + 1;
       newSR.ColumnStart = 1;
     }
-    if (SR.LineEnd == NextTokLine) {
+    if (SM.isWrittenInSameFile(LocEnd, NextTokLoc) &&
+        SR.LineEnd == NextTokLine) {
       newSR.LineEnd = SR.LineEnd - 1;
       newSR.ColumnEnd = SR.ColumnStart + 1;
     }
@@ -354,14 +359,13 @@
       auto CovFileID = getCoverageFileID(LocStart);
       if (!CovFileID)
         continue;
-      SpellingRegion SR{SM, LocStart, LocEnd};
-      if (Optional<SpellingRegion> res =
-              adjustSkippedRange(SM, SR, I.PrevTokLoc, I.NextTokLoc))
-        SR = res.getValue();
-      else
+      Optional<SpellingRegion> SR =
+          adjustSkippedRange(SM, LocStart, LocEnd, I.PrevTokLoc, I.NextTokLoc);
+      if (!SR.hasValue())
         continue;
       auto Region = CounterMappingRegion::makeSkipped(
-          *CovFileID, SR.LineStart, SR.ColumnStart, SR.LineEnd, SR.ColumnEnd);
+          *CovFileID, SR->LineStart, SR->ColumnStart, SR->LineEnd,
+          SR->ColumnEnd);
       // Make sure that we only collect the regions that are inside
       // the source code of this function.
       if (Region.LineStart >= FileLineRanges[*CovFileID].first &&


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D86116.286167.patch
Type: text/x-patch
Size: 3563 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200817/e033a197/attachment.bin>