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 529ffd0db97..00a393e0e01 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TypeHierarchyResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TypeHierarchyResolver.java @@ -33,6 +33,7 @@ import org.jetbrains.jet.lang.resolve.scopes.JetScope; import org.jetbrains.jet.lang.resolve.scopes.WritableScope; import org.jetbrains.jet.lang.resolve.scopes.WriteThroughScope; import org.jetbrains.jet.lang.types.JetType; +import org.jetbrains.jet.lang.types.TypeConstructor; import org.jetbrains.jet.lang.types.lang.KotlinBuiltIns; import org.jetbrains.jet.utils.DFS; @@ -247,88 +248,75 @@ public class TypeHierarchyResolver { private void detectAndDisconnectLoops(@NotNull TopDownAnalysisContext c) { // Loop detection and disconnection - Set visited = Sets.newHashSet(); - Set beingProcessed = Sets.newHashSet(); - List currentPath = Lists.newArrayList(); - for (MutableClassDescriptorLite klass : c.getClassesTopologicalOrder()) { - traverseTypeHierarchy(klass, visited, beingProcessed, currentPath); + List tasks = new ArrayList(); + for (final MutableClassDescriptorLite klass : c.getClassesTopologicalOrder()) { + for (final JetType supertype : klass.getSupertypes()) { + ClassifierDescriptor supertypeDescriptor = supertype.getConstructor().getDeclarationDescriptor(); + if (supertypeDescriptor instanceof MutableClassDescriptorLite) { + MutableClassDescriptorLite superclass = (MutableClassDescriptorLite) supertypeDescriptor; + if (isReachable(superclass, klass, new HashSet())) { + tasks.add(new Runnable() { + @Override + public void run() { + klass.getSupertypes().remove(supertype); + } + }); + reportCyclicInheritanceHierarchyError(trace, klass, superclass); + } + } + } + } + + for (Runnable task : tasks) { + task.run(); } } - private void traverseTypeHierarchy( - MutableClassDescriptorLite currentClass, - Set visited, - Set beingProcessed, - List currentPath + // Temporary. Duplicates logic from LazyClassTypeConstructor.isReachable + private static boolean isReachable(MutableClassDescriptorLite from, MutableClassDescriptorLite to, Set visited) { + if (!visited.add(from)) return false; + for (JetType supertype : from.getSupertypes()) { + TypeConstructor supertypeConstructor = supertype.getConstructor(); + if (supertypeConstructor.getDeclarationDescriptor() == to) { + return true; + } + ClassifierDescriptor superclass = supertypeConstructor.getDeclarationDescriptor(); + if (superclass instanceof MutableClassDescriptorLite && isReachable((MutableClassDescriptorLite) superclass, to, visited)) { + return true; + } + } + return false; + } + + public static void reportCyclicInheritanceHierarchyError( + @NotNull BindingTrace trace, + @NotNull ClassDescriptor classDescriptor, + @NotNull ClassDescriptor superclass ) { - 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; - } + PsiElement psiElement = BindingContextUtils.classDescriptorToDeclaration(trace.getBindingContext(), classDescriptor); - 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); + PsiElement elementToMark = null; + if (psiElement instanceof JetClassOrObject) { + JetClassOrObject classOrObject = (JetClassOrObject) psiElement; + for (JetDelegationSpecifier delegationSpecifier : classOrObject.getDelegationSpecifiers()) { + JetTypeReference typeReference = delegationSpecifier.getTypeReference(); + if (typeReference == null) continue; + JetType supertype = trace.get(TYPE, typeReference); + if (supertype != null && supertype.getConstructor() == superclass.getTypeConstructor()) { + elementToMark = typeReference; + } } } - beingProcessed.remove(currentClass); - currentPath.remove(currentPath.size() - 1); - } - - private void markCycleErrors(List currentPath, @NotNull ClassDescriptor current) { - int size = currentPath.size(); - for (int i = size - 1; i >= 0; i--) { - ClassDescriptor classDescriptor = currentPath.get(i); - - ClassDescriptor superclass = (i < size - 1) ? currentPath.get(i + 1) : current; - PsiElement psiElement = BindingContextUtils.classDescriptorToDeclaration(trace.getBindingContext(), classDescriptor); - - PsiElement elementToMark = null; - if (psiElement instanceof JetClassOrObject) { - JetClassOrObject classOrObject = (JetClassOrObject) psiElement; - for (JetDelegationSpecifier delegationSpecifier : classOrObject.getDelegationSpecifiers()) { - JetTypeReference typeReference = delegationSpecifier.getTypeReference(); - if (typeReference == null) continue; - JetType supertype = trace.get(TYPE, typeReference); - if (supertype != null && supertype.getConstructor() == superclass.getTypeConstructor()) { - elementToMark = typeReference; - } - } - } - if (elementToMark == null && psiElement instanceof PsiNameIdentifierOwner) { - PsiNameIdentifierOwner namedElement = (PsiNameIdentifierOwner) psiElement; - PsiElement nameIdentifier = namedElement.getNameIdentifier(); - if (nameIdentifier != null) { - elementToMark = nameIdentifier; - } - } - if (elementToMark != null) { - trace.report(CYCLIC_INHERITANCE_HIERARCHY.on(elementToMark)); - } - - if (classDescriptor == current) { - // Beginning of cycle is found - break; + if (elementToMark == null && psiElement instanceof PsiNameIdentifierOwner) { + PsiNameIdentifierOwner namedElement = (PsiNameIdentifierOwner) psiElement; + PsiElement nameIdentifier = namedElement.getNameIdentifier(); + if (nameIdentifier != null) { + elementToMark = nameIdentifier; } } + if (elementToMark != null) { + trace.report(CYCLIC_INHERITANCE_HIERARCHY.on(elementToMark)); + } } private void checkTypesInClassHeaders(@NotNull TopDownAnalysisContext c) { diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/lazy/descriptors/LazyClassDescriptor.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/lazy/descriptors/LazyClassDescriptor.java index 4728ca3ae87..9def0f7508f 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/lazy/descriptors/LazyClassDescriptor.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/lazy/descriptors/LazyClassDescriptor.java @@ -24,6 +24,7 @@ import kotlin.Function0; import kotlin.Function1; import kotlin.Unit; import kotlin.KotlinPackage; +import org.jetbrains.annotations.Mutable; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.lang.descriptors.*; @@ -33,6 +34,7 @@ import org.jetbrains.jet.lang.psi.*; import org.jetbrains.jet.lang.resolve.AnnotationResolver; import org.jetbrains.jet.lang.resolve.BindingContext; import org.jetbrains.jet.lang.resolve.DescriptorUtils; +import org.jetbrains.jet.lang.resolve.TypeHierarchyResolver; import org.jetbrains.jet.lang.resolve.lazy.ForceResolveUtil; import org.jetbrains.jet.lang.resolve.lazy.LazyEntity; import org.jetbrains.jet.lang.resolve.lazy.ResolveSession; @@ -383,44 +385,65 @@ public class LazyClassDescriptor extends ClassDescriptorBase implements LazyEnti getVisibility(); } + private static class Supertypes { + @Mutable + public final Collection trueSupertypes; + @Mutable + public final Collection cyclicSupertypes; + + private Supertypes(@Mutable @NotNull Collection trueSupertypes) { + this(trueSupertypes, new ArrayList(0)); + } + + private Supertypes(@Mutable @NotNull Collection trueSupertypes, @Mutable @NotNull Collection cyclicSupertypes) { + this.trueSupertypes = trueSupertypes; + this.cyclicSupertypes = cyclicSupertypes; + } + + @NotNull + public Collection getAllSupertypes() { + return KotlinPackage.plus(trueSupertypes, cyclicSupertypes); + } + } + private class LazyClassTypeConstructor implements LazyEntity, TypeConstructor { - private final NotNullLazyValue> supertypes = resolveSession.getStorageManager().createLazyValueWithPostCompute( - new Function0>() { + private final NotNullLazyValue supertypes = resolveSession.getStorageManager().createLazyValueWithPostCompute( + new Function0() { @Override - public Collection invoke() { + public Supertypes invoke() { if (KotlinBuiltIns.isSpecialClassWithNoSupertypes(LazyClassDescriptor.this)) { - return Collections.emptyList(); + return new Supertypes(Collections.emptyList()); } JetClassLikeInfo info = declarationProvider.getOwnerInfo(); if (info instanceof SyntheticClassObjectInfo) { LazyClassDescriptor descriptor = ((SyntheticClassObjectInfo) info).getClassDescriptor(); if (descriptor.getKind().isSingleton()) { - return Collections.singleton(descriptor.getDefaultType()); + return new Supertypes(Collections.singleton(descriptor.getDefaultType())); } } JetClassOrObject classOrObject = info.getCorrespondingClassOrObject(); if (classOrObject == null) { - return Collections.singleton(KotlinBuiltIns.getInstance().getAnyType()); + return new Supertypes(Collections.singleton(KotlinBuiltIns.getInstance().getAnyType())); } List allSupertypes = resolveSession.getDescriptorResolver() .resolveSupertypes(getScopeForClassHeaderResolution(), LazyClassDescriptor.this, classOrObject, resolveSession.getTrace()); - return Lists.newArrayList(Collections2.filter(allSupertypes, VALID_SUPERTYPE)); + return new Supertypes(Lists.newArrayList(Collections2.filter(allSupertypes, VALID_SUPERTYPE))); } }, - new Function1>() { + new Function1() { @Override - public Collection invoke(Boolean firstTime) { - return Collections.emptyList(); + public Supertypes invoke(Boolean firstTime) { + return new Supertypes(Collections.emptyList()); } }, - new Function1, Unit>() { + new Function1() { @Override - public Unit invoke(@NotNull Collection supertypes) { + public Unit invoke(@NotNull Supertypes supertypes) { findAndDisconnectLoopsInTypeHierarchy(supertypes); return Unit.VALUE; } @@ -460,21 +483,32 @@ public class LazyClassDescriptor extends ClassDescriptorBase implements LazyEnti @NotNull @Override public Collection getSupertypes() { - return supertypes.invoke(); + return supertypes.invoke().trueSupertypes; } - private void findAndDisconnectLoopsInTypeHierarchy(Collection supertypes) { - for (Iterator iterator = supertypes.iterator(); iterator.hasNext(); ) { + private void findAndDisconnectLoopsInTypeHierarchy(Supertypes supertypes) { + for (Iterator iterator = supertypes.trueSupertypes.iterator(); iterator.hasNext(); ) { JetType supertype = iterator.next(); if (isReachable(supertype.getConstructor(), this, new HashSet())) { iterator.remove(); + supertypes.cyclicSupertypes.add(supertype); + + ClassifierDescriptor supertypeDescriptor = supertype.getConstructor().getDeclarationDescriptor(); + if (supertypeDescriptor instanceof ClassDescriptor) { + ClassDescriptor superclass = (ClassDescriptor) supertypeDescriptor; + TypeHierarchyResolver.reportCyclicInheritanceHierarchyError(resolveSession.getTrace(), LazyClassDescriptor.this, + superclass); + } } } } private boolean isReachable(TypeConstructor from, TypeConstructor to, Set visited) { if (!visited.add(from)) return false; - for (JetType supertype : from.getSupertypes()) { + Collection supertypes = from instanceof LazyClassTypeConstructor + ? ((LazyClassTypeConstructor) from).supertypes.invoke().getAllSupertypes() + : from.getSupertypes(); + for (JetType supertype : supertypes) { TypeConstructor supertypeConstructor = supertype.getConstructor(); if (supertypeConstructor == to) { return true; diff --git a/compiler/testData/diagnostics/tests/CyclicHierarchy.kt b/compiler/testData/diagnostics/tests/CyclicHierarchy.kt index c3a1650653b..318d67fd867 100644 --- a/compiler/testData/diagnostics/tests/CyclicHierarchy.kt +++ b/compiler/testData/diagnostics/tests/CyclicHierarchy.kt @@ -2,10 +2,10 @@ trait A { fun foo() {} } trait B : A, E {} -trait C : B {} +trait C : B {} trait D : B {} trait E : F {} -trait F : D, C {} +trait F : D, C {} trait G : F {} trait H : F {} @@ -21,10 +21,10 @@ val h : H? = null fun test() { a?.foo() b?.foo() - c?.foo() - d?.foo() + c?.foo() + d?.foo() e?.foo() - f?.foo() - g?.foo() - h?.foo() -} + f?.foo() + g?.foo() + h?.foo() +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/cast/IsRecursionSustainable.kt b/compiler/testData/diagnostics/tests/cast/IsRecursionSustainable.kt index f194a077389..73faa4af355 100644 --- a/compiler/testData/diagnostics/tests/cast/IsRecursionSustainable.kt +++ b/compiler/testData/diagnostics/tests/cast/IsRecursionSustainable.kt @@ -3,4 +3,4 @@ open class RecB: RecA() open class SelfR: SelfR() fun test(f: SelfR) = f is RecA -fun test(f: RecB) = f is RecA \ No newline at end of file +fun test(f: RecB) = f is RecA \ No newline at end of file diff --git a/compiler/testData/lazyResolve/recursiveComparator/CyclicHierarchy.kt b/compiler/testData/lazyResolve/recursiveComparator/CyclicHierarchy.kt new file mode 100644 index 00000000000..9a060d9a1e7 --- /dev/null +++ b/compiler/testData/lazyResolve/recursiveComparator/CyclicHierarchy.kt @@ -0,0 +1,10 @@ +package test + +trait A +trait B : A, E +trait C : B +trait D : B +trait E : F +trait F : D, C +trait G : F +trait H : F \ No newline at end of file diff --git a/compiler/testData/lazyResolve/recursiveComparator/CyclicHierarchy.txt b/compiler/testData/lazyResolve/recursiveComparator/CyclicHierarchy.txt new file mode 100644 index 00000000000..2b5136f8c42 --- /dev/null +++ b/compiler/testData/lazyResolve/recursiveComparator/CyclicHierarchy.txt @@ -0,0 +1,25 @@ +package test + +internal trait A { +} + +internal trait B : test.A { +} + +internal trait C { +} + +internal trait D { +} + +internal trait E { +} + +internal trait F { +} + +internal trait G : test.F { +} + +internal trait H : test.F { +} diff --git a/compiler/tests/org/jetbrains/jet/lang/resolve/lazy/LazyResolveRecursiveComparingTestGenerated.java b/compiler/tests/org/jetbrains/jet/lang/resolve/lazy/LazyResolveRecursiveComparingTestGenerated.java index 4adb3d4aae6..ee301bf5481 100644 --- a/compiler/tests/org/jetbrains/jet/lang/resolve/lazy/LazyResolveRecursiveComparingTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/lang/resolve/lazy/LazyResolveRecursiveComparingTestGenerated.java @@ -2515,6 +2515,11 @@ public class LazyResolveRecursiveComparingTestGenerated extends AbstractLazyReso doTestCheckingPrimaryConstructors("compiler/testData/lazyResolve/recursiveComparator/classObjectHeader.kt"); } + @TestMetadata("CyclicHierarchy.kt") + public void testCyclicHierarchy() throws Exception { + doTestCheckingPrimaryConstructors("compiler/testData/lazyResolve/recursiveComparator/CyclicHierarchy.kt"); + } + @TestMetadata("enum.kt") public void testEnum() throws Exception { doTestCheckingPrimaryConstructors("compiler/testData/lazyResolve/recursiveComparator/enum.kt"); diff --git a/idea/testData/checker/CyclicHierarchy.kt b/idea/testData/checker/CyclicHierarchy.kt index f63abefd2be..24c7565d5ec 100644 --- a/idea/testData/checker/CyclicHierarchy.kt +++ b/idea/testData/checker/CyclicHierarchy.kt @@ -2,10 +2,10 @@ trait A { fun foo() {} } trait B : A, E {} -trait C : B {} +trait C : B {} trait D : B {} trait E : F {} -trait F : D, C {} +trait F : D, C {} trait G : F {} trait H : F {} @@ -21,10 +21,10 @@ val h : H? = null fun test() { a?.foo() b?.foo() - c?.foo() - d?.foo() + c?.foo() + d?.foo() e?.foo() - f?.foo() - g?.foo() - h?.foo() + f?.foo() + g?.foo() + h?.foo() } \ No newline at end of file