From 583733be8da2f96336cb0158d51f5cc94356e010 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Wed, 13 Apr 2016 15:59:42 +0300 Subject: [PATCH] KT-11645 properly handle private property getter name mangling in reflection --- .../codegen/state/KotlinTypeMapper.java | 12 +++--- .../javaFieldForVarAndConstVal.kt | 20 ++++++++++ .../bytecodeListing/emptyMultifileFacade.txt | 2 +- .../multifileFacadeMembers.json | 9 +++-- .../reflect/jvm/internal/RuntimeTypeMapper.kt | 38 +++++++++++-------- 5 files changed, 56 insertions(+), 25 deletions(-) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/state/KotlinTypeMapper.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/state/KotlinTypeMapper.java index 8d6d34dda83..178d67823d1 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/state/KotlinTypeMapper.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/state/KotlinTypeMapper.java @@ -34,6 +34,7 @@ import org.jetbrains.kotlin.codegen.signature.BothSignatureWriter; import org.jetbrains.kotlin.codegen.signature.JvmSignatureWriter; import org.jetbrains.kotlin.descriptors.*; import org.jetbrains.kotlin.fileClasses.FileClasses; +import org.jetbrains.kotlin.fileClasses.JvmFileClassInfo; import org.jetbrains.kotlin.fileClasses.JvmFileClassUtil; import org.jetbrains.kotlin.fileClasses.JvmFileClassesProvider; import org.jetbrains.kotlin.load.java.BuiltinMethodsWithSpecialGenericSignature; @@ -924,7 +925,7 @@ public class KotlinTypeMapper { ? JvmAbi.getterName(propertyName) : JvmAbi.setterName(propertyName); - return updateMemberNameIfInternal(isAccessor ? "access$" + accessorName : accessorName, descriptor); + return mangleMemberNameIfRequired(isAccessor ? "access$" + accessorName : accessorName, descriptor); } else if (isFunctionLiteral(descriptor)) { PsiElement element = DescriptorToSourceUtils.getSourceFromDescriptor(descriptor); @@ -944,7 +945,7 @@ public class KotlinTypeMapper { return OperatorNameConventions.INVOKE.asString(); } else { - return updateMemberNameIfInternal(descriptor.getName().asString(), descriptor); + return mangleMemberNameIfRequired(descriptor.getName().asString(), descriptor); } } @@ -973,7 +974,7 @@ public class KotlinTypeMapper { } @NotNull - private String updateMemberNameIfInternal(@NotNull String name, @NotNull CallableMemberDescriptor descriptor) { + private String mangleMemberNameIfRequired(@NotNull String name, @NotNull CallableMemberDescriptor descriptor) { if (descriptor.getContainingDeclaration() instanceof ScriptDescriptor) { //script properties should be public return name; @@ -983,8 +984,9 @@ public class KotlinTypeMapper { if (Visibilities.isPrivate(descriptor.getVisibility()) && !(descriptor instanceof ConstructorDescriptor) && !"".equals(name)) { KtFile containingFile = DescriptorToSourceUtils.getContainingFile(descriptor); assert containingFile != null : "Private descriptor accessed outside of corresponding file scope: " + descriptor; - if (JvmFileClassUtil.isFromMultifileClass(containingFile, descriptor)) { - return name + "$" + JvmAbi.sanitizeAsJavaIdentifier(containingFile.getName()); + JvmFileClassInfo fileClassInfo = JvmFileClassUtil.getFileClassInfoNoResolve(containingFile); + if (fileClassInfo.getWithJvmMultifileClass()) { + return name + "$" + fileClassInfo.getFileClassFqName().shortName().asString(); } } return name; diff --git a/compiler/testData/codegen/box/reflection/multifileClasses/javaFieldForVarAndConstVal.kt b/compiler/testData/codegen/box/reflection/multifileClasses/javaFieldForVarAndConstVal.kt index 870043e54a1..5dacf17ff86 100644 --- a/compiler/testData/codegen/box/reflection/multifileClasses/javaFieldForVarAndConstVal.kt +++ b/compiler/testData/codegen/box/reflection/multifileClasses/javaFieldForVarAndConstVal.kt @@ -36,9 +36,26 @@ fun testY() { assertEquals("I am const y", field.get(null)) } +fun testZ() { + val field = refZ.javaField ?: throw AssertionError("No java field for ${refZ.name}") + + + try { + field.get(null) + throw AssertionError("IllegalAccessError expected") + } + catch (e: IllegalAccessException) { + // OK + } + + field.setAccessible(true) + assertEquals("I am private const val Z", field.get(null)) +} + fun box(): String { testX() testY() + testZ() return x } @@ -50,3 +67,6 @@ package test var x = "I am x" const val y = "I am const y" +private const val z = "I am private const val Z" + +val refZ = ::z \ No newline at end of file diff --git a/compiler/testData/codegen/bytecodeListing/emptyMultifileFacade.txt b/compiler/testData/codegen/bytecodeListing/emptyMultifileFacade.txt index 054e3710133..96d030eeca5 100644 --- a/compiler/testData/codegen/bytecodeListing/emptyMultifileFacade.txt +++ b/compiler/testData/codegen/bytecodeListing/emptyMultifileFacade.txt @@ -3,5 +3,5 @@ public final class test/Foo @kotlin.Metadata synthetic final class test/Foo__EmptyMultifileFacadeKt { - private final static method privateOnly$emptyMultifileFacade_kt(): void + private final static method privateOnly$Foo__EmptyMultifileFacadeKt(): void } \ No newline at end of file diff --git a/compiler/testData/codegen/dumpDeclarations/multifileFacadeMembers.json b/compiler/testData/codegen/dumpDeclarations/multifileFacadeMembers.json index f7415a6e796..25f5440d53b 100644 --- a/compiler/testData/codegen/dumpDeclarations/multifileFacadeMembers.json +++ b/compiler/testData/codegen/dumpDeclarations/multifileFacadeMembers.json @@ -14,10 +14,10 @@ {"visibility": "internal", "declaration": "fun (): kotlin.Long", "name": "getInternalVar", "desc": "()J"}, {"visibility": "internal", "declaration": "fun (: kotlin.Long): kotlin.Unit", "name": "setInternalVar", "desc": "(J)V"}, {"visibility": "private", "declaration": "val privateVal: kotlin.Any?", "name": "privateVal", "desc": "Ljava/lang/Object;"}, - {"visibility": "private", "declaration": "fun (): kotlin.Any?", "name": "getPrivateVal$Part1_kt", "desc": "()Ljava/lang/Object;"}, + {"visibility": "private", "declaration": "fun (): kotlin.Any?", "name": "getPrivateVal$MultifileFacade__Part1Kt", "desc": "()Ljava/lang/Object;"}, {"visibility": "private", "declaration": "var privateVar: kotlin.Any?", "name": "privateVar", "desc": "Ljava/lang/Object;"}, - {"visibility": "private", "declaration": "fun (): kotlin.Any?", "name": "getPrivateVar$Part1_kt", "desc": "()Ljava/lang/Object;"}, - {"visibility": "private", "declaration": "fun (: kotlin.Any?): kotlin.Unit", "name": "setPrivateVar$Part1_kt", "desc": "(Ljava/lang/Object;)V"}, + {"visibility": "private", "declaration": "fun (): kotlin.Any?", "name": "getPrivateVar$MultifileFacade__Part1Kt", "desc": "()Ljava/lang/Object;"}, + {"visibility": "private", "declaration": "fun (: kotlin.Any?): kotlin.Unit", "name": "setPrivateVar$MultifileFacade__Part1Kt", "desc": "(Ljava/lang/Object;)V"}, {"declaration": "package-fragment ", "name": "", "desc": "()V"} ] }, @@ -34,9 +34,10 @@ "declaration": "package-fragment ", "class": "MultifileFacade__Part2Kt", "members": [ + {"visibility": "private", "declaration": "const val privateConst: kotlin.Int", "name": "privateConst", "desc": "I"}, {"visibility": "public", "declaration": "fun publicFun(): kotlin.Unit", "name": "publicFun", "desc": "()V"}, {"visibility": "internal", "declaration": "fun internalFun(param1: kotlin.Int): kotlin.Unit", "name": "internalFun", "desc": "(I)V"}, - {"visibility": "private", "declaration": "fun privateFun(x: kotlin.Any): kotlin.Unit", "name": "privateFun$Part2_kt", "desc": "(Ljava/lang/Object;)V"}, + {"visibility": "private", "declaration": "fun privateFun(x: kotlin.Any): kotlin.Unit", "name": "privateFun$MultifileFacade__Part2Kt", "desc": "(Ljava/lang/Object;)V"}, {"visibility": "private", "declaration": "fun privateFun(x: kotlin.Any): kotlin.Unit", "name": "access$privateFun", "desc": "(Ljava/lang/Object;)V"}, {"visibility": "private", "declaration": "fun (): kotlin.Int", "name": "access$getPrivateConst$p", "desc": "()I"} ] diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/RuntimeTypeMapper.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/RuntimeTypeMapper.kt index 3eeee629d53..a1b6cab2489 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/RuntimeTypeMapper.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/RuntimeTypeMapper.kt @@ -19,6 +19,7 @@ package kotlin.reflect.jvm.internal import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.builtins.PrimitiveType import org.jetbrains.kotlin.descriptors.FunctionDescriptor +import org.jetbrains.kotlin.descriptors.PackageFragmentDescriptor import org.jetbrains.kotlin.descriptors.PropertyDescriptor import org.jetbrains.kotlin.descriptors.Visibilities import org.jetbrains.kotlin.load.java.JvmAbi @@ -27,6 +28,7 @@ import org.jetbrains.kotlin.load.java.descriptors.JavaMethodDescriptor import org.jetbrains.kotlin.load.java.descriptors.JavaPropertyDescriptor import org.jetbrains.kotlin.load.java.sources.JavaSourceElement import org.jetbrains.kotlin.load.java.structure.reflect.* +import org.jetbrains.kotlin.load.kotlin.JvmPackagePartSource import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.platform.JavaToKotlinClassMap import org.jetbrains.kotlin.resolve.DescriptorUtils @@ -108,24 +110,30 @@ internal sealed class JvmPropertySignature { val (name, desc) = JvmProtoBufUtil.getJvmFieldSignature(proto, nameResolver, typeTable) ?: throw KotlinReflectionInternalError("No field signature for property: $descriptor") - - val moduleSuffix = - if (descriptor.visibility == Visibilities.INTERNAL && - descriptor.containingDeclaration is DeserializedClassDescriptor) { - val classProto = (descriptor.containingDeclaration as DeserializedClassDescriptor).classProto - val moduleName = - if (classProto.hasExtension(JvmProtoBuf.classModuleName)) - nameResolver.getString(classProto.getExtension(JvmProtoBuf.classModuleName)) - else JvmAbi.DEFAULT_MODULE_NAME - "$" + JvmAbi.sanitizeAsJavaIdentifier(moduleName) - } - else { - "" - } - string = JvmAbi.getterName(name) + moduleSuffix + "()" + desc + string = JvmAbi.getterName(name) + getManglingSuffix() + "()" + desc } } + private fun getManglingSuffix(): String { + val containingDeclaration = descriptor.containingDeclaration + if (descriptor.visibility == Visibilities.INTERNAL && containingDeclaration is DeserializedClassDescriptor) { + val classProto = containingDeclaration.classProto + val moduleName = + if (classProto.hasExtension(JvmProtoBuf.classModuleName)) + nameResolver.getString(classProto.getExtension(JvmProtoBuf.classModuleName)) + else JvmAbi.DEFAULT_MODULE_NAME + return "$" + JvmAbi.sanitizeAsJavaIdentifier(moduleName) + } + if (descriptor.visibility == Visibilities.PRIVATE && containingDeclaration is PackageFragmentDescriptor) { + val packagePartSource = (descriptor as DeserializedPropertyDescriptor).packagePartSource + if (packagePartSource is JvmPackagePartSource && packagePartSource.facadeClassName != null) { + return "$" + packagePartSource.simpleName.asString() + } + } + + return "" + } + override fun asString(): String = string }