From 708816f09ce4e856bac8b80fba3ff05bd4f4380f Mon Sep 17 00:00:00 2001 From: Evgeny Gerashchenko Date: Thu, 10 Jul 2014 17:26:40 +0400 Subject: [PATCH] Fixed generating method calls from same module when inlining. --- .../jet/codegen/ExpressionCodegen.java | 2 +- .../jetbrains/jet/codegen/JvmCodegenUtil.java | 65 ++++++++++++++++++- .../jet/codegen/inline/InlineCodegen.java | 2 +- .../jet/codegen/state/GenerationState.java | 9 +++ .../jet/codegen/state/JetTypeMapper.java | 40 ++++++------ .../state/JetTypeMapperWithOutDirectory.java | 40 ++---------- .../bytecodeText/inlineFromOtherModule.kt | 7 ++ .../codegen/BytecodeTextTestGenerated.java | 5 ++ .../jet/lang/resolve/DescriptorUtils.java | 2 + 9 files changed, 111 insertions(+), 61 deletions(-) create mode 100644 compiler/testData/codegen/bytecodeText/inlineFromOtherModule.kt diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java index fea26907ebf..6340571cc6e 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java @@ -1899,7 +1899,7 @@ public class ExpressionCodegen extends JetVisitor implem propertyDescriptor = DescriptorUtils.unwrapFakeOverride(propertyDescriptor); if (callableMethod == null) { owner = typeMapper.mapOwner(isBackingFieldInAnotherClass ? propertyDescriptor.getContainingDeclaration() : propertyDescriptor, - isCallInsideSameModuleAsDeclared(propertyDescriptor, context)); + isCallInsideSameModuleAsDeclared(propertyDescriptor, context, state.getOutDirectory())); } else { owner = callableMethod.getOwner(); diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/JvmCodegenUtil.java b/compiler/backend/src/org/jetbrains/jet/codegen/JvmCodegenUtil.java index 32a665261ba..6325f904ea6 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/JvmCodegenUtil.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/JvmCodegenUtil.java @@ -16,6 +16,9 @@ package org.jetbrains.jet.codegen; +import com.intellij.openapi.vfs.StandardFileSystems; +import com.intellij.openapi.vfs.VfsUtilCore; +import com.intellij.openapi.vfs.VirtualFile; import com.intellij.util.containers.Stack; import kotlin.Function1; import kotlin.KotlinPackage; @@ -26,20 +29,26 @@ import org.jetbrains.jet.codegen.context.CodegenContext; import org.jetbrains.jet.codegen.context.MethodContext; import org.jetbrains.jet.codegen.context.PackageContext; import org.jetbrains.jet.codegen.state.JetTypeMapper; -import org.jetbrains.jet.config.IncrementalCompilation; +import org.jetbrains.jet.descriptors.serialization.descriptors.DeserializedCallableMemberDescriptor; import org.jetbrains.jet.lang.descriptors.*; import org.jetbrains.jet.lang.descriptors.annotations.Annotations; import org.jetbrains.jet.lang.descriptors.impl.SimpleFunctionDescriptorImpl; import org.jetbrains.jet.lang.descriptors.impl.TypeParameterDescriptorImpl; import org.jetbrains.jet.lang.resolve.DescriptorUtils; import org.jetbrains.jet.lang.resolve.calls.CallResolverUtil; +import org.jetbrains.jet.lang.resolve.java.descriptor.JavaPackageFragmentDescriptor; +import org.jetbrains.jet.lang.resolve.java.lazy.descriptors.LazyPackageFragmentScopeForJavaPackage; +import org.jetbrains.jet.lang.resolve.kotlin.KotlinJvmBinaryClass; +import org.jetbrains.jet.lang.resolve.kotlin.VirtualFileKotlinClass; import org.jetbrains.jet.lang.resolve.kotlin.incremental.IncrementalPackageFragmentProvider; import org.jetbrains.jet.lang.resolve.name.Name; +import org.jetbrains.jet.lang.resolve.scopes.JetScope; import org.jetbrains.jet.lang.types.JetType; import org.jetbrains.jet.lang.types.TypeUtils; import org.jetbrains.jet.lang.types.checker.JetTypeChecker; import org.jetbrains.jet.lang.types.lang.KotlinBuiltIns; +import java.io.File; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -173,12 +182,55 @@ public class JvmCodegenUtil { return false; } - public static boolean isCallInsideSameModuleAsDeclared(CallableMemberDescriptor declarationDescriptor, CodegenContext context) { + public static boolean isCallInsideSameModuleAsDeclared( + @NotNull CallableMemberDescriptor declarationDescriptor, + @NotNull CodegenContext context, + @Nullable File outDirectory + ) { if (context == CodegenContext.STATIC) { return true; } DeclarationDescriptor contextDescriptor = context.getContextDescriptor(); - return DescriptorUtils.areInSameModule(declarationDescriptor, contextDescriptor); + + CallableMemberDescriptor directMember = getDirectMember(declarationDescriptor); + if (directMember instanceof DeserializedCallableMemberDescriptor) { + return isContainedByCompiledPartOfOurModule(((DeserializedCallableMemberDescriptor) directMember), outDirectory); + } + else { + return DescriptorUtils.areInSameModule(directMember, contextDescriptor); + } + } + + private static boolean isContainedByCompiledPartOfOurModule( + @NotNull DeserializedCallableMemberDescriptor descriptor, + @Nullable File outDirectory + ) { + if (descriptor.getContainingDeclaration() instanceof IncrementalPackageFragmentProvider.IncrementalPackageFragment) { + return true; + } + + if (outDirectory == null) { + return false; + } + + if (!(descriptor.getContainingDeclaration() instanceof JavaPackageFragmentDescriptor)) { + return false; + } + JavaPackageFragmentDescriptor packageFragment = (JavaPackageFragmentDescriptor) descriptor.getContainingDeclaration(); + JetScope packageScope = packageFragment.getMemberScope(); + if (!(packageScope instanceof LazyPackageFragmentScopeForJavaPackage)) { + return false; + } + KotlinJvmBinaryClass binaryClass = ((LazyPackageFragmentScopeForJavaPackage) packageScope).getKotlinBinaryClass(); + + if (binaryClass instanceof VirtualFileKotlinClass) { + VirtualFile file = ((VirtualFileKotlinClass) binaryClass).getFile(); + if (file.getFileSystem().getProtocol() == StandardFileSystems.FILE_PROTOCOL) { + File ioFile = VfsUtilCore.virtualToIoFile(file); + return ioFile.getAbsolutePath().startsWith(outDirectory.getAbsolutePath() + File.separator); + } + } + return false; } public static boolean hasAbstractMembers(@NotNull ClassDescriptor classDescriptor) { @@ -267,4 +319,11 @@ public class JvmCodegenUtil { return "values".equals(functionDescriptor.getName().asString()) && methodTypeParameters.isEmpty(); } + + @NotNull + public static CallableMemberDescriptor getDirectMember(@NotNull CallableMemberDescriptor descriptor) { + return descriptor instanceof PropertyAccessorDescriptor + ? ((PropertyAccessorDescriptor) descriptor).getCorrespondingProperty() + : descriptor; + } } diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/inline/InlineCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/inline/InlineCodegen.java index c7b4876826f..0183c738e5b 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/inline/InlineCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/inline/InlineCodegen.java @@ -103,7 +103,7 @@ public class InlineCodegen implements CallGenerator { codegen.getContext().isInlineFunction() ? InlineStrategy.IN_PLACE : functionDescriptor.getInlineStrategy(); this.asFunctionInline = false; - isSameModule = JvmCodegenUtil.isCallInsideSameModuleAsDeclared(functionDescriptor, codegen.getContext()); + isSameModule = JvmCodegenUtil.isCallInsideSameModuleAsDeclared(functionDescriptor, codegen.getContext(), state.getOutDirectory()); } @Override diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/state/GenerationState.java b/compiler/backend/src/org/jetbrains/jet/codegen/state/GenerationState.java index 7e055673a0e..248a23d9172 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/state/GenerationState.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/state/GenerationState.java @@ -105,6 +105,9 @@ public class GenerationState { @Nullable private final String moduleId; // for PackageCodegen in incremental compilation mode + @Nullable + private final File outDirectory; // TODO: temporary hack, see JetTypeMapperWithOutDirectory state for details + public GenerationState( @NotNull Project project, @NotNull ClassBuilderFactory builderFactory, @@ -144,6 +147,7 @@ public class GenerationState { this.bindingTrace = new DelegatingBindingTrace(bindingContext, "trace in GenerationState"); this.bindingContext = bindingTrace.getBindingContext(); + this.outDirectory = outDirectory; this.typeMapper = new JetTypeMapperWithOutDirectory(this.bindingContext, classBuilderMode, outDirectory); this.intrinsics = new IntrinsicMethods(); @@ -269,4 +273,9 @@ public class GenerationState { public String getModuleId() { return moduleId; } + + @Nullable + public File getOutDirectory() { + return outDirectory; + } } diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/state/JetTypeMapper.java b/compiler/backend/src/org/jetbrains/jet/codegen/state/JetTypeMapper.java index b5141f46694..ff591c45c19 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/state/JetTypeMapper.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/state/JetTypeMapper.java @@ -46,7 +46,6 @@ import org.jetbrains.jet.lang.resolve.java.jvmSignature.JvmMethodParameterSignat import org.jetbrains.jet.lang.resolve.java.jvmSignature.JvmMethodSignature; import org.jetbrains.jet.lang.resolve.java.mapping.KotlinToJavaTypesMap; import org.jetbrains.jet.lang.resolve.kotlin.PackagePartClassUtils; -import org.jetbrains.jet.lang.resolve.kotlin.incremental.IncrementalPackageFragmentProvider; import org.jetbrains.jet.lang.resolve.name.FqName; import org.jetbrains.jet.lang.resolve.name.FqNameUnsafe; import org.jetbrains.jet.lang.types.*; @@ -54,6 +53,7 @@ import org.jetbrains.jet.lang.types.lang.KotlinBuiltIns; import org.jetbrains.org.objectweb.asm.Type; import org.jetbrains.org.objectweb.asm.commons.Method; +import java.io.File; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -110,7 +110,11 @@ public class JetTypeMapper { return mapClass(((JavaClassStaticsPackageFragmentDescriptor) container).getCorrespondingClass()); } else if (container instanceof PackageFragmentDescriptor) { - return Type.getObjectType(internalNameForPackage((PackageFragmentDescriptor) container, descriptor, isInsideModule)); + return Type.getObjectType(internalNameForPackage( + (PackageFragmentDescriptor) container, + (CallableMemberDescriptor) descriptor, + isInsideModule + )); } else if (container instanceof ClassDescriptor) { return mapClass((ClassDescriptor) container); @@ -123,14 +127,10 @@ public class JetTypeMapper { } } - protected boolean isContainedByCompiledPartOfOurModule(@NotNull DeclarationDescriptor descriptor) { - return false; - } - @NotNull - private String internalNameForPackage( + private static String internalNameForPackage( @NotNull PackageFragmentDescriptor packageFragment, - @NotNull DeclarationDescriptor descriptor, + @NotNull CallableMemberDescriptor descriptor, boolean insideModule ) { if (insideModule) { @@ -139,17 +139,11 @@ public class JetTypeMapper { return PackagePartClassUtils.getPackagePartInternalName(file); } - DeclarationDescriptor descriptorToCheck = descriptor instanceof PropertyAccessorDescriptor - ? ((PropertyAccessorDescriptor) descriptor).getCorrespondingProperty() - : descriptor; + CallableMemberDescriptor directMember = getDirectMember(descriptor); - if (descriptorToCheck instanceof DeserializedCallableMemberDescriptor) { - // TODO Temporary hack until modules infrastructure is implemented. See JetTypeMapperWithOutDirectory for details - if (descriptor.getContainingDeclaration() instanceof IncrementalPackageFragmentProvider.IncrementalPackageFragment || - isContainedByCompiledPartOfOurModule(descriptor)) { - FqName packagePartFqName = PackagePartClassUtils.getPackagePartFqName((DeserializedCallableMemberDescriptor) descriptorToCheck); - return AsmUtil.internalNameByFqNameWithoutInnerClasses(packagePartFqName); - } + if (directMember instanceof DeserializedCallableMemberDescriptor) { + FqName packagePartFqName = PackagePartClassUtils.getPackagePartFqName((DeserializedCallableMemberDescriptor) directMember); + return AsmUtil.internalNameByFqNameWithoutInnerClasses(packagePartFqName); } } @@ -469,7 +463,7 @@ public class JetTypeMapper { } else { signature = mapSignature(functionDescriptor.getOriginal()); - owner = mapOwner(functionDescriptor, isCallInsideSameModuleAsDeclared(functionDescriptor, context)); + owner = mapOwner(functionDescriptor, isCallInsideSameModuleAsDeclared(functionDescriptor, context, getOutDirectory())); ownerForDefaultParam = owner; ownerForDefaultImpl = owner; if (functionParent instanceof PackageFragmentDescriptor) { @@ -620,7 +614,7 @@ public class JetTypeMapper { @NotNull public Method mapDefaultMethod(@NotNull FunctionDescriptor functionDescriptor, @NotNull OwnerKind kind, @NotNull CodegenContext context) { Method jvmSignature = mapSignature(functionDescriptor, kind).getAsmMethod(); - Type ownerType = mapOwner(functionDescriptor, isCallInsideSameModuleAsDeclared(functionDescriptor, context)); + Type ownerType = mapOwner(functionDescriptor, isCallInsideSameModuleAsDeclared(functionDescriptor, context, getOutDirectory())); String descriptor = jvmSignature.getDescriptor().replace(")", "I)"); boolean isConstructor = "".equals(jvmSignature.getName()); if (!isStatic(kind) && !isConstructor) { @@ -865,4 +859,10 @@ public class JetTypeMapper { } return null; } + + // TODO Temporary hack until modules infrastructure is implemented. See JetTypeMapperWithOutDirectory for details + @Nullable + protected File getOutDirectory() { + return null; + } } diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/state/JetTypeMapperWithOutDirectory.java b/compiler/backend/src/org/jetbrains/jet/codegen/state/JetTypeMapperWithOutDirectory.java index fb1a1a00e78..c040a622b28 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/state/JetTypeMapperWithOutDirectory.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/state/JetTypeMapperWithOutDirectory.java @@ -16,22 +16,12 @@ package org.jetbrains.jet.codegen.state; -import com.intellij.openapi.vfs.StandardFileSystems; -import com.intellij.openapi.vfs.VfsUtilCore; -import com.intellij.openapi.vfs.VirtualFile; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.codegen.ClassBuilderMode; -import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor; import org.jetbrains.jet.lang.resolve.BindingContext; -import org.jetbrains.jet.lang.resolve.java.descriptor.JavaPackageFragmentDescriptor; -import org.jetbrains.jet.lang.resolve.java.lazy.descriptors.LazyPackageFragmentScopeForJavaPackage; -import org.jetbrains.jet.lang.resolve.kotlin.KotlinJvmBinaryClass; -import org.jetbrains.jet.lang.resolve.kotlin.VirtualFileKotlinClass; -import org.jetbrains.jet.lang.resolve.scopes.JetScope; import java.io.File; -import java.util.Set; // TODO Temporary hack until modules infrastructure is implemented. // This class is necessary for detecting if compiled class is from the same module as callee. @@ -42,37 +32,15 @@ public class JetTypeMapperWithOutDirectory extends JetTypeMapper { public JetTypeMapperWithOutDirectory( @NotNull BindingContext bindingContext, @NotNull ClassBuilderMode classBuilderMode, - @Nullable File outDirectory + @NotNull File outDirectory ) { super(bindingContext, classBuilderMode); this.outDirectory = outDirectory; } + @Nullable @Override - protected boolean isContainedByCompiledPartOfOurModule(@NotNull DeclarationDescriptor descriptor) { - if (outDirectory == null) { - return false; - } - - if (!(descriptor.getContainingDeclaration() instanceof JavaPackageFragmentDescriptor)) { - return false; - } - JavaPackageFragmentDescriptor packageFragment = (JavaPackageFragmentDescriptor) descriptor.getContainingDeclaration(); - JetScope packageScope = packageFragment.getMemberScope(); - if (!(packageScope instanceof LazyPackageFragmentScopeForJavaPackage)) { - return false; - } - KotlinJvmBinaryClass binaryClass = ((LazyPackageFragmentScopeForJavaPackage) packageScope).getKotlinBinaryClass(); - - if (binaryClass instanceof VirtualFileKotlinClass) { - VirtualFile file = ((VirtualFileKotlinClass) binaryClass).getFile(); - if (file.getFileSystem().getProtocol() == StandardFileSystems.FILE_PROTOCOL) { - File ioFile = VfsUtilCore.virtualToIoFile(file); - return ioFile.getAbsolutePath().startsWith(outDirectory.getAbsolutePath() + File.separator); - } - } - return false; + public File getOutDirectory() { + return outDirectory; } - - } diff --git a/compiler/testData/codegen/bytecodeText/inlineFromOtherModule.kt b/compiler/testData/codegen/bytecodeText/inlineFromOtherModule.kt new file mode 100644 index 00000000000..0d4d499ae61 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/inlineFromOtherModule.kt @@ -0,0 +1,7 @@ +fun foo() { + assert(1 == 1) { "Hahaha" } +} + +// assert function will be inlined, and we assure that there are no calls via package part, but is call via package facade in inlined code +// 0 INVOKESTATIC kotlin\/KotlinPackage-[^-]+-[^-]+.getASSERTIONS_ENABLED \(\)Z +// 1 INVOKESTATIC kotlin\/KotlinPackage.getASSERTIONS_ENABLED \(\)Z \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/jet/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/jet/codegen/BytecodeTextTestGenerated.java index 269eef9a9de..0a4192ea02a 100644 --- a/compiler/tests/org/jetbrains/jet/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/codegen/BytecodeTextTestGenerated.java @@ -77,6 +77,11 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { doTest("compiler/testData/codegen/bytecodeText/defaultDelegation.kt"); } + @TestMetadata("inlineFromOtherModule.kt") + public void testInlineFromOtherModule() throws Exception { + doTest("compiler/testData/codegen/bytecodeText/inlineFromOtherModule.kt"); + } + @TestMetadata("intConstantNotNull.kt") public void testIntConstantNotNull() throws Exception { doTest("compiler/testData/codegen/bytecodeText/intConstantNotNull.kt"); diff --git a/core/descriptors/src/org/jetbrains/jet/lang/resolve/DescriptorUtils.java b/core/descriptors/src/org/jetbrains/jet/lang/resolve/DescriptorUtils.java index dfa6c4fddc5..9843a2ebdb0 100644 --- a/core/descriptors/src/org/jetbrains/jet/lang/resolve/DescriptorUtils.java +++ b/core/descriptors/src/org/jetbrains/jet/lang/resolve/DescriptorUtils.java @@ -117,6 +117,8 @@ public class DescriptorUtils { return descriptor.getContainingDeclaration() instanceof PackageFragmentDescriptor; } + // WARNING! Don't use this method in JVM backend, use JvmCodegenUtil.isCallInsideSameModuleAsDeclared() instead. + // The latter handles compilation against compiled part of our module correctly. public static boolean areInSameModule(@NotNull DeclarationDescriptor first, @NotNull DeclarationDescriptor second) { return getContainingModule(first).equals(getContainingModule(second)); }