From 910ba57f8aca24a4c973b95933901cc92f88d675 Mon Sep 17 00:00:00 2001 From: Andrey Breslav Date: Tue, 13 Sep 2011 17:17:45 +0400 Subject: [PATCH] KT-303 Stack overflow on a cyclic class hierarchy --- .../descriptors/MutableClassDescriptor.java | 13 +- .../lang/resolve/ClassDescriptorResolver.java | 63 ++++---- .../lang/resolve/TypeHierarchyResolver.java | 138 +++++++++++++++++- .../jet/lang/types/TypeConstructorImpl.java | 9 +- idea/testData/checker/CyclicHierarchy.jet | 30 ++++ idea/testData/checker/regression/kt303.jet | 11 ++ 6 files changed, 222 insertions(+), 42 deletions(-) create mode 100644 idea/testData/checker/CyclicHierarchy.jet create mode 100644 idea/testData/checker/regression/kt303.jet diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/MutableClassDescriptor.java b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/MutableClassDescriptor.java index 008c53e20bb..6168cd1bb91 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/MutableClassDescriptor.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/MutableClassDescriptor.java @@ -125,9 +125,8 @@ public class MutableClassDescriptor extends MutableDeclarationDescriptor impleme public void addSupertype(@NotNull JetType supertype) { if (!ErrorUtils.isErrorType(supertype)) { - scopeForMemberLookup.importScope(supertype.getMemberScope()); + supertypes.add(supertype); } - supertypes.add(supertype); } public void setTypeParameterDescriptors(List typeParameters) { @@ -166,6 +165,12 @@ public class MutableClassDescriptor extends MutableDeclarationDescriptor impleme } } + public void addSupertypesToScopeForMemberLookup() { + for (JetType supertype : supertypes) { + scopeForMemberLookup.importScope(supertype.getMemberScope()); + } + } + @NotNull @Override public JetScope getMemberScope(List typeArguments) { @@ -278,4 +283,8 @@ public class MutableClassDescriptor extends MutableDeclarationDescriptor impleme public String toString() { return DescriptorRenderer.TEXT.render(this) + "[" + getClass().getCanonicalName() + "@" + System.identityHashCode(this) + "]"; } + + public Collection getSupertypes() { + return supertypes; + } } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/ClassDescriptorResolver.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/ClassDescriptorResolver.java index 2b0b2645e6a..c0c71b2eed3 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/ClassDescriptorResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/ClassDescriptorResolver.java @@ -66,28 +66,52 @@ public class ClassDescriptorResolver { public void resolveSupertypes(@NotNull JetClassOrObject jetClass, @NotNull MutableClassDescriptor descriptor) { List delegationSpecifiers = jetClass.getDelegationSpecifiers(); - JetType defaultSupertype = JetStandardClasses.getAnyType(); + if (delegationSpecifiers.isEmpty()) { + descriptor.addSupertype(getDefaultSupertype(jetClass)); + } + else { + Collection supertypes = resolveDelegationSpecifiers( + descriptor.getScopeForSupertypeResolution(), + delegationSpecifiers, + typeResolverNotCheckingBounds); + for (JetType supertype : supertypes) { + descriptor.addSupertype(supertype); + } + } + + } + + private JetType getDefaultSupertype(JetClassOrObject jetClass) { // TODO : beautify if (jetClass instanceof JetEnumEntry) { JetClassOrObject parent = PsiTreeUtil.getParentOfType(jetClass, JetClassOrObject.class); ClassDescriptor parentDescriptor = trace.getBindingContext().get(BindingContext.CLASS, parent); if (parentDescriptor.getTypeConstructor().getParameters().isEmpty()) { - defaultSupertype = parentDescriptor.getDefaultType(); + return parentDescriptor.getDefaultType(); } - else if (delegationSpecifiers.isEmpty()) { + else { trace.getErrorHandler().genericError(((JetEnumEntry) jetClass).getNameIdentifier().getNode(), "generic arguments of the base type must be specified"); + return ErrorUtils.createErrorType("Supertype not specified"); } } - Collection supertypes = delegationSpecifiers.isEmpty() - ? Collections.singleton(defaultSupertype) - : resolveDelegationSpecifiers( - descriptor.getScopeForSupertypeResolution(), - delegationSpecifiers, - typeResolverNotCheckingBounds); + return JetStandardClasses.getAnyType(); + } - for (JetType supertype : supertypes) { - descriptor.addSupertype(supertype); + public Collection resolveDelegationSpecifiers(JetScope extensibleScope, List delegationSpecifiers, @NotNull TypeResolver resolver) { + if (delegationSpecifiers.isEmpty()) { + return Collections.emptyList(); } + Collection result = Lists.newArrayList(); + for (JetDelegationSpecifier delegationSpecifier : delegationSpecifiers) { + JetTypeReference typeReference = delegationSpecifier.getTypeReference(); + if (typeReference != null) { + result.add(resolver.resolveType(extensibleScope, typeReference)); + } + else { + result.add(ErrorUtils.createErrorType("No type reference")); + } + } + return result; } @NotNull @@ -315,23 +339,6 @@ public class ClassDescriptorResolver { return jetType; } - public Collection resolveDelegationSpecifiers(JetScope extensibleScope, List delegationSpecifiers, @NotNull TypeResolver resolver) { - if (delegationSpecifiers.isEmpty()) { - return Collections.emptyList(); - } - Collection result = new ArrayList(); - for (JetDelegationSpecifier delegationSpecifier : delegationSpecifiers) { - JetTypeReference typeReference = delegationSpecifier.getTypeReference(); - if (typeReference != null) { - result.add(resolver.resolveType(extensibleScope, typeReference)); - } - else { - result.add(ErrorUtils.createErrorType("No type reference")); - } - } - return result; - } - @NotNull public VariableDescriptor resolveLocalVariableDescriptor(@NotNull DeclarationDescriptor containingDeclaration, @NotNull JetScope scope, @NotNull JetParameter parameter) { JetType type = resolveParameterType(scope, parameter); diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TypeHierarchyResolver.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TypeHierarchyResolver.java index 451fcf52346..700f8522c75 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TypeHierarchyResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TypeHierarchyResolver.java @@ -1,5 +1,10 @@ package org.jetbrains.jet.lang.resolve; +import com.google.common.collect.Lists; +import com.google.common.collect.Sets; +import com.intellij.lang.ASTNode; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiNameIdentifierOwner; import org.jetbrains.annotations.NotNull; import org.jetbrains.jet.lang.cfg.JetFlowInformationProvider; import org.jetbrains.jet.lang.descriptors.*; @@ -9,10 +14,10 @@ import org.jetbrains.jet.lang.types.JetType; import org.jetbrains.jet.lang.types.JetTypeInferrer; import org.jetbrains.jet.lexer.JetTokens; -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.Map; +import java.util.*; + +import static org.jetbrains.jet.lang.resolve.BindingContext.DESCRIPTOR_TO_DECLARATION; +import static org.jetbrains.jet.lang.resolve.BindingContext.TYPE; /** * @author abreslav @@ -27,8 +32,16 @@ public class TypeHierarchyResolver { public void process(@NotNull JetScope outerScope, NamespaceLike owner, @NotNull List declarations) { collectNamespacesAndClassifiers(outerScope, owner, declarations); // namespaceScopes, classes - createTypeConstructors(); // create type constructors for classes and generic parameters + createTypeConstructors(); // create type constructors for classes and generic parameters, supertypes are not filled in resolveTypesInClassHeaders(); // Generic bounds and types in supertype lists (no expressions or constructor resolution) + + // Detect and disconnect all loops in the hierarchy + detectAndDisconnectLoops(); + + // Add supertypes to resolution scopes of classes + addSupertypesToScopes(); + + checkTypesInClassHeaders(); // Generic bounds and supertype lists } @@ -256,6 +269,115 @@ public class TypeHierarchyResolver { } } + private void detectAndDisconnectLoops() { + // A topsort is needed only for better diagnostics: + // edges that get removed to disconnect loops are more reasonable in this case + LinkedList topologicalOrder = Lists.newLinkedList(); + Set visited = Sets.newHashSet(); + for (MutableClassDescriptor mutableClassDescriptor : context.getClasses().values()) { + topologicallySort(mutableClassDescriptor, visited, topologicalOrder); + } + + // Loop detection and disconnection + visited.clear(); + Set beingProcessed = Sets.newHashSet(); + List currentPath = Lists.newArrayList(); + for (MutableClassDescriptor mutableClassDescriptor : topologicalOrder) { + traverseTypeHierarchy(mutableClassDescriptor, visited, beingProcessed, currentPath); + } + } + + private static void topologicallySort(MutableClassDescriptor mutableClassDescriptor, Set visited, LinkedList topologicalOrder) { + if (!visited.add(mutableClassDescriptor)) { + return; + } + for (JetType supertype : mutableClassDescriptor.getSupertypes()) { + DeclarationDescriptor declarationDescriptor = supertype.getConstructor().getDeclarationDescriptor(); + if (declarationDescriptor instanceof MutableClassDescriptor) { + MutableClassDescriptor classDescriptor = (MutableClassDescriptor) declarationDescriptor; + topologicallySort(classDescriptor, visited, topologicalOrder); + } + } + topologicalOrder.addFirst(mutableClassDescriptor); + } + + private void traverseTypeHierarchy(MutableClassDescriptor currentClass, Set visited, Set beingProcessed, List currentPath) { + if (!visited.add(currentClass)) { + if (beingProcessed.contains(currentClass)) { + markCycleErrors(currentPath, currentClass); + assert !currentPath.isEmpty() : "Cycle cannot be found on an empty currentPath"; + ClassDescriptor subclassOfCurrent = currentPath.get(currentPath.size() - 1); + assert subclassOfCurrent instanceof MutableClassDescriptor; + // Disconnect the loop + for (Iterator iterator = ((MutableClassDescriptor) subclassOfCurrent).getSupertypes().iterator(); iterator.hasNext(); ) { + JetType type = iterator.next(); + if (type.getConstructor() == currentClass.getTypeConstructor()) { + iterator.remove(); + break; + } + } + } + return; + } + + beingProcessed.add(currentClass); + currentPath.add(currentClass); + for (JetType supertype : Lists.newArrayList(currentClass.getSupertypes())) { + DeclarationDescriptor declarationDescriptor = supertype.getConstructor().getDeclarationDescriptor(); + if (declarationDescriptor instanceof MutableClassDescriptor) { + MutableClassDescriptor mutableClassDescriptor = (MutableClassDescriptor) declarationDescriptor; + traverseTypeHierarchy(mutableClassDescriptor, visited, beingProcessed, currentPath); + } + } + beingProcessed.remove(currentClass); + currentPath.remove(currentPath.size() - 1); + } + + private void markCycleErrors(List currentPath, @NotNull ClassDescriptor current) { + int size = currentPath.size(); + boolean found = false; + for (int i = 0; i < size; i++) { + ClassDescriptor classDescriptor = currentPath.get(i); + if (classDescriptor == current) found = true; + if (!found) continue; + + ClassDescriptor superclass = (i < size - 1) ? currentPath.get(i + 1) : current; + PsiElement psiElement = context.getTrace().get(DESCRIPTOR_TO_DECLARATION, classDescriptor); + + ASTNode node = null; + if (psiElement instanceof JetClassOrObject) { + JetClassOrObject classOrObject = (JetClassOrObject) psiElement; + for (JetDelegationSpecifier delegationSpecifier : classOrObject.getDelegationSpecifiers()) { + JetTypeReference typeReference = delegationSpecifier.getTypeReference(); + if (typeReference == null) continue; + JetType supertype = context.getTrace().get(TYPE, typeReference); + if (supertype != null && supertype.getConstructor() == superclass.getTypeConstructor()) { + node = typeReference.getNode(); + } + } + } + if (node == null && psiElement instanceof PsiNameIdentifierOwner) { + PsiNameIdentifierOwner namedElement = (PsiNameIdentifierOwner) psiElement; + PsiElement nameIdentifier = namedElement.getNameIdentifier(); + if (nameIdentifier != null) { + node = nameIdentifier.getNode(); + } + } + if (node != null) { + context.getTrace().getErrorHandler().genericError(node, "There's a cycle in the inheritance hierarchy for this type"); + } + } + } + + private void addSupertypesToScopes() { + for (MutableClassDescriptor mutableClassDescriptor : context.getClasses().values()) { + mutableClassDescriptor.addSupertypesToScopeForMemberLookup(); + } + for (MutableClassDescriptor mutableClassDescriptor : context.getObjects().values()) { + mutableClassDescriptor.addSupertypesToScopeForMemberLookup(); + } + } + private void checkTypesInClassHeaders() { for (Map.Entry entry : context.getClasses().entrySet()) { JetClass jetClass = entry.getKey(); @@ -263,7 +385,7 @@ public class TypeHierarchyResolver { for (JetDelegationSpecifier delegationSpecifier : jetClass.getDelegationSpecifiers()) { JetTypeReference typeReference = delegationSpecifier.getTypeReference(); if (typeReference != null) { - JetType type = context.getTrace().getBindingContext().get(BindingContext.TYPE, typeReference); + JetType type = context.getTrace().getBindingContext().get(TYPE, typeReference); if (type != null) { context.getClassDescriptorResolver().checkBounds(typeReference, type); } @@ -273,7 +395,7 @@ public class TypeHierarchyResolver { for (JetTypeParameter jetTypeParameter : jetClass.getTypeParameters()) { JetTypeReference extendsBound = jetTypeParameter.getExtendsBound(); if (extendsBound != null) { - JetType type = context.getTrace().getBindingContext().get(BindingContext.TYPE, extendsBound); + JetType type = context.getTrace().getBindingContext().get(TYPE, extendsBound); if (type != null) { context.getClassDescriptorResolver().checkBounds(extendsBound, type); } @@ -283,7 +405,7 @@ public class TypeHierarchyResolver { for (JetTypeConstraint constraint : jetClass.getTypeConstaints()) { JetTypeReference extendsBound = constraint.getBoundTypeReference(); if (extendsBound != null) { - JetType type = context.getTrace().getBindingContext().get(BindingContext.TYPE, extendsBound); + JetType type = context.getTrace().getBindingContext().get(TYPE, extendsBound); if (type != null) { context.getClassDescriptorResolver().checkBounds(extendsBound, type); } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/types/TypeConstructorImpl.java b/compiler/frontend/src/org/jetbrains/jet/lang/types/TypeConstructorImpl.java index cb5167563de..ed11cc2e5d4 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/types/TypeConstructorImpl.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/types/TypeConstructorImpl.java @@ -2,13 +2,14 @@ package org.jetbrains.jet.lang.types; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.jetbrains.jet.lang.descriptors.annotations.AnnotatedImpl; -import org.jetbrains.jet.lang.descriptors.annotations.AnnotationDescriptor; import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor; import org.jetbrains.jet.lang.descriptors.TypeParameterDescriptor; +import org.jetbrains.jet.lang.descriptors.annotations.AnnotatedImpl; +import org.jetbrains.jet.lang.descriptors.annotations.AnnotationDescriptor; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; /** @@ -34,8 +35,8 @@ public class TypeConstructorImpl extends AnnotatedImpl implements TypeConstructo this.declarationDescriptor = declarationDescriptor; this.sealed = sealed; this.debugName = debugName; - this.parameters = new ArrayList(parameters); - this.supertypes = supertypes; + this.parameters = Collections.unmodifiableList(new ArrayList(parameters)); + this.supertypes = Collections.unmodifiableCollection(supertypes); } @Override diff --git a/idea/testData/checker/CyclicHierarchy.jet b/idea/testData/checker/CyclicHierarchy.jet new file mode 100644 index 00000000000..431db452077 --- /dev/null +++ b/idea/testData/checker/CyclicHierarchy.jet @@ -0,0 +1,30 @@ +open trait A { + fun foo() {} +} +open trait B : A, E {} +open trait C : B {} +open trait D : B {} +open trait E : F {} +open trait F : D, C {} +open trait G : F {} +open trait H : F {} + +val a : A? = null +val b : B? = null +val c : C? = null +val d : D? = null +val e : E? = null +val f : F? = null +val g : G? = null +val h : H? = null + +fun test() { + a?.foo() + b?.foo() + c?.foo() + d?.foo() + e?.foo() + f?.foo() + g?.foo() + h?.foo() +} \ No newline at end of file diff --git a/idea/testData/checker/regression/kt303.jet b/idea/testData/checker/regression/kt303.jet new file mode 100644 index 00000000000..e7b10295990 --- /dev/null +++ b/idea/testData/checker/regression/kt303.jet @@ -0,0 +1,11 @@ +// KT-303 Stack overflow on a cyclic class hierarchy + +open class Foo() : Bar() { + val a : Int = 1 +} + +open class Bar() : Foo() { + +} + +val x : Int = Foo() \ No newline at end of file