From 41933facbb353cb18f0c44d86925330248feeeaa Mon Sep 17 00:00:00 2001 From: "Denis.Zharkov" Date: Fri, 9 Jun 2023 11:36:47 +0200 Subject: [PATCH] K2: Fix ISE from inference on delegate vars with implicit types See the test data. ISE happened because at some point after incomplete `getValue` resolution of `a` property, we updated in the `transformAccessors` the property type to the `Variable(Y)` type and then used it as a 3rd argument for `setValue` call which is incorrect because the variable belongs to a different constraint system (from `getValue`). Mostly, the fix is just a repeating K1 behavior, namely postponing `setValue` resolution until delegate inference is completed. ^KT-59066 Fixed --- ...CompilerTestFE10TestdataTestGenerated.java | 12 +++++ ...sticCompilerFE10TestDataTestGenerated.java | 12 +++++ ...eeOldFrontendDiagnosticsTestGenerated.java | 12 +++++ ...siOldFrontendDiagnosticsTestGenerated.java | 12 +++++ .../FirDeclarationsResolveTransformer.kt | 47 ++++++++++++++++--- ...nferenceFromGetValueThroughSetValue.fir.kt | 24 ++++++++++ .../noInferenceFromGetValueThroughSetValue.kt | 24 ++++++++++ ...arySetterInferenceForImplicitlyTypedVar.kt | 27 +++++++++++ .../test/runners/DiagnosticTestGenerated.java | 12 +++++ 9 files changed, 175 insertions(+), 7 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/delegatedProperty/noInferenceFromGetValueThroughSetValue.fir.kt create mode 100644 compiler/testData/diagnostics/tests/delegatedProperty/noInferenceFromGetValueThroughSetValue.kt create mode 100644 compiler/testData/diagnostics/tests/delegatedProperty/noPreliminarySetterInferenceForImplicitlyTypedVar.kt diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java index 0019e3fbf94..e9eaeddb992 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java @@ -8974,12 +8974,24 @@ public class DiagnosticCompilerTestFE10TestdataTestGenerated extends AbstractDia runTest("compiler/testData/diagnostics/tests/delegatedProperty/missedSetter.kt"); } + @Test + @TestMetadata("noInferenceFromGetValueThroughSetValue.kt") + public void testNoInferenceFromGetValueThroughSetValue() throws Exception { + runTest("compiler/testData/diagnostics/tests/delegatedProperty/noInferenceFromGetValueThroughSetValue.kt"); + } + @Test @TestMetadata("noInferenceFromWrappedDelegate.kt") public void testNoInferenceFromWrappedDelegate() throws Exception { runTest("compiler/testData/diagnostics/tests/delegatedProperty/noInferenceFromWrappedDelegate.kt"); } + @Test + @TestMetadata("noPreliminarySetterInferenceForImplicitlyTypedVar.kt") + public void testNoPreliminarySetterInferenceForImplicitlyTypedVar() throws Exception { + runTest("compiler/testData/diagnostics/tests/delegatedProperty/noPreliminarySetterInferenceForImplicitlyTypedVar.kt"); + } + @Test @TestMetadata("nonDefaultAccessors.kt") public void testNonDefaultAccessors() throws Exception { diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java index 8d075a3a110..78ab557eee6 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java @@ -8974,12 +8974,24 @@ public class LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated runTest("compiler/testData/diagnostics/tests/delegatedProperty/missedSetter.kt"); } + @Test + @TestMetadata("noInferenceFromGetValueThroughSetValue.kt") + public void testNoInferenceFromGetValueThroughSetValue() throws Exception { + runTest("compiler/testData/diagnostics/tests/delegatedProperty/noInferenceFromGetValueThroughSetValue.kt"); + } + @Test @TestMetadata("noInferenceFromWrappedDelegate.kt") public void testNoInferenceFromWrappedDelegate() throws Exception { runTest("compiler/testData/diagnostics/tests/delegatedProperty/noInferenceFromWrappedDelegate.kt"); } + @Test + @TestMetadata("noPreliminarySetterInferenceForImplicitlyTypedVar.kt") + public void testNoPreliminarySetterInferenceForImplicitlyTypedVar() throws Exception { + runTest("compiler/testData/diagnostics/tests/delegatedProperty/noPreliminarySetterInferenceForImplicitlyTypedVar.kt"); + } + @Test @TestMetadata("nonDefaultAccessors.kt") public void testNonDefaultAccessors() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java index 3c2344afd9a..5e5aa13efe4 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java @@ -8974,12 +8974,24 @@ public class FirLightTreeOldFrontendDiagnosticsTestGenerated extends AbstractFir runTest("compiler/testData/diagnostics/tests/delegatedProperty/missedSetter.kt"); } + @Test + @TestMetadata("noInferenceFromGetValueThroughSetValue.kt") + public void testNoInferenceFromGetValueThroughSetValue() throws Exception { + runTest("compiler/testData/diagnostics/tests/delegatedProperty/noInferenceFromGetValueThroughSetValue.kt"); + } + @Test @TestMetadata("noInferenceFromWrappedDelegate.kt") public void testNoInferenceFromWrappedDelegate() throws Exception { runTest("compiler/testData/diagnostics/tests/delegatedProperty/noInferenceFromWrappedDelegate.kt"); } + @Test + @TestMetadata("noPreliminarySetterInferenceForImplicitlyTypedVar.kt") + public void testNoPreliminarySetterInferenceForImplicitlyTypedVar() throws Exception { + runTest("compiler/testData/diagnostics/tests/delegatedProperty/noPreliminarySetterInferenceForImplicitlyTypedVar.kt"); + } + @Test @TestMetadata("nonDefaultAccessors.kt") public void testNonDefaultAccessors() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java index ba4874c848f..0ee36840bd2 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java @@ -8980,12 +8980,24 @@ public class FirPsiOldFrontendDiagnosticsTestGenerated extends AbstractFirPsiDia runTest("compiler/testData/diagnostics/tests/delegatedProperty/missedSetter.kt"); } + @Test + @TestMetadata("noInferenceFromGetValueThroughSetValue.kt") + public void testNoInferenceFromGetValueThroughSetValue() throws Exception { + runTest("compiler/testData/diagnostics/tests/delegatedProperty/noInferenceFromGetValueThroughSetValue.kt"); + } + @Test @TestMetadata("noInferenceFromWrappedDelegate.kt") public void testNoInferenceFromWrappedDelegate() throws Exception { runTest("compiler/testData/diagnostics/tests/delegatedProperty/noInferenceFromWrappedDelegate.kt"); } + @Test + @TestMetadata("noPreliminarySetterInferenceForImplicitlyTypedVar.kt") + public void testNoPreliminarySetterInferenceForImplicitlyTypedVar() throws Exception { + runTest("compiler/testData/diagnostics/tests/delegatedProperty/noPreliminarySetterInferenceForImplicitlyTypedVar.kt"); + } + @Test @TestMetadata("nonDefaultAccessors.kt") public void testNonDefaultAccessors() throws Exception { diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirDeclarationsResolveTransformer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirDeclarationsResolveTransformer.kt index 445af430466..b57e222ba11 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirDeclarationsResolveTransformer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirDeclarationsResolveTransformer.kt @@ -181,7 +181,9 @@ open class FirDeclarationsResolveTransformer( val propertyTypeIsKnown = propertyTypeRefAfterResolve is FirResolvedTypeRef val mayResolveGetter = mayResolveSetter || !propertyTypeIsKnown if (mayResolveGetter) { - property.transformAccessors(mayResolveSetter) + property.transformAccessors( + if (mayResolveSetter) SetterResolutionMode.FULLY_RESOLVE else SetterResolutionMode.ONLY_IMPLICIT_PARAMETER_TYPE + ) property.replaceBodyResolveState( if (mayResolveSetter) FirPropertyBodyResolveState.EVERYTHING_RESOLVED else FirPropertyBodyResolveState.INITIALIZER_AND_GETTER_RESOLVED @@ -277,6 +279,8 @@ open class FirDeclarationsResolveTransformer( } private fun transformPropertyAccessorsWithDelegate(property: FirProperty, delegate: FirExpression) { + val isImplicitTypedProperty = property.returnTypeRef is FirImplicitTypeRef + context.forPropertyDelegateAccessors(property, resolutionContext, callCompleter) { dataFlowAnalyzer.enterDelegateExpression() // Resolve delegate expression, after that, delegate will contain either expr.provideDelegate or expr @@ -288,7 +292,12 @@ open class FirDeclarationsResolveTransformer( } } - property.transformAccessors() + // We don't use inference from setValue calls (i.e. don't resolve setters until the delegate inference is completed), + // when property doesn't have explicit type. + // It's necessary because we need to supply the property type as the 3rd argument for `setValue` and there might be uninferred + // variables from `getValue`. + // The same logic was used at K1 (see org.jetbrains.kotlin.resolve.DelegatedPropertyResolver.inferDelegateTypeFromGetSetValueMethods) + property.transformAccessors(if (isImplicitTypedProperty) SetterResolutionMode.SKIP else SetterResolutionMode.FULLY_RESOLVE) val completedCalls = completeCandidates() val finalSubstitutor = createFinalSubstitutor() @@ -310,10 +319,14 @@ open class FirDeclarationsResolveTransformer( completedCalls.forEach { it.transformSingle(callCompletionResultsWriter, null) } - - dataFlowAnalyzer.exitDelegateExpression(delegate) - property } + + // `isImplicitTypedProperty` means we haven't run setter resolution yet (see its second usage) + if (isImplicitTypedProperty) { + property.resolveSetter(property.returnTypeRef, mayResolveSetterBody = true) + } + + dataFlowAnalyzer.exitDelegateExpression(delegate) } override fun transformPropertyAccessor(propertyAccessor: FirPropertyAccessor, data: ResolutionMode): FirStatement { @@ -420,7 +433,17 @@ open class FirDeclarationsResolveTransformer( return variable } - private fun FirProperty.transformAccessors(mayResolveSetter: Boolean = true) { + // TODO: This enum might actually be a boolean (resolve setter/don't resolve setter) + // But for some reason, in IDE there's a need to resolve setter's parameter types on the implicit-resolution stage + // See ad183434137939a0c9eeea2f7df9ef522672a18e commit. + // But for delegate inference case, we don't need both body of the setter and its parameter resolved (SKIP mode) + private enum class SetterResolutionMode { + FULLY_RESOLVE, ONLY_IMPLICIT_PARAMETER_TYPE, SKIP + } + + private fun FirProperty.transformAccessors( + setterResolutionMode: SetterResolutionMode = SetterResolutionMode.FULLY_RESOLVE + ) { var enhancedTypeRef = returnTypeRef if (bodyResolveState < FirPropertyBodyResolveState.INITIALIZER_AND_GETTER_RESOLVED) { getter?.let { @@ -433,10 +456,20 @@ open class FirDeclarationsResolveTransformer( // We need update type of getter for case when its type was approximated getter?.transformTypeWithPropertyType(enhancedTypeRef, forceUpdateForNonImplicitTypes = true) } + + if (setterResolutionMode != SetterResolutionMode.SKIP) { + resolveSetter(enhancedTypeRef, mayResolveSetterBody = setterResolutionMode == SetterResolutionMode.FULLY_RESOLVE) + } + } + + private fun FirProperty.resolveSetter( + enhancedTypeRef: FirTypeRef, + mayResolveSetterBody: Boolean, + ) { setter?.let { it.transformTypeWithPropertyType(enhancedTypeRef) - if (mayResolveSetter) { + if (mayResolveSetterBody) { transformAccessor(it, enhancedTypeRef, this) } } diff --git a/compiler/testData/diagnostics/tests/delegatedProperty/noInferenceFromGetValueThroughSetValue.fir.kt b/compiler/testData/diagnostics/tests/delegatedProperty/noInferenceFromGetValueThroughSetValue.fir.kt new file mode 100644 index 00000000000..2258aa5c332 --- /dev/null +++ b/compiler/testData/diagnostics/tests/delegatedProperty/noInferenceFromGetValueThroughSetValue.fir.kt @@ -0,0 +1,24 @@ +import kotlin.reflect.KProperty + +// Definitions +class M + +operator fun M.getValue(thisRef: Any?, property: KProperty<*>): String = "value" +operator fun M.setValue(thisRef: Any?, property: KProperty<*>, value: Z) {} + +fun m(): M = M() + +// We don't allow to infer type of a delegate expression through a setValue, where the argument (value) is constrained by the return type of a getValue +var a by m() + +// We infer type of delegate expression through a setValue from the explicit property type +var b: String by m() + +fun takeString(v: String) {} + +fun main() { + takeString(a) + a = "a" + takeString(b) + b = "b" +} diff --git a/compiler/testData/diagnostics/tests/delegatedProperty/noInferenceFromGetValueThroughSetValue.kt b/compiler/testData/diagnostics/tests/delegatedProperty/noInferenceFromGetValueThroughSetValue.kt new file mode 100644 index 00000000000..e5acc373ab4 --- /dev/null +++ b/compiler/testData/diagnostics/tests/delegatedProperty/noInferenceFromGetValueThroughSetValue.kt @@ -0,0 +1,24 @@ +import kotlin.reflect.KProperty + +// Definitions +class M + +operator fun M.getValue(thisRef: Any?, property: KProperty<*>): String = "value" +operator fun M.setValue(thisRef: Any?, property: KProperty<*>, value: Z) {} + +fun m(): M = M() + +// We don't allow to infer type of a delegate expression through a setValue, where the argument (value) is constrained by the return type of a getValue +var a by m() + +// We infer type of delegate expression through a setValue from the explicit property type +var b: String by m() + +fun takeString(v: String) {} + +fun main() { + takeString(a) + a = "a" + takeString(b) + b = "b" +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/delegatedProperty/noPreliminarySetterInferenceForImplicitlyTypedVar.kt b/compiler/testData/diagnostics/tests/delegatedProperty/noPreliminarySetterInferenceForImplicitlyTypedVar.kt new file mode 100644 index 00000000000..aff9023b407 --- /dev/null +++ b/compiler/testData/diagnostics/tests/delegatedProperty/noPreliminarySetterInferenceForImplicitlyTypedVar.kt @@ -0,0 +1,27 @@ +// FIR_IDENTICAL +// WITH_STDLIB +// WITH_REFLECT +// ISSUE: KT-59066 + +import kotlin.reflect.KProperty + +interface M< E> + +public operator fun M.getValue(thisRef: Any?, property: KProperty<*>): Y = + TODO() + +public operator fun M.setValue(thisRef: Any?, property: KProperty<*>, value: Z) {} + +fun m(u: U): M = TODO() + +var a by m("") + +var b by mutableMapOf("" to 1) + +fun main() { + a.length + a = "b" + + b = 23 + b.and(4) +} \ No newline at end of file diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java index 1991ce2029a..5d3f0feced6 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java @@ -8980,12 +8980,24 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { runTest("compiler/testData/diagnostics/tests/delegatedProperty/missedSetter.kt"); } + @Test + @TestMetadata("noInferenceFromGetValueThroughSetValue.kt") + public void testNoInferenceFromGetValueThroughSetValue() throws Exception { + runTest("compiler/testData/diagnostics/tests/delegatedProperty/noInferenceFromGetValueThroughSetValue.kt"); + } + @Test @TestMetadata("noInferenceFromWrappedDelegate.kt") public void testNoInferenceFromWrappedDelegate() throws Exception { runTest("compiler/testData/diagnostics/tests/delegatedProperty/noInferenceFromWrappedDelegate.kt"); } + @Test + @TestMetadata("noPreliminarySetterInferenceForImplicitlyTypedVar.kt") + public void testNoPreliminarySetterInferenceForImplicitlyTypedVar() throws Exception { + runTest("compiler/testData/diagnostics/tests/delegatedProperty/noPreliminarySetterInferenceForImplicitlyTypedVar.kt"); + } + @Test @TestMetadata("nonDefaultAccessors.kt") public void testNonDefaultAccessors() throws Exception {