From 35a1240dc7ace8048f19da35286f7d845dc718e3 Mon Sep 17 00:00:00 2001 From: Alex Tkachman Date: Fri, 20 Jan 2012 14:21:38 +0100 Subject: [PATCH] KT-1018: better handling of property visibility --- .../jet/codegen/ExpressionCodegen.java | 23 +++++------ .../jetbrains/jet/codegen/JetTypeMapper.java | 24 ++++++++---- .../jet/codegen/PropertyCodegen.java | 39 ++++++++++--------- .../testData/codegen/regressions/kt1018.kt | 12 ++++++ .../testData/codegen/super/traitproperty.jet | 2 +- .../jetbrains/jet/codegen/ClassGenTest.java | 6 ++- .../jet/codegen/PropertyGenTest.java | 22 ++++++++++- 7 files changed, 84 insertions(+), 44 deletions(-) create mode 100644 compiler/testData/codegen/regressions/kt1018.kt diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java index dbf018ec21a..ae8f3ccfbf8 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java @@ -965,14 +965,16 @@ public class ExpressionCodegen extends JetVisitor { final boolean directToField = expression.getReferencedNameElementType() == JetTokens.FIELD_IDENTIFIER && contextKind() != OwnerKind.TRAIT_IMPL ; JetExpression r = getReceiverForSelector(expression); final boolean isSuper = r instanceof JetSuperExpression; - if(propertyDescriptor.getVisibility() == Visibility.PRIVATE && context.getClassOrNamespaceDescriptor() != propertyDescriptor.getContainingDeclaration()) { - DeclarationDescriptor enclosed = propertyDescriptor.getContainingDeclaration(); - if(enclosed != null && enclosed != context.getThisDescriptor()) { - CodegenContext c = context; - while(c.getContextDescriptor() != enclosed) { - c = c.getParentContext(); + if(propertyDescriptor.getVisibility() == Visibility.PRIVATE && !CodegenUtil.isClassObject(propertyDescriptor.getContainingDeclaration())) { + if(context.getClassOrNamespaceDescriptor() != propertyDescriptor.getContainingDeclaration()) { + DeclarationDescriptor enclosed = propertyDescriptor.getContainingDeclaration(); + if(enclosed != null && enclosed != context.getThisDescriptor()) { + CodegenContext c = context; + while(c.getContextDescriptor() != enclosed) { + c = c.getParentContext(); + } + propertyDescriptor = (PropertyDescriptor) c.getAccessor(propertyDescriptor); } - propertyDescriptor = (PropertyDescriptor) c.getAccessor(propertyDescriptor); } } final StackValue.Property iValue = intermediateValueForProperty(propertyDescriptor, directToField, isSuper ? (JetSuperExpression)r : null); @@ -1080,12 +1082,7 @@ public class ExpressionCodegen extends JetVisitor { } else { owner = typeMapper.getOwner(functionDescriptor, OwnerKind.IMPLEMENTATION); - if (containingDeclaration instanceof ClassDescriptor && ((ClassDescriptor) containingDeclaration).getKind() == ClassKind.TRAIT) { - isInterface = true; - } - else { - isInterface = false; - } + isInterface = CodegenUtil.isInterface(containingDeclaration); } v.visitMethodInsn(isStatic ? Opcodes.INVOKESTATIC : isInterface ? Opcodes.INVOKEINTERFACE : Opcodes.INVOKEVIRTUAL, owner, functionDescriptor.getName(), typeMapper.mapSignature(functionDescriptor.getName(),functionDescriptor).getAsmMethod().getDescriptor()); diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/JetTypeMapper.java b/compiler/backend/src/org/jetbrains/jet/codegen/JetTypeMapper.java index fcc7bc2a303..d997b8f7c4c 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/JetTypeMapper.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/JetTypeMapper.java @@ -684,18 +684,26 @@ public class JetTypeMapper { return new CallableMethod(owner, method, INVOKESPECIAL); } - static int getAccessModifiers(JetDeclaration p, int defaultFlags) { - int flags = 0; - if (p.hasModifier(JetTokens.PUBLIC_KEYWORD)) { - flags |= ACC_PUBLIC; + static int getAccessModifiers(DeclarationDescriptorWithVisibility p, int defaultFlags) { + DeclarationDescriptor declaration = p.getContainingDeclaration(); + if(CodegenUtil.isInterface(declaration)) { + return ACC_PUBLIC; } - else if (p.hasModifier(JetTokens.PRIVATE_KEYWORD)) { - flags |= ACC_PRIVATE; + if (p.getVisibility() == Visibility.PUBLIC) { + return ACC_PUBLIC; + } + else if (p.getVisibility() == Visibility.PROTECTED) { + return ACC_PROTECTED; + } + else if (p.getVisibility() == Visibility.PRIVATE) { + if(CodegenUtil.isClassObject(declaration)) { + return defaultFlags; + } + return ACC_PRIVATE; } else { - flags |= defaultFlags; + return defaultFlags; } - return flags; } String classNameForAnonymousClass(JetExpression expression) { diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/PropertyCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/PropertyCodegen.java index fefe4b7d5f5..691f904dd92 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/PropertyCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/PropertyCodegen.java @@ -3,10 +3,7 @@ package org.jetbrains.jet.codegen; import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.PsiElement; import org.jetbrains.annotations.NotNull; -import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor; -import org.jetbrains.jet.lang.descriptors.PropertyDescriptor; -import org.jetbrains.jet.lang.descriptors.PropertySetterDescriptor; -import org.jetbrains.jet.lang.descriptors.VariableDescriptor; +import org.jetbrains.jet.lang.descriptors.*; import org.jetbrains.jet.lang.psi.*; import org.jetbrains.jet.lang.resolve.BindingContext; import org.jetbrains.jet.lang.resolve.constants.CompileTimeConstant; @@ -70,7 +67,7 @@ public class PropertyCodegen { } int modifiers; if (kind == OwnerKind.NAMESPACE) { - int access = isExternallyAccessible(p) ? Opcodes.ACC_PUBLIC : Opcodes.ACC_PRIVATE; + int access = JetTypeMapper.getAccessModifiers(propertyDescriptor, 0); modifiers = access | Opcodes.ACC_STATIC; } else { @@ -90,17 +87,17 @@ public class PropertyCodegen { JvmPropertyAccessorSignature signature = state.getTypeMapper().mapGetterSignature(propertyDescriptor, kind); functionCodegen.generateMethod(getter, signature.getJvmMethodSignature(), true, signature.getPropertyTypeKotlinSignature(), propertyDescriptor.getGetter()); } - else if (!getter.hasModifier(JetTokens.PRIVATE_KEYWORD)) { - generateDefaultGetter(p, getter); + else if (isExternallyAccessible(propertyDescriptor)) { + generateDefaultGetter(p); } } - else if (isExternallyAccessible(p)) { - generateDefaultGetter(p, p); + else if (isExternallyAccessible(propertyDescriptor)) { + generateDefaultGetter(p); } } - private static boolean isExternallyAccessible(JetProperty p) { - return !p.hasModifier(JetTokens.PRIVATE_KEYWORD); + private static boolean isExternallyAccessible(PropertyDescriptor p) { + return p.getVisibility() != Visibility.PRIVATE || CodegenUtil.isClassObject(p.getContainingDeclaration()); } private void generateSetter(JetProperty p, PropertyDescriptor propertyDescriptor) { @@ -112,18 +109,18 @@ public class PropertyCodegen { JvmPropertyAccessorSignature signature = state.getTypeMapper().mapSetterSignature(propertyDescriptor, kind); functionCodegen.generateMethod(setter, signature.getJvmMethodSignature(), true, signature.getPropertyTypeKotlinSignature(), setterDescriptor); } - else if (!p.hasModifier(JetTokens.PRIVATE_KEYWORD)) { - generateDefaultSetter(p, setter); + else if (isExternallyAccessible(propertyDescriptor)) { + generateDefaultSetter(p); } } - else if (isExternallyAccessible(p) && p.isVar()) { - generateDefaultSetter(p, p); + else if (isExternallyAccessible(propertyDescriptor) && propertyDescriptor.isVar()) { + generateDefaultSetter(p); } } - private void generateDefaultGetter(JetProperty p, JetDeclaration declaration) { + private void generateDefaultGetter(JetProperty p) { final PropertyDescriptor propertyDescriptor = (PropertyDescriptor) state.getBindingContext().get(BindingContext.VARIABLE, p); - int flags = JetTypeMapper.getAccessModifiers(declaration, Opcodes.ACC_PUBLIC); + int flags = JetTypeMapper.getAccessModifiers(propertyDescriptor, 0); generateDefaultGetter(propertyDescriptor, flags, p); } @@ -176,9 +173,13 @@ public class PropertyCodegen { aw.visitEnd(); } - private void generateDefaultSetter(JetProperty p, JetDeclaration declaration) { + private void generateDefaultSetter(JetProperty p) { final PropertyDescriptor propertyDescriptor = (PropertyDescriptor) state.getBindingContext().get(BindingContext.VARIABLE, p); - int flags = JetTypeMapper.getAccessModifiers(declaration, Opcodes.ACC_PUBLIC); + assert propertyDescriptor != null; + + int modifiers = JetTypeMapper.getAccessModifiers(propertyDescriptor, 0); + PropertySetterDescriptor setter = propertyDescriptor.getSetter(); + int flags = setter == null ? modifiers : JetTypeMapper.getAccessModifiers(setter, modifiers); generateDefaultSetter(propertyDescriptor, flags, p); } diff --git a/compiler/testData/codegen/regressions/kt1018.kt b/compiler/testData/codegen/regressions/kt1018.kt new file mode 100644 index 00000000000..5e96b01c0e5 --- /dev/null +++ b/compiler/testData/codegen/regressions/kt1018.kt @@ -0,0 +1,12 @@ +public class StockMarketTableModel() { + + public fun getColumnCount() : Int { + return COLUMN_TITLES?.size.sure() + } + + class object { + private val COLUMN_TITLES : Array = Array(10) + } +} + +fun box() : String = if(StockMarketTableModel().getColumnCount()==10) "OK" else "fail" \ No newline at end of file diff --git a/compiler/testData/codegen/super/traitproperty.jet b/compiler/testData/codegen/super/traitproperty.jet index 962f71c5dc9..6b61b588ef4 100644 --- a/compiler/testData/codegen/super/traitproperty.jet +++ b/compiler/testData/codegen/super/traitproperty.jet @@ -8,7 +8,7 @@ trait M { } class N() : M { - override var backingB : Int = 0 + public override var backingB : Int = 0 val a : Int get() { diff --git a/compiler/tests/org/jetbrains/jet/codegen/ClassGenTest.java b/compiler/tests/org/jetbrains/jet/codegen/ClassGenTest.java index e2a33a757cd..d94efb6af13 100644 --- a/compiler/tests/org/jetbrains/jet/codegen/ClassGenTest.java +++ b/compiler/tests/org/jetbrains/jet/codegen/ClassGenTest.java @@ -199,7 +199,7 @@ public class ClassGenTest extends CodegenTestCase { public void testKt508 () throws Exception { loadFile("regressions/kt508.jet"); -// System.out.println(generateToText()); + System.out.println(generateToText()); blackBox(); } @@ -245,4 +245,8 @@ public class ClassGenTest extends CodegenTestCase { public void testKt940 () throws Exception { blackBoxFile("regressions/kt940.kt"); } + + public void testKt1018 () throws Exception { + blackBoxFile("regressions/kt1018.kt"); + } } diff --git a/compiler/tests/org/jetbrains/jet/codegen/PropertyGenTest.java b/compiler/tests/org/jetbrains/jet/codegen/PropertyGenTest.java index 52ce1309e36..36c65f6982d 100644 --- a/compiler/tests/org/jetbrains/jet/codegen/PropertyGenTest.java +++ b/compiler/tests/org/jetbrains/jet/codegen/PropertyGenTest.java @@ -50,7 +50,7 @@ public class PropertyGenTest extends CodegenTestCase { assertNotNull(findMethodByName(aClass, "setFoo")); } - public void testPropertyInNamespace() throws Exception { + public void testPrivatePropertyInNamespace() throws Exception { loadText("private val x = 239"); final Class nsClass = generateNamespaceClass(); final Field[] fields = nsClass.getDeclaredFields(); @@ -62,6 +62,18 @@ public class PropertyGenTest extends CodegenTestCase { assertEquals(239, field.get(null)); } + public void testProtectedPropertyInNamespace() throws Exception { + loadText("protected val x : Int = 239"); + final Class nsClass = generateNamespaceClass(); + final Field[] fields = nsClass.getDeclaredFields(); + assertEquals(1, fields.length); + final Field field = fields[0]; + field.setAccessible(true); + assertEquals("x", field.getName()); + assertEquals(Modifier.PROTECTED | Modifier.STATIC | Modifier.FINAL, field.getModifiers()); + assertEquals(239, field.get(null)); + } + public void testFieldPropertyAccess() throws Exception { loadFile("properties/fieldPropertyAccess.jet"); // System.out.println(generateToText()); @@ -96,12 +108,16 @@ public class PropertyGenTest extends CodegenTestCase { } public void testAccessorsWithoutBody() throws Exception { - loadText("class AccessorsWithoutBody() { public var foo: Int = 349\n get\n private set\n fun setter() { foo = 610; } } "); + loadText("class AccessorsWithoutBody() { protected var foo: Int = 349\n get\n private set\n fun setter() { foo = 610; } } "); + System.out.println(generateToText()); final Class aClass = loadImplementationClass(generateClassesInFile(), "AccessorsWithoutBody"); final Object instance = aClass.newInstance(); final Method getFoo = findMethodByName(aClass, "getFoo"); + getFoo.setAccessible(true); + assertTrue((getFoo.getModifiers() & Modifier.PROTECTED) != 0); assertEquals(349, getFoo.invoke(instance)); final Method setFoo = findMethodByName(aClass, "setFoo"); + setFoo.setAccessible(true); assertTrue((setFoo.getModifiers() & Modifier.PRIVATE) != 0); final Method setter = findMethodByName(aClass, "setter"); setter.invoke(instance); @@ -111,6 +127,7 @@ public class PropertyGenTest extends CodegenTestCase { public void testInitializersForNamespaceProperties() throws Exception { loadText("val x = System.currentTimeMillis()"); final Method method = generateFunction("getX"); + method.setAccessible(true); assertIsCurrentTime((Long) method.invoke(null)); } @@ -144,6 +161,7 @@ public class PropertyGenTest extends CodegenTestCase { public void testKt160() throws Exception { loadText("internal val s = java.lang.Double.toString(1.0)"); final Method method = generateFunction("getS"); + method.setAccessible(true); assertEquals(method.invoke(null), "1.0"); } }