Update filter for Kotlin coroutines that restore state (bazelbuild#803)

gergelyfabian · Dec 26, 2018 · 3b8df21 · 3b8df21
1 parent ccad8eb
commit 3b8df21
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 28 deletions.
diff --git a/...dation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinCoroutineTarget.kt b/...dation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinCoroutineTarget.kt
@@ -26,9 +26,10 @@ object KotlinCoroutineTarget {
     fun main(args: Array<String>) {
 
         runBlocking { // assertFullyCovered()
-            nop() // assertFullyCovered()
+            val x = 42
+            nop(x) // assertFullyCovered()
             suspendingFunction() // assertFullyCovered()
-            nop() // assertFullyCovered()
+            nop(x) // assertFullyCovered()
         } // assertFullyCovered()
 
     }

diff --git a/...oco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinCoroutineFilterTest.java b/...oco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinCoroutineFilterTest.java
@@ -31,9 +31,10 @@ public class KotlinCoroutineFilterTest extends FilterTestBase {
 	/**
 	 * <pre>
 	 *     runBlocking {
-	 *         nop()
+	 *         val x = 42
+	 *         nop(x)
 	 *         suspendingFunction()
-	 *         nop()
+	 *         nop(x)
 	 *     }
 	 * </pre>
 	 */
@@ -46,11 +47,11 @@ public void should_filter() {
 		m.visitMethodInsn(Opcodes.INVOKESTATIC,
 				"kotlin/coroutines/intrinsics/IntrinsicsKt",
 				"getCOROUTINE_SUSPENDED", "()Ljava/lang/Object;", false);
-		m.visitVarInsn(Opcodes.ASTORE, 3);
+		m.visitVarInsn(Opcodes.ASTORE, 4);
 
 		m.visitVarInsn(Opcodes.ALOAD, 0);
 		// line of "runBlocking"
-		m.visitFieldInsn(Opcodes.GETFIELD, "", "label", "I");
+		m.visitFieldInsn(Opcodes.GETFIELD, "Target", "label", "I");
 		final Label dflt = new Label();
 		final Label state0 = new Label();
 		final Label state1 = new Label();
@@ -85,15 +86,19 @@ public void should_filter() {
 		m.visitInsn(Opcodes.DUP);
 		final Range range2 = new Range();
 		range2.fromInclusive = m.instructions.getLast();
-		m.visitVarInsn(Opcodes.ALOAD, 3);
+		m.visitVarInsn(Opcodes.ALOAD, 4);
 		final Label continuationLabelAfterLoadedResult = new Label();
 		m.visitJumpInsn(Opcodes.IF_ACMPNE, continuationLabelAfterLoadedResult);
 		// line of "runBlocking"
-		m.visitVarInsn(Opcodes.ALOAD, 3);
+		m.visitVarInsn(Opcodes.ALOAD, 4);
 		m.visitInsn(Opcodes.ARETURN);
 
 		m.visitLabel(state1);
 
+		m.visitVarInsn(Opcodes.ALOAD, 0);
+		m.visitFieldInsn(Opcodes.GETFIELD, "Target", "I$0", "I");
+		m.visitVarInsn(Opcodes.ISTORE, 3);
+
 		{
 			m.visitVarInsn(Opcodes.ALOAD, 1);
 			m.visitInsn(Opcodes.DUP);

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
@@ -158,11 +158,21 @@ final void next() {
 	 * {@link AbstractInsnNode#LABEL}, {@link AbstractInsnNode#LINE}.
 	 */
 	final void skipNonOpcodes() {
+		cursor = skipNonOpcodes(cursor);
+	}
+
+	/**
+	 * Returns first instruction from given and following it that is not
+	 * {@link AbstractInsnNode#FRAME}, {@link AbstractInsnNode#LABEL},
+	 * {@link AbstractInsnNode#LINE}.
+	 */
+	static AbstractInsnNode skipNonOpcodes(AbstractInsnNode cursor) {
 		while (cursor != null && (cursor.getType() == AbstractInsnNode.FRAME
 				|| cursor.getType() == AbstractInsnNode.LABEL
 				|| cursor.getType() == AbstractInsnNode.LINE)) {
 			cursor = cursor.getNext();
 		}
+		return cursor;
 	}
 
 }
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
@@ -16,6 +16,7 @@
 
 import org.objectweb.asm.Opcodes;
 import org.objectweb.asm.tree.AbstractInsnNode;
+import org.objectweb.asm.tree.JumpInsnNode;
 import org.objectweb.asm.tree.LdcInsnNode;
 import org.objectweb.asm.tree.MethodNode;
 import org.objectweb.asm.tree.TableSwitchInsnNode;
@@ -72,32 +73,45 @@ private void match(final MethodNode methodNode,
 			ignore.add(s);
 			ignore.add(cursor);
 
-			for (AbstractInsnNode i = methodNode.instructions
-					.getFirst(); i != null; i = i.getNext()) {
+			int suspensionPoint = 1;
+			for (AbstractInsnNode i = cursor; i != null
+					&& suspensionPoint < s.labels.size(); i = i.getNext()) {
 				cursor = i;
 				nextIsVar(Opcodes.ALOAD, "COROUTINE_SUSPENDED");
 				nextIs(Opcodes.IF_ACMPNE);
+				if (cursor == null) {
+					continue;
+				}
+				final AbstractInsnNode continuationAfterLoadedResult = skipNonOpcodes(
+						(((JumpInsnNode) cursor)).label);
 				nextIsVar(Opcodes.ALOAD, "COROUTINE_SUSPENDED");
 				nextIs(Opcodes.ARETURN);
-
-				nextIs(Opcodes.ALOAD);
-				nextIs(Opcodes.DUP);
-				nextIsType(Opcodes.INSTANCEOF, "kotlin/Result$Failure");
-				nextIs(Opcodes.IFEQ);
-				nextIsType(Opcodes.CHECKCAST, "kotlin/Result$Failure");
-				nextIs(Opcodes.GETFIELD);
-				nextIs(Opcodes.ATHROW);
-				nextIs(Opcodes.POP);
-
-				nextIs(Opcodes.ALOAD);
-				if (cursor != null) {
-					ignore.add(i);
-					ignore.add(cursor);
+				if (cursor == null
+						|| skipNonOpcodes(cursor.getNext()) != skipNonOpcodes(
+								s.labels.get(suspensionPoint))) {
+					continue;
 				}
-			}
 
-			if (ignore.size() != s.labels.size() * 2) {
-				return;
+				for (AbstractInsnNode j = i; j != null; j = j.getNext()) {
+					cursor = j;
+					nextIs(Opcodes.ALOAD);
+					nextIs(Opcodes.DUP);
+					nextIsType(Opcodes.INSTANCEOF, "kotlin/Result$Failure");
+					nextIs(Opcodes.IFEQ);
+					nextIsType(Opcodes.CHECKCAST, "kotlin/Result$Failure");
+					nextIs(Opcodes.GETFIELD);
+					nextIs(Opcodes.ATHROW);
+					nextIs(Opcodes.POP);
+
+					nextIs(Opcodes.ALOAD);
+					if (cursor != null && skipNonOpcodes(cursor
+							.getNext()) == continuationAfterLoadedResult) {
+						ignore.add(i);
+						ignore.add(cursor);
+						suspensionPoint++;
+						break;
+					}
+				}
 			}
 
 			cursor = s.dflt;

diff --git a/org.jacoco.doc/docroot/doc/changes.html b/org.jacoco.doc/docroot/doc/changes.html
@@ -32,7 +32,8 @@ <h3>New Features</h3>
       (GitHub <a href="https://github.com/jacoco/jacoco/issues/764">#764</a>).</li>
   <li>Branches added by the Kotlin compiler for coroutines are filtered out
       during generation of report
-      (GitHub <a href="https://github.com/jacoco/jacoco/issues/802">#802</a>).</li>
+      (GitHub <a href="https://github.com/jacoco/jacoco/issues/802">#802</a>,
+      <a href="https://github.com/jacoco/jacoco/issues/803">#803</a>).</li>
   <li>HTML report shows message when source file can't be found
       (GitHub <a href="https://github.com/jacoco/jacoco/issues/801">#801</a>).</li>
 </ul>