From f1c66fa6b095843e31b007f5591e8d3d59e67898 Mon Sep 17 00:00:00 2001 From: Andrey Breslav Date: Sun, 5 Oct 2014 22:18:15 +0400 Subject: [PATCH] Assertions on approximation of platform types to non-null types - for most expressions (ExpressionCodegen.genQualified) --- .../org/jetbrains/jet/codegen/AsmUtil.java | 31 ++++++++++- .../jet/codegen/ExpressionCodegen.java | 10 +++- .../jet/lang/resolve/BindingContext.java | 2 + .../lang/types/expressions/DataFlowUtils.java | 49 +++++++++++++++--- .../testData/codegen/notNullAssertions/A.java | 10 ++++ .../notNullAssertions/AssertionChecker.kt | 35 +++++++++---- .../codegen/notNullAssertions/arrayListGet.kt | 4 +- .../GenerateNotNullAssertionsTest.java | 4 +- .../jetbrains/jet/lang/types/flexibleTypes.kt | 51 ++++++++++++++++++- .../src/kotlin/jvm/internal/Intrinsics.java | 7 +++ spec-docs/flexible-java-types.md | 14 ++++- 11 files changed, 193 insertions(+), 24 deletions(-) diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/AsmUtil.java b/compiler/backend/src/org/jetbrains/jet/codegen/AsmUtil.java index 78edd474512..b262d843388 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/AsmUtil.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/AsmUtil.java @@ -37,7 +37,9 @@ import org.jetbrains.jet.lang.resolve.java.*; import org.jetbrains.jet.lang.resolve.java.descriptor.JavaCallableMemberDescriptor; import org.jetbrains.jet.lang.resolve.kotlin.PackagePartClassUtils; import org.jetbrains.jet.lang.resolve.name.FqName; +import org.jetbrains.jet.lang.types.Approximation; import org.jetbrains.jet.lang.types.JetType; +import org.jetbrains.jet.lang.types.TypesPackage; import org.jetbrains.jet.lang.types.lang.KotlinBuiltIns; import org.jetbrains.jet.lexer.JetTokens; import org.jetbrains.org.objectweb.asm.*; @@ -608,12 +610,15 @@ public class AsmUtil { @NotNull CallableDescriptor descriptor, @NotNull String assertMethodToCall ) { + // Assertions are generated elsewhere for platform types + if (JavaPackage.getPLATFORM_TYPES()) return; + if (!state.isCallAssertionsEnabled()) return; if (!isDeclaredInJava(descriptor)) return; JetType type = descriptor.getReturnType(); - if (type == null || isNullableType(type)) return; + if (type == null || isNullableType(TypesPackage.lowerIfFlexible(type))) return; Type asmType = state.getTypeMapper().mapReturnType(descriptor); if (asmType.getSort() == Type.OBJECT || asmType.getSort() == Type.ARRAY) { @@ -625,6 +630,30 @@ public class AsmUtil { } } + @NotNull + public static StackValue genNotNullAssertions( + @NotNull GenerationState state, + @NotNull final StackValue stackValue, + @Nullable final Approximation.Info approximationInfo + ) { + if (!state.isCallAssertionsEnabled()) return stackValue; + if (approximationInfo == null || !TypesPackage.assertNotNull(approximationInfo)) return stackValue; + + return new StackValue(stackValue.type) { + + @Override + public void put(Type type, InstructionAdapter v) { + stackValue.put(type, v); + if (type.getSort() == Type.OBJECT || type.getSort() == Type.ARRAY) { + v.dup(); + v.visitLdcInsn(approximationInfo.getMessage()); + v.invokestatic("kotlin/jvm/internal/Intrinsics", "checkExpressionValueIsNotNull", + "(Ljava/lang/Object;Ljava/lang/String;)V", false); + } + } + }; + } + private static boolean isDeclaredInJava(@NotNull CallableDescriptor callableDescriptor) { CallableDescriptor descriptor = callableDescriptor; while (true) { diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java index 847ec6302ff..20435e4e5b0 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java @@ -61,6 +61,7 @@ import org.jetbrains.jet.lang.resolve.java.descriptor.SamConstructorDescriptor; import org.jetbrains.jet.lang.resolve.java.jvmSignature.JvmMethodSignature; import org.jetbrains.jet.lang.resolve.name.Name; import org.jetbrains.jet.lang.resolve.scopes.receivers.*; +import org.jetbrains.jet.lang.types.Approximation; import org.jetbrains.jet.lang.types.JetType; import org.jetbrains.jet.lang.types.TypeUtils; import org.jetbrains.jet.lang.types.checker.JetTypeChecker; @@ -242,7 +243,14 @@ public class ExpressionCodegen extends JetVisitor implem } } - return selector.accept(visitor, receiver); + StackValue stackValue = selector.accept(visitor, receiver); + + Approximation.Info approximationInfo = null; + if (selector instanceof JetExpression) { + approximationInfo = bindingContext.get(BindingContext.EXPRESSION_RESULT_APPROXIMATION, (JetExpression) selector); + } + + return genNotNullAssertions(state, stackValue, approximationInfo); } catch (ProcessCanceledException e) { throw e; diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BindingContext.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BindingContext.java index 05bb76f9233..c3083b5f650 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BindingContext.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BindingContext.java @@ -35,6 +35,7 @@ import org.jetbrains.jet.lang.resolve.name.FqName; import org.jetbrains.jet.lang.resolve.name.FqNameUnsafe; import org.jetbrains.jet.lang.resolve.scopes.JetScope; import org.jetbrains.jet.lang.resolve.scopes.receivers.Qualifier; +import org.jetbrains.jet.lang.types.Approximation; import org.jetbrains.jet.lang.types.DeferredType; import org.jetbrains.jet.lang.types.JetType; import org.jetbrains.jet.lang.types.expressions.CaptureKind; @@ -83,6 +84,7 @@ public interface BindingContext { WritableSlice EXPRESSION_TYPE = new BasicWritableSlice(DO_NOTHING); WritableSlice EXPECTED_EXPRESSION_TYPE = new BasicWritableSlice(DO_NOTHING); WritableSlice EXPRESSION_DATA_FLOW_INFO = new BasicWritableSlice(DO_NOTHING); + WritableSlice EXPRESSION_RESULT_APPROXIMATION = new BasicWritableSlice(DO_NOTHING); WritableSlice DATAFLOW_INFO_AFTER_CONDITION = Slices.createSimpleSlice(); /** diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/DataFlowUtils.java b/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/DataFlowUtils.java index 7ea9dfa8aeb..e25b1f67e21 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/DataFlowUtils.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/DataFlowUtils.java @@ -17,6 +17,7 @@ package org.jetbrains.jet.lang.types.expressions; import com.intellij.openapi.util.Ref; +import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.tree.IElementType; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -33,13 +34,13 @@ import org.jetbrains.jet.lang.resolve.calls.smartcasts.SmartCastUtils; import org.jetbrains.jet.lang.resolve.constants.CompileTimeConstant; import org.jetbrains.jet.lang.resolve.constants.CompileTimeConstantChecker; import org.jetbrains.jet.lang.resolve.constants.IntegerValueTypeConstant; -import org.jetbrains.jet.lang.types.JetType; -import org.jetbrains.jet.lang.types.JetTypeInfo; -import org.jetbrains.jet.lang.types.TypeUtils; +import org.jetbrains.jet.lang.types.*; import org.jetbrains.jet.lang.types.checker.JetTypeChecker; import org.jetbrains.jet.lang.types.lang.KotlinBuiltIns; import org.jetbrains.jet.lexer.JetTokens; +import java.util.Set; + import static org.jetbrains.jet.lang.diagnostics.Errors.*; import static org.jetbrains.jet.lang.resolve.calls.context.ContextDependency.INDEPENDENT; import static org.jetbrains.jet.lang.types.TypeUtils.*; @@ -157,14 +158,47 @@ public class DataFlowUtils { } @Nullable - public static JetType checkType(@Nullable JetType expressionType, @NotNull JetExpression expressionToCheck, - @NotNull JetType expectedType, @NotNull DataFlowInfo dataFlowInfo, @NotNull BindingTrace trace + public static JetType checkType(@Nullable final JetType expressionType, @NotNull JetExpression expressionToCheck, + @NotNull JetType expectedType, @NotNull final DataFlowInfo dataFlowInfo, @NotNull final BindingTrace trace ) { - JetExpression expression = JetPsiUtil.safeDeparenthesize(expressionToCheck, false); + final JetExpression expression = JetPsiUtil.safeDeparenthesize(expressionToCheck, false); recordExpectedType(trace, expression, expectedType); - if (expressionType == null || noExpectedType(expectedType) || !expectedType.getConstructor().isDenotable() || + if (expressionType == null) return null; + + if (noExpectedType(expectedType) || !expectedType.getConstructor().isDenotable() || JetTypeChecker.DEFAULT.isSubtypeOf(expressionType, expectedType)) { + + if (!noExpectedType(expectedType)) { + Approximation.Info approximationInfo = TypesPackage.getApproximationTo(expressionType, expectedType, + new Approximation.DataFlowExtras() { + private DataFlowValue getDataFlowValue() { + return DataFlowValueFactory.createDataFlowValue(expression, expressionType, trace.getBindingContext()); + } + + @Override + public boolean getCanBeNull() { + return dataFlowInfo.getNullability(getDataFlowValue()).canBeNull(); + } + + @NotNull + @Override + public Set getPossibleTypes() { + return dataFlowInfo.getPossibleTypes(getDataFlowValue()); + } + + @NotNull + @Override + public String getPresentableText() { + return StringUtil.trimMiddle(expression.getText(), 50); + } + } + ); + if (approximationInfo != null) { + trace.record(BindingContext.EXPRESSION_RESULT_APPROXIMATION, expression, approximationInfo); + } + } + return expressionType; } @@ -178,6 +212,7 @@ public class DataFlowUtils { } DataFlowValue dataFlowValue = DataFlowValueFactory.createDataFlowValue(expression, expressionType, trace.getBindingContext()); + for (JetType possibleType : dataFlowInfo.getPossibleTypes(dataFlowValue)) { if (JetTypeChecker.DEFAULT.isSubtypeOf(possibleType, expectedType)) { SmartCastUtils.recordCastOrError(expression, possibleType, trace, dataFlowValue.isStableIdentifier(), false); diff --git a/compiler/testData/codegen/notNullAssertions/A.java b/compiler/testData/codegen/notNullAssertions/A.java index 8e9bd8ae717..631e18c33ac 100644 --- a/compiler/testData/codegen/notNullAssertions/A.java +++ b/compiler/testData/codegen/notNullAssertions/A.java @@ -32,4 +32,14 @@ public class A { public Object get(Object o) { return null; } + + public A a() { return this; } + + public static class B { + public static B b() { return null; } + } + + public static class C { + public static C c() { return null; } + } } diff --git a/compiler/testData/codegen/notNullAssertions/AssertionChecker.kt b/compiler/testData/codegen/notNullAssertions/AssertionChecker.kt index da58066e402..40eb76742af 100644 --- a/compiler/testData/codegen/notNullAssertions/AssertionChecker.kt +++ b/compiler/testData/codegen/notNullAssertions/AssertionChecker.kt @@ -1,5 +1,5 @@ class AssertionChecker(val illegalStateExpected: Boolean) { - fun invoke(name: String, f: () -> Unit) { + fun invoke(name: String, f: () -> Any) { try { f() } catch (e: IllegalStateException) { @@ -44,15 +44,32 @@ fun checkAssertions(illegalStateExpected: Boolean) { // binary expression check("plus") { A() + A() } - // postfix expression - check("inc") { var a = A(); a++ } - - // prefix expression - check("inc") { var a = A(); ++a } - // field - check("NULL") { val a = A().NULL } + check("NULL") { A().NULL } // static field - check("STATIC_NULL") { val a = A.STATIC_NULL } + check("STATIC_NULL") { A.STATIC_NULL } + + // postfix expression + // TODO: +// check("inc") { var a = A().a(); a++ } + + // prefix expression + check("inc-b") { var a = A.B.b(); a++ } + + // prefix expression + check("inc-c") { var a = A.C.c(); a++ } + + // prefix expression + check("inc") { var a = A().a(); ++a } + + // prefix expression + check("inc-b") { var a = A.B.b(); ++a } + + // prefix expression + // TODO: +// check("inc-c") { var a = A.C.c(); ++a } } + +fun A.C.inc(): A.C = A.C() +fun T.inc(): T = null as T diff --git a/compiler/testData/codegen/notNullAssertions/arrayListGet.kt b/compiler/testData/codegen/notNullAssertions/arrayListGet.kt index faa7b14f68b..bd32e460fc1 100644 --- a/compiler/testData/codegen/notNullAssertions/arrayListGet.kt +++ b/compiler/testData/codegen/notNullAssertions/arrayListGet.kt @@ -1,8 +1,8 @@ import java.util.ArrayList -fun foo() { +fun foo(): Any { val a = ArrayList() - a.get(0) + return a.get(0) } fun bar(a: ArrayList) { diff --git a/compiler/tests/org/jetbrains/jet/codegen/GenerateNotNullAssertionsTest.java b/compiler/tests/org/jetbrains/jet/codegen/GenerateNotNullAssertionsTest.java index d450773a711..c46db0b03b4 100644 --- a/compiler/tests/org/jetbrains/jet/codegen/GenerateNotNullAssertionsTest.java +++ b/compiler/tests/org/jetbrains/jet/codegen/GenerateNotNullAssertionsTest.java @@ -120,7 +120,7 @@ public class GenerateNotNullAssertionsTest extends CodegenTestCase { loadFile("notNullAssertions/arrayListGet.kt"); String text = generateToText(); - assertTrue(text.contains("checkReturnedValueIsNotNull")); + assertTrue(text.contains("checkExpressionValueIsNotNull")); assertTrue(text.contains("checkParameterIsNotNull")); } @@ -131,7 +131,7 @@ public class GenerateNotNullAssertionsTest extends CodegenTestCase { loadFile("notNullAssertions/javaMultipleSubstitutions.kt"); String text = generateToText(); - assertEquals(3, StringUtil.getOccurrenceCount(text, "checkReturnedValueIsNotNull")); + assertEquals(3, StringUtil.getOccurrenceCount(text, "checkExpressionValueIsNotNull")); assertEquals(3, StringUtil.getOccurrenceCount(text, "checkParameterIsNotNull")); } diff --git a/core/descriptors/src/org/jetbrains/jet/lang/types/flexibleTypes.kt b/core/descriptors/src/org/jetbrains/jet/lang/types/flexibleTypes.kt index 2f91c2745b6..b218a9cdb6c 100644 --- a/core/descriptors/src/org/jetbrains/jet/lang/types/flexibleTypes.kt +++ b/core/descriptors/src/org/jetbrains/jet/lang/types/flexibleTypes.kt @@ -17,6 +17,7 @@ package org.jetbrains.jet.lang.types import org.jetbrains.jet.lang.types.checker.JetTypeChecker +import org.jetbrains.jet.lang.types.Approximation.DataFlowExtras public trait Flexibility : TypeCapability { // lowerBound is a subtype of upperBound @@ -29,15 +30,49 @@ public fun JetType.isFlexible(): Boolean = this.getCapability(javaClass())!! public fun JetType.lowerIfFlexible(): JetType = if (this.isFlexible()) this.flexibility().getLowerBound() else this +public fun JetType.upperIfFlexible(): JetType = if (this.isFlexible()) this.flexibility().getUpperBound() else this public trait NullAwareness : TypeCapability { public fun makeNullableAsSpecified(nullable: Boolean): JetType } +public trait Approximation : TypeCapability { + public class Info(val from: JetType, val to: JetType, val message: String) + public trait DataFlowExtras { + object EMPTY : DataFlowExtras { + override val canBeNull: Boolean get() = true + override val possibleTypes: Set get() = setOf() + override val presentableText: String = "" + } + + class OnlyMessage(message: String) : DataFlowExtras { + override val canBeNull: Boolean get() = true + override val possibleTypes: Set get() = setOf() + override val presentableText: String = message + } + + val canBeNull: Boolean + val possibleTypes: Set + val presentableText: String + } + + public fun approximateToExpectedType(expectedType: JetType, dataFlowExtras: DataFlowExtras): Info? +} + +fun Approximation.Info.assertNotNull(): Boolean { + return from.upperIfFlexible().isNullable() && !TypeUtils.isNullableType(to) +} + +public fun JetType.getApproximationTo( + expectedType: JetType, + extras: Approximation.DataFlowExtras = Approximation.DataFlowExtras.EMPTY +): Approximation.Info? = this.getCapability(javaClass())?.approximateToExpectedType(expectedType, extras) + + public open class DelegatingFlexibleType protected ( private val _lowerBound: JetType, private val _upperBound: JetType -) : DelegatingType(), NullAwareness, Flexibility { +) : DelegatingType(), NullAwareness, Flexibility, Approximation { class object { public fun create(lowerBound: JetType, upperBound: JetType): JetType { if (lowerBound == upperBound) return lowerBound @@ -65,6 +100,20 @@ public open class DelegatingFlexibleType protected ( return create(TypeUtils.makeNullableAsSpecified(_lowerBound, nullable), TypeUtils.makeNullableAsSpecified(_upperBound, nullable)) } + override fun approximateToExpectedType(expectedType: JetType, dataFlowExtras: Approximation.DataFlowExtras): Approximation.Info? { + // val foo: Any? = foo() : Foo! + if (JetTypeChecker.DEFAULT.isSubtypeOf(getUpperBound(), expectedType)) return null + + // if (foo : Foo! != null) { + // val bar: Any = foo + // } + if (!dataFlowExtras.canBeNull && JetTypeChecker.DEFAULT.isSubtypeOf(TypeUtils.makeNotNullable(getUpperBound()), expectedType)) return null + + // TODO: maybe check possibleTypes to avoid extra approximations + + return Approximation.Info(this, expectedType, dataFlowExtras.presentableText) + } + override fun getDelegate() = _lowerBound override fun toString() = "('$_lowerBound'..'$_upperBound')" diff --git a/core/runtime.jvm/src/kotlin/jvm/internal/Intrinsics.java b/core/runtime.jvm/src/kotlin/jvm/internal/Intrinsics.java index efeab4ff4c4..efc55a73d3c 100644 --- a/core/runtime.jvm/src/kotlin/jvm/internal/Intrinsics.java +++ b/core/runtime.jvm/src/kotlin/jvm/internal/Intrinsics.java @@ -37,6 +37,13 @@ public class Intrinsics { throw new KotlinNullPointerException(); } + public static void checkExpressionValueIsNotNull(Object value, String message) { + if (value == null) { + IllegalStateException exception = new IllegalStateException(message + " must not be null"); + throw sanitizeStackTrace(exception); + } + } + public static void checkReturnedValueIsNotNull(Object value, String className, String methodName) { if (value == null) { IllegalStateException exception = diff --git a/spec-docs/flexible-java-types.md b/spec-docs/flexible-java-types.md index 336ce9f1c15..7e449bfa3b0 100644 --- a/spec-docs/flexible-java-types.md +++ b/spec-docs/flexible-java-types.md @@ -102,4 +102,16 @@ val arr: Array = javaArrayMethod() // assert value "is Bar[]" `a++` stands for `a = a.inc()`, so - check a to satisfy the `a.inc()` conditions for receiver -- check `a.inc()` result for assignability to `a` \ No newline at end of file +- check `a.inc()` result for assignability to `a` + +## Assertion Generation + +Constructs in question: anything that provides an expected type, i.e. + - assignments + - parameter default values + - delegation by: supertypes and properties + - dereferencing: x.foo + - all kinds of calls (foo, foo(), x[], x foo y, x + y, x++, x += 3, for loop, multi-declarations, invoke-convention, ...) + - explicit expected type (foo: Bar) + - for booleans: if (foo), foo || bar, foo && bar (!foo is a call) + - argument of throw \ No newline at end of file