[clang] [Coverage] Don't mark code next to comments as skipped (PR #233137)
Fabian Meumertzheim via cfe-commits
cfe-commits at lists.llvm.org
Sat Oct 17 15:53:04 PDT 2026
https://github.com/fmeum updated https://github.com/llvm/llvm-project/pull/233137
>From 7b3b1f6ef0a5f739ab180cd2b93b75def2683de8 Mon Sep 17 00:00:00 2001
From: Fabian Meumertzheim <fabian at meumertzhe.im>
Date: Sat, 17 Oct 2026 23:00:03 +0200
Subject: [PATCH] [Coverage] Don't mark code next to comments as skipped
adjustSkippedRange missed two cases in which the skipped region of a
comment covered code on the same line:
* The adjacent token came from a macro expansion. Use the location of
the macro invocation instead.
* An empty line or #if 0 block merged with a following comment kept its
kind and thus wasn't adjusted. Adjust the end of all ranges and the
start only for comments.
The second case already affects used functions. Both cases matter for
unused functions with skipped regions.
Assisted-by: Claude Code, OpenAI Codex
---
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..a0e352e0ecd114 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++;
+ // Use the locations of the macro invocations for tokens from macro
+ // expansions.
+ 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++;
+ }
+ // Any range can end with a merged comment that is followed by code.
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 at 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
More information about the cfe-commits
mailing list