llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-codegen
Author: Fabian Meumertzheim (fmeum)
<details>
<summary>Changes</summary>
`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, e.g. llvm-cov reports no code
on the `return 0;` line in:
```cpp
int g() {
/* Merged with the empty line above. */ return 0;
}
```
Both cases matter for unused functions with skipped regions (#<!-- -->232831).
Assisted-by: Claude Code, OpenAI Codex
---
Full diff: https://github.com/llvm/llvm-project/pull/233137.diff
3 Files Affected:
- (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+22-20)
- (modified) clang/lib/CodeGen/CoverageMappingGen.h (+3-3)
- (modified) clang/test/CoverageMapping/comment.cpp (+28)
``````````diff
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 970812f7b0f75..a0e352e0ecd11 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 0ed50597e1dc3..7d5f7cf741e69 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 f8e4b4912e182..9356f1f7c1f9c 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
``````````
</details>
https://github.com/llvm/llvm-project/pull/233137
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
