Skip to content

Commit ce8c72f

Browse files
Fix dlang#18337 - Emit coverage counters at call sites of inlined functions (dlang#22922)
inlined functions always showed 0000000 in -cov reports even when called. the standalone function body is never executed at runtime (it's inlined at each call site), so its counters stayed zero. fixes https://issues.dlang.org/show_bug.cgi?id=5848 fixes dlang#18337
1 parent a529ad8 commit ce8c72f

9 files changed

Lines changed: 108 additions & 6 deletions

File tree

changelog/dmd.cov-inline-pragma.dd

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
Coverage counters are now emitted at call sites of inlined functions
2+
3+
Functions marked `pragma(inline, true)` (and functions inlined via the
4+
`-inline` flag) would always show `0000000` in `-cov` coverage reports
5+
even when executed. The inlined function body is never called as a
6+
standalone function at runtime - only the inlined copy inside the caller
7+
runs, so its coverage counters stayed at zero.
8+
9+
Coverage counters are now emitted at each call site where a function is
10+
inlined, so the function's lines correctly reflect how many times they
11+
were executed.
12+
13+
Fixes $(LINK2 https://issues.dlang.org/show_bug.cgi?id=5848, Issue 5848) /
14+
$(LINK2 https://github.com/dlang/dmd/issues/18337, #18337)

compiler/include/dmd/expression.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -948,6 +948,7 @@ class CommaExp final : public BinExp
948948
{
949949
public:
950950
d_bool isGenerated;
951+
d_bool isInlineSequence;
951952
d_bool allowCommaExp;
952953
Expression* originalExp;
953954
void accept(Visitor *v) override { v->visit(this); }

compiler/src/dmd/astbase.d

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5701,6 +5701,7 @@ struct ASTBase
57015701
extern (C++) final class CommaExp : BinExp
57025702
{
57035703
const bool isGenerated;
5704+
bool isInlineSequence;
57045705
bool allowCommaExp;
57055706

57065707
extern (D) this(Loc loc, Expression e1, Expression e2, bool generated = true)

compiler/src/dmd/expression.d

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2829,6 +2829,9 @@ extern (C++) final class CommaExp : BinExp
28292829
/// to trigger the deprecation.
28302830
const bool isGenerated;
28312831

2832+
/// `true` if this comma chain was introduced by inline expansion.
2833+
bool isInlineSequence;
2834+
28322835
/// Temporary variable to enable / disable deprecation of comma expression
28332836
/// depending on the context.
28342837
/// Since most constructor calls are rewritting, the only place where

compiler/src/dmd/frontend.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2649,6 +2649,7 @@ class CommaExp final : public BinExp
26492649
{
26502650
public:
26512651
const bool isGenerated;
2652+
bool isInlineSequence;
26522653
bool allowCommaExp;
26532654
Expression* originalExp;
26542655
void accept(Visitor* v) override;

compiler/src/dmd/glue/e2ir.d

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3292,8 +3292,27 @@ elem* toElem(Expression e, ref IRState irs)
32923292
elem* visitComma(CommaExp ce)
32933293
{
32943294
assert(ce.e1 && ce.e2);
3295+
elem* inlineCoverage(Expression e)
3296+
{
3297+
if (!ce.isInlineSequence || !irs.params.cov || !e || !e.loc.isValid())
3298+
return null;
3299+
3300+
// Nested inline-sequence commas are accounted for recursively.
3301+
if (auto nested = e.isCommaExp())
3302+
{
3303+
if (nested.isInlineSequence)
3304+
return null;
3305+
}
3306+
3307+
return incUsageElem(irs, e.loc);
3308+
}
3309+
32953310
elem* eleft = toElem(ce.e1, irs);
3311+
eleft = el_combine(inlineCoverage(ce.e1), eleft);
3312+
32963313
elem* eright = toElem(ce.e2, irs);
3314+
eright = el_combine(inlineCoverage(ce.e2), eright);
3315+
32973316
elem* e = el_combine(eleft, eright);
32983317
if (e)
32993318
elem_setLoc(e, ce.loc);

compiler/src/dmd/inline.d

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,15 @@ private final class InlineDoState
120120
}
121121
}
122122

123+
private Expression combineInlineSequence(Expression e1, Expression e2)
124+
{
125+
auto result = Expression.combine(e1, e2);
126+
if (result)
127+
if (auto ce = result.isCommaExp())
128+
ce.isInlineSequence = true;
129+
return result;
130+
}
131+
123132
/***********************************************************
124133
* Perform the inlining from (Statement or Expression) to (Statement or Expression).
125134
*
@@ -218,13 +227,13 @@ public:
218227
e2.type = Type.tvoid;
219228
e.type = Type.tvoid;
220229
}
221-
result = Expression.combine(result, e);
230+
result = combineInlineSequence(result, e);
222231
}
223232
else
224233
{
225234
ids.foundReturn = false;
226235
auto e = doInlineAs!Expression(sx, ids);
227-
result = Expression.combine(result, e);
236+
result = combineInlineSequence(result, e);
228237
}
229238
}
230239

@@ -253,7 +262,7 @@ public:
253262
static if (asStatements)
254263
as.push(r);
255264
else
256-
result = Expression.combine(result, r);
265+
result = combineInlineSequence(result, r);
257266

258267
if (ids.foundReturn)
259268
break;
@@ -2350,7 +2359,7 @@ private void expandInline(CallExp ecall, FuncDeclaration fd, FuncDeclaration par
23502359
ids.from.push(vfrom);
23512360
ids.to.push(vto);
23522361

2353-
auto de = new DeclarationExp(vto.loc, vto);
2362+
auto de = new DeclarationExp(Loc.initial, vto);
23542363
de.type = Type.tvoid;
23552364
eparams = Expression.combine(eparams, de);
23562365

@@ -2445,8 +2454,10 @@ private void expandInline(CallExp ecall, FuncDeclaration fd, FuncDeclaration par
24452454
e.type = Type.tvoid;
24462455
}
24472456

2448-
eresult = Expression.combine(eresult, eret, ethis, eparams);
2449-
eresult = Expression.combine(eresult, e);
2457+
eresult = combineInlineSequence(eresult, eret);
2458+
eresult = combineInlineSequence(eresult, ethis);
2459+
eresult = combineInlineSequence(eresult, eparams);
2460+
eresult = combineInlineSequence(eresult, e);
24502461

24512462
if (ecall.rvalue || tf.isRvalue)
24522463
eresult.rvalue = true;
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
|// PERMUTE_ARGS:
2+
|// REQUIRED_ARGS: -cov
3+
|// POST_SCRIPT: runnable/extra-files/coverage-postscript.sh
4+
|// EXECUTE_ARGS: ${RESULTS_DIR}/runnable
5+
|
6+
|extern(C) void dmd_coverDestPath(string path);
7+
|
8+
|pragma(inline, true)
9+
|int square(int x)
10+
|{
11+
2| int y = x * x;
12+
2| return y;
13+
|}
14+
|
15+
|pragma(inline, true)
16+
|int addSquares(int a, int b)
17+
|{
18+
1| return square(a) + square(b);
19+
|}
20+
|
21+
|void main(string[] args)
22+
|{
23+
1| dmd_coverDestPath(args[1]);
24+
1| auto value = addSquares(2, 3);
25+
1| assert(value == 13);
26+
|}

compiler/test/runnable/test18337.d

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// PERMUTE_ARGS:
2+
// REQUIRED_ARGS: -cov
3+
// POST_SCRIPT: runnable/extra-files/coverage-postscript.sh
4+
// EXECUTE_ARGS: ${RESULTS_DIR}/runnable
5+
6+
extern(C) void dmd_coverDestPath(string path);
7+
8+
pragma(inline, true)
9+
int square(int x)
10+
{
11+
int y = x * x;
12+
return y;
13+
}
14+
15+
pragma(inline, true)
16+
int addSquares(int a, int b)
17+
{
18+
return square(a) + square(b);
19+
}
20+
21+
void main(string[] args)
22+
{
23+
dmd_coverDestPath(args[1]);
24+
auto value = addSquares(2, 3);
25+
assert(value == 13);
26+
}

0 commit comments

Comments
 (0)