From de3f5d0e2d06be42fa41b8f64fd0c9bcad69b1b4 Mon Sep 17 00:00:00 2001 From: Dmitry Jemerov Date: Wed, 11 Jul 2012 16:59:01 +0200 Subject: [PATCH] avoid coercing 'this' to a supertype; cleaner handling of stackvalues for 'this' and 'super' (KT-2395) --- .../jet/codegen/ClosureAnnotator.java | 4 +- .../jet/codegen/ExpressionCodegen.java | 50 +++++++++---------- .../jetbrains/jet/codegen/JetTypeMapper.java | 2 +- .../jetbrains/jet/codegen/ScriptCodegen.java | 3 +- .../org/jetbrains/jet/codegen/StackValue.java | 3 +- .../testData/codegen/regressions/kt2395.kt | 11 ++++ .../jetbrains/jet/codegen/ClassGenTest.java | 5 ++ 7 files changed, 45 insertions(+), 33 deletions(-) create mode 100644 compiler/testData/codegen/regressions/kt2395.kt diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/ClosureAnnotator.java b/compiler/backend/src/org/jetbrains/jet/codegen/ClosureAnnotator.java index 74fe392840e..83863901f75 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/ClosureAnnotator.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/ClosureAnnotator.java @@ -127,7 +127,7 @@ public class ClosureAnnotator { } @NotNull - public ClassDescriptor classDescriptorForScrpitDescriptor(@NotNull ScriptDescriptor scriptDescriptor) { + public ClassDescriptor classDescriptorForScriptDescriptor(@NotNull ScriptDescriptor scriptDescriptor) { ClassDescriptorImpl classDescriptor = classesForFunctions.get(scriptDescriptor); if (classDescriptor == null) { throw new IllegalStateException("Class for script is not registered: " + scriptDescriptor); @@ -146,7 +146,7 @@ public class ClosureAnnotator { @NotNull public JvmClassName classNameForScriptDescriptor(@NotNull ScriptDescriptor scriptDescriptor) { - return classNameForClassDescriptor(classDescriptorForScrpitDescriptor(scriptDescriptor)); + return classNameForClassDescriptor(classDescriptorForScriptDescriptor(scriptDescriptor)); } private void mapFilesToNamespaces(Collection files) { diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java index b170238a693..227253d510e 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java @@ -35,11 +35,7 @@ import org.jetbrains.jet.lang.resolve.DescriptorUtils; import org.jetbrains.jet.lang.resolve.calls.*; import org.jetbrains.jet.lang.resolve.constants.CompileTimeConstant; import org.jetbrains.jet.lang.resolve.java.JvmClassName; -import org.jetbrains.jet.lang.resolve.scopes.receivers.ClassReceiver; -import org.jetbrains.jet.lang.resolve.scopes.receivers.ExpressionReceiver; -import org.jetbrains.jet.lang.resolve.scopes.receivers.ExtensionReceiver; -import org.jetbrains.jet.lang.resolve.scopes.receivers.ReceiverDescriptor; -import org.jetbrains.jet.lang.resolve.scopes.receivers.ScriptReceiver; +import org.jetbrains.jet.lang.resolve.scopes.receivers.*; import org.jetbrains.jet.lang.types.JetType; import org.jetbrains.jet.lang.types.lang.JetStandardClasses; import org.jetbrains.jet.lang.types.lang.JetStandardLibraryNames; @@ -136,7 +132,10 @@ public class ExpressionCodegen extends JetVisitor { assert provided instanceof ClassDescriptor; if (!CodegenUtil.isInterface(provided) && CodegenUtil.isInterface(required)) { - v.checkcast(asmType(required.getDefaultType())); + inner.put(TYPE_OBJECT, v); + final Type type = asmType(required.getDefaultType()); + v.checkcast(type); + return StackValue.onStack(type); } return inner; @@ -199,7 +198,7 @@ public class ExpressionCodegen extends JetVisitor { public StackValue visitSuperExpression(JetSuperExpression expression, StackValue data) { final DeclarationDescriptor descriptor = bindingContext.get(BindingContext.REFERENCE_TARGET, expression.getInstanceReference()); if (descriptor instanceof ClassDescriptor) { - return generateThisOrOuter((ClassDescriptor) descriptor); + return StackValue.thisOrOuter(this, (ClassDescriptor) descriptor); } else { JetType type = context.getThisDescriptor().getDefaultType(); @@ -1161,9 +1160,12 @@ public class ExpressionCodegen extends JetVisitor { if (descriptor instanceof ValueParameterDescriptor && descriptor.getContainingDeclaration() instanceof ScriptDescriptor) { ScriptDescriptor scriptDescriptor = (ScriptDescriptor) descriptor.getContainingDeclaration(); - JvmClassName scriptClassName = state.getInjector().getClosureAnnotator().classNameForScriptDescriptor(scriptDescriptor); + final ClosureAnnotator closureAnnotator = state.getInjector().getClosureAnnotator(); + JvmClassName scriptClassName = closureAnnotator.classNameForScriptDescriptor(scriptDescriptor); ValueParameterDescriptor valueParameterDescriptor = (ValueParameterDescriptor) descriptor; - generateThisOrOuter(state.getInjector().getClosureAnnotator().classDescriptorForScrpitDescriptor(scriptDescriptor)); + final ClassDescriptor scriptClass = closureAnnotator.classDescriptorForScriptDescriptor(scriptDescriptor); + final StackValue script = StackValue.thisOrOuter(this, scriptClass); + script.put(script.type, v); Type fieldType = typeMapper.mapType(valueParameterDescriptor.getType(), MapTypeMode.VALUE); return StackValue.field(fieldType, scriptClassName, valueParameterDescriptor.getName().getIdentifier(), false); } @@ -1513,27 +1515,23 @@ public class ExpressionCodegen extends JetVisitor { } else { v.getstatic(classObjType.getInternalName(), "$classobj", exprType.getDescriptor()); } + StackValue.onStack(exprType).put(type, v); } else { - generateThisOrOuter(classReceiverDeclarationDescriptor).put(exprType, v); + StackValue.thisOrOuter(this, classReceiverDeclarationDescriptor).put(type, v); } - StackValue.onStack(exprType).put(type, v); } else if (descriptor instanceof ScriptReceiver) { generateScript((ScriptReceiver) descriptor); } else if (descriptor instanceof ExtensionReceiver) { - Type exprType = asmType(descriptor.getType()); ExtensionReceiver extensionReceiver = (ExtensionReceiver) descriptor; - generateReceiver(extensionReceiver.getDeclarationDescriptor()).put(exprType, v); - StackValue.onStack(exprType).put(type, v); + generateReceiver(extensionReceiver.getDeclarationDescriptor()).put(type, v); } else if (descriptor instanceof ExpressionReceiver) { ExpressionReceiver expressionReceiver = (ExpressionReceiver) descriptor; JetExpression expr = expressionReceiver.getExpression(); - Type exprType = asmType(expressionReceiver.getType()); - gen(expr, exprType); - StackValue.onStack(exprType).put(type, v); + gen(expr, type); } else if (descriptor instanceof AutoCastReceiver) { AutoCastReceiver autoCastReceiver = (AutoCastReceiver) descriptor; @@ -1602,25 +1600,23 @@ public class ExpressionCodegen extends JetVisitor { cur = cur.getParentContext(); } - throw new UnsupportedOperationException(); } - - public StackValue generateThisOrOuter(@NotNull ClassDescriptor calleeContainingClass) { - CodegenContext cur = context; + throw new UnsupportedOperationException(); + } + public StackValue generateThisOrOuter(@NotNull final ClassDescriptor calleeContainingClass) { PsiElement psiElement = BindingContextUtils.classDescriptorToDeclaration(bindingContext, calleeContainingClass); boolean isObject = psiElement instanceof JetClassOrObject && CodegenUtil.isNonLiteralObject((JetClassOrObject) psiElement); - cur = context; - StackValue result = StackValue.local(0, TYPE_OBJECT); + CodegenContext cur = context; + Type type = asmType(calleeContainingClass.getDefaultType()); + StackValue result = StackValue.local(0, type); while (cur != null) { if (cur instanceof CodegenContexts.MethodContext && !(cur instanceof CodegenContexts.ConstructorContext)) cur = cur.getParentContext(); if (DescriptorUtils.isSubclass(cur.getThisDescriptor(), calleeContainingClass)) { - Type type = asmType(calleeContainingClass.getDefaultType()); if (!isObject || (cur.getThisDescriptor() == calleeContainingClass)) { - result.put(TYPE_OBJECT, v); - return castToRequiredTypeOfInterfaceIfNeeded(StackValue.onStack(type), cur.getThisDescriptor(), calleeContainingClass); + return castToRequiredTypeOfInterfaceIfNeeded(result, cur.getThisDescriptor(), calleeContainingClass); } else { v.getstatic(type.getInternalName(), "$instance", type.getDescriptor()); @@ -2697,7 +2693,7 @@ public class ExpressionCodegen extends JetVisitor { public StackValue visitThisExpression(JetThisExpression expression, StackValue receiver) { final DeclarationDescriptor descriptor = bindingContext.get(BindingContext.REFERENCE_TARGET, expression.getInstanceReference()); if (descriptor instanceof ClassDescriptor) { - return generateThisOrOuter((ClassDescriptor) descriptor); + return StackValue.thisOrOuter(this, (ClassDescriptor) descriptor); } else { if (descriptor instanceof FunctionDescriptor || descriptor instanceof PropertyDescriptor) { diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/JetTypeMapper.java b/compiler/backend/src/org/jetbrains/jet/codegen/JetTypeMapper.java index dfba49abfec..8c3fc02c1f4 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/JetTypeMapper.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/JetTypeMapper.java @@ -953,7 +953,7 @@ public class JetTypeMapper { for (ScriptDescriptor importedScript : importedScripts) { signatureWriter.writeParameterType(JvmMethodParameterKind.VALUE); - mapType(closureAnnotator.classDescriptorForScrpitDescriptor(importedScript).getDefaultType(), signatureWriter, MapTypeMode.VALUE); + mapType(closureAnnotator.classDescriptorForScriptDescriptor(importedScript).getDefaultType(), signatureWriter, MapTypeMode.VALUE); signatureWriter.writeParameterTypeEnd(); } diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/ScriptCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/ScriptCodegen.java index 22cd6a33882..32b89ed607b 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/ScriptCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/ScriptCodegen.java @@ -35,7 +35,6 @@ import org.objectweb.asm.Type; import org.objectweb.asm.commons.InstructionAdapter; import javax.inject.Inject; -import java.util.Collections; import java.util.List; /** @@ -99,7 +98,7 @@ public class ScriptCodegen { ScriptDescriptor scriptDescriptor = (ScriptDescriptor) state.getBindingContext().get(BindingContext.SCRIPT, scriptDeclaration); - ClassDescriptor classDescriptorForScript = closureAnnotator.classDescriptorForScrpitDescriptor(scriptDescriptor); + ClassDescriptor classDescriptorForScript = closureAnnotator.classDescriptorForScriptDescriptor(scriptDescriptor); CodegenContexts.ScriptContext context = (CodegenContexts.ScriptContext) CodegenContexts.STATIC.intoScript(scriptDescriptor, classDescriptorForScript); diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/StackValue.java b/compiler/backend/src/org/jetbrains/jet/codegen/StackValue.java index 8cb3527c54f..6923f95f423 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/StackValue.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/StackValue.java @@ -1087,7 +1087,8 @@ public abstract class StackValue { @Override public void put(Type type, InstructionAdapter v) { - codegen.generateThisOrOuter(descriptor); + final StackValue stackValue = codegen.generateThisOrOuter(descriptor); + stackValue.put(stackValue.type, v); // no coercion here } } diff --git a/compiler/testData/codegen/regressions/kt2395.kt b/compiler/testData/codegen/regressions/kt2395.kt new file mode 100644 index 00000000000..7a0401ec171 --- /dev/null +++ b/compiler/testData/codegen/regressions/kt2395.kt @@ -0,0 +1,11 @@ +import java.util.AbstractList + +class MyList(): AbstractList() { + public fun getModificationCount(): Int = modCount + public override fun get(index: Int): String? = "" + public override fun size(): Int = 0 +} + +fun box(): String { + return if (MyList().getModificationCount() == 0) "OK" else "fail" +} diff --git a/compiler/tests/org/jetbrains/jet/codegen/ClassGenTest.java b/compiler/tests/org/jetbrains/jet/codegen/ClassGenTest.java index 8a5afb6b9a1..5276334f2ec 100644 --- a/compiler/tests/org/jetbrains/jet/codegen/ClassGenTest.java +++ b/compiler/tests/org/jetbrains/jet/codegen/ClassGenTest.java @@ -471,4 +471,9 @@ public class ClassGenTest extends CodegenTestCase { createEnvironmentWithMockJdkAndIdeaAnnotations(ConfigurationKind.JDK_ONLY); blackBoxMultiFile("regressions/kt2060_1.kt", "regressions/kt2060.kt"); } + + public void testKt2395() { + createEnvironmentWithMockJdkAndIdeaAnnotations(ConfigurationKind.JDK_ONLY); + blackBoxMultiFile("regressions/kt2395.kt"); + } }