From 60b227c5193cd6ef38c5561a68e06a33509918d8 Mon Sep 17 00:00:00 2001 From: Kirill Rakhman Date: Wed, 22 Mar 2023 12:10:48 +0100 Subject: [PATCH] [FIR] Sort HMPP dependencies topologically for symbol providers This fixes an issue where an actual class from an intermediate module has more supertypes than its expect declaration which leads to a false-positive resolution error because a type reference resolves to the expect class. The fix is to sort the dependencies topologically from "most actual" to "most expect" when creating the list of symbol providers. #KT-57369 Fixed --- .../kotlin/container/DataStructures.kt | 6 ++-- ...DiagnosticsWithLightTreeTestGenerated.java | 6 ++++ ...endMPPDiagnosticsWithPsiTestGenerated.java | 6 ++++ .../fir/session/FirAbstractSessionFactory.kt | 9 ++++- ...ediateActualHasAdditionalSupertypes.fir.kt | 30 ++++++++++++++++ ...termediateActualHasAdditionalSupertypes.kt | 30 ++++++++++++++++ .../test/runners/DiagnosticTestGenerated.java | 6 ++++ .../test/frontend/fir/FirFrontendFacade.kt | 18 ++-------- .../kotlin/gradle/HierarchicalMppIT.kt | 11 ++++++ .../build.gradle.kts | 34 +++++++++++++++++++ .../settings.gradle.kts | 1 + .../src/commonMain/kotlin/Main.common.kt | 12 +++++++ .../concurrentMain/kotlin/Main.concurrent.kt | 13 +++++++ .../src/jvmMain/kotlin/Main.kt | 9 +++++ .../compilation/TestCompilationFactory.kt | 16 ++------- 15 files changed, 174 insertions(+), 33 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/multiplatform/hmpp/intermediateActualHasAdditionalSupertypes.fir.kt create mode 100644 compiler/testData/diagnostics/tests/multiplatform/hmpp/intermediateActualHasAdditionalSupertypes.kt create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/hierarchical-mpp-actual-has-additional-supertypes/build.gradle.kts create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/hierarchical-mpp-actual-has-additional-supertypes/settings.gradle.kts create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/hierarchical-mpp-actual-has-additional-supertypes/src/commonMain/kotlin/Main.common.kt create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/hierarchical-mpp-actual-has-additional-supertypes/src/concurrentMain/kotlin/Main.concurrent.kt create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/hierarchical-mpp-actual-has-additional-supertypes/src/jvmMain/kotlin/Main.kt diff --git a/compiler/container/src/org/jetbrains/kotlin/container/DataStructures.kt b/compiler/container/src/org/jetbrains/kotlin/container/DataStructures.kt index 712221c0f41..53117787d88 100644 --- a/compiler/container/src/org/jetbrains/kotlin/container/DataStructures.kt +++ b/compiler/container/src/org/jetbrains/kotlin/container/DataStructures.kt @@ -19,7 +19,7 @@ package org.jetbrains.kotlin.container import java.util.ArrayList import java.util.HashSet -fun topologicalSort(items: Iterable, dependencies: (T) -> Iterable): List { +fun topologicalSort(items: Iterable, reverseOrder: Boolean = false, dependencies: (T) -> Iterable): List { val itemsInProgress = HashSet() val completedItems = HashSet() val result = ArrayList() @@ -45,7 +45,7 @@ fun topologicalSort(items: Iterable, dependencies: (T) -> Iterable): L for (item in items) DfsVisit(item) - return result.apply { reverse() } + return result.apply { if (!reverseOrder) reverse() } } -class CycleInTopoSortException : Exception() \ No newline at end of file +class CycleInTopoSortException : Exception() diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated.java index 7928dc90f27..3cdd0b3d85d 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated.java @@ -550,6 +550,12 @@ public class FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated extends Abst KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/multiplatform/hmpp"), Pattern.compile("^(.*)\\.kts?$"), Pattern.compile("^(.+)\\.(reversed|fir|ll)\\.kts?$"), TargetBackend.JVM_IR, true); } + @Test + @TestMetadata("intermediateActualHasAdditionalSupertypes.kt") + public void testIntermediateActualHasAdditionalSupertypes() throws Exception { + runTest("compiler/testData/diagnostics/tests/multiplatform/hmpp/intermediateActualHasAdditionalSupertypes.kt"); + } + @Test @TestMetadata("kt-55570.kt") public void testKt_55570() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithPsiTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithPsiTestGenerated.java index 7da0165b417..4452703e208 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithPsiTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithPsiTestGenerated.java @@ -550,6 +550,12 @@ public class FirOldFrontendMPPDiagnosticsWithPsiTestGenerated extends AbstractFi KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/multiplatform/hmpp"), Pattern.compile("^(.*)\\.kts?$"), Pattern.compile("^(.+)\\.(reversed|fir|ll)\\.kts?$"), TargetBackend.JVM_IR, true); } + @Test + @TestMetadata("intermediateActualHasAdditionalSupertypes.kt") + public void testIntermediateActualHasAdditionalSupertypes() throws Exception { + runTest("compiler/testData/diagnostics/tests/multiplatform/hmpp/intermediateActualHasAdditionalSupertypes.kt"); + } + @Test @TestMetadata("kt-55570.kt") public void testKt_55570() throws Exception { diff --git a/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/session/FirAbstractSessionFactory.kt b/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/session/FirAbstractSessionFactory.kt index 206f2974009..025ae048330 100644 --- a/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/session/FirAbstractSessionFactory.kt +++ b/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/session/FirAbstractSessionFactory.kt @@ -6,6 +6,7 @@ package org.jetbrains.kotlin.fir.session import org.jetbrains.kotlin.config.LanguageVersionSettings +import org.jetbrains.kotlin.container.topologicalSort import org.jetbrains.kotlin.fir.* import org.jetbrains.kotlin.fir.checkers.registerCommonCheckers import org.jetbrains.kotlin.fir.deserialization.ModuleDataProvider @@ -139,7 +140,13 @@ abstract class FirAbstractSessionFactory { private fun FirSession.computeDependencyProviderList(moduleData: FirModuleData): List { val visited = mutableSetOf() - return (moduleData.dependencies + moduleData.friendDependencies + moduleData.dependsOnDependencies) + + // dependsOnDependencies can actualize declarations from their dependencies. Because actual declarations can be more specific + // (e.g. have additional supertypes), the modules must be ordered from most specific (i.e. actual) to most generic (i.e. expect) + // to prevent false positive resolution errors (see KT-57369 for an example). + val dependsOnDependencies = topologicalSort(moduleData.dependsOnDependencies) { it.dependsOnDependencies } + + return (moduleData.dependencies + moduleData.friendDependencies + dependsOnDependencies) .mapNotNull { sessionProvider?.getSession(it) } .map { it.symbolProvider } .flatMap { it.flatten(visited, collectSourceProviders = it.session.kind == FirSession.Kind.Source) } diff --git a/compiler/testData/diagnostics/tests/multiplatform/hmpp/intermediateActualHasAdditionalSupertypes.fir.kt b/compiler/testData/diagnostics/tests/multiplatform/hmpp/intermediateActualHasAdditionalSupertypes.fir.kt new file mode 100644 index 00000000000..00a23bfaed3 --- /dev/null +++ b/compiler/testData/diagnostics/tests/multiplatform/hmpp/intermediateActualHasAdditionalSupertypes.fir.kt @@ -0,0 +1,30 @@ +// ISSUE: KT-57369 + +// MODULE: common +// TARGET_PLATFORM: Common +interface CompletionHandler { + fun foo() +} + +expect class CompletionHandlerBase() + +fun invokeOnCompletion(handler: CompletionHandler) {} + +// MODULE: intermediate()()(common) +// TARGET_PLATFORM: Common +// actual has an additional super type +actual class CompletionHandlerBase : CompletionHandler { + override fun foo() {} +} + +fun cancelFutureOnCompletionAlt(handlerBase: CompletionHandlerBase) { + invokeOnCompletion(handlerBase) + handlerBase.foo() +} + +// MODULE: main()()(common, intermediate) +// the order of dependencies is important to reproduce KT-57369 +fun cancelFutureOnCompletion(handlerBase: CompletionHandlerBase) { + invokeOnCompletion(handlerBase) + handlerBase.foo() +} diff --git a/compiler/testData/diagnostics/tests/multiplatform/hmpp/intermediateActualHasAdditionalSupertypes.kt b/compiler/testData/diagnostics/tests/multiplatform/hmpp/intermediateActualHasAdditionalSupertypes.kt new file mode 100644 index 00000000000..c82c6537d19 --- /dev/null +++ b/compiler/testData/diagnostics/tests/multiplatform/hmpp/intermediateActualHasAdditionalSupertypes.kt @@ -0,0 +1,30 @@ +// ISSUE: KT-57369 + +// MODULE: common +// TARGET_PLATFORM: Common +interface CompletionHandler { + fun foo() +} + +expect class CompletionHandlerBase() + +fun invokeOnCompletion(handler: CompletionHandler) {} + +// MODULE: intermediate()()(common) +// TARGET_PLATFORM: Common +// actual has an additional super type +actual class CompletionHandlerBase : CompletionHandler { + override fun foo() {} +} + +fun cancelFutureOnCompletionAlt(handlerBase: CompletionHandlerBase) { + invokeOnCompletion(handlerBase) + handlerBase.foo() +} + +// MODULE: main()()(common, intermediate) +// the order of dependencies is important to reproduce KT-57369 +fun cancelFutureOnCompletion(handlerBase: CompletionHandlerBase) { + invokeOnCompletion(handlerBase) + handlerBase.foo() +} 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 646fe33592c..02dc2ebe982 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 @@ -22209,6 +22209,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/multiplatform/hmpp"), Pattern.compile("^(.*)\\.kts?$"), Pattern.compile("^(.+)\\.(reversed|fir|ll)\\.kts?$"), true); } + @Test + @TestMetadata("intermediateActualHasAdditionalSupertypes.kt") + public void testIntermediateActualHasAdditionalSupertypes() throws Exception { + runTest("compiler/testData/diagnostics/tests/multiplatform/hmpp/intermediateActualHasAdditionalSupertypes.kt"); + } + @Test @TestMetadata("kt-55570.kt") public void testKt_55570() throws Exception { diff --git a/compiler/tests-common-new/tests/org/jetbrains/kotlin/test/frontend/fir/FirFrontendFacade.kt b/compiler/tests-common-new/tests/org/jetbrains/kotlin/test/frontend/fir/FirFrontendFacade.kt index 8a4b8ca0789..6444947ec06 100644 --- a/compiler/tests-common-new/tests/org/jetbrains/kotlin/test/frontend/fir/FirFrontendFacade.kt +++ b/compiler/tests-common-new/tests/org/jetbrains/kotlin/test/frontend/fir/FirFrontendFacade.kt @@ -20,6 +20,7 @@ import org.jetbrains.kotlin.cli.jvm.config.jvmModularRoots import org.jetbrains.kotlin.config.CompilerConfiguration import org.jetbrains.kotlin.config.JVMConfigurationKeys import org.jetbrains.kotlin.config.LanguageFeature +import org.jetbrains.kotlin.container.topologicalSort import org.jetbrains.kotlin.fir.* import org.jetbrains.kotlin.fir.checkers.registerExtendedCommonCheckers import org.jetbrains.kotlin.fir.deserialization.ModuleDataProvider @@ -105,22 +106,9 @@ open class FirFrontendFacade( } protected fun sortDependsOnTopologically(module: TestModule): List { - val sortedModules = mutableListOf() - val visitedModules = mutableSetOf() - val modulesQueue = ArrayDeque() - modulesQueue.add(module) - - while (modulesQueue.isNotEmpty()) { - val currentModule = modulesQueue.removeFirst() - if (!visitedModules.add(currentModule)) continue - sortedModules.add(currentModule) - - for (dependency in currentModule.dependsOnDependencies) { - modulesQueue.add(testServices.dependencyProvider.getTestModule(dependency.moduleName)) - } + return topologicalSort(listOf(module), reverseOrder = true) { item -> + item.dependsOnDependencies.map { testServices.dependencyProvider.getTestModule(it.moduleName) } } - - return sortedModules.reversed() } private fun initializeModuleData(modules: List): Pair, ModuleDataProvider> { diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/HierarchicalMppIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/HierarchicalMppIT.kt index 48a8138e627..190b84277cc 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/HierarchicalMppIT.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/HierarchicalMppIT.kt @@ -334,6 +334,17 @@ open class HierarchicalMppIT : KGPBaseTest() { } } + @GradleTest + @DisplayName("KT-57369 K2/MPP: supertypes established in actual-classifiers from other source sets are not visible") + fun testHmppActualHasAdditionalSuperTypes(gradleVersion: GradleVersion) { + project( + "hierarchical-mpp-actual-has-additional-supertypes", + gradleVersion + ) { + build("assemble") + } + } + @GradleTest @DisplayName("Test that disambiguation attribute of Kotlin JVM Target is propagated to Java configurations") fun testMultipleJvmTargetsWithJavaAndDisambiguationAttributeKt31468(gradleVersion: GradleVersion) { diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/hierarchical-mpp-actual-has-additional-supertypes/build.gradle.kts b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/hierarchical-mpp-actual-has-additional-supertypes/build.gradle.kts new file mode 100644 index 00000000000..f138e8b7910 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/hierarchical-mpp-actual-has-additional-supertypes/build.gradle.kts @@ -0,0 +1,34 @@ +import org.jetbrains.kotlin.gradle.dsl.KotlinVersion +import org.jetbrains.kotlin.gradle.tasks.KotlinCompilationTask + +plugins { + kotlin("multiplatform") +} + +repositories { + mavenLocal() + maven("") + mavenCentral() +} + +kotlin { + jvm {} + sourceSets { + val common = maybeCreate("commonMain") + val concurrent = maybeCreate("concurrentMain") + val jvm = maybeCreate("jvmMain") + + concurrent.dependsOn(common) + jvm.dependsOn(concurrent) + jvm.dependsOn(common) + } +} + +tasks.withType>().configureEach { + compilerOptions { + with(project.providers) { + languageVersion.set(KotlinVersion.KOTLIN_2_0) + apiVersion.set(KotlinVersion.KOTLIN_2_0) + } + } +} diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/hierarchical-mpp-actual-has-additional-supertypes/settings.gradle.kts b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/hierarchical-mpp-actual-has-additional-supertypes/settings.gradle.kts new file mode 100644 index 00000000000..f02088a5132 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/hierarchical-mpp-actual-has-additional-supertypes/settings.gradle.kts @@ -0,0 +1 @@ +rootProject.name = "mpp-sandbox" diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/hierarchical-mpp-actual-has-additional-supertypes/src/commonMain/kotlin/Main.common.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/hierarchical-mpp-actual-has-additional-supertypes/src/commonMain/kotlin/Main.common.kt new file mode 100644 index 00000000000..9fb006ca6be --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/hierarchical-mpp-actual-has-additional-supertypes/src/commonMain/kotlin/Main.common.kt @@ -0,0 +1,12 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +interface CompletionHandler { + fun foo() +} + +expect class CompletionHandlerBase() + +fun invokeOnCompletion(handler: CompletionHandler) {} diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/hierarchical-mpp-actual-has-additional-supertypes/src/concurrentMain/kotlin/Main.concurrent.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/hierarchical-mpp-actual-has-additional-supertypes/src/concurrentMain/kotlin/Main.concurrent.kt new file mode 100644 index 00000000000..f731c07e8a2 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/hierarchical-mpp-actual-has-additional-supertypes/src/concurrentMain/kotlin/Main.concurrent.kt @@ -0,0 +1,13 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +actual class CompletionHandlerBase: CompletionHandler { + override fun foo() {} +} + +fun cancelFutureOnCompletionAlt(handlerBase: CompletionHandlerBase) { + invokeOnCompletion(handlerBase) + handlerBase.foo() +} diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/hierarchical-mpp-actual-has-additional-supertypes/src/jvmMain/kotlin/Main.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/hierarchical-mpp-actual-has-additional-supertypes/src/jvmMain/kotlin/Main.kt new file mode 100644 index 00000000000..e2dc7069fc3 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/hierarchical-mpp-actual-has-additional-supertypes/src/jvmMain/kotlin/Main.kt @@ -0,0 +1,9 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +fun cancelFutureOnCompletion(handlerBase: CompletionHandlerBase) { + invokeOnCompletion(handlerBase) + handlerBase.foo() +} diff --git a/native/native.tests/tests/org/jetbrains/kotlin/konan/blackboxtest/support/compilation/TestCompilationFactory.kt b/native/native.tests/tests/org/jetbrains/kotlin/konan/blackboxtest/support/compilation/TestCompilationFactory.kt index 3f537936eee..429c1c3331d 100644 --- a/native/native.tests/tests/org/jetbrains/kotlin/konan/blackboxtest/support/compilation/TestCompilationFactory.kt +++ b/native/native.tests/tests/org/jetbrains/kotlin/konan/blackboxtest/support/compilation/TestCompilationFactory.kt @@ -5,6 +5,7 @@ package org.jetbrains.kotlin.konan.blackboxtest.support.compilation +import org.jetbrains.kotlin.container.topologicalSort import org.jetbrains.kotlin.konan.blackboxtest.support.PackageName import org.jetbrains.kotlin.konan.blackboxtest.support.TestCase import org.jetbrains.kotlin.konan.blackboxtest.support.TestCase.* @@ -205,21 +206,8 @@ internal class TestCompilationFactory { return CompilationDependencies(klibDependencies, staticCacheDependencies) } - // mimics FirFrontendFacade.sortDependsOnTopologically(org.jetbrains.kotlin.test.model.TestModule) private fun sortDependsOnTopologically(module: TestModule): List { - val sortedModules = mutableListOf() - val visitedModules = mutableSetOf() - val modulesQueue = ArrayDeque() - modulesQueue.add(module) - - while (modulesQueue.isNotEmpty()) { - val currentModule = modulesQueue.removeFirst() - if (!visitedModules.add(currentModule)) continue - sortedModules.add(currentModule) - modulesQueue += currentModule.allDependsOn - } - - return sortedModules.reversed() + return topologicalSort(listOf(module), reverseOrder = true) { it.allDependsOn } } companion object {