From 0b33e9430baa7fb16dfc2c89aff7412a7305c12e Mon Sep 17 00:00:00 2001 From: Pavel Kirpichenkov Date: Wed, 20 May 2020 12:49:38 +0300 Subject: [PATCH] [NI] Report unstable smart cast directly instead of using SmartCastManager Fix compilation errors, revealed by this fix. SmartCastManager is unnecessary for error reporting, intermediate diagnostics from the NI contain all required infromation. When SmartCastManager is used it leads to missing unstable smart casts in case of expressions with captured types. This happens, because data flow info is recorded for original expression without captured types, which is used as a key. DataFlowValues created from receivers with captured types can't be used to retrieve that info. ^KT-39010 Fixed --- ...irOldFrontendDiagnosticsTestGenerated.java | 10 ++++++++ .../kotlin/fir/backend/ConversionUtils.kt | 12 ++++----- ...egerLiteralTypeApproximationTransformer.kt | 4 +-- .../impl/AbstractFirUseSiteMemberScope.kt | 2 +- .../expressions/impl/FirWhenExpressionImpl.kt | 5 ++-- .../tree/generator/printer/implementation.kt | 5 ++-- .../DiagnosticReporterByTrackingStrategy.kt | 16 ++++++++++-- .../tests/smartCasts/inference/kt39010.fir.kt | 11 ++++++++ .../tests/smartCasts/inference/kt39010.kt | 11 ++++++++ .../tests/smartCasts/inference/kt39010.txt | 18 +++++++++++++ .../smartCasts/inference/kt39010_2.fir.kt | 16 ++++++++++++ .../tests/smartCasts/inference/kt39010_2.kt | 16 ++++++++++++ .../tests/smartCasts/inference/kt39010_2.txt | 25 +++++++++++++++++++ .../checkers/DiagnosticsTestGenerated.java | 10 ++++++++ .../DiagnosticsUsingJavacTestGenerated.java | 10 ++++++++ 15 files changed, 156 insertions(+), 15 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/smartCasts/inference/kt39010.fir.kt create mode 100644 compiler/testData/diagnostics/tests/smartCasts/inference/kt39010.kt create mode 100644 compiler/testData/diagnostics/tests/smartCasts/inference/kt39010.txt create mode 100644 compiler/testData/diagnostics/tests/smartCasts/inference/kt39010_2.fir.kt create mode 100644 compiler/testData/diagnostics/tests/smartCasts/inference/kt39010_2.kt create mode 100644 compiler/testData/diagnostics/tests/smartCasts/inference/kt39010_2.txt diff --git a/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirOldFrontendDiagnosticsTestGenerated.java index 96ff152e587..8b68a7ef490 100644 --- a/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirOldFrontendDiagnosticsTestGenerated.java @@ -21545,6 +21545,16 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirOldFronte runTest("compiler/testData/diagnostics/tests/smartCasts/inference/kt29767.kt"); } + @TestMetadata("kt39010.kt") + public void testKt39010() throws Exception { + runTest("compiler/testData/diagnostics/tests/smartCasts/inference/kt39010.kt"); + } + + @TestMetadata("kt39010_2.kt") + public void testKt39010_2() throws Exception { + runTest("compiler/testData/diagnostics/tests/smartCasts/inference/kt39010_2.kt"); + } + @TestMetadata("kt4009.kt") public void testKt4009() throws Exception { runTest("compiler/testData/diagnostics/tests/smartCasts/inference/kt4009.kt"); diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/ConversionUtils.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/ConversionUtils.kt index 46359257ca8..04f9e4a8168 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/ConversionUtils.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/ConversionUtils.kt @@ -415,19 +415,19 @@ private val nameToOperationConventionOrigin = mutableMapOf( internal fun FirReference.statementOrigin(): IrStatementOrigin? { return when (this) { is FirPropertyFromParameterResolvedNamedReference -> IrStatementOrigin.INITIALIZE_PROPERTY_FROM_PARAMETER - is FirResolvedNamedReference -> when (resolvedSymbol) { + is FirResolvedNamedReference -> when (val symbol = resolvedSymbol) { is AccessorSymbol, is SyntheticPropertySymbol -> IrStatementOrigin.GET_PROPERTY is FirNamedFunctionSymbol -> when { - resolvedSymbol.callableId.isInvoke() -> + symbol.callableId.isInvoke() -> IrStatementOrigin.INVOKE - source?.elementType == KtNodeTypes.FOR && resolvedSymbol.callableId.isIteratorNext() -> + source?.elementType == KtNodeTypes.FOR && symbol.callableId.isIteratorNext() -> IrStatementOrigin.FOR_LOOP_NEXT - source?.elementType == KtNodeTypes.FOR && resolvedSymbol.callableId.isIteratorHasNext() -> + source?.elementType == KtNodeTypes.FOR && symbol.callableId.isIteratorHasNext() -> IrStatementOrigin.FOR_LOOP_HAS_NEXT - source?.elementType == KtNodeTypes.FOR && resolvedSymbol.callableId.isIterator() -> + source?.elementType == KtNodeTypes.FOR && symbol.callableId.isIterator() -> IrStatementOrigin.FOR_LOOP_ITERATOR source?.elementType == KtNodeTypes.OPERATION_REFERENCE -> - nameToOperationConventionOrigin[resolvedSymbol.callableId.callableName] + nameToOperationConventionOrigin[symbol.callableId.callableName] else -> null } diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/IntegerLiteralTypeApproximationTransformer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/IntegerLiteralTypeApproximationTransformer.kt index a59597bb6f3..7440171ae7d 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/IntegerLiteralTypeApproximationTransformer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/IntegerLiteralTypeApproximationTransformer.kt @@ -76,7 +76,7 @@ class IntegerLiteralTypeApproximationTransformer( // check black box tests // e.g. Byte doesn't have `and` in member scope. It's an extension if (resultSymbol == null) return functionCall.compose() - functionCall.resultType = data?.let { functionCall.resultType.resolvedTypeFromPrototype(it) } ?: resultSymbol.fir.returnTypeRef + functionCall.resultType = data?.let { functionCall.resultType.resolvedTypeFromPrototype(it) } ?: resultSymbol!!.fir.returnTypeRef // If the original call has argument mapping, values in that mapping refer to value parameters in that original symbol. We should // map those original value parameters back to indices, and then renew the argument mapping with new value parameters in the result // symbol. Otherwise, while putting the value argument to the converted IR call, it will encounter an unknown value parameter, @@ -84,7 +84,7 @@ class IntegerLiteralTypeApproximationTransformer( val newArgumentMapping = functionCall.argumentMapping?.mapValues { (_, oldValueParameter) -> val index = operator.valueParameters.indexOf(oldValueParameter) - if (index != -1) resultSymbol.fir.valueParameters[index] else oldValueParameter + if (index != -1) resultSymbol!!.fir.valueParameters[index] else oldValueParameter } return functionCall.transformCalleeReference( StoreCalleeReference, diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/scopes/impl/AbstractFirUseSiteMemberScope.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/scopes/impl/AbstractFirUseSiteMemberScope.kt index e6072df136e..ef3bc2f5ca9 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/scopes/impl/AbstractFirUseSiteMemberScope.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/scopes/impl/AbstractFirUseSiteMemberScope.kt @@ -79,7 +79,7 @@ abstract class AbstractFirUseSiteMemberScope( createFunctionCopy(firSimpleFunction, newSymbol).apply { resolvePhase = firSimpleFunction.resolvePhase typeParameters += firSimpleFunction.typeParameters - valueParameters += firSimpleFunction.valueParameters.zip(foundFir.valueParameters) + valueParameters += firSimpleFunction.valueParameters.zip(foundFir!!.valueParameters) .map { (overrideParameter, overriddenParameter) -> if (overriddenParameter.defaultValue != null) createValueParameterCopy(overrideParameter, overriddenParameter.defaultValue).apply { diff --git a/compiler/fir/tree/gen/org/jetbrains/kotlin/fir/expressions/impl/FirWhenExpressionImpl.kt b/compiler/fir/tree/gen/org/jetbrains/kotlin/fir/expressions/impl/FirWhenExpressionImpl.kt index daf83431096..da4f9003e5c 100644 --- a/compiler/fir/tree/gen/org/jetbrains/kotlin/fir/expressions/impl/FirWhenExpressionImpl.kt +++ b/compiler/fir/tree/gen/org/jetbrains/kotlin/fir/expressions/impl/FirWhenExpressionImpl.kt @@ -34,8 +34,9 @@ internal class FirWhenExpressionImpl( typeRef.accept(visitor, data) annotations.forEach { it.accept(visitor, data) } calleeReference.accept(visitor, data) - if (subjectVariable != null) { - subjectVariable.accept(visitor, data) + val subjectVariable_ = subjectVariable + if (subjectVariable_ != null) { + subjectVariable_.accept(visitor, data) } else { subject?.accept(visitor, data) } diff --git a/compiler/fir/tree/tree-generator/src/org/jetbrains/kotlin/fir/tree/generator/printer/implementation.kt b/compiler/fir/tree/tree-generator/src/org/jetbrains/kotlin/fir/tree/generator/printer/implementation.kt index 2ee87e96557..2a36122290e 100644 --- a/compiler/fir/tree/tree-generator/src/org/jetbrains/kotlin/fir/tree/generator/printer/implementation.kt +++ b/compiler/fir/tree/tree-generator/src/org/jetbrains/kotlin/fir/tree/generator/printer/implementation.kt @@ -150,8 +150,9 @@ fun SmartPrinter.printImplementation(implementation: Implementation) { if (type == "FirWhenExpressionImpl" && field.name == "subject") { println( """ - |if (subjectVariable != null) { - | subjectVariable.accept(visitor, data) + |val subjectVariable_ = subjectVariable + | if (subjectVariable_ != null) { + | subjectVariable_.accept(visitor, data) | } else { | subject?.accept(visitor, data) | } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/DiagnosticReporterByTrackingStrategy.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/DiagnosticReporterByTrackingStrategy.kt index 65162d29666..13bef2ddf2e 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/DiagnosticReporterByTrackingStrategy.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/DiagnosticReporterByTrackingStrategy.kt @@ -32,6 +32,7 @@ import org.jetbrains.kotlin.resolve.constants.evaluate.ConstantExpressionEvaluat import org.jetbrains.kotlin.resolve.descriptorUtil.module import org.jetbrains.kotlin.resolve.scopes.receivers.ExpressionReceiver import org.jetbrains.kotlin.types.KotlinType +import org.jetbrains.kotlin.types.checker.intersectWrappedTypes import org.jetbrains.kotlin.types.expressions.ControlStructureTypingUtils import org.jetbrains.kotlin.types.typeUtil.isNullableNothing import org.jetbrains.kotlin.types.typeUtil.makeNullable @@ -283,8 +284,19 @@ class DiagnosticReporterByTrackingStrategy( } private fun reportUnstableSmartCast(unstableSmartCast: UnstableSmartCast) { - // todo hack -- remove it after removing SmartCastManager - reportSmartCast(SmartCastDiagnostic(unstableSmartCast.argument, unstableSmartCast.targetType, null)) + val dataFlowValue = dataFlowValueFactory.createDataFlowValue(unstableSmartCast.argument.receiver.receiverValue, context) + val possibleTypes = unstableSmartCast.argument.receiver.typesFromSmartCasts + val argumentExpression = unstableSmartCast.argument.psiExpression ?: return + + require(possibleTypes.isNotEmpty()) { "Receiver for unstable smart cast without possible types" } + trace.report( + SMARTCAST_IMPOSSIBLE.on( + argumentExpression, + intersectWrappedTypes(possibleTypes), + argumentExpression.text, + dataFlowValue.kind.description + ) + ) } override fun constraintError(diagnostic: KotlinCallDiagnostic) { diff --git a/compiler/testData/diagnostics/tests/smartCasts/inference/kt39010.fir.kt b/compiler/testData/diagnostics/tests/smartCasts/inference/kt39010.fir.kt new file mode 100644 index 00000000000..39ab0627006 --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/inference/kt39010.fir.kt @@ -0,0 +1,11 @@ +class A { + fun foo(): E = TODO() +} + +class B(var a: A<*>?) { + fun bar() { + if (a != null) { + a.foo() + } + } +} diff --git a/compiler/testData/diagnostics/tests/smartCasts/inference/kt39010.kt b/compiler/testData/diagnostics/tests/smartCasts/inference/kt39010.kt new file mode 100644 index 00000000000..6ab66160644 --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/inference/kt39010.kt @@ -0,0 +1,11 @@ +class A { + fun foo(): E = TODO() +} + +class B(var a: A<*>?) { + fun bar() { + if (a != null) { + a.foo() + } + } +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/smartCasts/inference/kt39010.txt b/compiler/testData/diagnostics/tests/smartCasts/inference/kt39010.txt new file mode 100644 index 00000000000..190b7c55733 --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/inference/kt39010.txt @@ -0,0 +1,18 @@ +package + +public final class A { + public constructor A() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final fun foo(): E + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class B { + public constructor B(/*0*/ a: A<*>?) + public final var a: A<*>? + public final fun bar(): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/testData/diagnostics/tests/smartCasts/inference/kt39010_2.fir.kt b/compiler/testData/diagnostics/tests/smartCasts/inference/kt39010_2.fir.kt new file mode 100644 index 00000000000..29b5304ca7b --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/inference/kt39010_2.fir.kt @@ -0,0 +1,16 @@ +open class A { +} + +class B : A() { + fun foo() {} +} + +interface KI { + val a: A<*> +} + +fun KI.bar() { + if (a is B) { + a.foo() + } +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/smartCasts/inference/kt39010_2.kt b/compiler/testData/diagnostics/tests/smartCasts/inference/kt39010_2.kt new file mode 100644 index 00000000000..92ad9e96636 --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/inference/kt39010_2.kt @@ -0,0 +1,16 @@ +open class A { +} + +class B : A() { + fun foo() {} +} + +interface KI { + val a: A<*> +} + +fun KI.bar() { + if (a is B) { + a.foo() + } +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/smartCasts/inference/kt39010_2.txt b/compiler/testData/diagnostics/tests/smartCasts/inference/kt39010_2.txt new file mode 100644 index 00000000000..bfe56d0868f --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/inference/kt39010_2.txt @@ -0,0 +1,25 @@ +package + +public fun KI.bar(): kotlin.Unit + +public open class A { + public constructor A() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class B : A { + public constructor B() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final fun foo(): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public interface KI { + public abstract val a: A<*> + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index 1e8b4f0ae85..8427a89ee0b 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -21622,6 +21622,16 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTestWithFirVali runTest("compiler/testData/diagnostics/tests/smartCasts/inference/kt29767.kt"); } + @TestMetadata("kt39010.kt") + public void testKt39010() throws Exception { + runTest("compiler/testData/diagnostics/tests/smartCasts/inference/kt39010.kt"); + } + + @TestMetadata("kt39010_2.kt") + public void testKt39010_2() throws Exception { + runTest("compiler/testData/diagnostics/tests/smartCasts/inference/kt39010_2.kt"); + } + @TestMetadata("kt4009.kt") public void testKt4009() throws Exception { runTest("compiler/testData/diagnostics/tests/smartCasts/inference/kt4009.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java index e319c370084..e989bf3a23d 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java @@ -21547,6 +21547,16 @@ public class DiagnosticsUsingJavacTestGenerated extends AbstractDiagnosticsUsing runTest("compiler/testData/diagnostics/tests/smartCasts/inference/kt29767.kt"); } + @TestMetadata("kt39010.kt") + public void testKt39010() throws Exception { + runTest("compiler/testData/diagnostics/tests/smartCasts/inference/kt39010.kt"); + } + + @TestMetadata("kt39010_2.kt") + public void testKt39010_2() throws Exception { + runTest("compiler/testData/diagnostics/tests/smartCasts/inference/kt39010_2.kt"); + } + @TestMetadata("kt4009.kt") public void testKt4009() throws Exception { runTest("compiler/testData/diagnostics/tests/smartCasts/inference/kt4009.kt");