From 8dd27aa298eab2cb475e39db646f4b5e4f31a869 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Fri, 12 Jul 2013 22:39:48 +0400 Subject: [PATCH] Java serialization maps descriptors correctly Instead of determining JVM method names, fields, types heuristically (mapType), do it correctly: in the place where a member is generated, its descriptor and signature are stored into MemberMap, which is queried later when we serialize its container --- .../jet/codegen/ClassBodyCodegen.java | 5 + .../jetbrains/jet/codegen/ClassBuilder.java | 7 ++ .../jet/codegen/FunctionCodegen.java | 2 +- .../codegen/ImplementationBodyCodegen.java | 11 +-- .../jet/codegen/JavaSerializerExtension.java | 35 +++---- .../org/jetbrains/jet/codegen/MemberMap.java | 92 +++++++++++++++++++ .../jet/codegen/NamespaceCodegen.java | 82 +++++++++-------- .../jet/codegen/PropertyCodegen.java | 6 +- 8 files changed, 179 insertions(+), 61 deletions(-) create mode 100644 compiler/backend/src/org/jetbrains/jet/codegen/MemberMap.java diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/ClassBodyCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/ClassBodyCodegen.java index 97fd5c0da3c..c1fe753b8a8 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/ClassBodyCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/ClassBodyCodegen.java @@ -73,10 +73,15 @@ public abstract class ClassBodyCodegen extends MemberCodegen { generateStaticInitializer(); generateRemoveInIterator(); + + generateKotlinAnnotation(); } protected abstract void generateDeclaration(); + protected void generateKotlinAnnotation() { + } + protected void generateSyntheticParts() { } diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/ClassBuilder.java b/compiler/backend/src/org/jetbrains/jet/codegen/ClassBuilder.java index c0227c94eda..dcb6c102e34 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/ClassBuilder.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/ClassBuilder.java @@ -27,6 +27,8 @@ import org.jetbrains.asm4.MethodVisitor; public abstract class ClassBuilder { private String thisName; + private final MemberMap members = new MemberMap(); + public static class Concrete extends ClassBuilder { private final ClassVisitor v; @@ -63,6 +65,11 @@ public abstract class ClassBuilder { return getVisitor().visitMethod(access, name, desc, signature, exceptions); } + @NotNull + public MemberMap getMemberMap() { + return members; + } + @NotNull public AnnotationVisitor newAnnotation(@NotNull String desc, boolean visible) { return getVisitor().visitAnnotation(desc, visible); diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/FunctionCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/FunctionCodegen.java index db7b855ba78..d5aa7f2fba4 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/FunctionCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/FunctionCodegen.java @@ -104,7 +104,6 @@ public class FunctionCodegen extends GenerationStateAware { @NotNull MethodContext methodContext, @NotNull FunctionGenerationStrategy strategy ) { - Method asmMethod = jvmSignature.getAsmMethod(); MethodVisitor mv = v.newMethod(origin, @@ -113,6 +112,7 @@ public class FunctionCodegen extends GenerationStateAware { asmMethod.getDescriptor(), jvmSignature.getGenericsSignature(), null); + v.getMemberMap().recordMethodOfDescriptor(functionDescriptor, asmMethod); AnnotationCodegen.forMethod(mv, typeMapper).genAnnotations(functionDescriptor); if (state.getClassBuilderMode() == ClassBuilderMode.SIGNATURES) return; diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/ImplementationBodyCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/ImplementationBodyCodegen.java index 97b64bed30b..bcca3760be8 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/ImplementationBodyCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/ImplementationBodyCodegen.java @@ -213,15 +213,14 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { AnnotationCodegen.forClass(v.getVisitor(), typeMapper).genAnnotations(descriptor); - if (isTopLevelOrInnerClass(descriptor)) { - writeKotlinInfo(); - } - writeClassSignatureIfNeeded(signature); } - private void writeKotlinInfo() { - DescriptorSerializer serializer = new DescriptorSerializer(new JavaSerializerExtension(typeMapper)); + @Override + protected void generateKotlinAnnotation() { + if (!isTopLevelOrInnerClass(descriptor)) return; + + DescriptorSerializer serializer = new DescriptorSerializer(new JavaSerializerExtension(v.getMemberMap())); ProtoBuf.Class classProto = serializer.classProto(descriptor).build(); diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/JavaSerializerExtension.java b/compiler/backend/src/org/jetbrains/jet/codegen/JavaSerializerExtension.java index 4b7792d6f3e..c966b2adcf2 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/JavaSerializerExtension.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/JavaSerializerExtension.java @@ -16,25 +16,23 @@ package org.jetbrains.jet.codegen; +import com.intellij.openapi.util.Pair; import org.jetbrains.annotations.NotNull; import org.jetbrains.asm4.Type; import org.jetbrains.asm4.commons.Method; -import org.jetbrains.jet.codegen.state.JetTypeMapper; import org.jetbrains.jet.descriptors.serialization.JavaProtoBufUtil; import org.jetbrains.jet.descriptors.serialization.NameTable; import org.jetbrains.jet.descriptors.serialization.ProtoBuf; import org.jetbrains.jet.descriptors.serialization.SerializerExtension; import org.jetbrains.jet.lang.descriptors.*; -import org.jetbrains.jet.lang.resolve.java.JvmAbi; public class JavaSerializerExtension extends SerializerExtension { - private final JetTypeMapper typeMapper; + private final MemberMap memberMap; - public JavaSerializerExtension(@NotNull JetTypeMapper typeMapper) { - this.typeMapper = typeMapper; + public JavaSerializerExtension(@NotNull MemberMap memberMap) { + this.memberMap = memberMap; } - // TODO: mapSignature should be done upon generation of the member instead, because we don't know enough at this point to map correctly @Override public void serializeCallable( @NotNull CallableMemberDescriptor callable, @@ -42,30 +40,35 @@ public class JavaSerializerExtension extends SerializerExtension { @NotNull NameTable nameTable ) { if (callable instanceof FunctionDescriptor) { - Method method = typeMapper.mapSignature((FunctionDescriptor) callable).getAsmMethod(); - JavaProtoBufUtil.saveMethodSignature(proto, method, nameTable); + Method method = memberMap.getMethodOfDescriptor(callable); + if (method != null) { + JavaProtoBufUtil.saveMethodSignature(proto, method, nameTable); + } } else if (callable instanceof PropertyDescriptor) { PropertyDescriptor property = (PropertyDescriptor) callable; - Type type = typeMapper.mapType(property.getType()); + PropertyGetterDescriptor getter = property.getGetter(); PropertySetterDescriptor setter = property.getSetter(); - Method getterMethod = getter == null ? null : typeMapper.mapGetterSignature(property, OwnerKind.IMPLEMENTATION).getAsmMethod(); - Method setterMethod = setter == null ? null : typeMapper.mapSetterSignature(property, OwnerKind.IMPLEMENTATION).getAsmMethod(); + Method getterMethod = getter == null ? null : memberMap.getMethodOfDescriptor(getter); + Method setterMethod = setter == null ? null : memberMap.getMethodOfDescriptor(setter); - // This is very wrong, see above todo + Pair field = memberMap.getFieldOfProperty(property); + Type fieldType; String fieldName; String syntheticMethodName; - if ((getter == null || getter.isDefault()) && (setter == null || setter.isDefault())) { - fieldName = property.getName().asString(); + if (field != null) { + fieldType = field.first; + fieldName = field.second; syntheticMethodName = null; } else { + fieldType = null; fieldName = null; - syntheticMethodName = JvmAbi.getSyntheticMethodNameForAnnotatedProperty(property.getName()); + syntheticMethodName = memberMap.getSyntheticMethodNameOfProperty(property); } - JavaProtoBufUtil.savePropertySignature(proto, type, fieldName, syntheticMethodName, getterMethod, setterMethod, nameTable); + JavaProtoBufUtil.savePropertySignature(proto, fieldType, fieldName, syntheticMethodName, getterMethod, setterMethod, nameTable); } } } diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/MemberMap.java b/compiler/backend/src/org/jetbrains/jet/codegen/MemberMap.java new file mode 100644 index 00000000000..7fa37b431d7 --- /dev/null +++ b/compiler/backend/src/org/jetbrains/jet/codegen/MemberMap.java @@ -0,0 +1,92 @@ +/* + * Copyright 2010-2013 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jetbrains.jet.codegen; + +import com.intellij.openapi.util.Pair; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.jetbrains.asm4.Type; +import org.jetbrains.asm4.commons.Method; +import org.jetbrains.jet.lang.descriptors.CallableMemberDescriptor; +import org.jetbrains.jet.lang.descriptors.PropertyDescriptor; + +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; + +public final class MemberMap { + private final Map methodForFunction = new HashMap(); + private final Map> fieldForProperty = new HashMap>(); + private final Map syntheticMethodNameForProperty = new HashMap(); + + @NotNull + public static MemberMap union(@NotNull Collection maps) { + MemberMap result = new MemberMap(); + for (MemberMap map : maps) { + for (Map.Entry entry : map.methodForFunction.entrySet()) { + result.recordMethodOfDescriptor(entry.getKey(), entry.getValue()); + } + + for (Map.Entry> entry : map.fieldForProperty.entrySet()) { + result.recordFieldOfProperty(entry.getKey(), entry.getValue().first, entry.getValue().second); + } + + for (Map.Entry entry : map.syntheticMethodNameForProperty.entrySet()) { + result.recordSyntheticMethodNameOfProperty(entry.getKey(), entry.getValue()); + } + } + + return result; + } + + public void recordMethodOfDescriptor(@NotNull CallableMemberDescriptor descriptor, @NotNull Method method) { + Method old = methodForFunction.put(descriptor, method); + assert old == null : "Duplicate method for callable member: " + descriptor + "; " + old; + } + + public void recordFieldOfProperty(@NotNull PropertyDescriptor descriptor, @NotNull Type type, @NotNull String fieldName) { + Pair old = fieldForProperty.put(descriptor, Pair.create(type, fieldName)); + assert old == null : "Duplicate field for property: " + descriptor + "; " + old; + } + + public void recordSyntheticMethodNameOfProperty(@NotNull PropertyDescriptor descriptor, @NotNull String name) { + String old = syntheticMethodNameForProperty.put(descriptor, name); + assert old == null : "Duplicate synthetic method for property: " + descriptor + "; " + old; + } + + @Nullable + public Method getMethodOfDescriptor(@NotNull CallableMemberDescriptor descriptor) { + return methodForFunction.get(descriptor); + } + + @Nullable + public Pair getFieldOfProperty(@NotNull PropertyDescriptor descriptor) { + return fieldForProperty.get(descriptor); + } + + @Nullable + public String getSyntheticMethodNameOfProperty(@NotNull PropertyDescriptor descriptor) { + return syntheticMethodNameForProperty.get(descriptor); + } + + @Override + public String toString() { + return "Functions: " + methodForFunction.size() + + ", fields: " + fieldForProperty.size() + + ", synthetic methods: " + syntheticMethodNameForProperty.size(); + } +} diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/NamespaceCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/NamespaceCodegen.java index 825d2ee525c..292bb117fd9 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/NamespaceCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/NamespaceCodegen.java @@ -24,6 +24,7 @@ import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.PsiFile; import com.intellij.util.PathUtil; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.jetbrains.asm4.AnnotationVisitor; import org.jetbrains.asm4.MethodVisitor; import org.jetbrains.asm4.Type; @@ -47,6 +48,7 @@ import org.jetbrains.jet.lang.resolve.java.PackageClassUtils; import org.jetbrains.jet.lang.resolve.name.FqName; import org.jetbrains.jet.lang.resolve.name.Name; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.List; @@ -96,7 +98,7 @@ public class NamespaceCodegen extends MemberCodegen { }); } - public void generate(CompilationErrorHandler errorHandler) { + public void generate(@NotNull CompilationErrorHandler errorHandler) { if (shouldGenerateNSClass(files)) { AnnotationVisitor packageClassAnnotation = v.getClassBuilder().newAnnotation(JvmStdlibNames.JET_PACKAGE_CLASS.getDescriptor(), true); @@ -104,18 +106,21 @@ public class NamespaceCodegen extends MemberCodegen { packageClassAnnotation.visitEnd(); } - writeKotlinInfoIfNeeded(); + List namespaceMembers = new ArrayList(); for (JetFile file : files) { - VirtualFile vFile = file.getVirtualFile(); try { - generate(file); + ClassBuilder builder = generate(file); + if (builder != null) { + namespaceMembers.add(builder.getMemberMap()); + } } catch (ProcessCanceledException e) { throw e; } catch (Throwable e) { - if (errorHandler != null) errorHandler.reportException(e, vFile == null ? "no file" : vFile.getUrl()); + VirtualFile vFile = file.getVirtualFile(); + errorHandler.reportException(e, vFile == null ? "no file" : vFile.getUrl()); DiagnosticUtils.throwIfRunningOnServer(e); if (ApplicationManager.getApplication().isInternal()) { //noinspection CallToPrintStackTrace @@ -124,11 +129,13 @@ public class NamespaceCodegen extends MemberCodegen { } } + writeKotlinInfoIfNeeded(MemberMap.union(namespaceMembers)); + assert v.isActivated() == shouldGenerateNSClass(files) : "Different algorithms for generating namespace class and for heuristics"; } - private void writeKotlinInfoIfNeeded() { - DescriptorSerializer serializer = new DescriptorSerializer(new JavaSerializerExtension(typeMapper)); + private void writeKotlinInfoIfNeeded(@NotNull MemberMap members) { + DescriptorSerializer serializer = new DescriptorSerializer(new JavaSerializerExtension(members)); ProtoBuf.Package.Builder packageProto = ProtoBuf.Package.newBuilder(); boolean writeAnnotation = false; @@ -171,13 +178,14 @@ public class NamespaceCodegen extends MemberCodegen { av.visitEnd(); } - private void generate(JetFile file) { + @Nullable + private ClassBuilder generate(@NotNull JetFile file) { NamespaceDescriptor descriptor = state.getBindingContext().get(BindingContext.FILE_TO_NAMESPACE, file); assert descriptor != null : "No namespace found for file " + file + " declared package: " + file.getPackageName(); - int countOfDeclarationsInSrcClass = 0; + boolean generateSrcClass = false; for (JetDeclaration declaration : file.getDeclarations()) { if (declaration instanceof JetProperty || declaration instanceof JetNamedFunction) { - countOfDeclarationsInSrcClass++; + generateSrcClass = true; } else if (declaration instanceof JetClassOrObject) { if (state.isGenerateDeclaredClasses()) { @@ -189,39 +197,39 @@ public class NamespaceCodegen extends MemberCodegen { } } - if (countOfDeclarationsInSrcClass > 0) { - String namespaceInternalName = JvmClassName.byFqNameWithoutInnerClasses( - PackageClassUtils.getPackageClassFqName(name)).getInternalName(); - String className = getMultiFileNamespaceInternalName(namespaceInternalName, file); - ClassBuilder builder = state.getFactory().forNamespacepart(className, file); + if (!generateSrcClass) return null; - builder.defineClass(file, V1_6, - ACC_PUBLIC | ACC_FINAL, - className, - null, - //"jet/lang/Namespace", - "java/lang/Object", - new String[0] - ); - builder.visitSource(file.getName(), null); + String namespaceInternalName = JvmClassName.byFqNameWithoutInnerClasses( + PackageClassUtils.getPackageClassFqName(name)).getInternalName(); + String className = getMultiFileNamespaceInternalName(namespaceInternalName, file); + ClassBuilder builder = state.getFactory().forNamespacepart(className, file); - FieldOwnerContext nameSpaceContext = - CodegenContext.STATIC.intoNamespace(descriptor); + builder.defineClass(file, V1_6, + ACC_PUBLIC | ACC_FINAL, + className, + null, + //"jet/lang/Namespace", + "java/lang/Object", + new String[0] + ); + builder.visitSource(file.getName(), null); - FieldOwnerContext nameSpacePart = - CodegenContext.STATIC.intoNamespacePart(className, descriptor); + FieldOwnerContext nameSpaceContext = CodegenContext.STATIC.intoNamespace(descriptor); - for (JetDeclaration declaration : file.getDeclarations()) { - if (declaration instanceof JetNamedFunction || declaration instanceof JetProperty) { - genFunctionOrProperty(nameSpaceContext, (JetTypeParameterListOwner) declaration, builder); - genFunctionOrProperty(nameSpacePart, (JetTypeParameterListOwner) declaration, v.getClassBuilder()); - } + FieldOwnerContext nameSpacePart = CodegenContext.STATIC.intoNamespacePart(className, descriptor); + + for (JetDeclaration declaration : file.getDeclarations()) { + if (declaration instanceof JetNamedFunction || declaration instanceof JetProperty) { + genFunctionOrProperty(nameSpaceContext, (JetTypeParameterListOwner) declaration, builder); + genFunctionOrProperty(nameSpacePart, (JetTypeParameterListOwner) declaration, v.getClassBuilder()); } - - generateStaticInitializers(descriptor, builder, file, nameSpaceContext); - - builder.done(); } + + generateStaticInitializers(descriptor, builder, file, nameSpaceContext); + + builder.done(); + + return builder; } public void generateClassOrObject(@NotNull NamespaceDescriptor descriptor, @NotNull JetClassOrObject classOrObject) { diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/PropertyCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/PropertyCodegen.java index f7a808e3e03..c5d10137b7b 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/PropertyCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/PropertyCodegen.java @@ -137,12 +137,14 @@ public class PropertyCodegen extends GenerationStateAware { else if (!propertyDescriptor.getAnnotations().isEmpty()) { // Annotations on properties without backing fields are stored in bytecode on an empty synthetic method. This way they're still // accessible via reflection, and 'deprecated' and 'private' flags prevent this method from being called accidentally + String methodName = JvmAbi.getSyntheticMethodNameForAnnotatedProperty(propertyDescriptor.getName()); MethodVisitor mv = v.newMethod(null, ACC_DEPRECATED | ACC_FINAL | ACC_PRIVATE | ACC_STATIC | ACC_SYNTHETIC, - JvmAbi.getSyntheticMethodNameForAnnotatedProperty(propertyDescriptor.getName()), + methodName, JvmAbi.ANNOTATED_PROPERTY_METHOD_SIGNATURE, null, null); + v.getMemberMap().recordSyntheticMethodNameOfProperty(propertyDescriptor, methodName); AnnotationCodegen.forMethod(mv, typeMapper).genAnnotations(propertyDescriptor); mv.visitCode(); mv.visitInsn(Opcodes.RETURN); @@ -188,6 +190,8 @@ public class PropertyCodegen extends GenerationStateAware { String name = backingFieldContext.getFieldName(propertyDescriptor, isDelegate); + builder.getMemberMap().recordFieldOfProperty(propertyDescriptor, type, name); + return builder.newField(element, modifiers, name, type.getDescriptor(), typeMapper.mapFieldSignature(jetType), defaultValue); }