Skip to content

Commit a26cc75

Browse files
hanickadothanickadot
authored
[clang][coverage] Fix "if constexpr" and "if consteval" coverage report (#77214)
Replace the discarded statement by an empty compound statement so we can keep track of the whole source range we need to skip in coverage Fixes #54419
1 parent e22cb93 commit a26cc75

File tree

5 files changed

+121
-22
lines changed

5 files changed

+121
-22
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -705,6 +705,9 @@ Bug Fixes in This Version
705705
- Fix assertion crash due to failed scope restoring caused by too-early VarDecl
706706
invalidation by invalid initializer Expr.
707707
Fixes (`#30908 <https://github.com/llvm/llvm-project/issues/30908>`_)
708+
- Clang now emits correct source location for code-coverage regions in `if constexpr`
709+
and `if consteval` branches.
710+
Fixes (`#54419 <https://github.com/llvm/llvm-project/issues/54419>`_)
708711

709712

710713
Bug Fixes to Compiler Builtins

‎clang/include/clang/AST/Stmt.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1631,8 +1631,10 @@ class CompoundStmt final
16311631
SourceLocation RB);
16321632

16331633
// Build an empty compound statement with a location.
1634-
explicit CompoundStmt(SourceLocation Loc)
1635-
: Stmt(CompoundStmtClass), LBraceLoc(Loc), RBraceLoc(Loc) {
1634+
explicit CompoundStmt(SourceLocation Loc) : CompoundStmt(Loc, Loc) {}
1635+
1636+
CompoundStmt(SourceLocation Loc, SourceLocation EndLoc)
1637+
: Stmt(CompoundStmtClass), LBraceLoc(Loc), RBraceLoc(EndLoc) {
16361638
CompoundStmtBits.NumStmts = 0;
16371639
CompoundStmtBits.HasFPFeatures = 0;
16381640
}

‎clang/lib/CodeGen/CoverageMappingGen.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1712,7 +1712,11 @@ struct CounterCoverageMappingBuilder
17121712
extendRegion(S->getCond());
17131713

17141714
Counter ParentCount = getRegion().getCounter();
1715-
Counter ThenCount = getRegionCounter(S);
1715+
1716+
// If this is "if !consteval" the then-branch will never be taken, we don't
1717+
// need to change counter
1718+
Counter ThenCount =
1719+
S->isNegatedConsteval() ? ParentCount : getRegionCounter(S);
17161720

17171721
if (!S->isConsteval()) {
17181722
// Emitting a counter for the condition makes it easier to interpret the
@@ -1729,7 +1733,12 @@ struct CounterCoverageMappingBuilder
17291733
extendRegion(S->getThen());
17301734
Counter OutCount = propagateCounts(ThenCount, S->getThen());
17311735

1732-
Counter ElseCount = subtractCounters(ParentCount, ThenCount);
1736+
// If this is "if consteval" the else-branch will never be taken, we don't
1737+
// need to change counter
1738+
Counter ElseCount = S->isNonNegatedConsteval()
1739+
? ParentCount
1740+
: subtractCounters(ParentCount, ThenCount);
1741+
17331742
if (const Stmt *Else = S->getElse()) {
17341743
bool ThenHasTerminateStmt = HasTerminateStmt;
17351744
HasTerminateStmt = false;

‎clang/lib/Sema/TreeTransform.h

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7739,7 +7739,11 @@ TreeTransform<Derived>::TransformIfStmt(IfStmt *S) {
77397739
if (Then.isInvalid())
77407740
return StmtError();
77417741
} else {
7742-
Then = new (getSema().Context) NullStmt(S->getThen()->getBeginLoc());
7742+
// Discarded branch is replaced with empty CompoundStmt so we can keep
7743+
// proper source location for start and end of original branch, so
7744+
// subsequent transformations like CoverageMapping work properly
7745+
Then = new (getSema().Context)
7746+
CompoundStmt(S->getThen()->getBeginLoc(), S->getThen()->getEndLoc());
77437747
}
77447748

77457749
// Transform the "else" branch.
@@ -7748,6 +7752,13 @@ TreeTransform<Derived>::TransformIfStmt(IfStmt *S) {
77487752
Else = getDerived().TransformStmt(S->getElse());
77497753
if (Else.isInvalid())
77507754
return StmtError();
7755+
} else if (S->getElse() && ConstexprConditionValue &&
7756+
*ConstexprConditionValue) {
7757+
// Same thing here as with <then> branch, we are discarding it, we can't
7758+
// replace it with NULL nor NullStmt as we need to keep for source location
7759+
// range, for CoverageMapping
7760+
Else = new (getSema().Context)
7761+
CompoundStmt(S->getElse()->getBeginLoc(), S->getElse()->getEndLoc());
77517762
}
77527763

77537764
if (!getDerived().AlwaysRebuild() &&

‎clang/test/CoverageMapping/if.cpp

Lines changed: 91 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,49 @@ void foo() { // CHECK-NEXT: Gap,File 0, [[@LINE+1]]:21 -> [[@
2323
} // CHECK-NEXT: [[@LINE-2]]:9 -> [[@LINE-1]]:5 = #1
2424
// CHECK-NEXT: [[@LINE-2]]:5 -> [[@LINE-2]]:8 = #1
2525

26-
// FIXME: Do not generate coverage for discarded branches in if consteval and if constexpr statements
27-
constexpr int check_consteval(int i) {
28-
if consteval {
29-
i++;
30-
}
31-
if !consteval {
32-
i++;
33-
}
34-
if consteval {
35-
return 42;
36-
} else {
37-
return i;
38-
}
26+
// FIXME: Do not generate coverage for discarded branches in if constexpr
27+
// CHECK-LABEL: _Z30check_constexpr_true_with_elsei:
28+
int check_constexpr_true_with_else(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
29+
// CHECK-NEXT: [[@LINE+2]]:16 -> [[@LINE+2]]:20 = #0
30+
// CHECK-NEXT: Branch,File 0, [[@LINE+1]]:16 -> [[@LINE+1]]:20 = 0, 0
31+
if constexpr(true) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:21 -> [[@LINE]]:22 = #1
32+
i *= 3; // CHECK-NEXT: [[@LINE-1]]:22 -> [[@LINE+1]]:4 = #1
33+
} else { // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 -> [[@LINE]]:10 = (#0 - #1)
34+
i *= 5; // CHECK-NEXT: [[@LINE-1]]:10 -> [[@LINE+1]]:4 = (#0 - #1)
35+
}
36+
return i;
37+
}
38+
39+
// CHECK-LABEL: _Z33check_constexpr_true_without_elsei:
40+
int check_constexpr_true_without_else(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
41+
// CHECK-NEXT: [[@LINE+2]]:16 -> [[@LINE+2]]:20 = #0
42+
// CHECK-NEXT: Branch,File 0, [[@LINE+1]]:16 -> [[@LINE+1]]:20 = 0, 0
43+
if constexpr(true) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:21 -> [[@LINE]]:22 = #1
44+
i *= 3; // CHECK-NEXT: [[@LINE-1]]:22 -> [[@LINE+1]]:4 = #1
45+
}
46+
return i;
47+
}
48+
49+
// CHECK-LABEL: _Z31check_constexpr_false_with_elsei:
50+
int check_constexpr_false_with_else(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
51+
// CHECK-NEXT: [[@LINE+2]]:16 -> [[@LINE+2]]:21 = #0
52+
// CHECK-NEXT: Branch,File 0, [[@LINE+1]]:16 -> [[@LINE+1]]:21 = 0, 0
53+
if constexpr(false) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:22 -> [[@LINE]]:23 = #1
54+
i *= 3; // CHECK-NEXT: File 0, [[@LINE-1]]:23 -> [[@LINE+1]]:4 = #1
55+
} else { // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 -> [[@LINE]]:10 = (#0 - #1)
56+
i *= 5; // CHECK-NEXT: File 0, [[@LINE-1]]:10 -> [[@LINE+1]]:4 = (#0 - #1)
57+
}
58+
return i;
59+
}
60+
61+
// CHECK-LABEL: _Z34check_constexpr_false_without_elsei:
62+
int check_constexpr_false_without_else(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
63+
// CHECK-NEXT: [[@LINE+2]]:16 -> [[@LINE+2]]:21 = #0
64+
// CHECK-NEXT: Branch,File 0, [[@LINE+1]]:16 -> [[@LINE+1]]:21 = 0, 0
65+
if constexpr(false) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:22 -> [[@LINE]]:23 = #1
66+
i *= 3; // CHECK-NEXT: File 0, [[@LINE-1]]:23 -> [[@LINE+1]]:4 = #1
67+
}
68+
return i;
3969
}
4070

4171
// CHECK-LABEL: main:
@@ -75,10 +105,6 @@ int main() { // CHECK: File 0, [[@LINE]]:12 -> {{[0-9]+}}:2 =
75105
// CHECK-NEXT: File 0, [[@LINE+1]]:14 -> [[@LINE+1]]:20 = #6
76106
i = i == 0?i + 12:i + 10; // CHECK-NEXT: File 0, [[@LINE]]:21 -> [[@LINE]]:27 = (#0 - #6)
77107

78-
// GH-57377
79-
constexpr int c_i = check_consteval(0);
80-
check_consteval(i);
81-
82108
// GH-45481
83109
S s;
84110
s.the_prop = 0? 1 : 2; // CHECK-NEXT: File 0, [[@LINE]]:16 -> [[@LINE]]:17 = #0
@@ -98,3 +124,51 @@ int main() { // CHECK: File 0, [[@LINE]]:12 -> {{[0-9]+}}:2 =
98124
void ternary() {
99125
true ? FOO : FOO; // CHECK-NOT: Gap,{{.*}}, [[@LINE]]:8 ->
100126
}
127+
128+
// GH-57377
129+
// CHECK-LABEL: _Z40check_consteval_with_else_discarded_theni:
130+
// FIXME: Do not generate coverage for discarded <then> branch in if consteval
131+
constexpr int check_consteval_with_else_discarded_then(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
132+
if consteval {
133+
i *= 3; // CHECK-NEXT: [[@LINE-1]]:16 -> [[@LINE+1]]:4 = #1
134+
} else { // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 -> [[@LINE]]:10 = #0
135+
i *= 5; // CHECK-NEXT: [[@LINE-1]]:10 -> [[@LINE+1]]:4 = #0
136+
}
137+
return i; // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE]]:11 = (#0 + #1)
138+
}
139+
140+
// CHECK-LABEL: _Z43check_notconsteval_with_else_discarded_elsei:
141+
// FIXME: Do not generate coverage for discarded <else> branch in if consteval
142+
constexpr int check_notconsteval_with_else_discarded_else(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
143+
if !consteval {
144+
i *= 3; // CHECK-NEXT: [[@LINE-1]]:17 -> [[@LINE+1]]:4 = #0
145+
} else { // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 -> [[@LINE]]:10 = 0
146+
i *= 5; // CHECK-NEXT: [[@LINE-1]]:10 -> [[@LINE+1]]:4 = 0
147+
}
148+
return i;
149+
}
150+
151+
// CHECK-LABEL: _Z32check_consteval_branch_discardedi:
152+
// FIXME: Do not generate coverage for discarded <then> branch in if consteval
153+
constexpr int check_consteval_branch_discarded(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
154+
if consteval {
155+
i *= 3; // CHECK-NEXT: [[@LINE-1]]:16 -> [[@LINE+1]]:4 = #1
156+
}
157+
return i; // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE]]:11 = (#0 + #1)
158+
}
159+
160+
// CHECK-LABEL: _Z30check_notconsteval_branch_kepti:
161+
constexpr int check_notconsteval_branch_kept(int i) { // CHECK-NEXT: [[@LINE]]:{{[0-9]+}} -> {{[0-9]+}}:2 = #0
162+
if !consteval {
163+
i *= 3; // CHECK-NEXT: [[@LINE-1]]:17 -> [[@LINE+1]]:4 = #0
164+
}
165+
return i;
166+
}
167+
168+
int instantiate_consteval(int i) {
169+
i *= check_consteval_with_else_discarded_then(i);
170+
i *= check_notconsteval_with_else_discarded_else(i);
171+
i *= check_consteval_branch_discarded(i);
172+
i *= check_notconsteval_branch_kept(i);
173+
return i;
174+
}

0 commit comments

Comments
 (0)