[llvm] [clang] [coverage] skipping code coverage for 'if constexpr' and 'if consteval' [WIP] (PR #78033)
Hana Dusíková via cfe-commits
cfe-commits at lists.llvm.org
Sun Jan 21 15:00:18 PST 2024
https://github.com/hanickadot updated https://github.com/llvm/llvm-project/pull/78033
>From 587228bf8c59813f70b9cfbc42a88076027e1422 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hana=20Dusi=CC=81kova=CC=81?= <hanicka at hanicka.net>
Date: Sun, 21 Jan 2024 23:13:55 +0100
Subject: [PATCH 1/3] [coverage] skipping code coverage for 'if constexpr' and
'if consteval'
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 167 ++++++++++++++----
.../CoverageMapping/branch-constfolded.cpp | 8 +-
clang/test/CoverageMapping/if.cpp | 119 +++++++++----
.../ProfileData/Coverage/CoverageMapping.cpp | 14 +-
.../ProfileData/CoverageMappingTest.cpp | 24 ++-
5 files changed, 253 insertions(+), 79 deletions(-)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 4a44d113ddec9e8..ff5bc6941cc5c85 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -119,12 +119,16 @@ class SourceMappingRegion {
/// as the line execution count if there are no other regions on the line.
bool GapRegion;
+ /// Whetever this region is skipped ('if constexpr' or 'if consteval' untaken
+ /// branch, or anything skipped but not empty line / comments)
+ bool SkippedRegion;
+
public:
SourceMappingRegion(Counter Count, std::optional<SourceLocation> LocStart,
std::optional<SourceLocation> LocEnd,
bool GapRegion = false)
- : Count(Count), LocStart(LocStart), LocEnd(LocEnd), GapRegion(GapRegion) {
- }
+ : Count(Count), LocStart(LocStart), LocEnd(LocEnd), GapRegion(GapRegion),
+ SkippedRegion(false) {}
SourceMappingRegion(Counter Count, std::optional<Counter> FalseCount,
MCDCParameters MCDCParams,
@@ -132,13 +136,14 @@ class SourceMappingRegion {
std::optional<SourceLocation> LocEnd,
bool GapRegion = false)
: Count(Count), FalseCount(FalseCount), MCDCParams(MCDCParams),
- LocStart(LocStart), LocEnd(LocEnd), GapRegion(GapRegion) {}
+ LocStart(LocStart), LocEnd(LocEnd), GapRegion(GapRegion),
+ SkippedRegion(false) {}
SourceMappingRegion(MCDCParameters MCDCParams,
std::optional<SourceLocation> LocStart,
std::optional<SourceLocation> LocEnd)
: MCDCParams(MCDCParams), LocStart(LocStart), LocEnd(LocEnd),
- GapRegion(false) {}
+ GapRegion(false), SkippedRegion(false) {}
const Counter &getCounter() const { return Count; }
@@ -174,6 +179,10 @@ class SourceMappingRegion {
void setGap(bool Gap) { GapRegion = Gap; }
+ bool isSkipped() const { return SkippedRegion; }
+
+ void setSkipped(bool Skipped) { SkippedRegion = Skipped; }
+
bool isBranch() const { return FalseCount.has_value(); }
bool isMCDCDecision() const { return MCDCParams.NumConditions != 0; }
@@ -468,6 +477,10 @@ class CoverageMappingBuilder {
MappingRegions.push_back(CounterMappingRegion::makeGapRegion(
Region.getCounter(), *CovFileID, SR.LineStart, SR.ColumnStart,
SR.LineEnd, SR.ColumnEnd));
+ } else if (Region.isSkipped()) {
+ MappingRegions.push_back(CounterMappingRegion::makeSkipped(
+ *CovFileID, SR.LineStart, SR.ColumnStart, SR.LineEnd,
+ SR.ColumnEnd));
} else if (Region.isBranch()) {
MappingRegions.push_back(CounterMappingRegion::makeBranchRegion(
Region.getCounter(), Region.getFalseCounter(),
@@ -1251,6 +1264,23 @@ struct CounterCoverageMappingBuilder
popRegions(Index);
}
+ /// Emit a skip region between \p StartLoc and \p EndLoc with the given count.
+ void markSkipArea(SourceLocation StartLoc, SourceLocation EndLoc) {
+ if (StartLoc == EndLoc)
+ return;
+ assert(SpellingRegion(SM, StartLoc, EndLoc).isInSourceOrder());
+ handleFileExit(StartLoc);
+ size_t Index = pushRegion({}, StartLoc, EndLoc);
+ getRegion().setSkipped(true);
+ handleFileExit(EndLoc);
+ popRegions(Index);
+ }
+
+ void findGapAreaBetweenAndMarkSkipArea(SourceLocation AfterLoc,
+ SourceLocation BeforeLoc) {
+ markSkipArea(AfterLoc, BeforeLoc);
+ }
+
/// Keep counts of breaks and continues inside loops.
struct BreakContinue {
Counter BreakCount;
@@ -1700,43 +1730,118 @@ struct CounterCoverageMappingBuilder
Visit(S->getSubStmt());
}
+ void CoverIfConsteval(const IfStmt *S) {
+ assert(S->isConsteval());
+
+ const auto *Then = S->getThen();
+ const auto *Else = S->getElse();
+
+ // I'm using 'propagateCounts' later as new region is better and allows me
+ // to properly calculate line coverage in llvm-cov utility
+ const Counter ParentCount = getRegion().getCounter();
+
+ extendRegion(S);
+
+ if (S->isNegatedConsteval()) {
+ // ignore 'if consteval'
+ findGapAreaBetweenAndMarkSkipArea(S->getIfLoc(), getStart(Then));
+ propagateCounts(ParentCount, Then);
+
+ if (Else) {
+ // ignore 'else <else>'
+ findGapAreaBetweenAndMarkSkipArea(getEnd(Then), getEnd(Else));
+ }
+ } else {
+ assert(S->isNonNegatedConsteval());
+ // ignore 'if consteval <then> [else]'
+ findGapAreaBetweenAndMarkSkipArea(S->getIfLoc(),
+ Else ? getStart(Else) : getEnd(Then));
+
+ if (Else)
+ propagateCounts(ParentCount, Else);
+ }
+ }
+
+ void CoverIfConstexpr(const IfStmt *S) {
+ assert(S->isConstexpr());
+
+ // evaluate constant condition...
+ const auto *E = dyn_cast<ConstantExpr>(S->getCond());
+ assert(E != nullptr);
+ const bool isTrue = E->getResultAsAPSInt().getExtValue();
+
+ extendRegion(S);
+
+ const auto *Init = S->getInit();
+ const auto *Then = S->getThen();
+ const auto *Else = S->getElse();
+
+ // I'm using 'propagateCounts' later as new region is better and allows me
+ // to properly calculate line coverage in llvm-cov utility
+ const Counter ParentCount = getRegion().getCounter();
+
+ // ignore 'if constexpr ('
+ SourceLocation startOfSkipped = S->getIfLoc();
+
+ if (Init) {
+ // don't mark initialisation as ignored
+ findGapAreaBetweenAndMarkSkipArea(startOfSkipped, getStart(Init));
+ propagateCounts(ParentCount, Init);
+ // ignore after initialisation: '; <condition>)'...
+ startOfSkipped = getEnd(Init);
+ }
+
+ if (isTrue) {
+ // ignore '<condition>)'
+ findGapAreaBetweenAndMarkSkipArea(startOfSkipped, getStart(Then));
+ propagateCounts(ParentCount, Then);
+
+ if (Else)
+ // ignore 'else <else>'
+ findGapAreaBetweenAndMarkSkipArea(getEnd(Then), getEnd(Else));
+ } else {
+ // ignore '<condition>) <then> [else]'
+ findGapAreaBetweenAndMarkSkipArea(startOfSkipped,
+ Else ? getStart(Else) : getEnd(Then));
+
+ if (Else) {
+ propagateCounts(ParentCount, Else);
+ }
+ }
+ }
+
void VisitIfStmt(const IfStmt *S) {
+ // "if constexpr" and "if consteval" are not normal conditional statements,
+ // they should behave more like a preprocessor conditions
+ if (S->isConsteval())
+ return CoverIfConsteval(S);
+ else if (S->isConstexpr())
+ return CoverIfConstexpr(S);
+
extendRegion(S);
if (S->getInit())
Visit(S->getInit());
// Extend into the condition before we propagate through it below - this is
// needed to handle macros that generate the "if" but not the condition.
- if (!S->isConsteval())
- extendRegion(S->getCond());
+ extendRegion(S->getCond());
Counter ParentCount = getRegion().getCounter();
+ Counter ThenCount = getRegionCounter(S);
- // If this is "if !consteval" the then-branch will never be taken, we don't
- // need to change counter
- Counter ThenCount =
- S->isNegatedConsteval() ? ParentCount : getRegionCounter(S);
+ // Emitting a counter for the condition makes it easier to interpret the
+ // counter for the body when looking at the coverage.
+ propagateCounts(ParentCount, S->getCond());
- if (!S->isConsteval()) {
- // Emitting a counter for the condition makes it easier to interpret the
- // counter for the body when looking at the coverage.
- propagateCounts(ParentCount, S->getCond());
-
- // The 'then' count applies to the area immediately after the condition.
- std::optional<SourceRange> Gap =
- findGapAreaBetween(S->getRParenLoc(), getStart(S->getThen()));
- if (Gap)
- fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
- }
+ // The 'then' count applies to the area immediately after the condition.
+ std::optional<SourceRange> Gap =
+ findGapAreaBetween(S->getRParenLoc(), getStart(S->getThen()));
+ if (Gap)
+ fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
extendRegion(S->getThen());
Counter OutCount = propagateCounts(ThenCount, S->getThen());
-
- // If this is "if consteval" the else-branch will never be taken, we don't
- // need to change counter
- Counter ElseCount = S->isNonNegatedConsteval()
- ? ParentCount
- : subtractCounters(ParentCount, ThenCount);
+ Counter ElseCount = subtractCounters(ParentCount, ThenCount);
if (const Stmt *Else = S->getElse()) {
bool ThenHasTerminateStmt = HasTerminateStmt;
@@ -1759,11 +1864,9 @@ struct CounterCoverageMappingBuilder
GapRegionCounter = OutCount;
}
- if (!S->isConsteval()) {
- // Create Branch Region around condition.
- createBranchRegion(S->getCond(), ThenCount,
- subtractCounters(ParentCount, ThenCount));
- }
+ // Create Branch Region around condition.
+ createBranchRegion(S->getCond(), ThenCount,
+ subtractCounters(ParentCount, ThenCount));
}
void VisitCXXTryStmt(const CXXTryStmt *S) {
diff --git a/clang/test/CoverageMapping/branch-constfolded.cpp b/clang/test/CoverageMapping/branch-constfolded.cpp
index 4fdb640506c9d31..c8755d5d752b638 100644
--- a/clang/test/CoverageMapping/branch-constfolded.cpp
+++ b/clang/test/CoverageMapping/branch-constfolded.cpp
@@ -90,10 +90,10 @@ bool for_7(bool a) { // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1
} // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:19 = 0, 0
// CHECK-LABEL: _Z5for_8b:
-bool for_8(bool a) { // MCDC: Decision,File 0, [[@LINE+3]]:17 -> [[@LINE+3]]:30 = M:0, C:2
- // CHECK: Branch,File 0, [[@LINE+2]]:17 -> [[@LINE+2]]:21 = 0, 0
- // CHECK: Branch,File 0, [[@LINE+1]]:25 -> [[@LINE+1]]:30 = 0, 0
- if constexpr (true && false)
+bool for_8(bool a) { // MCDC: Decision,File 0, [[@LINE+3]]:7 -> [[@LINE+3]]:20 = M:0, C:2
+ // CHECK: Branch,File 0, [[@LINE+2]]:7 -> [[@LINE+2]]:11 = 0, 0
+ // CHECK: Branch,File 0, [[@LINE+1]]:15 -> [[@LINE+1]]:20 = 0, 0
+ if (true && false)
return true;
else
return false;
diff --git a/clang/test/CoverageMapping/if.cpp b/clang/test/CoverageMapping/if.cpp
index 92d560be01f3b7f..72240a729e82fb4 100644
--- a/clang/test/CoverageMapping/if.cpp
+++ b/clang/test/CoverageMapping/if.cpp
@@ -23,47 +23,90 @@ void foo() { // CHECK-NEXT: Gap,File 0, [[@LINE+1]]:21 -> [[@
} // CHECK-NEXT: [[@LINE-2]]:9 -> [[@LINE-1]]:5 = #1
// CHECK-NEXT: [[@LINE-2]]:5 -> [[@LINE-2]]:8 = #1
-// FIXME: Do not generate coverage for discarded branches in if constexpr
+// GH-57377
// CHECK-LABEL: _Z30check_constexpr_true_with_elsei:
int check_constexpr_true_with_else(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
- // CHECK-NEXT: [[@LINE+2]]:16 -> [[@LINE+2]]:20 = #0
- // CHECK-NEXT: Branch,File 0, [[@LINE+1]]:16 -> [[@LINE+1]]:20 = 0, 0
- if constexpr(true) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:21 -> [[@LINE]]:22 = #1
- i *= 3; // CHECK-NEXT: [[@LINE-1]]:22 -> [[@LINE+1]]:4 = #1
- } else { // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 -> [[@LINE]]:10 = (#0 - #1)
- i *= 5; // CHECK-NEXT: [[@LINE-1]]:10 -> [[@LINE+1]]:4 = (#0 - #1)
+ if constexpr(true) { // CHECK-NEXT: Skipped,File 0, [[@LINE]]:3 -> [[@LINE]]:22 = 0
+ i *= 3; // CHECK-NEXT: File 0, [[@LINE-1]]:22 -> [[@LINE+1]]:4 = #0
+ } else { // CHECK-NEXT: Skipped,File 0, [[@LINE]]:4 -> [[@LINE+2]]:4 = 0
+ i *= 5;
}
return i;
}
+// GH-57377
// CHECK-LABEL: _Z33check_constexpr_true_without_elsei:
int check_constexpr_true_without_else(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
- // CHECK-NEXT: [[@LINE+2]]:16 -> [[@LINE+2]]:20 = #0
- // CHECK-NEXT: Branch,File 0, [[@LINE+1]]:16 -> [[@LINE+1]]:20 = 0, 0
- if constexpr(true) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:21 -> [[@LINE]]:22 = #1
- i *= 3; // CHECK-NEXT: [[@LINE-1]]:22 -> [[@LINE+1]]:4 = #1
+ if constexpr(true) { // CHECK-NEXT: Skipped,File 0, [[@LINE]]:3 -> [[@LINE]]:22 = 0
+ i *= 3; // CHECK-NEXT: File 0, [[@LINE-1]]:22 -> [[@LINE+1]]:4 = #0
}
return i;
}
+// GH-57377
// CHECK-LABEL: _Z31check_constexpr_false_with_elsei:
int check_constexpr_false_with_else(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
- // CHECK-NEXT: [[@LINE+2]]:16 -> [[@LINE+2]]:21 = #0
- // CHECK-NEXT: Branch,File 0, [[@LINE+1]]:16 -> [[@LINE+1]]:21 = 0, 0
- if constexpr(false) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:22 -> [[@LINE]]:23 = #1
- i *= 3; // CHECK-NEXT: File 0, [[@LINE-1]]:23 -> [[@LINE+1]]:4 = #1
- } else { // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 -> [[@LINE]]:10 = (#0 - #1)
- i *= 5; // CHECK-NEXT: File 0, [[@LINE-1]]:10 -> [[@LINE+1]]:4 = (#0 - #1)
+ if constexpr(false) { // CHECK-NEXT: Skipped,File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = 0
+ i *= 3;
+ } else { // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+2]]:4 = #0
+ i *= 5;
}
return i;
}
+// GH-57377
// CHECK-LABEL: _Z34check_constexpr_false_without_elsei:
int check_constexpr_false_without_else(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
- // CHECK-NEXT: [[@LINE+2]]:16 -> [[@LINE+2]]:21 = #0
- // CHECK-NEXT: Branch,File 0, [[@LINE+1]]:16 -> [[@LINE+1]]:21 = 0, 0
- if constexpr(false) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:22 -> [[@LINE]]:23 = #1
- i *= 3; // CHECK-NEXT: File 0, [[@LINE-1]]:23 -> [[@LINE+1]]:4 = #1
+ if constexpr(false) { // CHECK-NEXT: Skipped,File 0, [[@LINE]]:3 -> [[@LINE+2]]:4 = 0
+ i *= 3;
+ }
+ return i;
+}
+
+// GH-57377
+// CHECK-LABEL: _Z35check_constexpr_init_true_with_elsei:
+int check_constexpr_init_true_with_else(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
+ if constexpr(int j = i; true) { // CHECK-NEXT: Skipped,File 0, [[@LINE]]:3 -> [[@LINE]]:16 = 0
+ // CHECK-NEXT: File 0, [[@LINE-1]]:16 -> [[@LINE-1]]:26 = #0
+ // CHECK-NEXT: Skipped,File 0, [[@LINE-2]]:26 -> [[@LINE-2]]:33 = 0
+ // CHECK-NEXT: File 0, [[@LINE-3]]:33 -> [[@LINE+2]]:4 = #0
+ i *= j;
+ } else { // CHECK-NEXT: Skipped,File 0, [[@LINE]]:4 -> [[@LINE+2]]:4 = 0
+ i *= j;
+ }
+ return i;
+}
+
+// GH-57377
+// CHECK-LABEL: _Z38check_constexpr_init_true_without_elsei:
+int check_constexpr_init_true_without_else(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
+ if constexpr(int j = i; true) { // CHECK-NEXT: Skipped,File 0, [[@LINE]]:3 -> [[@LINE]]:16 = 0
+ // CHECK-NEXT: File 0, [[@LINE-1]]:16 -> [[@LINE-1]]:26 = #0
+ // CHECK-NEXT: Skipped,File 0, [[@LINE-2]]:26 -> [[@LINE-2]]:33 = 0
+ // CHECK-NEXT: File 0, [[@LINE-3]]:33 -> [[@LINE+2]]:4 = #0
+ i *= j;
+ }
+ return i;
+}
+
+// GH-57377
+// CHECK-LABEL: _Z36check_constexpr_init_false_with_elsei:
+int check_constexpr_init_false_with_else(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
+ if constexpr(int j = i; false) { // CHECK-NEXT: Skipped,File 0, [[@LINE]]:3 -> [[@LINE]]:16 = 0
+ // CHECK-NEXT: File 0, [[@LINE-1]]:16 -> [[@LINE-1]]:26 = #0
+ i *= j; // CHECK-NEXT: Skipped,File 0, [[@LINE-2]]:26 -> [[@LINE+1]]:10 = 0
+ } else { // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+2]]:4 = #0
+ i *= j;
+ }
+ return i;
+}
+
+// GH-57377
+// CHECK-LABEL: _Z39check_constexpr_init_false_without_elsei:
+int check_constexpr_init_false_without_else(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
+ if constexpr(int j = i; false) { // CHECK-NEXT: Skipped,File 0, [[@LINE]]:3 -> [[@LINE]]:16 = 0
+ // CHECK-NEXT: File 0, [[@LINE-1]]:16 -> [[@LINE-1]]:26 = #0
+ i *= j; // CHECK-NEXT: Skipped,File 0, [[@LINE-2]]:26 -> [[@LINE+1]]:4 = 0
}
return i;
}
@@ -127,40 +170,40 @@ void ternary() {
// GH-57377
// CHECK-LABEL: _Z40check_consteval_with_else_discarded_theni:
-// FIXME: Do not generate coverage for discarded <then> branch in if consteval
constexpr int check_consteval_with_else_discarded_then(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
- if consteval {
- i *= 3; // CHECK-NEXT: [[@LINE-1]]:16 -> [[@LINE+1]]:4 = #1
- } else { // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 -> [[@LINE]]:10 = #0
- i *= 5; // CHECK-NEXT: [[@LINE-1]]:10 -> [[@LINE+1]]:4 = #0
+ if consteval { // CHECK-NEXT: Skipped,File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = 0
+ i *= 3;
+ } else { // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+2]]:4 = #0
+ i *= 5;
}
- return i; // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE]]:11 = (#0 + #1)
+ return i;
}
+// GH-57377
// CHECK-LABEL: _Z43check_notconsteval_with_else_discarded_elsei:
-// FIXME: Do not generate coverage for discarded <else> branch in if consteval
constexpr int check_notconsteval_with_else_discarded_else(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
- if !consteval {
- i *= 3; // CHECK-NEXT: [[@LINE-1]]:17 -> [[@LINE+1]]:4 = #0
- } else { // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 -> [[@LINE]]:10 = 0
- i *= 5; // CHECK-NEXT: [[@LINE-1]]:10 -> [[@LINE+1]]:4 = 0
+ if !consteval { // CHECK-NEXT: Skipped,File 0, [[@LINE]]:3 -> [[@LINE]]:17 = 0
+ i *= 3;
+ } else { // CHECK-NEXT: File 0, [[@LINE-2]]:17 -> [[@LINE]]:4 = #0
+ i *= 5; // CHECK-NEXT: Skipped,File 0, [[@LINE-1]]:4 -> [[@LINE+1]]:4 = 0
}
return i;
}
+// GH-57377
// CHECK-LABEL: _Z32check_consteval_branch_discardedi:
-// FIXME: Do not generate coverage for discarded <then> branch in if consteval
constexpr int check_consteval_branch_discarded(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
- if consteval {
- i *= 3; // CHECK-NEXT: [[@LINE-1]]:16 -> [[@LINE+1]]:4 = #1
+ if consteval { // CHECK-NEXT: Skipped,File 0, [[@LINE]]:3 -> [[@LINE+2]]:4 = 0
+ i *= 3;
}
- return i; // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE]]:11 = (#0 + #1)
+ return i;
}
+// GH-57377
// CHECK-LABEL: _Z30check_notconsteval_branch_kepti:
constexpr int check_notconsteval_branch_kept(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
- if !consteval {
- i *= 3; // CHECK-NEXT: [[@LINE-1]]:17 -> [[@LINE+1]]:4 = #0
+ if !consteval { // CHECK-NEXT: Skipped,File 0, [[@LINE]]:3 -> [[@LINE]]:17 = 0
+ i *= 3; // CHECK-NEXT: File 0, [[@LINE-1]]:17 -> [[@LINE+1]]:4 = #0
}
return i;
}
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index eece6a2cc717977..2d7fcf2eac2d39c 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -1319,8 +1319,20 @@ LineCoverageStats::LineCoverageStats(
!StartOfSkippedRegion &&
((WrappedSegment && WrappedSegment->HasCount) || (MinRegionCount > 0));
- if (!Mapped)
+ // if there is any starting segment at this line with a counter, it must be
+ // mapped
+ Mapped |= std::any_of(
+ LineSegments.begin(), LineSegments.end(),
+ [](const auto *Seq) { return Seq->IsRegionEntry && Seq->HasCount; });
+
+ // make sure last line of a block is covered if at that line there is also
+ // skipped region
+ Mapped |= WrappedSegment && WrappedSegment->IsRegionEntry &&
+ WrappedSegment->HasCount;
+
+ if (!Mapped) {
return;
+ }
// Pick the max count from the non-gap, region entry segments and the
// wrapped count.
diff --git a/llvm/unittests/ProfileData/CoverageMappingTest.cpp b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
index 1cf497cbdc2e61f..23f66a0232ddbb8 100644
--- a/llvm/unittests/ProfileData/CoverageMappingTest.cpp
+++ b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
@@ -187,6 +187,11 @@ struct CoverageMappingTest : ::testing::TestWithParam<std::tuple<bool, bool>> {
: CounterMappingRegion::makeRegion(C, FileID, LS, CS, LE, CE));
}
+ void addSkipped(StringRef File, unsigned LS, unsigned CS, unsigned LE,
+ unsigned CE) {
+ addCMR(Counter::getZero(), File, LS, CS, LE, CE, true);
+ }
+
void addMCDCDecisionCMR(unsigned Mask, unsigned NC, StringRef File,
unsigned LS, unsigned CS, unsigned LE, unsigned CE) {
auto &Regions = InputFunctions.back().Regions;
@@ -700,22 +705,33 @@ TEST_P(CoverageMappingTest, test_line_coverage_iterator) {
startFunction("func", 0x1234);
addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
+ addSkipped("file1", 1, 3, 1, 8); // skipped region inside previous block
addCMR(Counter::getCounter(1), "file1", 1, 1, 4, 7);
+ addSkipped("file1", 4, 1, 4, 20); // skipped line
addCMR(Counter::getCounter(2), "file1", 5, 8, 9, 1);
+ addSkipped("file1", 10, 1, 12,
+ 20); // skipped region which contains next region
addCMR(Counter::getCounter(3), "file1", 10, 10, 11, 11);
EXPECT_THAT_ERROR(loadCoverageMapping(), Succeeded());
-
CoverageData Data = LoadedCoverage->getCoverageForFile("file1");
unsigned Line = 0;
- unsigned LineCounts[] = {20, 20, 20, 20, 30, 10, 10, 10, 10, 0, 0};
+ const unsigned LineCounts[] = {20, 20, 20, 0, 30, 10, 10, 10, 10, 0, 0, 0, 0};
+ const bool MappedLines[] = {1, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 0, 0};
+ ASSERT_EQ(std::size(LineCounts), std::size(MappedLines));
+
for (const auto &LCS : getLineCoverageStats(Data)) {
+ ASSERT_LT(Line, std::size(LineCounts));
+ ASSERT_LT(Line, std::size(MappedLines));
+
ASSERT_EQ(Line + 1, LCS.getLine());
- errs() << "Line: " << Line + 1 << ", count = " << LCS.getExecutionCount() << "\n";
+ errs() << "Line: " << Line + 1 << ", count = " << LCS.getExecutionCount()
+ << ", mapped = " << LCS.isMapped() << "\n";
ASSERT_EQ(LineCounts[Line], LCS.getExecutionCount());
+ ASSERT_EQ(MappedLines[Line], LCS.isMapped());
++Line;
}
- ASSERT_EQ(11U, Line);
+ ASSERT_EQ(12U, Line);
// Check that operator->() works / compiles.
ASSERT_EQ(1U, LineCoverageIterator(Data)->getLine());
>From 0a496937777437553183f104e96b61d6c34cbff0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hana=20Dusi=CC=81kova=CC=81?= <hanicka at hanicka.net>
Date: Sun, 21 Jan 2024 23:48:35 +0100
Subject: [PATCH 2/3] Handle macros properly.
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 80 ++++++++++++++++++------
1 file changed, 61 insertions(+), 19 deletions(-)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index ff5bc6941cc5c85..cda3a0d5db42801 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1264,23 +1264,66 @@ struct CounterCoverageMappingBuilder
popRegions(Index);
}
- /// Emit a skip region between \p StartLoc and \p EndLoc with the given count.
- void markSkipArea(SourceLocation StartLoc, SourceLocation EndLoc) {
- if (StartLoc == EndLoc)
+ /// Find a valid range starting with \p StartingLoc and ending before \p BeforeLoc.
+ std::optional<SourceRange> findAreaStartingFromTo(SourceLocation StartingLoc, SourceLocation BeforeLoc) {
+ // If StartingLoc is in function-like macro, use its start location.
+ if (StartingLoc.isMacroID()) {
+ FileID FID = SM.getFileID(StartingLoc);
+ const SrcMgr::ExpansionInfo *EI = &SM.getSLocEntry(FID).getExpansion();
+ if (EI->isFunctionMacroExpansion())
+ StartingLoc = EI->getExpansionLocStart();
+ }
+
+ size_t StartDepth = locationDepth(StartingLoc);
+ size_t EndDepth = locationDepth(BeforeLoc);
+ while (!SM.isWrittenInSameFile(StartingLoc, BeforeLoc)) {
+ bool UnnestStart = StartDepth >= EndDepth;
+ bool UnnestEnd = EndDepth >= StartDepth;
+ if (UnnestEnd) {
+ assert(SM.isWrittenInSameFile(getStartOfFileOrMacro(BeforeLoc), BeforeLoc));
+
+ BeforeLoc = getIncludeOrExpansionLoc(BeforeLoc);
+ assert(BeforeLoc.isValid());
+ EndDepth--;
+ }
+ if (UnnestStart) {
+ assert(SM.isWrittenInSameFile(StartingLoc, getStartOfFileOrMacro(StartingLoc)));
+
+ StartingLoc = getIncludeOrExpansionLoc(StartingLoc);
+ assert(StartingLoc.isValid());
+ StartDepth--;
+ }
+ }
+ // If the start and end locations of the gap are both within the same macro
+ // file, the range may not be in source order.
+ if (StartingLoc.isMacroID() || BeforeLoc.isMacroID())
+ return std::nullopt;
+ if (!SM.isWrittenInSameFile(StartingLoc, BeforeLoc) ||
+ !SpellingRegion(SM, StartingLoc, BeforeLoc).isInSourceOrder())
+ return std::nullopt;
+ return {{StartingLoc, BeforeLoc}};
+ }
+
+ void markSkipped(SourceLocation StartLoc, SourceLocation BeforeLoc) {
+ const auto Skipped = findAreaStartingFromTo(StartLoc, BeforeLoc);
+
+ if (!Skipped) {
return;
- assert(SpellingRegion(SM, StartLoc, EndLoc).isInSourceOrder());
- handleFileExit(StartLoc);
- size_t Index = pushRegion({}, StartLoc, EndLoc);
+ }
+
+ const auto NewStartLoc = Skipped->getBegin();
+ const auto EndLoc = Skipped->getEnd();
+
+ if (NewStartLoc == EndLoc)
+ return;
+ assert(SpellingRegion(SM, NewStartLoc, EndLoc).isInSourceOrder());
+ handleFileExit(NewStartLoc);
+ size_t Index = pushRegion({}, NewStartLoc, EndLoc);
getRegion().setSkipped(true);
handleFileExit(EndLoc);
popRegions(Index);
}
- void findGapAreaBetweenAndMarkSkipArea(SourceLocation AfterLoc,
- SourceLocation BeforeLoc) {
- markSkipArea(AfterLoc, BeforeLoc);
- }
-
/// Keep counts of breaks and continues inside loops.
struct BreakContinue {
Counter BreakCount;
@@ -1744,17 +1787,17 @@ struct CounterCoverageMappingBuilder
if (S->isNegatedConsteval()) {
// ignore 'if consteval'
- findGapAreaBetweenAndMarkSkipArea(S->getIfLoc(), getStart(Then));
+ markSkipped(S->getIfLoc(), getStart(Then));
propagateCounts(ParentCount, Then);
if (Else) {
// ignore 'else <else>'
- findGapAreaBetweenAndMarkSkipArea(getEnd(Then), getEnd(Else));
+ markSkipped(getEnd(Then), getEnd(Else));
}
} else {
assert(S->isNonNegatedConsteval());
// ignore 'if consteval <then> [else]'
- findGapAreaBetweenAndMarkSkipArea(S->getIfLoc(),
+ markSkipped(S->getIfLoc(),
Else ? getStart(Else) : getEnd(Then));
if (Else)
@@ -1785,7 +1828,7 @@ struct CounterCoverageMappingBuilder
if (Init) {
// don't mark initialisation as ignored
- findGapAreaBetweenAndMarkSkipArea(startOfSkipped, getStart(Init));
+ markSkipped(startOfSkipped, getStart(Init));
propagateCounts(ParentCount, Init);
// ignore after initialisation: '; <condition>)'...
startOfSkipped = getEnd(Init);
@@ -1793,16 +1836,15 @@ struct CounterCoverageMappingBuilder
if (isTrue) {
// ignore '<condition>)'
- findGapAreaBetweenAndMarkSkipArea(startOfSkipped, getStart(Then));
+ markSkipped(startOfSkipped, getStart(Then));
propagateCounts(ParentCount, Then);
if (Else)
// ignore 'else <else>'
- findGapAreaBetweenAndMarkSkipArea(getEnd(Then), getEnd(Else));
+ markSkipped(getEnd(Then), getEnd(Else));
} else {
// ignore '<condition>) <then> [else]'
- findGapAreaBetweenAndMarkSkipArea(startOfSkipped,
- Else ? getStart(Else) : getEnd(Then));
+ markSkipped(startOfSkipped, Else ? getStart(Else) : getEnd(Then));
if (Else) {
propagateCounts(ParentCount, Else);
>From cc8350e6679fe4cc4a7034c7e124539d501bafc1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hana=20Dusi=CC=81kova=CC=81?= <hanicka at hanicka.net>
Date: Mon, 22 Jan 2024 00:00:07 +0100
Subject: [PATCH 3/3] Handle macros properly (tests)
---
clang/test/CoverageMapping/if.cpp | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/clang/test/CoverageMapping/if.cpp b/clang/test/CoverageMapping/if.cpp
index 72240a729e82fb4..bd23088ae652749 100644
--- a/clang/test/CoverageMapping/if.cpp
+++ b/clang/test/CoverageMapping/if.cpp
@@ -111,6 +111,15 @@ int check_constexpr_init_false_without_else(int i) { // CHECK-NEXT: [[@LINE]]:
return i;
}
+// CHECK-LABEL: _Z32check_macro_constexpr_if_skippedi:
+int check_macro_constexpr_if_skipped(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
+#define IF_CONSTEXPR if constexpr // CHECK-NEXT: Expansion,File 0, [[@LINE+1]]:3 -> [[@LINE+1]]:15 = #0 (Expanded file = 1)
+ IF_CONSTEXPR(false) { // CHECK-NEXT: Skipped,File 0, [[@LINE]]:3 -> [[@LINE+2]]:4 = 0
+ i *= 2; // CHECK-NEXT: File 1, [[@LINE-2]]:22 -> [[@LINE-2]]:34 = #0
+ }
+ return i;
+}
+
// CHECK-LABEL: main:
int main() { // CHECK: File 0, [[@LINE]]:12 -> {{[0-9]+}}:2 = #0
int i = 0;
@@ -208,10 +217,20 @@ constexpr int check_notconsteval_branch_kept(int i) { // CHECK-NEXT: [[@LINE]]:{
return i;
}
+// CHECK-LABEL: _Z32check_macro_consteval_if_skippedi:
+constexpr int check_macro_consteval_if_skipped(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
+#define IF_RUNTIME if !consteval // CHECK-NEXT: Expansion,File 0, [[@LINE+1]]:3 -> [[@LINE+1]]:13 = #0 (Expanded file = 1)
+ IF_RUNTIME { // CHECK-NEXT: Skipped,File 0, [[@LINE]]:3 -> [[@LINE]]:14 = 0
+ i *= 2; // CHECK-NEXT: File 0, [[@LINE-1]]:14 -> [[@LINE+1]]:4 = #0
+ } // CHECK-NEXT: File 1, [[@LINE-3]]:20 -> [[@LINE-3]]:33 = #0
+ return i;
+}
+
int instantiate_consteval(int i) {
i *= check_consteval_with_else_discarded_then(i);
i *= check_notconsteval_with_else_discarded_else(i);
i *= check_consteval_branch_discarded(i);
i *= check_notconsteval_branch_kept(i);
+ i *= check_macro_consteval_if_skipped(i);
return i;
}
More information about the cfe-commits
mailing list