diff --git a/changelog/dmd.cov-inline-pragma.dd b/changelog/dmd.cov-inline-pragma.dd new file mode 100644 index 000000000000..04a5ba9a573f --- /dev/null +++ b/changelog/dmd.cov-inline-pragma.dd @@ -0,0 +1,14 @@ +Coverage counters are now emitted at call sites of inlined functions + +Functions marked `pragma(inline, true)` (and functions inlined via the +`-inline` flag) would always show `0000000` in `-cov` coverage reports +even when executed. The inlined function body is never called as a +standalone function at runtime - only the inlined copy inside the caller +runs, so its coverage counters stayed at zero. + +Coverage counters are now emitted at each call site where a function is +inlined, so the function's lines correctly reflect how many times they +were executed. + +Fixes $(LINK2 https://issues.dlang.org/show_bug.cgi?id=5848, Issue 5848) / +$(LINK2 https://github.com/dlang/dmd/issues/18337, #18337) diff --git a/compiler/src/dmd/astbase.d b/compiler/src/dmd/astbase.d index a840f4d185ee..69c27119c73c 100644 --- a/compiler/src/dmd/astbase.d +++ b/compiler/src/dmd/astbase.d @@ -5684,6 +5684,7 @@ struct ASTBase extern (C++) final class CommaExp : BinExp { const bool isGenerated; + bool isInlineSequence; bool allowCommaExp; extern (D) this(Loc loc, Expression e1, Expression e2, bool generated = true) diff --git a/compiler/src/dmd/expression.d b/compiler/src/dmd/expression.d index d51048346769..7defc086779b 100644 --- a/compiler/src/dmd/expression.d +++ b/compiler/src/dmd/expression.d @@ -2828,6 +2828,9 @@ extern (C++) final class CommaExp : BinExp /// to trigger the deprecation. const bool isGenerated; + /// `true` if this comma chain was introduced by inline expansion. + bool isInlineSequence; + /// Temporary variable to enable / disable deprecation of comma expression /// depending on the context. /// Since most constructor calls are rewritting, the only place where diff --git a/compiler/src/dmd/expression.h b/compiler/src/dmd/expression.h index acf9285161e7..15998ddc177a 100644 --- a/compiler/src/dmd/expression.h +++ b/compiler/src/dmd/expression.h @@ -946,6 +946,7 @@ class CommaExp final : public BinExp { public: d_bool isGenerated; + d_bool isInlineSequence; d_bool allowCommaExp; Expression* originalExp; void accept(Visitor *v) override { v->visit(this); } diff --git a/compiler/src/dmd/frontend.h b/compiler/src/dmd/frontend.h index a5b710cfd503..9d61af41b245 100644 --- a/compiler/src/dmd/frontend.h +++ b/compiler/src/dmd/frontend.h @@ -2647,6 +2647,7 @@ class CommaExp final : public BinExp { public: const bool isGenerated; + bool isInlineSequence; bool allowCommaExp; Expression* originalExp; void accept(Visitor* v) override; diff --git a/compiler/src/dmd/glue/e2ir.d b/compiler/src/dmd/glue/e2ir.d index 4ff0439f4330..cf7684b03492 100644 --- a/compiler/src/dmd/glue/e2ir.d +++ b/compiler/src/dmd/glue/e2ir.d @@ -3256,8 +3256,27 @@ elem* toElem(Expression e, ref IRState irs) elem* visitComma(CommaExp ce) { assert(ce.e1 && ce.e2); + elem* inlineCoverage(Expression e) + { + if (!ce.isInlineSequence || !irs.params.cov || !e || !e.loc.isValid()) + return null; + + // Nested inline-sequence commas are accounted for recursively. + if (auto nested = e.isCommaExp()) + { + if (nested.isInlineSequence) + return null; + } + + return incUsageElem(irs, e.loc); + } + elem* eleft = toElem(ce.e1, irs); + eleft = el_combine(inlineCoverage(ce.e1), eleft); + elem* eright = toElem(ce.e2, irs); + eright = el_combine(inlineCoverage(ce.e2), eright); + elem* e = el_combine(eleft, eright); if (e) elem_setLoc(e, ce.loc); diff --git a/compiler/src/dmd/inline.d b/compiler/src/dmd/inline.d index 311da43463a4..2123efdeeb59 100644 --- a/compiler/src/dmd/inline.d +++ b/compiler/src/dmd/inline.d @@ -120,6 +120,15 @@ private final class InlineDoState } } +private Expression combineInlineSequence(Expression e1, Expression e2) +{ + auto result = Expression.combine(e1, e2); + if (result) + if (auto ce = result.isCommaExp()) + ce.isInlineSequence = true; + return result; +} + /*********************************************************** * Perform the inlining from (Statement or Expression) to (Statement or Expression). * @@ -218,13 +227,13 @@ public: e2.type = Type.tvoid; e.type = Type.tvoid; } - result = Expression.combine(result, e); + result = combineInlineSequence(result, e); } else { ids.foundReturn = false; auto e = doInlineAs!Expression(sx, ids); - result = Expression.combine(result, e); + result = combineInlineSequence(result, e); } } @@ -253,7 +262,7 @@ public: static if (asStatements) as.push(r); else - result = Expression.combine(result, r); + result = combineInlineSequence(result, r); if (ids.foundReturn) break; @@ -2350,7 +2359,7 @@ private void expandInline(CallExp ecall, FuncDeclaration fd, FuncDeclaration par ids.from.push(vfrom); ids.to.push(vto); - auto de = new DeclarationExp(vto.loc, vto); + auto de = new DeclarationExp(Loc.initial, vto); de.type = Type.tvoid; eparams = Expression.combine(eparams, de); @@ -2445,8 +2454,10 @@ private void expandInline(CallExp ecall, FuncDeclaration fd, FuncDeclaration par e.type = Type.tvoid; } - eresult = Expression.combine(eresult, eret, ethis, eparams); - eresult = Expression.combine(eresult, e); + eresult = combineInlineSequence(eresult, eret); + eresult = combineInlineSequence(eresult, ethis); + eresult = combineInlineSequence(eresult, eparams); + eresult = combineInlineSequence(eresult, e); if (ecall.rvalue || tf.isRvalue) eresult.rvalue = true; diff --git a/compiler/test/runnable/extra-files/runnable-test18337.lst b/compiler/test/runnable/extra-files/runnable-test18337.lst new file mode 100644 index 000000000000..aa1da2078e50 --- /dev/null +++ b/compiler/test/runnable/extra-files/runnable-test18337.lst @@ -0,0 +1,26 @@ + |// PERMUTE_ARGS: + |// REQUIRED_ARGS: -cov + |// POST_SCRIPT: runnable/extra-files/coverage-postscript.sh + |// EXECUTE_ARGS: ${RESULTS_DIR}/runnable + | + |extern(C) void dmd_coverDestPath(string path); + | + |pragma(inline, true) + |int square(int x) + |{ + 2| int y = x * x; + 2| return y; + |} + | + |pragma(inline, true) + |int addSquares(int a, int b) + |{ + 1| return square(a) + square(b); + |} + | + |void main(string[] args) + |{ + 1| dmd_coverDestPath(args[1]); + 1| auto value = addSquares(2, 3); + 1| assert(value == 13); + |} diff --git a/compiler/test/runnable/test18337.d b/compiler/test/runnable/test18337.d new file mode 100644 index 000000000000..a99dfcf9e258 --- /dev/null +++ b/compiler/test/runnable/test18337.d @@ -0,0 +1,26 @@ +// PERMUTE_ARGS: +// REQUIRED_ARGS: -cov +// POST_SCRIPT: runnable/extra-files/coverage-postscript.sh +// EXECUTE_ARGS: ${RESULTS_DIR}/runnable + +extern(C) void dmd_coverDestPath(string path); + +pragma(inline, true) +int square(int x) +{ + int y = x * x; + return y; +} + +pragma(inline, true) +int addSquares(int a, int b) +{ + return square(a) + square(b); +} + +void main(string[] args) +{ + dmd_coverDestPath(args[1]); + auto value = addSquares(2, 3); + assert(value == 13); +}