azalea/kotlin-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix reflection on top level declarations from other modules

Browse Source 
The main problem here is that moduleName that is being passed to KPackageImpl
is useless: as can be seen in
ClosureCodegen.generateCallableReferenceDeclarationContainer, the name of the
current module is always written to the class file for a callable reference,
not the name of the module of the referenced declaration. This resulted in
reflection not loading the correct .kotlin_module file and subsequently not
finding the required file facade for a top-level function.

The commit does not fix the issue with the incorrect module name written in the
back-end, but workarounds it. It turns out, reflection can figure out the name
of the module of the referenced declaration itself by parsing the header from
the given java.lang.Class object for a single-file/multi-file package facade
and extract the package_module_name protobuf extension. Similar code was
already there in Member.getKPackage() in ReflectJvmMapping.kt but it did not
support multi-file classes, of which there are a lot in the standard library;
this is now supported

 #KT-12630 Fixed
 #KT-14731 Fixed
This commit is contained in:
Alexander Udalov
2017-02-03 18:48:03 +03:00
parent 8a79482c5c
commit 6d16ea0a3a
14 changed files with 144 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/backend/src/org/jetbrains/kotlin/codegen/ClosureCodegen.java
+3
View File
@@ -396,6 +396,9 @@ public class ClosureCodegen extends MemberCodegen<KtElement> {
}
else if (container instanceof PackageFragmentDescriptor) {
iv.aconst(state.getTypeMapper().mapOwner(descriptor));
// Note that this name is not used in reflection. There should be the name of the referenced declaration's module instead,
// but there's no nice API to obtain that name here yet
// TODO: write the referenced declaration's module name and use it in reflection
iv.aconst(state.getModuleName());
iv.invokestatic(REFLECTION, "getOrCreateKotlinPackage",
Type.getMethodDescriptor(K_DECLARATION_CONTAINER_TYPE, getType(Class.class), getType(String.class)), false);
compiler/testData/codegen/box/reflection/functions/functionFromStdlib.kt
Vendored
+4 -1
View File
@@ -1,6 +1,9 @@
// IGNORE_BACKEND: JS
// WITH_REFLECT
fun doStuff(fn: String.() -> String) = "ok".fn()
import kotlin.reflect.KFunction1
fun doStuff(fn: KFunction1<String, String>) = fn.call("ok")
fun box(): String {
return doStuff(String::toUpperCase)
compiler/testData/codegen/box/reflection/methodsFromAny/functionFromStdlibMultiFileFacade.kt
Vendored
+13
View File
@@ -0,0 +1,13 @@
// KT-12630 KotlinReflectionInternalError on referencing some functions from stdlib
// IGNORE_BACKEND: JS
// WITH_REFLECT
import kotlin.test.*
fun box(): String {
val asIterable = List<Int>::asIterable
assertEquals("fun kotlin.collections.Iterable<T>.asIterable(): kotlin.collections.Iterable<T>", asIterable.toString())
return "OK"
}
compiler/testData/codegen/box/reflection/methodsFromAny/functionFromStdlibSingleFileFacade.kt
Vendored
+13
View File
@@ -0,0 +1,13 @@
// KT-12630 KotlinReflectionInternalError on referencing some functions from stdlib
// IGNORE_BACKEND: JS
// WITH_REFLECT
import kotlin.test.*
fun box(): String {
val lazyOf: (String) -> Lazy<String> = ::lazyOf
assertEquals("fun lazyOf(T): kotlin.Lazy<T>", lazyOf.toString())
return "OK"
}
compiler/testData/codegen/light-analysis/reflection/functions/functionFromStdlib.txt
Vendored
+1 -1
View File
@@ -1,5 +1,5 @@
@kotlin.Metadata
public final class FunctionFromStdlibKt {
public final static @org.jetbrains.annotations.NotNull method box(): java.lang.String
public final static @org.jetbrains.annotations.NotNull method doStuff(@org.jetbrains.annotations.NotNull p0: kotlin.jvm.functions.Function1): java.lang.String
public final static @org.jetbrains.annotations.NotNull method doStuff(@org.jetbrains.annotations.NotNull p0: kotlin.reflect.KFunction): java.lang.String
}
compiler/testData/codegen/light-analysis/reflection/methodsFromAny/functionFromStdlibMultiFileFacade.txt
Vendored
+4
View File
@@ -0,0 +1,4 @@
@kotlin.Metadata
public final class FunctionFromStdlibMultiFileFacadeKt {
public final static @org.jetbrains.annotations.NotNull method box(): java.lang.String
}
compiler/testData/codegen/light-analysis/reflection/methodsFromAny/functionFromStdlibSingleFileFacade.txt
Vendored
+4
View File
@@ -0,0 +1,4 @@
@kotlin.Metadata
public final class FunctionFromStdlibSingleFileFacadeKt {
public final static @org.jetbrains.annotations.NotNull method box(): java.lang.String
}
compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java
+12
View File
@@ -14380,6 +14380,18 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
doTest(fileName);
}
@TestMetadata("functionFromStdlibMultiFileFacade.kt")
public void testFunctionFromStdlibMultiFileFacade() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/methodsFromAny/functionFromStdlibMultiFileFacade.kt");
doTest(fileName);
}
@TestMetadata("functionFromStdlibSingleFileFacade.kt")
public void testFunctionFromStdlibSingleFileFacade() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/methodsFromAny/functionFromStdlibSingleFileFacade.kt");
doTest(fileName);
}
@TestMetadata("functionToString.kt")
public void testFunctionToString() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/methodsFromAny/functionToString.kt");
compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java
+12
View File
@@ -14380,6 +14380,18 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
doTest(fileName);
}
@TestMetadata("functionFromStdlibMultiFileFacade.kt")
public void testFunctionFromStdlibMultiFileFacade() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/methodsFromAny/functionFromStdlibMultiFileFacade.kt");
doTest(fileName);
}
@TestMetadata("functionFromStdlibSingleFileFacade.kt")
public void testFunctionFromStdlibSingleFileFacade() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/methodsFromAny/functionFromStdlibSingleFileFacade.kt");
doTest(fileName);
}
@TestMetadata("functionToString.kt")
public void testFunctionToString() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/methodsFromAny/functionToString.kt");
core/descriptor.loader.java/src/org/jetbrains/kotlin/load/kotlin/header/KotlinClassHeader.kt
+4 -1
View File
@@ -55,7 +55,10 @@ class KotlinClassHeader(
}
val multifileClassName: String?
get() = if (kind == Kind.MULTIFILE_CLASS_PART) extraString else null
get() = extraString.takeIf { kind == Kind.MULTIFILE_CLASS_PART }
val multifilePartNames: List<String>
get() = data.takeIf { kind == Kind.MULTIFILE_CLASS }?.asList().orEmpty()
// TODO: use in incremental compilation
val multifileClassKind: MultifileClassKind?
core/reflection.jvm/src/kotlin/reflect/jvm/ReflectJvmMapping.kt
+3 -20
View File
@@ -17,18 +17,14 @@
@file:JvmName("ReflectJvmMapping")
package kotlin.reflect.jvm
import org.jetbrains.kotlin.load.java.JvmAbi
import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader
import org.jetbrains.kotlin.load.kotlin.reflect.ReflectKotlinClass
import org.jetbrains.kotlin.serialization.jvm.JvmProtoBuf
import org.jetbrains.kotlin.serialization.jvm.JvmProtoBufUtil
import java.lang.reflect.*
import java.util.*
import kotlin.jvm.internal.Reflection
import kotlin.reflect.*
import kotlin.reflect.full.companionObject
import kotlin.reflect.full.functions
import kotlin.reflect.full.memberProperties
import kotlin.reflect.jvm.internal.KPackageImpl
import kotlin.reflect.jvm.internal.KTypeImpl
import kotlin.reflect.jvm.internal.asKCallableImpl
import kotlin.reflect.jvm.internal.asKPropertyImpl
@@ -104,21 +100,8 @@ val Field.kotlinProperty: KProperty<*>?
}
private fun Member.getKPackage(): KDeclarationContainer? {
// TODO: support multifile classes
val header = ReflectKotlinClass.create(declaringClass)?.classHeader
if (header != null && header.kind == KotlinClassHeader.Kind.FILE_FACADE && header.metadataVersion.isCompatible()) {
// TODO: avoid reading and parsing metadata twice (here and later in KPackageImpl#descriptor)
val (nameResolver, proto) = JvmProtoBufUtil.readPackageDataFrom(header.data!!, header.strings!!)
val moduleName =
if (proto.hasExtension(JvmProtoBuf.packageModuleName))
nameResolver.getString(proto.getExtension(JvmProtoBuf.packageModuleName))
else JvmAbi.DEFAULT_MODULE_NAME
return Reflection.getOrCreateKotlinPackage(declaringClass, moduleName)
}
return null
}
private fun Member.getKPackage(): KDeclarationContainer? =
if (ReflectKotlinClass.create(declaringClass) != null) KPackageImpl(declaringClass) else null
/**
* Returns a [KFunction] instance corresponding to the given Java [Method] instance,
core/reflection.jvm/src/kotlin/reflect/jvm/internal/KPackageImpl.kt
+11 -4
View File
@@ -28,20 +28,27 @@ import org.jetbrains.kotlin.serialization.deserialization.descriptors.Deserializ
import kotlin.reflect.KCallable
import kotlin.reflect.jvm.internal.KDeclarationContainerImpl.MemberBelonginess.DECLARED
internal class KPackageImpl(override val jClass: Class<*>, val moduleName: String) : KDeclarationContainerImpl() {
internal class KPackageImpl(
override val jClass: Class<*>,
@Suppress("unused") val usageModuleName: String? = null // may be useful for debug
) : KDeclarationContainerImpl() {
private inner class Data : KDeclarationContainerImpl.Data() {
val kotlinClass: ReflectKotlinClass? by ReflectProperties.lazySoft {
// TODO: do not read ReflectKotlinClass multiple times
ReflectKotlinClass.create(jClass)
}
val descriptor: PackageViewDescriptor by ReflectProperties.lazySoft {
with(moduleData) {
packagePartProvider.registerModule(moduleName)
kotlinClass?.packageModuleName?.let(packagePartProvider::registerModule)
module.getPackage(jClass.classId.packageFqName)
}
}
val methodOwner: Class<*> by ReflectProperties.lazy {
val facadeName = ReflectKotlinClass.create(jClass)?.classHeader?.multifileClassName
val facadeName = kotlinClass?.classHeader?.multifileClassName
// We need to check isNotEmpty because this is the value read from the annotation which cannot be null.
// The default value for 'xs' is empty string, as declared in kotlin.Metadata
// TODO: do not read ReflectKotlinClass multiple times, obtain facade name from descriptor
if (facadeName != null && facadeName.isNotEmpty()) {
jClass.classLoader.loadClass(facadeName.replace('/', '.'))
}
core/reflection.jvm/src/kotlin/reflect/jvm/internal/util.kt
+29 -1
View File
@@ -20,22 +20,26 @@ import org.jetbrains.kotlin.descriptors.ClassDescriptor
import org.jetbrains.kotlin.descriptors.Visibilities
import org.jetbrains.kotlin.descriptors.Visibility
import org.jetbrains.kotlin.descriptors.annotations.Annotated
import org.jetbrains.kotlin.load.java.JvmAbi
import org.jetbrains.kotlin.load.java.components.RuntimeSourceElementFactory
import org.jetbrains.kotlin.load.java.reflect.tryLoadClass
import org.jetbrains.kotlin.load.java.structure.reflect.ReflectJavaAnnotation
import org.jetbrains.kotlin.load.java.structure.reflect.ReflectJavaClass
import org.jetbrains.kotlin.load.java.structure.reflect.safeClassLoader
import org.jetbrains.kotlin.load.kotlin.KotlinJvmBinarySourceElement
import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader
import org.jetbrains.kotlin.load.kotlin.reflect.ReflectAnnotationSource
import org.jetbrains.kotlin.load.kotlin.reflect.ReflectKotlinClass
import org.jetbrains.kotlin.name.FqName
import org.jetbrains.kotlin.platform.JavaToKotlinClassMap
import org.jetbrains.kotlin.resolve.DescriptorUtils
import org.jetbrains.kotlin.resolve.descriptorUtil.classId
import org.jetbrains.kotlin.serialization.jvm.JvmProtoBuf
import org.jetbrains.kotlin.serialization.jvm.JvmProtoBufUtil
import kotlin.jvm.internal.FunctionReference
import kotlin.jvm.internal.PropertyReference
import kotlin.reflect.full.IllegalCallableAccessException
import kotlin.reflect.KVisibility
import kotlin.reflect.full.IllegalCallableAccessException
internal val JVM_STATIC = FqName("kotlin.jvm.JvmStatic")
@@ -117,3 +121,27 @@ internal fun Any?.asKPropertyImpl(): KPropertyImpl<*>? =
internal fun Any?.asKCallableImpl(): KCallableImpl<*>? =
this as? KCallableImpl<*> ?: asKFunctionImpl() ?: asKPropertyImpl()
internal val ReflectKotlinClass.packageModuleName: String?
get() {
val header = classHeader
if (!header.metadataVersion.isCompatible()) return null
return when (header.kind) {
KotlinClassHeader.Kind.FILE_FACADE, KotlinClassHeader.Kind.MULTIFILE_CLASS_PART -> {
// TODO: avoid reading and parsing metadata twice (here and later in KPackageImpl#descriptor)
val (nameResolver, proto) = JvmProtoBufUtil.readPackageDataFrom(header.data!!, header.strings!!)
// If no packageModuleName extension is written, the name is assumed to be JvmAbi.DEFAULT_MODULE_NAME
// (see JvmSerializerExtension.serializePackage)
if (proto.hasExtension(JvmProtoBuf.packageModuleName))
nameResolver.getString(proto.getExtension(JvmProtoBuf.packageModuleName))
else JvmAbi.DEFAULT_MODULE_NAME
}
KotlinClassHeader.Kind.MULTIFILE_CLASS -> {
val partName = header.multifilePartNames.firstOrNull() ?: return null
ReflectKotlinClass.create(klass.classLoader.loadClass(partName.replace('/', '.')))?.packageModuleName
}
else -> null
}
}
js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java
+31 -1
View File
@@ -16859,7 +16859,13 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest {
@TestMetadata("functionFromStdlib.kt")
public void testFunctionFromStdlib() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/functions/functionFromStdlib.kt");
doTest(fileName);
try {
doTest(fileName);
}
catch (Throwable ignore) {
return;
}
throw new AssertionError("Looks like this test can be unmuted. Remove IGNORE_BACKEND directive for that.");
}
@TestMetadata("functionReferenceErasedToKFunction.kt")
@@ -17645,6 +17651,30 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest {
throw new AssertionError("Looks like this test can be unmuted. Remove IGNORE_BACKEND directive for that.");
}
@TestMetadata("functionFromStdlibMultiFileFacade.kt")
public void testFunctionFromStdlibMultiFileFacade() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/methodsFromAny/functionFromStdlibMultiFileFacade.kt");
try {
doTest(fileName);
}
catch (Throwable ignore) {
return;
}
throw new AssertionError("Looks like this test can be unmuted. Remove IGNORE_BACKEND directive for that.");
}
@TestMetadata("functionFromStdlibSingleFileFacade.kt")
public void testFunctionFromStdlibSingleFileFacade() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/methodsFromAny/functionFromStdlibSingleFileFacade.kt");
try {
doTest(fileName);
}
catch (Throwable ignore) {
return;
}
throw new AssertionError("Looks like this test can be unmuted. Remove IGNORE_BACKEND directive for that.");
}
@TestMetadata("functionToString.kt")
public void testFunctionToString() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/methodsFromAny/functionToString.kt");