[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
This commit is contained in:
Kirill Rakhman
2023-03-22 12:10:48 +01:00
committed by Space Team
parent 6fe0849402
commit 60b227c519
15 changed files with 174 additions and 33 deletions
@@ -19,7 +19,7 @@ package org.jetbrains.kotlin.container
import java.util.ArrayList import java.util.ArrayList
import java.util.HashSet import java.util.HashSet
fun <T> topologicalSort(items: Iterable<T>, dependencies: (T) -> Iterable<T>): List<T> { fun <T> topologicalSort(items: Iterable<T>, reverseOrder: Boolean = false, dependencies: (T) -> Iterable<T>): List<T> {
val itemsInProgress = HashSet<T>() val itemsInProgress = HashSet<T>()
val completedItems = HashSet<T>() val completedItems = HashSet<T>()
val result = ArrayList<T>() val result = ArrayList<T>()
@@ -45,7 +45,7 @@ fun <T> topologicalSort(items: Iterable<T>, dependencies: (T) -> Iterable<T>): L
for (item in items) for (item in items)
DfsVisit(item) DfsVisit(item)
return result.apply { reverse() } return result.apply { if (!reverseOrder) reverse() }
} }
class CycleInTopoSortException : Exception() class CycleInTopoSortException : Exception()
@@ -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); 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 @Test
@TestMetadata("kt-55570.kt") @TestMetadata("kt-55570.kt")
public void testKt_55570() throws Exception { public void testKt_55570() throws Exception {
@@ -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); 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 @Test
@TestMetadata("kt-55570.kt") @TestMetadata("kt-55570.kt")
public void testKt_55570() throws Exception { public void testKt_55570() throws Exception {
@@ -6,6 +6,7 @@
package org.jetbrains.kotlin.fir.session package org.jetbrains.kotlin.fir.session
import org.jetbrains.kotlin.config.LanguageVersionSettings import org.jetbrains.kotlin.config.LanguageVersionSettings
import org.jetbrains.kotlin.container.topologicalSort
import org.jetbrains.kotlin.fir.* import org.jetbrains.kotlin.fir.*
import org.jetbrains.kotlin.fir.checkers.registerCommonCheckers import org.jetbrains.kotlin.fir.checkers.registerCommonCheckers
import org.jetbrains.kotlin.fir.deserialization.ModuleDataProvider import org.jetbrains.kotlin.fir.deserialization.ModuleDataProvider
@@ -139,7 +140,13 @@ abstract class FirAbstractSessionFactory {
private fun FirSession.computeDependencyProviderList(moduleData: FirModuleData): List<FirSymbolProvider> { private fun FirSession.computeDependencyProviderList(moduleData: FirModuleData): List<FirSymbolProvider> {
val visited = mutableSetOf<FirSymbolProvider>() val visited = mutableSetOf<FirSymbolProvider>()
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) } .mapNotNull { sessionProvider?.getSession(it) }
.map { it.symbolProvider } .map { it.symbolProvider }
.flatMap { it.flatten(visited, collectSourceProviders = it.session.kind == FirSession.Kind.Source) } .flatMap { it.flatten(visited, collectSourceProviders = it.session.kind == FirSession.Kind.Source) }
@@ -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()
}
@@ -0,0 +1,30 @@
// ISSUE: KT-57369
// MODULE: common
// TARGET_PLATFORM: Common
interface CompletionHandler {
fun foo()
}
expect <!EXPECT_AND_ACTUAL_IN_THE_SAME_MODULE{COMMON}!>class CompletionHandlerBase<!>()
fun invokeOnCompletion(handler: CompletionHandler) {}
// MODULE: intermediate()()(common)
// TARGET_PLATFORM: Common
// actual has an additional super type
actual <!EXPECT_AND_ACTUAL_IN_THE_SAME_MODULE!>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()
}
@@ -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); 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 @Test
@TestMetadata("kt-55570.kt") @TestMetadata("kt-55570.kt")
public void testKt_55570() throws Exception { public void testKt_55570() throws Exception {
@@ -20,6 +20,7 @@ import org.jetbrains.kotlin.cli.jvm.config.jvmModularRoots
import org.jetbrains.kotlin.config.CompilerConfiguration import org.jetbrains.kotlin.config.CompilerConfiguration
import org.jetbrains.kotlin.config.JVMConfigurationKeys import org.jetbrains.kotlin.config.JVMConfigurationKeys
import org.jetbrains.kotlin.config.LanguageFeature import org.jetbrains.kotlin.config.LanguageFeature
import org.jetbrains.kotlin.container.topologicalSort
import org.jetbrains.kotlin.fir.* import org.jetbrains.kotlin.fir.*
import org.jetbrains.kotlin.fir.checkers.registerExtendedCommonCheckers import org.jetbrains.kotlin.fir.checkers.registerExtendedCommonCheckers
import org.jetbrains.kotlin.fir.deserialization.ModuleDataProvider import org.jetbrains.kotlin.fir.deserialization.ModuleDataProvider
@@ -105,22 +106,9 @@ open class FirFrontendFacade(
} }
protected fun sortDependsOnTopologically(module: TestModule): List<TestModule> { protected fun sortDependsOnTopologically(module: TestModule): List<TestModule> {
val sortedModules = mutableListOf<TestModule>() return topologicalSort(listOf(module), reverseOrder = true) { item ->
val visitedModules = mutableSetOf<TestModule>() item.dependsOnDependencies.map { testServices.dependencyProvider.getTestModule(it.moduleName) }
val modulesQueue = ArrayDeque<TestModule>()
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 sortedModules.reversed()
} }
private fun initializeModuleData(modules: List<TestModule>): Pair<Map<TestModule, FirModuleData>, ModuleDataProvider> { private fun initializeModuleData(modules: List<TestModule>): Pair<Map<TestModule, FirModuleData>, ModuleDataProvider> {
@@ -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 @GradleTest
@DisplayName("Test that disambiguation attribute of Kotlin JVM Target is propagated to Java configurations") @DisplayName("Test that disambiguation attribute of Kotlin JVM Target is propagated to Java configurations")
fun testMultipleJvmTargetsWithJavaAndDisambiguationAttributeKt31468(gradleVersion: GradleVersion) { fun testMultipleJvmTargetsWithJavaAndDisambiguationAttributeKt31468(gradleVersion: GradleVersion) {
@@ -0,0 +1,34 @@
import org.jetbrains.kotlin.gradle.dsl.KotlinVersion
import org.jetbrains.kotlin.gradle.tasks.KotlinCompilationTask
plugins {
kotlin("multiplatform")
}
repositories {
mavenLocal()
maven("<localRepo>")
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<KotlinCompilationTask<*>>().configureEach {
compilerOptions {
with(project.providers) {
languageVersion.set(KotlinVersion.KOTLIN_2_0)
apiVersion.set(KotlinVersion.KOTLIN_2_0)
}
}
}
@@ -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) {}
@@ -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()
}
@@ -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()
}
@@ -5,6 +5,7 @@
package org.jetbrains.kotlin.konan.blackboxtest.support.compilation 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.PackageName
import org.jetbrains.kotlin.konan.blackboxtest.support.TestCase import org.jetbrains.kotlin.konan.blackboxtest.support.TestCase
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) return CompilationDependencies(klibDependencies, staticCacheDependencies)
} }
// mimics FirFrontendFacade.sortDependsOnTopologically(org.jetbrains.kotlin.test.model.TestModule)
private fun sortDependsOnTopologically(module: TestModule): List<TestModule> { private fun sortDependsOnTopologically(module: TestModule): List<TestModule> {
val sortedModules = mutableListOf<TestModule>() return topologicalSort(listOf(module), reverseOrder = true) { it.allDependsOn }
val visitedModules = mutableSetOf<TestModule>()
val modulesQueue = ArrayDeque<TestModule>()
modulesQueue.add(module)
while (modulesQueue.isNotEmpty()) {
val currentModule = modulesQueue.removeFirst()
if (!visitedModules.add(currentModule)) continue
sortedModules.add(currentModule)
modulesQueue += currentModule.allDependsOn
}
return sortedModules.reversed()
} }
companion object { companion object {