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 <[email protected]> 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@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
