From b10cc6657e49f074c2ade08d1efa216f57bd24b0 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Wed, 19 Aug 2020 20:03:17 +0200 Subject: [PATCH] Fix loading of builtins resources in kotlin-reflect in Java modular mode The main change here is that when kotlin-reflect is being run under Java 9+, we use another implementation of BuiltInsResourceLoader (see libraries/reflect/api/src/java9) which loads .kotlin_builtins files from the java.lang.Module instance of kotlin-stdlib, instead of the class loader of kotlin-reflect, which may not have access to those resources. If the application is being run in the old (classpath) mode, that Module represents the unnamed module, which contains everything on the classpath, and everything works as before. But if it's being run in the modular mode, that Module instance is an encapsulated module for kotlin-stdlib classes and resources, exactly where .kotlin_builtins files are located. This fixes a regression in 1.4.0. Prior to 1.4, kotlin-reflect and kotlin-stdlib were not named modules (see KT-21266) and were loaded as _automatic_ modules even if the application was run in the modular mode. Apparently, they shared the same class loader in that case and looking up .kotlin_builtins resources worked. This change was supposed to go alongside 828cc6dbf3a0775963cae2d5bc4992a4af3cbcd6, but was overlooked because adding module-info to standard libraries was postponed at that point. Also slightly refactor Java9ModulesIntegrationTest to simplify running compiled code, and add a smoke test on using kotlin-reflect in modular mode. #KT-40842 Fixed --- .../coroutinesDebugMetadata/stdout.txt | 1 - .../testData/javaModules/reflection/usage.txt | 1 + .../reflection/usage/module-info.java | 5 ++ .../javaModules/reflection/usage/usage.kt | 61 +++++++++++++++++++ .../compiler/Java9ModulesIntegrationTest.kt | 42 ++++++++----- .../components/ReflectKotlinClassFinder.kt | 5 +- 6 files changed, 99 insertions(+), 16 deletions(-) delete mode 100644 compiler/testData/javaModules/coroutinesDebugMetadata/stdout.txt create mode 100644 compiler/testData/javaModules/reflection/usage.txt create mode 100644 compiler/testData/javaModules/reflection/usage/module-info.java create mode 100644 compiler/testData/javaModules/reflection/usage/usage.kt diff --git a/compiler/testData/javaModules/coroutinesDebugMetadata/stdout.txt b/compiler/testData/javaModules/coroutinesDebugMetadata/stdout.txt deleted file mode 100644 index 26db5b8efd5..00000000000 --- a/compiler/testData/javaModules/coroutinesDebugMetadata/stdout.txt +++ /dev/null @@ -1 +0,0 @@ -usage/some.module.withsome.packages.Test diff --git a/compiler/testData/javaModules/reflection/usage.txt b/compiler/testData/javaModules/reflection/usage.txt new file mode 100644 index 00000000000..d86bac9de59 --- /dev/null +++ b/compiler/testData/javaModules/reflection/usage.txt @@ -0,0 +1 @@ +OK diff --git a/compiler/testData/javaModules/reflection/usage/module-info.java b/compiler/testData/javaModules/reflection/usage/module-info.java new file mode 100644 index 00000000000..9d7a52523a4 --- /dev/null +++ b/compiler/testData/javaModules/reflection/usage/module-info.java @@ -0,0 +1,5 @@ +module usage { + requires kotlin.reflect; + + opens usage.test to kotlin.reflect; +} diff --git a/compiler/testData/javaModules/reflection/usage/usage.kt b/compiler/testData/javaModules/reflection/usage/usage.kt new file mode 100644 index 00000000000..e7384647a7b --- /dev/null +++ b/compiler/testData/javaModules/reflection/usage/usage.kt @@ -0,0 +1,61 @@ +package usage.test + +import java.io.Serializable +import kotlin.reflect.full.createInstance +import kotlin.reflect.full.superclasses +import kotlin.reflect.jvm.javaMethod +import kotlin.reflect.jvm.kotlinFunction + +typealias TA = List + +open class B { + fun inherited(): C? = null +} + +class C : B() { + fun function(s: S): TA = listOf(s) + var property: Int? = 42 + fun String.extensionFunction(): Int = length + val U.extensionProperty: Unit get() = Unit +} + +fun box(): String { + val members = C::class.members.joinToString("\n") + if (members != """ + var usage.test.C.property: kotlin.Int? + val usage.test.C.(U.)extensionProperty: kotlin.Unit + fun usage.test.C.function(S): usage.test.TA /* = kotlin.collections.List */ + fun usage.test.C.(kotlin.String.)extensionFunction(): kotlin.Int + fun usage.test.C.equals(kotlin.Any?): kotlin.Boolean + fun usage.test.C.hashCode(): kotlin.Int + fun usage.test.C.inherited(): usage.test.C? + fun usage.test.C.toString(): kotlin.String + """.trimIndent()) + return "Fail members toString: $members" + + + val c = C::class.createInstance() + c.property = 239 + val callResult = (C<*>::property).call(c) + if (callResult != 239) + return "Fail call: $callResult" + + + val stringSuperclasses = String::class.superclasses + if (stringSuperclasses != listOf(Comparable::class, CharSequence::class, Serializable::class, Any::class)) + return "Fail superclasses: $stringSuperclasses" + + + val function = B::inherited + val javaMethod = function.javaMethod!! + val kotlinFunction = javaMethod.kotlinFunction!! + if (function != kotlinFunction) + return "Fail javaMethod/kotlinFunction:\nfunction=$function\njavaMethod=$javaMethod\nkotlinFunction=$kotlinFunction" + + + return "OK" +} + +fun main() { + println(box()) +} diff --git a/compiler/tests/org/jetbrains/kotlin/jvm/compiler/Java9ModulesIntegrationTest.kt b/compiler/tests/org/jetbrains/kotlin/jvm/compiler/Java9ModulesIntegrationTest.kt index df38713c144..b9f63fd9298 100644 --- a/compiler/tests/org/jetbrains/kotlin/jvm/compiler/Java9ModulesIntegrationTest.kt +++ b/compiler/tests/org/jetbrains/kotlin/jvm/compiler/Java9ModulesIntegrationTest.kt @@ -6,7 +6,6 @@ package org.jetbrains.kotlin.jvm.compiler import com.intellij.openapi.util.io.FileUtil -import junit.framework.TestCase import org.jetbrains.kotlin.cli.AbstractCliTest import org.jetbrains.kotlin.codegen.forTestCompile.ForTestCompileRuntime import org.jetbrains.kotlin.test.KotlinTestUtils @@ -60,6 +59,23 @@ class Java9ModulesIntegrationTest : AbstractKotlinCompilerIntegrationTest() { KotlinTestUtils.assertEqualsToFile(File(testDataDirectory, "$moduleName.txt"), actual) } + private data class ModuleRunResult(val stdout: String, val stderr: String) + + private fun runModule(className: String, modulePath: List): ModuleRunResult { + val command = listOf( + File(KotlinTestUtils.getJdk9Home(), "bin/java").path, + "-p", (modulePath + ForTestCompileRuntime.runtimeJarForTests()).joinToString(File.pathSeparator, transform = File::getPath), + "-m", className + ) + + val process = ProcessBuilder().command(command).start() + process.waitFor(1, TimeUnit.MINUTES) + return ModuleRunResult( + process.inputStream.reader().readText().trimEnd(), + process.errorStream.reader().readText().trimEnd() + ) + } + private fun createMultiReleaseJar(jdk9Home: File, destination: File, mainRoot: File, java9Root: File): File { val command = listOf( File(jdk9Home, "bin/jar").path, @@ -244,19 +260,17 @@ class Java9ModulesIntegrationTest : AbstractKotlinCompilerIntegrationTest() { } fun testCoroutinesDebugMetadata() { - val jar = module("usage", listOf(ForTestCompileRuntime.runtimeJarForTests())) + val usage = module("usage") + val (stdout, stderr) = runModule("usage/some.module.withsome.packages.UsageKt", listOf(usage)) + assertEquals("", stderr) + assertEquals("usage/some.module.withsome.packages.Test", stdout) + } - val command = listOf( - File(KotlinTestUtils.getJdk9Home(), "bin/java").path, - "-p", - "${ForTestCompileRuntime.runtimeJarForTests().path}${File.pathSeparator}${jar.path}", - "-m", - "usage/some.module.withsome.packages.UsageKt" - ) - - val process = ProcessBuilder().command(command).start() - process.waitFor(1, TimeUnit.MINUTES) - val got = process.inputStream.reader().readText() - KotlinTestUtils.assertEqualsToFile(File("$testDataDirectory/stdout.txt"), got) + fun testReflection() { + val reflect = ForTestCompileRuntime.reflectJarForTests() + val usage = module("usage", listOf(reflect)) + val (stdout, stderr) = runModule("usage/usage.test.UsageKt", listOf(usage, reflect)) + assertEquals("", stderr) + assertEquals("OK", stdout) } } diff --git a/core/descriptors.runtime/src/org/jetbrains/kotlin/descriptors/runtime/components/ReflectKotlinClassFinder.kt b/core/descriptors.runtime/src/org/jetbrains/kotlin/descriptors/runtime/components/ReflectKotlinClassFinder.kt index 7e7e1c54afd..c73212f6053 100644 --- a/core/descriptors.runtime/src/org/jetbrains/kotlin/descriptors/runtime/components/ReflectKotlinClassFinder.kt +++ b/core/descriptors.runtime/src/org/jetbrains/kotlin/descriptors/runtime/components/ReflectKotlinClassFinder.kt @@ -23,9 +23,12 @@ import org.jetbrains.kotlin.load.kotlin.KotlinClassFinder.Result.KotlinClass import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.serialization.deserialization.builtins.BuiltInSerializerProtocol +import org.jetbrains.kotlin.serialization.deserialization.builtins.BuiltInsResourceLoader import java.io.InputStream class ReflectKotlinClassFinder(private val classLoader: ClassLoader) : KotlinClassFinder { + private val builtInsResourceLoader = BuiltInsResourceLoader() + private fun findKotlinClass(fqName: String): KotlinClassFinder.Result? { return classLoader.tryLoadClass(fqName)?.let { ReflectKotlinClass.create(it) }?.let(::KotlinClass) } @@ -46,7 +49,7 @@ class ReflectKotlinClassFinder(private val classLoader: ClassLoader) : KotlinCla override fun findBuiltInsData(packageFqName: FqName): InputStream? { if (!packageFqName.startsWith(KotlinBuiltIns.BUILT_INS_PACKAGE_NAME)) return null - return classLoader.getResourceAsStream(BuiltInSerializerProtocol.getBuiltInsFilePath(packageFqName)) + return builtInsResourceLoader.loadResource(BuiltInSerializerProtocol.getBuiltInsFilePath(packageFqName)) } }