From 00a78fce0cf7cd84f66a7bddac2e59e645f6ef56 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Thu, 27 Aug 2015 16:25:18 +0300 Subject: [PATCH] Get rid of special logic when check receiver Instead of manually checking nullability when reporing UNSAFE_CALL_ERROR just check if receiver type is subtype of receiver parameter Make it in two steps 1. Check subtype with respect to smart casts but ignoring nullability (if it's not satisfied -> ERROR) 2. Check subtype with respect to nullability and smartcasts (record latter if needed) --- .../kotlin/resolve/calls/CandidateResolver.kt | 50 +++++++++----- .../calls/smartcasts/SmartCastManager.java | 68 +------------------ idea/testData/checker/infos/SmartCasts.kt | 24 +++++-- .../checker/infos/SmartCastsWithSafeAccess.kt | 2 +- 4 files changed, 53 insertions(+), 91 deletions(-) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CandidateResolver.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CandidateResolver.kt index cab325c18df..e8aca3e0984 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CandidateResolver.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CandidateResolver.kt @@ -440,33 +440,49 @@ public class CandidateResolver( if (TypeUtils.dependsOnTypeParameters(receiverParameter.getType(), candidateDescriptor.getTypeParameters())) return SUCCESS val safeAccess = isExplicitReceiver && !implicitInvokeCheck && candidateCall.getCall().isExplicitSafeCall() - val isSubtypeBySmartCast = smartCastManager.isSubTypeBySmartCastIgnoringNullability( + val isSubtypeBySmartCastIgnoringNullability = smartCastManager.isSubTypeBySmartCastIgnoringNullability( receiverArgument, receiverParameter.getType(), this) - if (!isSubtypeBySmartCast) { + + if (!isSubtypeBySmartCastIgnoringNullability) { tracing.wrongReceiverType(trace, receiverParameter, receiverArgument) return OTHER_ERROR } - if (!smartCastManager.recordSmartCastIfNecessary(receiverArgument, receiverParameter.getType(), this, safeAccess)) { - return OTHER_ERROR - } - val receiverArgumentType = receiverArgument.getType() + // Here we know that receiver is OK ignoring nullability and check that nullability is OK too + // Doing it simply as full subtyping check (receiverValueType <: receiverParameterType) + val expectedReceiverParameterType = if (safeAccess) TypeUtils.makeNullable(receiverParameter.type) else receiverParameter.type + val smartCastNeeded = !ArgumentTypeResolver.isSubtypeOfForArgumentType(receiverArgument.type, expectedReceiverParameterType) + var reportUnsafeCall = false - val bindingContext = trace.getBindingContext() - // We just checked isSubtypeIgnoringNullability(receiverType, parameter) - // Here we do almost the same thing as isSubtypeOf does (but pay attention only to nullability): - // - find corresponding supertype, than check it's nullability is not weaker than parameter's one - // - if latter failed, check whether value can be smart cast - val commonReceiverType = TypeCheckingProcedure.findCorrespondingSupertype(receiverArgumentType, receiverParameter.type) - if (!safeAccess && !receiverParameter.type.isMarkedNullable && (commonReceiverType?.isMarkedNullable ?: false)) { - if (!smartCastManager.recordSmartCastToNotNullIfPossible(receiverArgument, this)) { - tracing.unsafeCall(trace, receiverArgumentType, implicitInvokeCheck) - return UNSAFE_CALL_ERROR + if (smartCastNeeded) { + // Look if smart cast has some useful nullability info + val expression = (receiverArgument as? ExpressionReceiver)?.expression + val dataFlowValue = DataFlowValueFactory.createDataFlowValue(receiverArgument, this) + + val smartCastResult = smartCastManager.checkAndRecordPossibleCast( + dataFlowValue, expectedReceiverParameterType, expression, this, /*recordType =*/ true + ) + + if (smartCastResult == null) { + reportUnsafeCall = true + } + else if (!smartCastResult.isCorrect) { + // Error about unstable smart cast reported within checkAndRecordPossibleCast + return OTHER_ERROR } } + + val receiverArgumentType = receiverArgument.type + + if (reportUnsafeCall) { + tracing.unsafeCall(trace, receiverArgumentType, implicitInvokeCheck) + return UNSAFE_CALL_ERROR + } + + val bindingContext = trace.bindingContext val receiverValue = DataFlowValueFactory.createDataFlowValue(receiverArgument, bindingContext, scope.getContainingDeclaration()) if (safeAccess && !dataFlowInfo.getNullability(receiverValue).canBeNull()) { - tracing.unnecessarySafeCall(trace, receiverArgumentType) + tracing.unnecessarySafeCall(trace, receiverArgument.type) } additionalTypeCheckers.forEach { it.checkReceiver(receiverParameter, receiverArgument, safeAccess, this) } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/SmartCastManager.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/SmartCastManager.java index dc2b0fb7339..b5ba038ed31 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/SmartCastManager.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/SmartCastManager.java @@ -153,31 +153,7 @@ public class SmartCastManager { return intersection; } - // Returns false when we need smart cast but cannot do it, otherwise true - public boolean recordSmartCastIfNecessary( - @NotNull ReceiverValue receiver, - @NotNull JetType receiverParameterType, - @NotNull ResolutionContext context, - boolean safeAccess - ) { - if (!(receiver instanceof ExpressionReceiver)) return true; - - receiverParameterType = safeAccess ? TypeUtils.makeNullable(receiverParameterType) : receiverParameterType; - if (ArgumentTypeResolver.isSubtypeOfForArgumentType(receiver.getType(), receiverParameterType)) { - return true; - } - - Collection smartCastTypesExcludingReceiver = getSmartCastVariantsExcludingReceiver(context, receiver); - JetType smartCastSubType = getSmartCastSubType(receiverParameterType, smartCastTypesExcludingReceiver); - if (smartCastSubType == null) return true; - - JetExpression expression = ((ExpressionReceiver) receiver).getExpression(); - DataFlowValue dataFlowValue = DataFlowValueFactory.createDataFlowValue(receiver, context); - - return recordCastOrError(expression, smartCastSubType, context.trace, dataFlowValue.isPredictable(), true); - } - - public boolean recordCastOrError( + private static void recordCastOrError( @NotNull JetExpression expression, @NotNull JetType type, @NotNull BindingTrace trace, @@ -195,7 +171,6 @@ public class SmartCastManager { else { trace.report(SMARTCAST_IMPOSSIBLE.on(expression, type, expression.getText())); } - return canBeCast; } @Nullable @@ -247,45 +222,4 @@ public class SmartCastManager { return null; } - - public boolean recordSmartCastToNotNullIfPossible( - @NotNull ReceiverValue receiver, - @NotNull ResolutionContext context - ) { - if (!TypeUtils.isNullableType(receiver.getType())) return true; - - DataFlowValue dataFlowValue = DataFlowValueFactory.createDataFlowValue( - receiver, - context.trace.getBindingContext(), - context.scope.getContainingDeclaration() - ); - - if (dataFlowValue == null) return false; - - if (!context.dataFlowInfo.getNullability(dataFlowValue).canBeNull()) { - JetExpression receiverExpression = getReceiverExpression(receiver); - - // report smart cast only on predictable expressions that were not reported before - if (receiverExpression != null - && dataFlowValue.isPredictable() - && context.trace.getBindingContext().get(SMARTCAST, receiverExpression) == null) { - recordCastOrError( - receiverExpression, receiver.getType(), context.trace, - /* canBeCast = */ true, /* recordExpressionType = */ false - ); - } - - return true; - } - - return !context.dataFlowInfo.getNullability(dataFlowValue).canBeNull(); - } - - @Nullable - private static JetExpression getReceiverExpression(@NotNull ReceiverValue value) { - if (value instanceof ExpressionReceiver) { - return ((ExpressionReceiver) value).getExpression(); - } - return null; - } } diff --git a/idea/testData/checker/infos/SmartCasts.kt b/idea/testData/checker/infos/SmartCasts.kt index 72266b9ad49..8441d34ec0c 100644 --- a/idea/testData/checker/infos/SmartCasts.kt +++ b/idea/testData/checker/infos/SmartCasts.kt @@ -11,7 +11,7 @@ fun f9(a : A?) { a?.bar() if (a is B) { a.bar() - a.foo() + a.foo() } a?.foo() a?.bar() @@ -26,7 +26,19 @@ fun f9(a : A?) { return; } a.bar() - a.foo() + a.foo() +} + +fun fAny(a : Any?) { + if (a is B) { + a.bar() + a.foo() + } + if (!(a is B)) { + return; + } + a.bar() + a.foo() } fun f10(a : A?) { @@ -80,7 +92,7 @@ fun f12(a : A?) { fun f13(a : A?) { if (a is B) { - a.foo() + a.foo() a.bar() } else { @@ -93,12 +105,12 @@ fun f13(a : A?) { a?.foo() } else { - a.foo() + a.foo() } a?.foo() - if (a is B && a.foo() == Unit) { - a.foo() + if (a is B && a.foo() == Unit) { + a.foo() a.bar() } else { diff --git a/idea/testData/checker/infos/SmartCastsWithSafeAccess.kt b/idea/testData/checker/infos/SmartCastsWithSafeAccess.kt index 19809df76c0..d8cf1fdd1c6 100644 --- a/idea/testData/checker/infos/SmartCastsWithSafeAccess.kt +++ b/idea/testData/checker/infos/SmartCastsWithSafeAccess.kt @@ -16,7 +16,7 @@ fun test(a: A?) { } if (a is B && a is C) { - a.foo() + a.foo() } if (a is B? && a is C?) {