Fix verify error with 'return when/if/try' in suspend function

The problem appears for tail-optimized suspend functions,
we erroneously assumed that when/if/try expressions in tail-call
position have simple types like `I` while actually, they can return SUSPENDED
object, i.e. must have `java/lang/Object` as return type.

But this only concerns branching operations in tail-call position,
so we have to make an additional analysis for remembering whether
a given expression is in a tail-call position.

Also, it's important here that we now assume
the return type of the current function  as `java/lang/Object`
that is necessary to avoid wrong checkcasts.

 #KT-15364 Fixed
This commit is contained in:
Denis Zharkov
2017-01-12 15:34:31 +03:00
parent de9c41c412
commit 2286027bed
14 changed files with 324 additions and 32 deletions
@@ -170,9 +170,8 @@ public class ExpressionCodegen extends KtVisitor<StackValue, StackValue> impleme
}
@NotNull
private Type getBoxedReturnTypeForSuspend(FunctionDescriptor descriptorForSuspend) {
assert descriptorForSuspend.getReturnType() != null : "Uninitialized suspend return type";
return AsmUtil.boxType(typeMapper.mapType(descriptorForSuspend.getReturnType()));
private static Type getBoxedReturnTypeForSuspend(FunctionDescriptor descriptorForSuspend) {
return AsmTypes.OBJECT_TYPE;
}
@Nullable
@@ -454,6 +453,16 @@ public class ExpressionCodegen extends KtVisitor<StackValue, StackValue> impleme
return typeMapper.mapType(type);
}
@NotNull
private Type expressionTypeForBranchingOperation(@Nullable KtExpression expression) {
if (context.getFunctionDescriptor().isSuspend() &&
!CoroutineCodegenUtilKt.isStateMachineNeeded(context.getFunctionDescriptor(), bindingContext) &&
Boolean.TRUE.equals(bindingContext.get(IS_TAIL_EXPRESSION_IN_SUSPEND_FUNCTION, expression))) {
return AsmTypes.OBJECT_TYPE;
}
return expressionType(expression);
}
@NotNull
public Type expressionType(@Nullable KtExpression expression) {
return CodegenUtilKt.asmType(expression, typeMapper, bindingContext);
@@ -494,7 +503,7 @@ public class ExpressionCodegen extends KtVisitor<StackValue, StackValue> impleme
}
/* package */ StackValue generateIfExpression(@NotNull final KtIfExpression expression, final boolean isStatement) {
final Type asmType = isStatement ? Type.VOID_TYPE : expressionType(expression);
final Type asmType = isStatement ? Type.VOID_TYPE : expressionTypeForBranchingOperation(expression);
final StackValue condition = gen(expression.getCondition());
final KtExpression thenExpression = expression.getThen();
@@ -4284,9 +4293,7 @@ The "returned" value of try expression with no finally is either the last expres
(or blocks).
*/
KotlinType jetType = bindingContext.getType(expression);
assert jetType != null;
final Type expectedAsmType = isStatement ? Type.VOID_TYPE : asmType(jetType);
final Type expectedAsmType = isStatement ? Type.VOID_TYPE : expressionTypeForBranchingOperation(expression);
return StackValue.operation(expectedAsmType, new Function1<InstructionAdapter, Unit>() {
@Override
@@ -4559,7 +4566,7 @@ The "returned" value of try expression with no finally is either the last expres
final KtExpression expr = expression.getSubjectExpression();
final Type subjectType = expressionType(expr);
final Type resultType = isStatement ? Type.VOID_TYPE : expressionType(expression);
final Type resultType = isStatement ? Type.VOID_TYPE : expressionTypeForBranchingOperation(expression);
return StackValue.operation(resultType, new Function1<InstructionAdapter, Unit>() {
@Override
@@ -30,10 +30,7 @@ import org.jetbrains.kotlin.cfg.pseudocode.instructions.jumps.*
import org.jetbrains.kotlin.cfg.pseudocode.instructions.special.MarkInstruction
import org.jetbrains.kotlin.cfg.pseudocode.instructions.special.VariableDeclarationInstruction
import org.jetbrains.kotlin.cfg.pseudocode.sideEffectFree
import org.jetbrains.kotlin.cfg.pseudocodeTraverser.Edges
import org.jetbrains.kotlin.cfg.pseudocodeTraverser.TraversalOrder
import org.jetbrains.kotlin.cfg.pseudocodeTraverser.traverse
import org.jetbrains.kotlin.cfg.pseudocodeTraverser.traverseFollowingInstructions
import org.jetbrains.kotlin.cfg.pseudocodeTraverser.*
import org.jetbrains.kotlin.descriptors.*
import org.jetbrains.kotlin.descriptors.impl.referencedProperty
import org.jetbrains.kotlin.diagnostics.Diagnostic
@@ -58,6 +55,7 @@ import org.jetbrains.kotlin.types.KotlinType
import org.jetbrains.kotlin.types.TypeUtils.*
import org.jetbrains.kotlin.types.expressions.ExpressionTypingUtils
import org.jetbrains.kotlin.util.OperatorNameConventions
import org.jetbrains.kotlin.utils.addToStdlib.safeAs
import java.util.*
class ControlFlowInformationProvider private constructor(
@@ -809,8 +807,8 @@ class ControlFlowInformationProvider private constructor(
}
private fun checkSuspendCalls(currentFunction: FunctionDescriptor) {
if (!currentFunction.isSuspend) return
var containsNonTailCalls = false
traverseCalls { instruction, resolvedCall ->
val calleeDescriptor = resolvedCall.resultingDescriptor as? FunctionDescriptor ?: return@traverseCalls
@@ -827,7 +825,28 @@ class ControlFlowInformationProvider private constructor(
val isUsedAsExpression = instruction.owner.getUsages(instruction.outputValue).isNotEmpty()
if (!isUsedAsExpression || !instruction.isTailCall(enclosingSuspendFunction) || isInsideTry(element)) {
trace.record(BindingContext.CONTAINS_NON_TAIL_SUSPEND_CALLS, currentFunction.original)
containsNonTailCalls = true
}
}
if (containsNonTailCalls) {
trace.record(BindingContext.CONTAINS_NON_TAIL_SUSPEND_CALLS, currentFunction.original)
}
else {
val tailInstructionDetector = TailInstructionDetector(subroutine)
traverseFollowingInstructions(
pseudocode.sinkInstruction,
order = TraversalOrder.BACKWARD
) { instruction ->
instruction.safeAs<KtElementInstruction>()?.element?.safeAs<KtExpression>()?.let { expression ->
trace.record(BindingContext.IS_TAIL_EXPRESSION_IN_SUSPEND_FUNCTION, expression)
}
if (instruction.accept(tailInstructionDetector))
TraverseInstructionResult.CONTINUE
else
TraverseInstructionResult.SKIP
}
}
}
@@ -895,13 +914,19 @@ class ControlFlowInformationProvider private constructor(
KtTryExpression::class.java, KtFunction::class.java, KtAnonymousInitializer::class.java
) is KtTryExpression
private fun CallInstruction.isTailCall(subroutine: KtElement = this@ControlFlowInformationProvider.subroutine) =
traverseFollowingInstructions(
this,
HashSet<Instruction>(),
TraversalOrder.FORWARD,
TailCallDetector(subroutine, this)
)
private fun CallInstruction.isTailCall(subroutine: KtElement = this@ControlFlowInformationProvider.subroutine): Boolean {
val tailInstructionDetector = TailInstructionDetector(subroutine)
return traverseFollowingInstructions(
this,
HashSet<Instruction>(),
TraversalOrder.FORWARD
) {
if (it == this@isTailCall || it.accept(tailInstructionDetector))
TraverseInstructionResult.CONTINUE
else
TraverseInstructionResult.HALT
}
}
private inline fun traverseCalls(crossinline onCall: (instruction: CallInstruction, resolvedCall: ResolvedCall<*>) -> Unit) {
pseudocode.traverse(TraversalOrder.FORWARD) { instruction ->
@@ -16,7 +16,6 @@
package org.jetbrains.kotlin.cfg;
import kotlin.jvm.functions.Function1;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.kotlin.cfg.pseudocode.instructions.Instruction;
import org.jetbrains.kotlin.cfg.pseudocode.instructions.InstructionVisitorWithResult;
@@ -27,21 +26,16 @@ import org.jetbrains.kotlin.cfg.pseudocode.instructions.jumps.ThrowExceptionInst
import org.jetbrains.kotlin.cfg.pseudocode.instructions.special.MarkInstruction;
import org.jetbrains.kotlin.cfg.pseudocode.instructions.special.SubroutineExitInstruction;
import org.jetbrains.kotlin.cfg.pseudocode.instructions.special.SubroutineSinkInstruction;
import org.jetbrains.kotlin.cfg.pseudocodeTraverser.TraverseInstructionResult;
import org.jetbrains.kotlin.psi.KtElement;
public class TailCallDetector extends InstructionVisitorWithResult<Boolean> implements Function1<Instruction, TraverseInstructionResult> {
/**
* Returns true when visited instruction may lie on a path from a tail-call-like operation to the sink of the subroutine
*/
public class TailInstructionDetector extends InstructionVisitorWithResult<Boolean> {
private final KtElement subroutine;
private final Instruction start;
public TailCallDetector(@NotNull KtElement subroutine, @NotNull Instruction start) {
public TailInstructionDetector(@NotNull KtElement subroutine) {
this.subroutine = subroutine;
this.start = start;
}
@Override
public TraverseInstructionResult invoke(@NotNull Instruction instruction) {
return instruction == start || instruction.accept(this) ? TraverseInstructionResult.CONTINUE : TraverseInstructionResult.HALT;
}
@Override
@@ -137,6 +137,7 @@ public interface BindingContext {
WritableSlice<Call, FunctionDescriptor> ENCLOSING_SUSPEND_FUNCTION_FOR_SUSPEND_FUNCTION_CALL = Slices.createSimpleSlice();
WritableSlice<FunctionDescriptor, Boolean> CONTAINS_NON_TAIL_SUSPEND_CALLS = Slices.createSimpleSetSlice();
WritableSlice<KtExpression, Boolean> IS_TAIL_EXPRESSION_IN_SUSPEND_FUNCTION = Slices.createSimpleSetSlice();
WritableSlice<VariableAccessorDescriptor, ResolvedCall<FunctionDescriptor>> DELEGATED_PROPERTY_RESOLVED_CALL = Slices.createSimpleSlice();
WritableSlice<VariableAccessorDescriptor, Call> DELEGATED_PROPERTY_CALL = Slices.createSimpleSlice();
@@ -0,0 +1,29 @@
// WITH_RUNTIME
// WITH_COROUTINES
import kotlin.coroutines.*
import kotlin.coroutines.intrinsics.*
suspend fun foo(x: Any): Int {
return if (x == "56") suspendHere() else 13
}
suspend fun suspendHere(): Int = suspendCoroutineOrReturn { x ->
x.resume(56)
SUSPENDED_MARKER
}
fun builder(c: suspend () -> Unit) {
c.startCoroutine(EmptyContinuation)
}
fun box(): String {
var result = -1
builder {
result = foo("56")
}
if (result != 56) return "fail 1: $result"
return "OK"
}
@@ -0,0 +1,33 @@
// WITH_RUNTIME
// WITH_COROUTINES
import kotlin.coroutines.*
import kotlin.coroutines.intrinsics.*
suspend fun foo(x: Any): Int {
return try {
suspendHere()
} catch (e: Throwable) {
13
}
}
suspend fun suspendHere(): Int = suspendCoroutineOrReturn { x ->
x.resume(56)
SUSPENDED_MARKER
}
fun builder(c: suspend () -> Unit) {
c.startCoroutine(EmptyContinuation)
}
fun box(): String {
var result = -1
builder {
result = foo("56")
}
if (result != 56) return "fail 1: $result"
return "OK"
}
@@ -0,0 +1,32 @@
// WITH_RUNTIME
// WITH_COROUTINES
import kotlin.coroutines.*
import kotlin.coroutines.intrinsics.*
suspend fun foo(x: Any): Int {
return when {
x == "56" -> suspendHere()
else -> 13
}
}
suspend fun suspendHere(): Int = suspendCoroutineOrReturn { x ->
x.resume(56)
SUSPENDED_MARKER
}
fun builder(c: suspend () -> Unit) {
c.startCoroutine(EmptyContinuation)
}
fun box(): String {
var result = -1
builder {
result = foo("56")
}
if (result != 56) return "fail 1: $result"
return "OK"
}
@@ -0,0 +1,21 @@
@kotlin.Metadata
public final class CoroutineUtilKt {
public final static @org.jetbrains.annotations.NotNull method handleExceptionContinuation(@org.jetbrains.annotations.NotNull p0: kotlin.jvm.functions.Function1): kotlin.coroutines.Continuation
public final static @org.jetbrains.annotations.NotNull method handleResultContinuation(@org.jetbrains.annotations.NotNull p0: kotlin.jvm.functions.Function1): kotlin.coroutines.Continuation
}
@kotlin.Metadata
public final class EmptyContinuation {
public final static field INSTANCE: EmptyContinuation
private method <init>(): void
public method resume(@org.jetbrains.annotations.Nullable p0: java.lang.Object): void
public method resumeWithException(@org.jetbrains.annotations.NotNull p0: java.lang.Throwable): void
}
@kotlin.Metadata
public final class SuspendWithIfKt {
public final static @org.jetbrains.annotations.NotNull method box(): java.lang.String
public final static method builder(@org.jetbrains.annotations.NotNull p0: kotlin.jvm.functions.Function1): void
public final static @org.jetbrains.annotations.Nullable method foo(@org.jetbrains.annotations.NotNull p0: java.lang.Object, @org.jetbrains.annotations.NotNull p1: kotlin.coroutines.Continuation): java.lang.Object
public final static @org.jetbrains.annotations.Nullable method suspendHere(@org.jetbrains.annotations.NotNull p0: kotlin.coroutines.Continuation): java.lang.Object
}
@@ -0,0 +1,21 @@
@kotlin.Metadata
public final class CoroutineUtilKt {
public final static @org.jetbrains.annotations.NotNull method handleExceptionContinuation(@org.jetbrains.annotations.NotNull p0: kotlin.jvm.functions.Function1): kotlin.coroutines.Continuation
public final static @org.jetbrains.annotations.NotNull method handleResultContinuation(@org.jetbrains.annotations.NotNull p0: kotlin.jvm.functions.Function1): kotlin.coroutines.Continuation
}
@kotlin.Metadata
public final class EmptyContinuation {
public final static field INSTANCE: EmptyContinuation
private method <init>(): void
public method resume(@org.jetbrains.annotations.Nullable p0: java.lang.Object): void
public method resumeWithException(@org.jetbrains.annotations.NotNull p0: java.lang.Throwable): void
}
@kotlin.Metadata
public final class SuspendWithTryCatchKt {
public final static @org.jetbrains.annotations.NotNull method box(): java.lang.String
public final static method builder(@org.jetbrains.annotations.NotNull p0: kotlin.jvm.functions.Function1): void
public final static @org.jetbrains.annotations.Nullable method foo(@org.jetbrains.annotations.NotNull p0: java.lang.Object, @org.jetbrains.annotations.NotNull p1: kotlin.coroutines.Continuation): java.lang.Object
public final static @org.jetbrains.annotations.Nullable method suspendHere(@org.jetbrains.annotations.NotNull p0: kotlin.coroutines.Continuation): java.lang.Object
}
@@ -0,0 +1,21 @@
@kotlin.Metadata
public final class CoroutineUtilKt {
public final static @org.jetbrains.annotations.NotNull method handleExceptionContinuation(@org.jetbrains.annotations.NotNull p0: kotlin.jvm.functions.Function1): kotlin.coroutines.Continuation
public final static @org.jetbrains.annotations.NotNull method handleResultContinuation(@org.jetbrains.annotations.NotNull p0: kotlin.jvm.functions.Function1): kotlin.coroutines.Continuation
}
@kotlin.Metadata
public final class EmptyContinuation {
public final static field INSTANCE: EmptyContinuation
private method <init>(): void
public method resume(@org.jetbrains.annotations.Nullable p0: java.lang.Object): void
public method resumeWithException(@org.jetbrains.annotations.NotNull p0: java.lang.Throwable): void
}
@kotlin.Metadata
public final class SuspendWithWhenKt {
public final static @org.jetbrains.annotations.NotNull method box(): java.lang.String
public final static method builder(@org.jetbrains.annotations.NotNull p0: kotlin.jvm.functions.Function1): void
public final static @org.jetbrains.annotations.Nullable method foo(@org.jetbrains.annotations.NotNull p0: java.lang.Object, @org.jetbrains.annotations.NotNull p1: kotlin.coroutines.Continuation): java.lang.Object
public final static @org.jetbrains.annotations.Nullable method suspendHere(@org.jetbrains.annotations.NotNull p0: kotlin.coroutines.Continuation): java.lang.Object
}
@@ -5135,6 +5135,33 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
}
}
@TestMetadata("compiler/testData/codegen/box/coroutines/tailOperations")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
public static class TailOperations extends AbstractIrBlackBoxCodegenTest {
public void testAllFilesPresentInTailOperations() throws Exception {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/coroutines/tailOperations"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true);
}
@TestMetadata("suspendWithIf.kt")
public void testSuspendWithIf() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/tailOperations/suspendWithIf.kt");
doTest(fileName);
}
@TestMetadata("suspendWithTryCatch.kt")
public void testSuspendWithTryCatch() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/tailOperations/suspendWithTryCatch.kt");
doTest(fileName);
}
@TestMetadata("suspendWithWhen.kt")
public void testSuspendWithWhen() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/tailOperations/suspendWithWhen.kt");
doTest(fileName);
}
}
@TestMetadata("compiler/testData/codegen/box/coroutines/unitTypeReturn")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
@@ -5135,6 +5135,33 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
}
}
@TestMetadata("compiler/testData/codegen/box/coroutines/tailOperations")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
public static class TailOperations extends AbstractBlackBoxCodegenTest {
public void testAllFilesPresentInTailOperations() throws Exception {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/coroutines/tailOperations"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true);
}
@TestMetadata("suspendWithIf.kt")
public void testSuspendWithIf() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/tailOperations/suspendWithIf.kt");
doTest(fileName);
}
@TestMetadata("suspendWithTryCatch.kt")
public void testSuspendWithTryCatch() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/tailOperations/suspendWithTryCatch.kt");
doTest(fileName);
}
@TestMetadata("suspendWithWhen.kt")
public void testSuspendWithWhen() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/tailOperations/suspendWithWhen.kt");
doTest(fileName);
}
}
@TestMetadata("compiler/testData/codegen/box/coroutines/unitTypeReturn")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
@@ -5135,6 +5135,33 @@ public class LightAnalysisModeCodegenTestGenerated extends AbstractLightAnalysis
}
}
@TestMetadata("compiler/testData/codegen/box/coroutines/tailOperations")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
public static class TailOperations extends AbstractLightAnalysisModeCodegenTest {
public void testAllFilesPresentInTailOperations() throws Exception {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/coroutines/tailOperations"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true);
}
@TestMetadata("suspendWithIf.kt")
public void testSuspendWithIf() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/tailOperations/suspendWithIf.kt");
doTest(fileName);
}
@TestMetadata("suspendWithTryCatch.kt")
public void testSuspendWithTryCatch() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/tailOperations/suspendWithTryCatch.kt");
doTest(fileName);
}
@TestMetadata("suspendWithWhen.kt")
public void testSuspendWithWhen() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/tailOperations/suspendWithWhen.kt");
doTest(fileName);
}
}
@TestMetadata("compiler/testData/codegen/box/coroutines/unitTypeReturn")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
@@ -5862,6 +5862,33 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest {
}
}
@TestMetadata("compiler/testData/codegen/box/coroutines/tailOperations")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
public static class TailOperations extends AbstractJsCodegenBoxTest {
public void testAllFilesPresentInTailOperations() throws Exception {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/coroutines/tailOperations"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JS, true);
}
@TestMetadata("suspendWithIf.kt")
public void testSuspendWithIf() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/tailOperations/suspendWithIf.kt");
doTest(fileName);
}
@TestMetadata("suspendWithTryCatch.kt")
public void testSuspendWithTryCatch() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/tailOperations/suspendWithTryCatch.kt");
doTest(fileName);
}
@TestMetadata("suspendWithWhen.kt")
public void testSuspendWithWhen() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/tailOperations/suspendWithWhen.kt");
doTest(fileName);
}
}
@TestMetadata("compiler/testData/codegen/box/coroutines/unitTypeReturn")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)