Skip to content

Commit

Permalink
Remove 'inner enum' hack & fix Java inner class resolve
Browse files Browse the repository at this point in the history 
Inner enum was placed into the class object of the outer class. Remove the
hack from frontend, frontend.java & backend. Fix tests

 #KT-1174 In Progress
  • Loading branch information
@udalov
udalov committed Jan 16, 2013
1 parent cc6bd62 commit b612989
Show file tree
Hide file tree
Showing 19 changed files with 70 additions and 236 deletions.
22 changes: 1 addition & 21 deletions compiler/backend/src/org/jetbrains/jet/codegen/ImplementationBodyCodegen.java
View file
Expand Up @@ -186,26 +186,12 @@ private void writeOuterClass() {
}

private void writeInnerClasses() {
// Inner enums are moved by frontend to a class object of outer class, but we want them to be inner for the outer class itself
// (to avoid publicly visible names like A$ClassObject$$B), so they are handled specially in this method

for (ClassDescriptor innerClass : getInnerClassesAndObjects(descriptor)) {
// If it's an inner enum inside a class object, don't write it
// (instead write it to inner classes of this class object's containing class)
if (!isEnumMovedToClassObject(innerClass)) {
writeInnerClass(innerClass, false);
}
writeInnerClass(innerClass, false /* TODO */);
}

ClassDescriptor classObjectDescriptor = descriptor.getClassObjectDescriptor();
if (classObjectDescriptor != null) {
// Process all enums here which were moved to our class object
for (ClassDescriptor innerClass : getInnerClassesAndObjects(classObjectDescriptor)) {
if (isEnumMovedToClassObject(innerClass)) {
writeInnerClass(innerClass, true);
}
}

int innerClassAccess = getVisibilityAccessFlag(classObjectDescriptor) | ACC_FINAL | ACC_STATIC;
v.visitInnerClass(classAsmType.getInternalName() + JvmAbi.CLASS_OBJECT_SUFFIX, classAsmType.getInternalName(),
JvmAbi.CLASS_OBJECT_CLASS_NAME,
Expand Down Expand Up @@ -240,12 +226,6 @@ else if (innerClass.getKind() == ClassKind.ENUM_CLASS) {
v.visitInnerClass(innerClassInternalName, outerClassInternalName, innerClass.getName().getName(), innerClassAccess);
}

private boolean isEnumMovedToClassObject(ClassDescriptor innerClass) {
// Checks if this is enum, moved to class object by frontend

return Boolean.TRUE.equals(bindingContext.get(BindingContext.IS_ENUM_MOVED_TO_CLASS_OBJECT, innerClass));
}

private void writeClassSignatureIfNeeded(JvmClassSignature signature) {
if (signature.getKotlinGenericSignature() != null || descriptor.getVisibility() != Visibilities.PUBLIC) {
AnnotationVisitor annotationVisitor = v.newAnnotation(JvmStdlibNames.JET_CLASS.getDescriptor(), true);
Expand Down
8 changes: 0 additions & 8 deletions compiler/frontend.java/src/org/jetbrains/jet/lang/resolve/java/DescriptorResolverUtils.java
View file
Expand Up @@ -50,14 +50,6 @@ public static boolean isKotlinClass(@NotNull PsiClass psiClass) {
return wrapper.getJetClass().isDefined() || wrapper.getJetPackageClass().isDefined();
}

public static boolean isInnerEnum(@NotNull PsiClass innerClass, @Nullable DeclarationDescriptor owner) {
if (!innerClass.isEnum()) return false;
if (!(owner instanceof ClassDescriptor)) return false;

ClassKind kind = ((ClassDescriptor) owner).getKind();
return kind == ClassKind.CLASS || kind == ClassKind.TRAIT || kind == ClassKind.ENUM_CLASS;
}

@NotNull
public static Collection<JetType> getSupertypes(@NotNull ClassOrNamespaceDescriptor classOrNamespaceDescriptor) {
if (classOrNamespaceDescriptor instanceof ClassDescriptor) {
Expand Down
7 changes: 2 additions & 5 deletions compiler/frontend.java/src/org/jetbrains/jet/lang/resolve/java/JavaDescriptorResolver.java
View file
Expand Up @@ -168,10 +168,7 @@ public Set<FunctionDescriptor> resolveFunctionGroup(
}

@NotNull
public List<ClassDescriptor> resolveInnerClasses(
@NotNull DeclarationDescriptor owner,
@NotNull ClassPsiDeclarationProvider declarationProvider)
{
return innerClassResolver.resolveInnerClasses(owner, declarationProvider);
public List<ClassDescriptor> resolveInnerClasses(@NotNull ClassPsiDeclarationProvider declarationProvider) {
return innerClassResolver.resolveInnerClasses(declarationProvider);
}
}
14 changes: 0 additions & 14 deletions ...ontend.java/src/org/jetbrains/jet/lang/resolve/java/resolver/JavaClassObjectResolver.java
View file
Expand Up @@ -46,7 +46,6 @@
import static org.jetbrains.jet.lang.resolve.DescriptorResolver.createEnumClassObjectValueOfMethod;
import static org.jetbrains.jet.lang.resolve.DescriptorResolver.createEnumClassObjectValuesMethod;
import static org.jetbrains.jet.lang.resolve.DescriptorUtils.getClassObjectName;
import static org.jetbrains.jet.lang.resolve.java.DescriptorResolverUtils.isInnerEnum;

public final class JavaClassObjectResolver {

Expand Down Expand Up @@ -84,10 +83,6 @@ public ClassDescriptorFromJvmBytecode createClassObjectDescriptor(
return null;
}

if (hasInnerEnums(containing, psiClass)) {
return createSyntheticClassObject(containing, psiClass);
}

PsiClass classObjectPsiClass = getClassObjectPsiClass(psiClass);
if (classObjectPsiClass == null) {
return null;
Expand Down Expand Up @@ -115,15 +110,6 @@ private ClassDescriptorFromJvmBytecode createClassObjectFromPsi(
return classObjectDescriptor;
}

private static boolean hasInnerEnums(@NotNull ClassDescriptor containing, @NotNull PsiClass psiClass) {
for (PsiClass innerClass : psiClass.getInnerClasses()) {
if (isInnerEnum(innerClass, containing)) {
return true;
}
}
return false;
}

@NotNull
private ClassDescriptorFromJvmBytecode createClassObjectDescriptorForEnum(
@NotNull ClassDescriptor containing,
Expand Down
7 changes: 0 additions & 7 deletions ...ler/frontend.java/src/org/jetbrains/jet/lang/resolve/java/resolver/JavaClassResolver.java
View file
Expand Up @@ -353,13 +353,6 @@ private ClassOrNamespaceDescriptor resolveParentClass(@NotNull PsiClass psiClass
throw new IllegalStateException(
"PsiClass not found by name " + containerFqName + ", required to be container declaration of " + getFqName(psiClass));
}
if (DescriptorResolverUtils.isInnerEnum(psiClass, parentClass) && DescriptorResolverUtils.isKotlinClass(psiClass)) {
ClassDescriptor classObjectDescriptor = parentClass.getClassObjectDescriptor();
if (classObjectDescriptor == null) {
throw new IllegalStateException("Class object for a class with inner enum should've been created earlier: " + parentClass);
}
return classObjectDescriptor;
}
return parentClass;
}

Expand Down
54 changes: 8 additions & 46 deletions ...rontend.java/src/org/jetbrains/jet/lang/resolve/java/resolver/JavaInnerClassResolver.java
View file
Expand Up @@ -16,14 +16,9 @@

package org.jetbrains.jet.lang.resolve.java.resolver;

import com.google.common.collect.Lists;
import com.intellij.psi.PsiClass;
import com.intellij.psi.PsiModifier;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.jet.lang.descriptors.ClassDescriptor;
import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor;
import org.jetbrains.jet.lang.resolve.DescriptorUtils;
import org.jetbrains.jet.lang.resolve.java.DescriptorResolverUtils;
import org.jetbrains.jet.lang.resolve.java.DescriptorSearchRule;
import org.jetbrains.jet.lang.resolve.java.JvmAbi;
import org.jetbrains.jet.lang.resolve.java.provider.ClassPsiDeclarationProvider;
Expand All @@ -34,8 +29,6 @@
import java.util.Collections;
import java.util.List;

import static org.jetbrains.jet.lang.resolve.java.provider.DeclarationOrigin.KOTLIN;

public final class JavaInnerClassResolver {

private JavaClassResolver classResolver;
Expand All @@ -49,23 +42,20 @@ public void setClassResolver(JavaClassResolver classResolver) {
}

@NotNull
public List<ClassDescriptor> resolveInnerClasses(
@NotNull DeclarationDescriptor owner,
@NotNull ClassPsiDeclarationProvider declarationProvider
) {
if (declarationProvider.isStaticMembers() && declarationProvider.getDeclarationOrigin() == KOTLIN) {
return resolveInnerClassesOfClassObject(owner, declarationProvider.getPsiClass());
public List<ClassDescriptor> resolveInnerClasses(@NotNull ClassPsiDeclarationProvider declarationProvider) {
if (declarationProvider.isStaticMembers()) {
return Collections.emptyList();
}

return resolveInnerClasses(owner, declarationProvider.getPsiClass(), declarationProvider.isStaticMembers());
return resolveInnerClasses(declarationProvider.getPsiClass());
}

@NotNull
private List<ClassDescriptor> resolveInnerClasses(@NotNull DeclarationDescriptor owner, @NotNull PsiClass psiClass, boolean isStatic) {
private List<ClassDescriptor> resolveInnerClasses(@NotNull PsiClass psiClass) {
PsiClass[] innerPsiClasses = psiClass.getInnerClasses();
List<ClassDescriptor> result = new ArrayList<ClassDescriptor>(innerPsiClasses.length);
for (PsiClass innerPsiClass : innerPsiClasses) {
if (shouldBeIgnored(owner, psiClass, innerPsiClass, isStatic)) {
if (shouldBeIgnored(innerPsiClass)) {
continue;
}
ClassDescriptor classDescriptor = resolveInnerClass(innerPsiClass);
Expand All @@ -74,36 +64,8 @@ private List<ClassDescriptor> resolveInnerClasses(@NotNull DeclarationDescriptor
return result;
}

private static boolean shouldBeIgnored(DeclarationDescriptor owner, PsiClass psiClass, PsiClass innerPsiClass, boolean isStatic) {
if (innerPsiClass.getName().equals(JvmAbi.CLASS_OBJECT_CLASS_NAME)
|| DescriptorResolverUtils.isInnerEnum(innerPsiClass, owner)) {
return true;
}
//TODO: this is a hack that puts interfaces and inner classes of interfaces (which are always static)
// inside NonStaticMemberScope
if (innerPsiClass.isInterface() || psiClass.isInterface()) {
return isStatic;
}
else {
return innerPsiClass.hasModifierProperty(PsiModifier.STATIC) != isStatic;
}
}

private List<ClassDescriptor> resolveInnerClassesOfClassObject(@NotNull DeclarationDescriptor classObject, @NotNull PsiClass psiClass) {
if (!DescriptorUtils.isClassObject(classObject)) {
return Collections.emptyList();
}

List<ClassDescriptor> result = Lists.newArrayList();
// If we're a class object, inner enums of our parent need to be put into us
DeclarationDescriptor containingDeclaration = classObject.getContainingDeclaration();
for (PsiClass innerPsiClass : psiClass.getInnerClasses()) {
if (DescriptorResolverUtils.isInnerEnum(innerPsiClass, containingDeclaration)) {
ClassDescriptor classDescriptor = resolveInnerClass(innerPsiClass);
result.add(classDescriptor);
}
}
return result;
private static boolean shouldBeIgnored(PsiClass innerPsiClass) {
return innerPsiClass.getName().equals(JvmAbi.CLASS_OBJECT_CLASS_NAME);
}

@NotNull
Expand Down
2 changes: 1 addition & 1 deletion ...er/frontend.java/src/org/jetbrains/jet/lang/resolve/java/scope/JavaClassMembersScope.java
View file
Expand Up @@ -71,7 +71,7 @@ private Map<Name, ClassDescriptor> getInnerClassesMap() {
@NotNull
@Override
protected Collection<ClassDescriptor> computeInnerClasses() {
return getResolver().resolveInnerClasses(descriptor, declarationProvider);
return getResolver().resolveInnerClasses(declarationProvider);
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion ...ntend.java/src/org/jetbrains/jet/lang/resolve/java/scope/JavaScopeForKotlinNamespace.java
View file
Expand Up @@ -51,6 +51,6 @@ protected Set<FunctionDescriptor> computeFunctionDescriptor(@NotNull Name name)
@NotNull
@Override
protected Collection<ClassDescriptor> computeInnerClasses() {
return getResolver().resolveInnerClasses(descriptor, declarationProvider);
return getResolver().resolveInnerClasses(declarationProvider);
}
}
2 changes: 0 additions & 2 deletions compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java
View file
Expand Up @@ -160,8 +160,6 @@ public interface Errors {

// Enum-specific

SimpleDiagnosticFactory<PsiElement> ENUM_NOT_ALLOWED = SimpleDiagnosticFactory.create(ERROR);

SimpleDiagnosticFactory<JetModifierListOwner> ILLEGAL_ENUM_ANNOTATION = SimpleDiagnosticFactory
.create(ERROR, modifierSetPosition(JetTokens.ENUM_KEYWORD));

Expand Down
1 change: 0 additions & 1 deletion compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/rendering/DefaultErrorMessages.java
View file
Expand Up @@ -162,7 +162,6 @@ public String render(@NotNull Collection<JetKeywordToken> tokens) {

MAP.put(ENUM_ENTRY_SHOULD_BE_INITIALIZED, "Missing delegation specifier ''{0}''", NAME);
MAP.put(ENUM_ENTRY_ILLEGAL_TYPE, "The type constructor of enum entry should be ''{0}''", NAME);
MAP.put(ENUM_NOT_ALLOWED, "Enum class is not allowed here");

MAP.put(UNINITIALIZED_VARIABLE, "Variable ''{0}'' must be initialized", NAME);
MAP.put(UNINITIALIZED_PARAMETER, "Parameter ''{0}'' is uninitialized here", NAME);
Expand Down
2 changes: 0 additions & 2 deletions compiler/frontend/src/org/jetbrains/jet/lang/resolve/BindingContext.java
View file
Expand Up @@ -228,8 +228,6 @@ public Boolean computeValue(SlicedMap map, JetFunctionLiteralExpression expressi
ReadOnlySlice<PsiElement, DeclarationDescriptor> DECLARATION_TO_DESCRIPTOR = Slices.<PsiElement, DeclarationDescriptor>sliceBuilder()
.setFurtherLookupSlices(DECLARATIONS_TO_DESCRIPTORS).build();

WritableSlice<ClassDescriptor, Boolean> IS_ENUM_MOVED_TO_CLASS_OBJECT = Slices.createSimpleSlice();

WritableSlice<JetReferenceExpression, PsiElement> LABEL_TARGET = Slices.<JetReferenceExpression, PsiElement>sliceBuilder().build();
WritableSlice<JetReferenceExpression, Collection<? extends PsiElement>> AMBIGUOUS_LABEL_TARGET =
Slices.<JetReferenceExpression, Collection<? extends PsiElement>>sliceBuilder().build();
Expand Down
63 changes: 0 additions & 63 deletions compiler/frontend/src/org/jetbrains/jet/lang/resolve/TypeHierarchyResolver.java
View file
Expand Up @@ -196,8 +196,6 @@ private Collection<JetDeclarationContainer> collectNamespacesAndClassifiers(
declaration.accept(collector);
}

collector.finishProcessing();

return forDeferredResolve;
}

Expand Down Expand Up @@ -470,8 +468,6 @@ private class ClassifierCollector extends JetVisitorVoid {
private final NamespaceLikeBuilder owner;
private final Collection<JetDeclarationContainer> forDeferredResolve;

private final List<JetClass> enumsToAddLater = new ArrayList<JetClass>(0);

public ClassifierCollector(@NotNull JetScope outerScope,
@NotNull NamespaceLikeBuilder owner,
@NotNull Collection<JetDeclarationContainer> forDeferredResolve
Expand Down Expand Up @@ -501,25 +497,11 @@ public void visitJetFile(JetFile file) {

@Override
public void visitClass(JetClass klass) {
if (getClassKind(klass) == ClassKind.ENUM_CLASS && inClass()) {
// Enums inside non-static context are put not to owner, but rather into its class object.
// This is handled after visiting all declarations in this class, because we may or may not
// encounter class object to put this enum into.
enumsToAddLater.add(klass);
return;
}

MutableClassDescriptor mutableClassDescriptor = createClassDescriptorForClass(klass, owner.getOwnerForChildren());

owner.addClassifierDescriptor(mutableClassDescriptor);
}

private boolean inClass() {
if (!(owner.getOwnerForChildren() instanceof ClassDescriptor)) return false;
ClassKind kind = ((ClassDescriptor) owner.getOwnerForChildren()).getKind();
return kind == ClassKind.CLASS || kind == ClassKind.ENUM_CLASS || kind == ClassKind.TRAIT;
}

@Override
public void visitObjectDeclaration(JetObjectDeclaration declaration) {
MutableClassDescriptor objectDescriptor =
Expand Down Expand Up @@ -673,50 +655,5 @@ private void prepareForDeferredCall(
context.normalScope.put(container, outerScope);
context.forDeferredResolver.put(container, descriptorForDeferredResolve);
}

@Nullable
private MutableClassDescriptor getOrCreateClassObjectDescriptor() {
ClassDescriptor ownerForChildren = (ClassDescriptor) owner.getOwnerForChildren();
MutableClassDescriptor classObjectDescriptor = (MutableClassDescriptor) ownerForChildren.getClassObjectDescriptor();

if (classObjectDescriptor == null) {
classObjectDescriptor = createClassObjectDescriptor(ownerForChildren, Visibilities.PUBLIC);
NamespaceLikeBuilder.ClassObjectStatus status = owner.setClassObjectDescriptor(classObjectDescriptor);
assert status != NamespaceLikeBuilder.ClassObjectStatus.DUPLICATE :
"Attempting to create an artificial class object where the real one exists: " + ownerForChildren;
if (status == NamespaceLikeBuilder.ClassObjectStatus.NOT_ALLOWED) {
return null;
}
}

return classObjectDescriptor;
}

private void putEnumsIntoOuterClassObject() {
if (enumsToAddLater.isEmpty()) return;

MutableClassDescriptor classObjectDescriptor = getOrCreateClassObjectDescriptor();
if (classObjectDescriptor == null) {
// A class object is not allowed in outer class, so report an error on every declared enum
for (JetClass klass : enumsToAddLater) {
JetModifierList modifierList = klass.getModifierList();
assert modifierList != null : "Enum class without modifier list: " + klass.getText();
ASTNode node = modifierList.getModifierNode(JetTokens.ENUM_KEYWORD);
assert node != null : "Enum class without enum modifier: " + klass.getText();
trace.report(ENUM_NOT_ALLOWED.on(node.getPsi()));
}
return;
}

for (JetClass klass : enumsToAddLater) {
MutableClassDescriptor mutableClassDescriptor = createClassDescriptorForClass(klass, classObjectDescriptor);
trace.record(BindingContext.IS_ENUM_MOVED_TO_CLASS_OBJECT, mutableClassDescriptor);
classObjectDescriptor.getBuilder().addClassifierDescriptor(mutableClassDescriptor);
}
}

private void finishProcessing() {
putEnumsIntoOuterClassObject();
}
}
}
4 changes: 2 additions & 2 deletions compiler/testData/diagnostics/tests/enum/inner/insideInnerClassNotAllowed.kt
View file
@@ -1,6 +1,6 @@
class A {
class B {
<!ENUM_NOT_ALLOWED!>enum<!> class E {
inner class B {
enum class <!NESTED_CLASS_NOT_ALLOWED!>E<!> {
ENTRY
}
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/testData/diagnostics/tests/enum/inner/twoEnumsInClassObjectAndInnerClass.kt
View file
Expand Up @@ -3,7 +3,7 @@ class A {
enum class E { ENTRY } // OK
}

class B {
<!ENUM_NOT_ALLOWED!>enum<!> class E { ENTRY }
inner class B {
enum class <!NESTED_CLASS_NOT_ALLOWED!>E<!> { ENTRY }
}
}
10 changes: 5 additions & 5 deletions compiler/testData/javaDescriptorResolver/staticMembers/Enum_non_static.txt
View file
Expand Up @@ -12,15 +12,15 @@ public final enum class Enum : jet.Enum<test.Enum> {
public final val C : test.Enum
public final fun valueOf(/*0*/ value : jet.String) : test.Enum
public final fun values() : jet.Array<test.Enum>

public open class Nested : java.lang.Object {
public constructor Nested()
public/*package*/ open fun foo() : Unit
}
}

public open inner class Inner : java.lang.Object {
public constructor Inner()
public/*package*/ open fun bar() : Unit
}

public open class Nested : java.lang.Object {
public constructor Nested()
public/*package*/ open fun foo() : Unit
}
}

0 comments on commit b612989

Please sign in to comment.