From b79b94fe75e79c910d919a78af696ed52d057298 Mon Sep 17 00:00:00 2001 From: Pavel Kirpichenkov Date: Tue, 3 Nov 2020 18:40:04 +0300 Subject: [PATCH] [MPP] Allow smart casts for properties from dependsOn modules Smartcasts for public properties from different module are not stable because module declaring a property in general can be compiled separately from the module using it. However, if client module has dependsOn relation with declaring module their simultaneous compilation is guaranteed which makes this smart cast safe. Cache all transitive 'expected by' modules in module dependencies. Extend test to check smart casts are allowed for properties from transitive 'expected by' dependencies and prohibited otherwise. ^KT-42754 Fixed --- .../fir/descriptors/FirModuleDescriptor.kt | 2 ++ .../kotlin/analyzer/AnalyzerFacade.kt | 9 +++++++- .../kotlin/analyzer/ModuleInfoUtils.kt | 14 ++++++++++++ .../smartcasts/DataFlowValueKindUtils.kt | 12 +++++++++- .../kotlin/descriptors/ModuleDescriptor.kt | 2 ++ .../descriptors/impl/ModuleDescriptorImpl.kt | 13 +++++++---- .../jetbrains/kotlin/types/ErrorUtils.java | 6 +++++ .../DebugLabelPropertyDescriptorProvider.kt | 3 +++ .../common/common.kt | 5 ----- .../common1/common.kt | 5 +++++ .../common2/common.kt | 5 +++++ .../common3/common.kt | 5 +++++ .../common4/common.kt | 5 +++++ .../dependencies.txt | 12 +++++++--- .../jvm2/jvm2.kt | 22 ++++++++++++++----- 15 files changed, 101 insertions(+), 19 deletions(-) delete mode 100644 idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/common/common.kt create mode 100644 idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/common1/common.kt create mode 100644 idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/common2/common.kt create mode 100644 idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/common3/common.kt create mode 100644 idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/common4/common.kt diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/descriptors/FirModuleDescriptor.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/descriptors/FirModuleDescriptor.kt index 388c4711214..d6a4f8ce3b1 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/descriptors/FirModuleDescriptor.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/descriptors/FirModuleDescriptor.kt @@ -42,6 +42,8 @@ class FirModuleDescriptor(val session: FirSession) : ModuleDescriptor { get() = TODO("not implemented") override val expectedByModules: List get() = TODO("not implemented") + override val allExpectedByModules: Set + get() = TODO("not implemented") override fun getCapability(capability: ModuleCapability): T? { return null diff --git a/compiler/frontend/src/org/jetbrains/kotlin/analyzer/AnalyzerFacade.kt b/compiler/frontend/src/org/jetbrains/kotlin/analyzer/AnalyzerFacade.kt index 1a078064c08..5dbdf8f0090 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/analyzer/AnalyzerFacade.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/analyzer/AnalyzerFacade.kt @@ -155,13 +155,20 @@ class LazyModuleDependencies( override val allDependencies: List get() = dependencies() - override val expectedByDependencies by storageManager.createLazyValue { + override val directExpectedByDependencies by storageManager.createLazyValue { module.expectedBy.map { @Suppress("UNCHECKED_CAST") resolverForProject.descriptorForModule(it as M) } } + override val allExpectedByDependencies: Set by storageManager.createLazyValue { + collectAllExpectedByModules(module).mapTo(HashSet()) { + @Suppress("UNCHECKED_CAST") + resolverForProject.descriptorForModule(it as M) + } + } + override val modulesWhoseInternalsAreVisible: Set get() = module.modulesWhoseInternalsAreVisible().mapTo(LinkedHashSet()) { diff --git a/compiler/frontend/src/org/jetbrains/kotlin/analyzer/ModuleInfoUtils.kt b/compiler/frontend/src/org/jetbrains/kotlin/analyzer/ModuleInfoUtils.kt index 955af9039b2..c5031a94500 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/analyzer/ModuleInfoUtils.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/analyzer/ModuleInfoUtils.kt @@ -9,3 +9,17 @@ import org.jetbrains.kotlin.descriptors.ModuleDescriptor val ModuleDescriptor.moduleInfo: ModuleInfo? get() = getCapability(ModuleInfo.Capability) + +internal fun collectAllExpectedByModules(entryModule: ModuleInfo): Set { + val unprocessedModules = ArrayDeque().apply { addAll(entryModule.expectedBy) } + val expectedByModules = HashSet() + + while (unprocessedModules.isNotEmpty()) { + val nextImplemented = unprocessedModules.removeFirst() + if (expectedByModules.add(nextImplemented)) { + unprocessedModules.addAll(nextImplemented.expectedBy) + } + } + + return expectedByModules +} diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DataFlowValueKindUtils.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DataFlowValueKindUtils.kt index a863014205e..54388a4e277 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DataFlowValueKindUtils.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DataFlowValueKindUtils.kt @@ -24,13 +24,23 @@ internal fun PropertyDescriptor.propertyKind(usageModule: ModuleDescriptor?): Da if (!hasDefaultGetter()) return DataFlowValue.Kind.PROPERTY_WITH_GETTER if (!isInvisibleFromOtherModules()) { val declarationModule = DescriptorUtils.getContainingModule(this) - if (usageModule == null || usageModule != declarationModule) { + if (!areCompiledTogether(usageModule, declarationModule)) { return DataFlowValue.Kind.ALIEN_PUBLIC_PROPERTY } } return DataFlowValue.Kind.STABLE_VALUE } +internal fun areCompiledTogether( + usageModule: ModuleDescriptor?, + declarationModule: ModuleDescriptor, +): Boolean { + if (usageModule == null) return false + if (usageModule == declarationModule) return true + + return declarationModule in usageModule.allExpectedByModules +} + internal fun VariableDescriptor.variableKind( usageModule: ModuleDescriptor?, bindingContext: BindingContext, diff --git a/core/descriptors/src/org/jetbrains/kotlin/descriptors/ModuleDescriptor.kt b/core/descriptors/src/org/jetbrains/kotlin/descriptors/ModuleDescriptor.kt index 5fed4e669cb..7222cad74c5 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/descriptors/ModuleDescriptor.kt +++ b/core/descriptors/src/org/jetbrains/kotlin/descriptors/ModuleDescriptor.kt @@ -52,6 +52,8 @@ interface ModuleDescriptor : DeclarationDescriptor { val expectedByModules: List + val allExpectedByModules: Set + fun getCapability(capability: ModuleCapability): T? class Capability(val name: String) { diff --git a/core/descriptors/src/org/jetbrains/kotlin/descriptors/impl/ModuleDescriptorImpl.kt b/core/descriptors/src/org/jetbrains/kotlin/descriptors/impl/ModuleDescriptorImpl.kt index 2d745b24777..c32cfaa0a93 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/descriptors/impl/ModuleDescriptorImpl.kt +++ b/core/descriptors/src/org/jetbrains/kotlin/descriptors/impl/ModuleDescriptorImpl.kt @@ -75,7 +75,10 @@ class ModuleDescriptorImpl @JvmOverloads constructor( get() = this.dependencies.sure { "Dependencies of module $id were not set" }.allDependencies.filter { it != this } override val expectedByModules: List - get() = this.dependencies.sure { "Dependencies of module $id were not set" }.expectedByDependencies + get() = this.dependencies.sure { "Dependencies of module $id were not set" }.directExpectedByDependencies + + override val allExpectedByModules: Set + get() = this.dependencies.sure { "Dependencies of module $id were not set" }.allExpectedByDependencies override fun getPackage(fqName: FqName): PackageViewDescriptor { assertValid() @@ -118,7 +121,7 @@ class ModuleDescriptorImpl @JvmOverloads constructor( } fun setDependencies(descriptors: List, friends: Set) { - setDependencies(ModuleDependenciesImpl(descriptors, friends, emptyList())) + setDependencies(ModuleDependenciesImpl(descriptors, friends, emptyList(), emptySet())) } override fun shouldSeeInternalsOf(targetModule: ModuleDescriptor): Boolean { @@ -154,11 +157,13 @@ class ModuleDescriptorImpl @JvmOverloads constructor( interface ModuleDependencies { val allDependencies: List val modulesWhoseInternalsAreVisible: Set - val expectedByDependencies: List + val directExpectedByDependencies: List + val allExpectedByDependencies: Set } class ModuleDependenciesImpl( override val allDependencies: List, override val modulesWhoseInternalsAreVisible: Set, - override val expectedByDependencies: List + override val directExpectedByDependencies: List, + override val allExpectedByDependencies: Set, ) : ModuleDependencies diff --git a/core/descriptors/src/org/jetbrains/kotlin/types/ErrorUtils.java b/core/descriptors/src/org/jetbrains/kotlin/types/ErrorUtils.java index 857768d90bc..9ac55d4ee28 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/types/ErrorUtils.java +++ b/core/descriptors/src/org/jetbrains/kotlin/types/ErrorUtils.java @@ -107,6 +107,12 @@ public class ErrorUtils { return emptyList(); } + @NotNull + @Override + public Set getAllExpectedByModules() { + return emptySet(); + } + @Override public R accept(@NotNull DeclarationDescriptorVisitor visitor, D data) { return null; diff --git a/idea/jvm-debugger/jvm-debugger-evaluation/src/org/jetbrains/kotlin/idea/debugger/evaluate/compilation/DebugLabelPropertyDescriptorProvider.kt b/idea/jvm-debugger/jvm-debugger-evaluation/src/org/jetbrains/kotlin/idea/debugger/evaluate/compilation/DebugLabelPropertyDescriptorProvider.kt index fc30b211325..4aea9e05a7a 100644 --- a/idea/jvm-debugger/jvm-debugger-evaluation/src/org/jetbrains/kotlin/idea/debugger/evaluate/compilation/DebugLabelPropertyDescriptorProvider.kt +++ b/idea/jvm-debugger/jvm-debugger-evaluation/src/org/jetbrains/kotlin/idea/debugger/evaluate/compilation/DebugLabelPropertyDescriptorProvider.kt @@ -189,6 +189,9 @@ private object DebugLabelModuleDescriptor : DeclarationDescriptorImpl(Annotation override val expectedByModules: List get() = emptyList() + override val allExpectedByModules: Set + get() = emptySet() + override fun getCapability(capability: ModuleCapability): T? = null override val isValid: Boolean diff --git a/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/common/common.kt b/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/common/common.kt deleted file mode 100644 index 154e1b86f72..00000000000 --- a/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/common/common.kt +++ /dev/null @@ -1,5 +0,0 @@ -data class CommonDataClass(val property: CommonObject?) - -object CommonObject { - fun doSomething() {} -} \ No newline at end of file diff --git a/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/common1/common.kt b/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/common1/common.kt new file mode 100644 index 00000000000..53d06499b90 --- /dev/null +++ b/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/common1/common.kt @@ -0,0 +1,5 @@ +data class CommonDataClass1(val property: CommonObject1?) + +object CommonObject1 { + fun doSomething() {} +} \ No newline at end of file diff --git a/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/common2/common.kt b/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/common2/common.kt new file mode 100644 index 00000000000..337cebf2d01 --- /dev/null +++ b/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/common2/common.kt @@ -0,0 +1,5 @@ +data class CommonDataClass2(val property: CommonObject2?) + +object CommonObject2 { + fun doSomething() {} +} \ No newline at end of file diff --git a/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/common3/common.kt b/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/common3/common.kt new file mode 100644 index 00000000000..b77ea5d8b16 --- /dev/null +++ b/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/common3/common.kt @@ -0,0 +1,5 @@ +data class CommonDataClass3(val property: CommonObject3?) + +object CommonObject3 { + fun doSomething() {} +} \ No newline at end of file diff --git a/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/common4/common.kt b/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/common4/common.kt new file mode 100644 index 00000000000..93036191e73 --- /dev/null +++ b/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/common4/common.kt @@ -0,0 +1,5 @@ +data class CommonDataClass4(val property: CommonObject4?) + +object CommonObject4 { + fun doSomething() {} +} \ No newline at end of file diff --git a/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/dependencies.txt b/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/dependencies.txt index c368ab48d9b..153ed8c60ff 100644 --- a/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/dependencies.txt +++ b/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/dependencies.txt @@ -1,7 +1,13 @@ -MODULE common { platform=[JVM, JS, Native] } +MODULE common1 { platform=[JVM, JS, Native] } +MODULE common2 { platform=[JVM, JS, Native] } +MODULE common3 { platform=[JVM, JS, Native] } +MODULE common4 { platform=[JVM, JS, Native] } MODULE jvm1 { platform=[JVM] } MODULE jvm2 { platform=[JVM] } -jvm1 -> common { kind=DEPENDS_ON } -jvm2 -> common { kind=DEPENDS_ON } +common2 -> common1 { kind=DEPENDS_ON } +common3 -> common1 { kind=DEPENDS_ON } +common4 -> common2, common3 { kind=DEPENDS_ON } +jvm1 -> common4 { kind=DEPENDS_ON } +jvm2 -> common4 { kind=DEPENDS_ON } jvm2 -> jvm1 { kind=DEPENDENCY } diff --git a/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/jvm2/jvm2.kt b/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/jvm2/jvm2.kt index f541db28ea9..9164f211aa7 100644 --- a/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/jvm2/jvm2.kt +++ b/idea/testData/multiplatform/smartCastOnPropertyFromDependentModule/jvm2/jvm2.kt @@ -1,9 +1,21 @@ -fun test(fromCommon: CommonDataClass, fromJvm: JvmDataClass) { - if (fromCommon.property != null) { - fromCommon.property.doSomething() +fun test(c1: CommonDataClass1, c2: CommonDataClass2, c3: CommonDataClass3, c4: CommonDataClass4, jvm: JvmDataClass) { + if (c1.property != null) { + c1.property.doSomething() } - if (fromJvm.property != null) { - fromJvm.property.doSomething() + if (c2.property != null) { + c2.property.doSomething() + } + + if (c3.property != null) { + c3.property.doSomething() + } + + if (c4.property != null) { + c4.property.doSomething() + } + + if (jvm.property != null) { + jvm.property.doSomething() } } \ No newline at end of file