https://github.com/fmeum created
https://github.com/llvm/llvm-project/pull/233137
`adjustSkippedRange` shrinks the skipped region of a comment so that it doesn't
cover lines with code before or after the comment, but missed two cases:
* If the token before or after the comment came from a macro expansion, it
wasn't considered to be in the same file as the comment, so the region still
covered the line of the macro invocation. Use the location of the macro
invocation instead.
* Consecutive skipped ranges without a token in between are merged and keep the
kind of the first range. If an empty line or a block skipped by the
preprocessor was followed by a comment and code on the same line, the merged
range wasn't adjusted at all and thus also covered the code. Since only the end
of such a range can be followed by code on the same line, adjust the end of all
ranges, but keep adjusting the start only for comments.
In used functions, the expansion region of a macro invocation keeps its line
mapped, so only the second case affects the line coverage reported by llvm-cov
today. For example, `llvm-cov show` currently reports the line with `return 0;`
as not containing code:
```cpp
int g() {
/* Merged with the empty line above. */ return 0;
}
```
Both cases matter for unused functions once their mappings include skipped
regions (#232831).
The `-dump-coverage-mapping` output for seven large LLVM source files
(including `SemaExpr.cpp`, `DAGCombiner.cpp` and `X86ISelLowering.cpp`) is
unchanged.
https://claude.ai/code/session_01SMjpGgdj23E2Nz1dKmfcyp
>From 59a6a63c17a2157b8216f6523863b58b7c310ace Mon Sep 17 00:00:00 2001
From: Fabian Meumertzheim <[email protected]>
Date: Sat, 17 Oct 2026 23:00:03 +0200
Subject: [PATCH] [Coverage] Don't mark code next to comments as skipped
adjustSkippedRange shrinks the skipped region of a comment so that it
doesn't cover lines with code before or after the comment, but missed
two cases:
* If the token before or after the comment came from a macro expansion,
it wasn't considered to be in the same file as the comment, so the
region still covered the line of the macro invocation. Use the
location of the macro invocation instead.
* Consecutive skipped ranges without a token in between are merged and
keep the kind of the first range. If an empty line or a block skipped
by the preprocessor was followed by a comment and code on the same
line, the merged range wasn't adjusted at all and thus also covered
the code. Since only the end of such a range can be followed by code
on the same line, adjust the end of all ranges, but keep adjusting the
start only for comments.
In used functions, the expansion region of a macro invocation keeps its
line mapped, so only the second case affects the line coverage reported
by llvm-cov. Both cases matter for unused functions once their mappings
include skipped regions.
Claude-Session: https://claude.ai/code/session_01SMjpGgdj23E2Nz1dKmfcyp
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 42 +++++++++++++-----------
clang/lib/CodeGen/CoverageMappingGen.h | 6 ++--
clang/test/CoverageMapping/comment.cpp | 28 ++++++++++++++++
3 files changed, 53 insertions(+), 23 deletions(-)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 970812f7b0f754..b4055a61935cb9 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -444,15 +444,26 @@ class CoverageMappingBuilder {
/// Note this function can potentially be expensive because
/// getSpellingLineNumber uses getLineNumber, which is expensive.
std::optional<SpellingRegion> adjustSkippedRange(SourceManager &SM,
- SourceLocation LocStart,
- SourceLocation LocEnd,
- SourceLocation PrevTokLoc,
- SourceLocation NextTokLoc) {
+ const SkippedRange &R) {
+ SourceLocation LocStart = R.Range.getBegin();
+ SourceLocation LocEnd = R.Range.getEnd();
SpellingRegion SR{SM, LocStart, LocEnd};
- SR.ColumnStart = 1;
- if (PrevTokLoc.isValid() && SM.isWrittenInSameFile(LocStart, PrevTokLoc) &&
- SR.LineStart == SM.getSpellingLineNumber(PrevTokLoc))
- SR.LineStart++;
+ // The surrounding tokens may come from macro expansions, in which case the
+ // lines of the macro invocations matter.
+ SourceLocation PrevTokLoc, NextTokLoc;
+ if (R.PrevTokLoc.isValid())
+ PrevTokLoc = SM.getExpansionRange(R.PrevTokLoc).getEnd();
+ if (R.NextTokLoc.isValid())
+ NextTokLoc = SM.getExpansionLoc(R.NextTokLoc);
+ // Only comments can be preceded by code on the same line.
+ if (R.isComment()) {
+ SR.ColumnStart = 1;
+ if (PrevTokLoc.isValid() &&
+ SM.isWrittenInSameFile(LocStart, PrevTokLoc) &&
+ SR.LineStart == SM.getSpellingLineNumber(PrevTokLoc))
+ SR.LineStart++;
+ }
+ // Other ranges may end with a comment that was merged into them.
if (NextTokLoc.isValid() && SM.isWrittenInSameFile(LocEnd, NextTokLoc) &&
SR.LineEnd == SM.getSpellingLineNumber(NextTokLoc)) {
SR.LineEnd--;
@@ -481,22 +492,13 @@ class CoverageMappingBuilder {
auto SkippedRanges = CVM.getSourceInfo().getSkippedRanges();
for (auto &I : SkippedRanges) {
- SourceRange Range = I.Range;
- auto LocStart = Range.getBegin();
- auto LocEnd = Range.getEnd();
- assert(SM.isWrittenInSameFile(LocStart, LocEnd) &&
+ assert(SM.isWrittenInSameFile(I.Range.getBegin(), I.Range.getEnd()) &&
"region spans multiple files");
- auto CovFileID = getCoverageFileID(LocStart);
+ auto CovFileID = getCoverageFileID(I.Range.getBegin());
if (!CovFileID)
continue;
- std::optional<SpellingRegion> SR;
- if (I.isComment())
- SR = adjustSkippedRange(SM, LocStart, LocEnd, I.PrevTokLoc,
- I.NextTokLoc);
- else if (I.isPPIfElse() || I.isEmptyLine())
- SR = {SM, LocStart, LocEnd};
-
+ std::optional<SpellingRegion> SR = adjustSkippedRange(SM, I);
if (!SR)
continue;
auto Region = CounterMappingRegion::makeSkipped(
diff --git a/clang/lib/CodeGen/CoverageMappingGen.h
b/clang/lib/CodeGen/CoverageMappingGen.h
index 0ed50597e1dc3e..7d5f7cf741e69d 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.h
+++ b/clang/lib/CodeGen/CoverageMappingGen.h
@@ -50,9 +50,9 @@ struct SkippedRange {
// The nature of this skipped range
Kind RangeKind;
- bool isComment() { return RangeKind == Comment; }
- bool isEmptyLine() { return RangeKind == EmptyLine; }
- bool isPPIfElse() { return RangeKind == PPIfElse; }
+ bool isComment() const { return RangeKind == Comment; }
+ bool isEmptyLine() const { return RangeKind == EmptyLine; }
+ bool isPPIfElse() const { return RangeKind == PPIfElse; }
SkippedRange(SourceRange Range, Kind K,
SourceLocation PrevTokLoc = SourceLocation(),
diff --git a/clang/test/CoverageMapping/comment.cpp
b/clang/test/CoverageMapping/comment.cpp
index f8e4b4912e182f..9356f1f7c1f9c4 100644
--- a/clang/test/CoverageMapping/comment.cpp
+++ b/clang/test/CoverageMapping/comment.cpp
@@ -11,3 +11,31 @@ int f() {
// CHECK-NEXT: Expansion,File 0, 5:10 -> 5:28 = #0
// CHECK-NEXT: Skipped,File 0, 6:1 -> 6:7 = 0
// CHECK-NEXT: File 1, 1:1 -> 7:1 = #0
+
+int g() {
+
+ /* Merged with the empty line above. */ return 0;
+}
+
+// CHECK-LABEL: {{_Z1gv|\?g@@YAHXZ}}:
+// CHECK-NEXT: File 0, [[@LINE-6]]:9 -> [[@LINE-3]]:2 = #0
+// CHECK-NEXT: Skipped,File 0, [[@LINE-6]]:1 -> [[@LINE-6]]:{{[0-9]+}} = 0
+
+#define RET(x) return x;
+#define RET0 return 0;
+
+int h(int x) {
+ RET(x) // Follows a macro.
+}
+
+// CHECK-LABEL: {{_Z1hi|\?h@@YAHH@Z}}:
+// CHECK-NEXT: File 0, [[@LINE-5]]:14 -> [[@LINE-3]]:2 = #0
+// CHECK-NEXT: Expansion,File 0, [[@LINE-5]]:3 -> [[@LINE-5]]:6 = #0
+
+int i() {
+ /* Precedes a macro. */ RET0
+}
+
+// CHECK-LABEL: {{_Z1iv|\?i@@YAHXZ}}:
+// CHECK-NEXT: File 0, [[@LINE-5]]:9 -> [[@LINE-3]]:2 = #0
+// CHECK-NEXT: Expansion,File 0, [[@LINE-5]]:27 -> [[@LINE-5]]:31 = #0
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
