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 &&
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 &&
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
