[IC] Baseline fix for common sources getting access to platform declarations

K2-only issue. In an incremental build, sourceSet boundary isn't
preserved in certain conditions:

SourceSet A depends on SourceSet B
A and B overload a function, for example:

A -> fun foo(p: Parent) // (1)
B -> fun foo(c: Child) // (2)

If some source file in A is calling foo(..), only (1) is supposed to be visible
for that call site.

However, if

a) it is an incremental build,
b) the declaration of (1) is not a part of the compilation set, and
c) call to foo(c: Child) is applicable,

then (2) would be called from the generated code. So, the build result is not
consistent between the full build and an incremental build.

As a workaround, we fallback to a non-incremental build, if any source from A
needs to be compiled.

To enable "risky" incremental builds, use Gradle property
kotlin.internal.incremental.enableUnsafeOptimizationsForMultiplatform=true

^KT-62686
^KT-63837 Fixed


Merge-request: KT-MR-13695
Merged-by: Evgenii Mazhukin <evgenii.mazhukin@jetbrains.com>
This commit is contained in:
Evgenii Mazhukin
2024-01-08 20:03:08 +00:00
committed by Space Team
parent ad6513e247
commit de4953adf6
26 changed files with 401 additions and 12 deletions
@@ -0,0 +1,156 @@
/*
* 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.
*/
package org.jetbrains.kotlin.gradle.mpp
import org.gradle.api.logging.LogLevel
import org.gradle.util.GradleVersion
import org.jetbrains.kotlin.build.report.metrics.BuildAttribute
import org.jetbrains.kotlin.gradle.testbase.*
import org.jetbrains.kotlin.gradle.util.replaceWithVersion
import org.jetbrains.kotlin.test.TestMetadata
import org.junit.jupiter.api.DisplayName
/**
* Normal behavior is tested by smokes: [org.jetbrains.kotlin.gradle.mpp.smoke.MultiModuleIncrementalCompilationIT]
*
* [CommonCodeWithPlatformSymbolsIT] should be removed with the IC option when the underlying compiler issue is fixed
*/
open class CommonCodeWithPlatformSymbolsITBase(val platformSourceSet: String, val taskToExecute: String) : KGPBaseTest() {
override val defaultBuildOptions: BuildOptions
get() = super.defaultBuildOptions.copy(
logLevel = LogLevel.DEBUG,
languageVersion = "2.0",
enableUnsafeIncrementalCompilationForMultiplatform = false
)
@GradleTest
@DisplayName("Baseline - compilation failure without hotfix")
@TestMetadata("kt-62686-mpp-source-set-boundary")
fun testBaselineForCommonSourceSetIC(gradleVersion: GradleVersion) {
project(
"kt-62686-mpp-source-set-boundary",
gradleVersion,
buildOptions = defaultBuildOptions.copy(enableUnsafeIncrementalCompilationForMultiplatform = true)
) {
// initial build is good
build(taskToExecute) {
assertTasksExecuted(taskToExecute)
}
projectPath.resolve("src/commonMain/kotlin/riskyCode.kt").replaceWithVersion("useMemberFunctionFromExpectClass")
// common source file is recompiled, and the incorrect overload resolution happens:
buildAndFail(taskToExecute) {
assertTasksFailed(taskToExecute)
assertOutputContains("Return type mismatch: expected 'kotlin.String', actual '${platformSourceSet}OnlyType'.")
}
// but changed code is valid for full compilation:
build("clean", taskToExecute) {
assertTasksExecuted(taskToExecute)
}
}
}
@GradleTest
@DisplayName("Use member function of an expect class that is overloaded in the actual class")
@TestMetadata("kt-62686-mpp-source-set-boundary")
fun testIncrementalBuildWithOverloadedMemberFunction(gradleVersion: GradleVersion) {
project(
"kt-62686-mpp-source-set-boundary", gradleVersion
) {
build(taskToExecute) {
assertTasksExecuted(taskToExecute)
}
projectPath.resolve("src/commonMain/kotlin/riskyCode.kt").replaceWithVersion("useMemberFunctionFromExpectClass")
build(taskToExecute) {
assertTasksExecuted(taskToExecute)
assertNonIncrementalCompilation(BuildAttribute.UNSAFE_INCREMENTAL_CHANGE_KT_62686)
}
}
}
@GradleTest
@DisplayName("Use top level function that is overloaded in platform sourceSet")
@TestMetadata("kt-62686-mpp-source-set-boundary")
fun testIncrementalBuildWithOverloadedTopLevelFunction(gradleVersion: GradleVersion) {
project(
"kt-62686-mpp-source-set-boundary", gradleVersion
) {
build(taskToExecute) {
assertTasksExecuted(taskToExecute)
}
projectPath.resolve("src/commonMain/kotlin/riskyCode.kt").replaceWithVersion("useTopLevelFunction")
build(taskToExecute) {
assertTasksExecuted(taskToExecute)
assertNonIncrementalCompilation(BuildAttribute.UNSAFE_INCREMENTAL_CHANGE_KT_62686)
}
}
}
@GradleTest
@DisplayName("Touch risky code by change in common sourceSet")
@TestMetadata("kt-62686-mpp-source-set-boundary")
fun testIncrementalBuildWhereRiskyCodeIsTouchedIndirectly(gradleVersion: GradleVersion) {
project(
"kt-62686-mpp-source-set-boundary", gradleVersion
) {
projectPath.resolve("src/commonMain/kotlin/riskyCode.kt").replaceWithVersion("useMemberFunctionFromExpectClass")
build(taskToExecute) {
assertTasksExecuted(taskToExecute)
}
projectPath.resolve("src/commonMain/kotlin/dependedOnByRiskyCode.kt")
.replaceWithVersion("changeType")
build(taskToExecute) {
assertTasksExecuted(taskToExecute)
assertNonIncrementalCompilation(BuildAttribute.UNSAFE_INCREMENTAL_CHANGE_KT_62686)
}
}
}
@GradleTest
@DisplayName("Touch risky code by change in platform sourceSet")
@TestMetadata("kt-62686-mpp-source-set-boundary")
fun testIncrementalBuildWhereRiskyCodeIsTouchedByChangeInPlatformSourceSet(gradleVersion: GradleVersion) {
project(
"kt-62686-mpp-source-set-boundary", gradleVersion
) {
projectPath.resolve("src/commonMain/kotlin/riskyCode.kt").replaceWithVersion("useMemberFunctionFromExpectClass")
build(taskToExecute) {
assertTasksExecuted(taskToExecute)
}
projectPath.resolve("src/$platformSourceSet/kotlin/dependedOnByActualDeclaration.kt")
.replaceWithVersion("changeType")
build(taskToExecute) {
assertTasksExecuted(taskToExecute)
assertNonIncrementalCompilation(BuildAttribute.UNSAFE_INCREMENTAL_CHANGE_KT_62686)
}
}
}
}
@MppGradlePluginTests
@DisplayName("Tests for IC with compatible overloads in common and platform sourceSets - Jvm")
class CommonCodeWithPlatformSymbolsJvmIT() : CommonCodeWithPlatformSymbolsITBase(
platformSourceSet = "jvmMain", taskToExecute = ":compileKotlinJvm"
)
@MppGradlePluginTests
@DisplayName("Tests for IC with compatible overloads in common and platform sourceSets - Js")
class CommonCodeWithPlatformSymbolsJsIT() : CommonCodeWithPlatformSymbolsITBase(
platformSourceSet = "jsMain", taskToExecute = ":compileKotlinJs"
)
@@ -20,7 +20,12 @@ import java.nio.file.Path
abstract class KmpIncrementalITBase : KGPBaseTest() {
override val defaultBuildOptions: BuildOptions
get() = super.defaultBuildOptions.copyEnsuringK2()
get() = super.defaultBuildOptions.copyEnsuringK2().copy(
/**
* disable IC-breaking feature; it's tested separately in [org.jetbrains.kotlin.gradle.mpp.CommonCodeWithPlatformSymbolsITBase]
*/
enableUnsafeIncrementalCompilationForMultiplatform = true
)
protected open val gradleTask = "assemble"
protected open val projectName = "generic-kmp-app-plus-lib-with-tests"
@@ -22,7 +22,10 @@ import org.junit.jupiter.api.DisplayName
@MppGradlePluginTests
open class ExpectActualIncrementalCompilationIT : KGPBaseTest() {
override val defaultBuildOptions: BuildOptions
get() = super.defaultBuildOptions.copyEnsuringK2()
get() = super.defaultBuildOptions.copyEnsuringK2().copy(
// disable IC-breaking feature; it's tested separately in [org.jetbrains.kotlin.gradle.mpp.CommonCodeWithPlatformSymbolsITBase]
enableUnsafeIncrementalCompilationForMultiplatform = true
)
@DisplayName("File with actual declaration needs recompiling")
@GradleTest
@@ -46,6 +46,7 @@ data class BuildOptions(
val statisticsForceValidation: Boolean = true,
val usePreciseOutputsBackup: Boolean? = null,
val keepIncrementalCompilationCachesInMemory: Boolean? = null,
val enableUnsafeIncrementalCompilationForMultiplatform: Boolean? = null,
val useDaemonFallbackStrategy: Boolean = false,
val useParsableDiagnosticsFormatting: Boolean = true,
val showDiagnosticsStacktrace: Boolean? = false, // false by default to not clutter the testdata + stacktraces change often
@@ -195,6 +196,10 @@ data class BuildOptions(
arguments.add("-Pkotlin.compiler.keepIncrementalCompilationCachesInMemory=$keepIncrementalCompilationCachesInMemory")
}
if (enableUnsafeIncrementalCompilationForMultiplatform != null) {
arguments.add("-Pkotlin.internal.incremental.enableUnsafeOptimizationsForMultiplatform=$enableUnsafeIncrementalCompilationForMultiplatform")
}
arguments.add("-Pkotlin.daemon.useFallbackStrategy=$useDaemonFallbackStrategy")
if (useParsableDiagnosticsFormatting) {
@@ -0,0 +1,18 @@
plugins {
kotlin("multiplatform")
}
repositories {
mavenLocal()
mavenCentral()
}
kotlin {
jvm().compilations.all {
compilerOptions.configure {
// log level isn't properly used to set `verbose` in the default configuration, fix is WIP in KT-64698
verbose.convention(true)
}
}
js(IR)
}
@@ -0,0 +1,6 @@
fun test(): String = "!" //A().foo(Child())
fun test2(): String = "?" //foo(Child())
private val dependency = dependedOnByRiskyCode
@@ -0,0 +1,6 @@
fun test(): String = A().foo(Child())
fun test2(): String = "?" //foo(Child())
private val dependency = dependedOnByRiskyCode
@@ -0,0 +1,6 @@
fun test(): String = "!" //A().foo(Child())
fun test2(): String = foo(Child())
private val dependency = dependedOnByRiskyCode
@@ -0,0 +1,9 @@
open class Base
class Child: Base()
fun foo(x: Base): String = ""
expect class A() {
fun foo(x: Base): String
}
@@ -0,0 +1,18 @@
class jsMainOnlyType()
actual class A {
actual fun foo(x: Base) = "aloha"
fun foo(x: Child) = jsMainOnlyType()
}
fun foo(x: Child) = jsMainOnlyType()
fun testPlatform() =
A().foo(Child())
fun main() {
println(test())
println(testPlatform())
}
private val dependency = dependedOnByActualDeclaration
@@ -0,0 +1,18 @@
class jvmMainOnlyType
actual class A {
actual fun foo(x: Base) = "aloha"
fun foo(x: Child) = jvmMainOnlyType()
}
fun foo(x: Child) = jvmMainOnlyType()
fun testPlatform() =
A().foo(Child())
fun main() {
println(test())
println(testPlatform())
}
private val dependency = dependedOnByActualDeclaration
@@ -540,6 +540,13 @@ internal class PropertiesProvider private constructor(private val project: Proje
.map { KotlinCompilerArgumentsLogLevel.fromPropertyValue(it) }
.orElse(KotlinCompilerArgumentsLogLevel.DEFAULT)
/**
* Without unsafe optimization: in k2, if common source is dirty, module will be rebuilt.
* With unsafe optimization: regular IC logic is used. Common sources might see declarations from platform sources. See KT-62686
*/
val enableUnsafeOptimizationsForMultiplatform: Boolean
get() = booleanProperty(PropertyNames.KOTLIN_UNSAFE_MULTIPLATFORM_INCREMENTAL_COMPILATION) ?: false
/**
* Retrieves a comma-separated list of browsers to use when running karma tests for [target]
* @see KOTLIN_JS_KARMA_BROWSERS
@@ -648,6 +655,7 @@ internal class PropertiesProvider private constructor(private val project: Proje
val KOTLIN_CREATE_ARCHIVE_TASKS_FOR_CUSTOM_COMPILATIONS =
property("$KOTLIN_INTERNAL_NAMESPACE.mpp.createArchiveTasksForCustomCompilations")
val KOTLIN_COMPILER_ARGUMENTS_LOG_LEVEL = property("$KOTLIN_INTERNAL_NAMESPACE.compiler.arguments.log.level")
val KOTLIN_UNSAFE_MULTIPLATFORM_INCREMENTAL_COMPILATION = property("$KOTLIN_INTERNAL_NAMESPACE.incremental.enableUnsafeOptimizationsForMultiplatform")
}
companion object {
@@ -218,6 +218,10 @@ abstract class AbstractKotlinCompile<T : CommonCompilerArguments> @Inject constr
@get:Internal
internal abstract val keepIncrementalCompilationCachesInMemory: Property<Boolean>
/** See [org.jetbrains.kotlin.incremental.IncrementalCompilationFeatures.enableUnsafeIncrementalCompilationForMultiplatform] */
@get:Internal
internal abstract val enableUnsafeIncrementalCompilationForMultiplatform: Property<Boolean>
/** Task outputs that we don't want to include in [TaskOutputsBackup] (see [TaskOutputsBackup.outputsToRestore] for more info). */
@get:Internal
internal abstract val taskOutputsBackupExcludes: SetProperty<File>
@@ -302,6 +306,7 @@ abstract class AbstractKotlinCompile<T : CommonCompilerArguments> @Inject constr
return IncrementalCompilationFeatures(
preciseCompilationResultsBackup = preciseCompilationResultsBackup.get(),
keepIncrementalCompilationCachesInMemory = keepIncrementalCompilationCachesInMemory.get(),
enableUnsafeIncrementalCompilationForMultiplatform = enableUnsafeIncrementalCompilationForMultiplatform.get(),
)
}
@@ -80,6 +80,9 @@ internal abstract class AbstractKotlinCompileConfig<TASK : AbstractKotlinCompile
task.taskOutputsBackupExcludes.addAll(task.keepIncrementalCompilationCachesInMemory.map {
if (it) listOf(task.taskBuildCacheableOutputDirectory.get().asFile) else emptyList()
})
task.enableUnsafeIncrementalCompilationForMultiplatform
.convention(propertiesProvider.enableUnsafeOptimizationsForMultiplatform)
.finalizeValueOnRead()
task.buildFinishedListenerService.value(buildFinishedListenerService).disallowChanges()
task.buildIdService.value(buildIdService).disallowChanges()