From 4759c3f86cd319cc0dbd99fec179eba7a04b81e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20R=C3=B6ssler?= Date: Fri, 7 Feb 2020 16:05:45 +0100 Subject: [PATCH 1/2] Filter return Unit from tail-optimized suspend funs --- .../kotlin/targets/KotlinCoroutineTarget.kt | 13 ++++--- .../filter/KotlinCoroutineFilterTest.java | 39 +++++++++++-------- .../analysis/filter/AbstractMatcher.java | 10 +++++ .../filter/KotlinCoroutineFilter.java | 33 +++++++++++++++- 4 files changed, 72 insertions(+), 23 deletions(-) diff --git a/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinCoroutineTarget.kt b/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinCoroutineTarget.kt index e242d788cb..aa25b73839 100644 --- a/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinCoroutineTarget.kt +++ b/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinCoroutineTarget.kt @@ -25,10 +25,12 @@ object KotlinCoroutineTarget { nop() // assertFullyCovered() } // assertFullyCovered() - private suspend fun suspendingFunctionWithTailCallOptimization() { // assertEmpty() - nop() // assertFullyCovered() - anotherSuspendingFunction() // assertFullyCovered() - } // assertFullyCovered() + private suspend fun suspendingFunctionWithTailCallOptimization(b: Boolean) { // assertEmpty() + if (b) + anotherSuspendingFunction() // assertFullyCovered() + else + anotherSuspendingFunction() // assertFullyCovered() + } // assertEmpty() private suspend fun anotherSuspendingFunction() { nop() // assertFullyCovered() @@ -42,7 +44,8 @@ object KotlinCoroutineTarget { nop(x) // assertFullyCovered() suspendingFunction() // assertFullyCovered() nop(x) // assertFullyCovered() - suspendingFunctionWithTailCallOptimization() + suspendingFunctionWithTailCallOptimization(true) // assertFullyCovered() + suspendingFunctionWithTailCallOptimization(false) // assertFullyCovered() } // assertFullyCovered() } diff --git a/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinCoroutineFilterTest.java b/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinCoroutineFilterTest.java index f96ed78e65..47b9e82e1c 100644 --- a/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinCoroutineFilterTest.java +++ b/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinCoroutineFilterTest.java @@ -391,11 +391,13 @@ public void should_filter_suspending_functions_with_tail_call_optimization() { context.classAnnotations .add(KotlinGeneratedFilter.KOTLIN_METADATA_DESC); - final Label exit = new Label(); + final Label[] labels = new Label[] { new Label(), new Label(), + new Label(), new Label(), new Label(), new Label() }; + m.visitLabel(labels[0]); m.visitVarInsn(Opcodes.ILOAD, 1); - final Label next = new Label(); - m.visitJumpInsn(Opcodes.IFEQ, next); + m.visitLabel(labels[2]); + m.visitJumpInsn(Opcodes.IFEQ, labels[1]); m.visitVarInsn(Opcodes.ALOAD, 0); m.visitVarInsn(Opcodes.ALOAD, 2); @@ -408,16 +410,15 @@ public void should_filter_suspending_functions_with_tail_call_optimization() { m.visitMethodInsn(Opcodes.INVOKESTATIC, "kotlin/coroutines/intrinsics/IntrinsicsKt", "getCOROUTINE_SUSPENDED", "()Ljava/lang/Object;", false); - final Label label = new Label(); - m.visitJumpInsn(Opcodes.IF_ACMPNE, label); + m.visitJumpInsn(Opcodes.IF_ACMPNE, labels[3]); m.visitInsn(Opcodes.ARETURN); - m.visitLabel(label); + m.visitLabel(labels[3]); m.visitInsn(Opcodes.POP); range1.toInclusive = m.instructions.getLast(); } - m.visitJumpInsn(Opcodes.GOTO, exit); - m.visitLabel(next); + m.visitJumpInsn(Opcodes.GOTO, labels[4]); + m.visitLabel(labels[1]); m.visitVarInsn(Opcodes.ALOAD, 0); m.visitVarInsn(Opcodes.ALOAD, 2); @@ -430,22 +431,26 @@ public void should_filter_suspending_functions_with_tail_call_optimization() { m.visitMethodInsn(Opcodes.INVOKESTATIC, "kotlin/coroutines/intrinsics/IntrinsicsKt", "getCOROUTINE_SUSPENDED", "()Ljava/lang/Object;", false); - final Label label = new Label(); - m.visitJumpInsn(Opcodes.IF_ACMPNE, label); + m.visitJumpInsn(Opcodes.IF_ACMPNE, labels[5]); m.visitInsn(Opcodes.ARETURN); - m.visitLabel(label); + m.visitLabel(labels[5]); m.visitInsn(Opcodes.POP); range2.toInclusive = m.instructions.getLast(); } - m.visitLabel(exit); - m.visitFieldInsn(Opcodes.GETSTATIC, "kotlin/Unit", "INSTANCE", - "Lkotlin/Unit;"); - m.visitInsn(Opcodes.ARETURN); + m.visitLabel(labels[4]); + final Range range3 = new Range(); + { + m.visitFieldInsn(Opcodes.GETSTATIC, "kotlin/Unit", "INSTANCE", + "Lkotlin/Unit;"); + range3.fromInclusive = m.instructions.getLast(); + m.visitInsn(Opcodes.ARETURN); + range3.toInclusive = m.instructions.getLast(); + } filter.filter(m, context, output); - assertIgnored(range1, range2); + // range3 is ignored twice, once for each suspend call + assertIgnored(range1, range3, range2, range3); } - } diff --git a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AbstractMatcher.java b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AbstractMatcher.java index e6b98fb4de..13fa6dc1eb 100644 --- a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AbstractMatcher.java +++ b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AbstractMatcher.java @@ -129,6 +129,16 @@ final void nextIsSwitch() { } } + final void nextIsLabel() { + if (cursor == null) { + return; + } + cursor = cursor.getNext(); + if (cursor != null && cursor.getType() != AbstractInsnNode.LABEL) { + cursor = null; + } + } + /** * Moves {@link #cursor} to next instruction if it has given opcode, * otherwise sets it to null. diff --git a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinCoroutineFilter.java b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinCoroutineFilter.java index 4b850f33e8..674792aeca 100644 --- a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinCoroutineFilter.java +++ b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinCoroutineFilter.java @@ -19,6 +19,7 @@ import org.objectweb.asm.Type; import org.objectweb.asm.tree.AbstractInsnNode; import org.objectweb.asm.tree.JumpInsnNode; +import org.objectweb.asm.tree.LabelNode; import org.objectweb.asm.tree.LdcInsnNode; import org.objectweb.asm.tree.MethodInsnNode; import org.objectweb.asm.tree.MethodNode; @@ -62,10 +63,40 @@ private void matchOptimizedTailCall(final MethodNode methodNode, "kotlin/coroutines/intrinsics/IntrinsicsKt", "getCOROUTINE_SUSPENDED", "()Ljava/lang/Object;"); nextIs(Opcodes.IF_ACMPNE); + JumpInsnNode jump = (JumpInsnNode) cursor; nextIs(Opcodes.ARETURN); + nextIsLabel(); + LabelNode label = (LabelNode) cursor; nextIs(Opcodes.POP); - if (cursor != null) { + if (cursor != null && jump.label == label) { output.ignore(i.getNext(), cursor); + AbstractInsnNode pop = cursor; + + // If we find a GOTO, follow it, the return Unit + // instructions may be there + nextIs(Opcodes.GOTO); + if (cursor != null) { + LabelNode gotoTarget = ((JumpInsnNode) cursor).label; + for (AbstractInsnNode j = cursor; j != null; j = j + .getNext()) { + if (j == gotoTarget) { + cursor = j; + break; + } + } + } else { + // else reset to the pop instruction + cursor = pop; + } + + // ignore the return-unit path as well + nextIsField(Opcodes.GETSTATIC, "kotlin/Unit", "INSTANCE", + "Lkotlin/Unit;"); + AbstractInsnNode getUnit = cursor; + nextIs(Opcodes.ARETURN); + if (cursor != null) { + output.ignore(getUnit, cursor); + } } } } From 2434b1feb8c6ed5214ed3e67c027febeb0116353 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20R=C3=B6ssler?= Date: Mon, 19 Dec 2022 17:19:40 +0100 Subject: [PATCH 2/2] add a comment to nextIsLabel --- .../jacoco/core/internal/analysis/filter/AbstractMatcher.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AbstractMatcher.java b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AbstractMatcher.java index 25db786815..262b0ed35e 100644 --- a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AbstractMatcher.java +++ b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AbstractMatcher.java @@ -130,6 +130,10 @@ final void nextIsSwitch() { } final void nextIsLabel() { + /** + * Moves {@link #cursor} to next instruction if it has given opcode, + * otherwise sets it to null. + */ if (cursor == null) { return; }