From cbe6e49a93e2d616befad79a399c234b3fb34d28 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Wed, 2 Apr 2014 22:51:26 +0400 Subject: [PATCH] Consider outer classes when topsorting hierarchy - when building a topsorted hierarchy, consider not only edges to the class' superclasses, but also from inner/nested classes to their container. This effectively means that a class cannot inherit from its nested class. Similar cases (see the tests) are disallowed in Java. - filter classes by their presence in TopDownAnalysisContext, not by being a MutableClassDescriptor (otherwise analysis for local classes ends up trying to do something with outer superclasses) - delete outdated comment: topsort is needed not only for better diagnostics, but also for correct order of fake overrides construction --- .../lang/resolve/TypeHierarchyResolver.java | 61 ++++++++----------- .../classIndirectlyInheritsNested.kt | 4 ++ .../cyclicHierarchy/classInheritsNested.kt | 3 + .../cyclicHierarchy/kotlinJavaNestedCycle.kt | 21 +++++++ .../twoClassesWithNestedCycle.kt | 6 ++ .../checkers/JetDiagnosticsTestGenerated.java | 20 ++++++ 6 files changed, 81 insertions(+), 34 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/cyclicHierarchy/classIndirectlyInheritsNested.kt create mode 100644 compiler/testData/diagnostics/tests/cyclicHierarchy/classInheritsNested.kt create mode 100644 compiler/testData/diagnostics/tests/cyclicHierarchy/kotlinJavaNestedCycle.kt create mode 100644 compiler/testData/diagnostics/tests/cyclicHierarchy/twoClassesWithNestedCycle.kt 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 6428b7360b0..e108b5e514a 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TypeHierarchyResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TypeHierarchyResolver.java @@ -16,10 +16,8 @@ package org.jetbrains.jet.lang.resolve; -import com.google.common.collect.Lists; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiNameIdentifierOwner; -import kotlin.KotlinPackage; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.lang.descriptors.*; @@ -32,7 +30,6 @@ 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; @@ -49,6 +46,25 @@ import static org.jetbrains.jet.lang.resolve.ModifiersChecker.resolveVisibilityF import static org.jetbrains.jet.lang.resolve.name.SpecialNames.getClassObjectName; public class TypeHierarchyResolver { + private static final DFS.Neighbors CLASS_INHERITANCE_EDGES = new DFS.Neighbors() { + @NotNull + @Override + public Iterable getNeighbors(ClassDescriptor current) { + List result = new ArrayList(); + for (JetType supertype : current.getDefaultType().getConstructor().getSupertypes()) { + DeclarationDescriptor descriptor = supertype.getConstructor().getDeclarationDescriptor(); + if (descriptor instanceof ClassDescriptor) { + result.add((ClassDescriptor) descriptor); + } + } + DeclarationDescriptor container = current.getContainingDeclaration(); + if (container instanceof ClassDescriptor) { + result.add((ClassDescriptor) container); + } + return result; + } + }; + @NotNull private ImportsResolver importsResolver; @NotNull @@ -220,28 +236,12 @@ public class TypeHierarchyResolver { } @NotNull + @SuppressWarnings("unchecked") private static List topologicallySortClassesAndObjects(@NotNull TopDownAnalysisContext c) { - // A topsort is needed only for better diagnostics: - // edges that get removed to disconnect loops are more reasonable in this case - //noinspection unchecked - List orderedClasses = DFS.topologicalOrder( - (Iterable) c.getAllClasses(), - new DFS.Neighbors() { - @NotNull - @Override - public Iterable getNeighbors(ClassDescriptor current) { - List result = Lists.newArrayList(); - for (JetType supertype : current.getDefaultType().getConstructor().getSupertypes()) { - DeclarationDescriptor descriptor = supertype.getConstructor().getDeclarationDescriptor(); - if (descriptor instanceof ClassDescriptor) { - result.add((ClassDescriptor) descriptor); - } - } - return result; - } - } - ); - return KotlinPackage.filterIsInstance(orderedClasses, MutableClassDescriptor.class); + Collection sourceClasses = (Collection) c.getAllClasses(); + List allClassesOrdered = DFS.topologicalOrder(sourceClasses, CLASS_INHERITANCE_EDGES); + allClassesOrdered.retainAll(sourceClasses); + return (List) allClassesOrdered; } private void detectAndDisconnectLoops(@NotNull TopDownAnalysisContext c) { @@ -269,22 +269,15 @@ public class TypeHierarchyResolver { } } - // Temporary. Duplicates logic from LazyClassTypeConstructor.isReachable + // TODO: use DFS and copy to LazyClassTypeConstructor.isReachable private static boolean isReachable( @NotNull ClassDescriptor from, @NotNull MutableClassDescriptor to, @NotNull Set visited ) { if (!visited.add(from)) return false; - for (JetType supertype : from.getDefaultType().getConstructor().getSupertypes()) { - TypeConstructor supertypeConstructor = supertype.getConstructor(); - if (supertypeConstructor.getDeclarationDescriptor() == to) { - return true; - } - ClassifierDescriptor superclass = supertypeConstructor.getDeclarationDescriptor(); - if (superclass instanceof ClassDescriptor && isReachable((ClassDescriptor) superclass, to, visited)) { - return true; - } + for (ClassDescriptor superclass : CLASS_INHERITANCE_EDGES.getNeighbors(from)) { + if (superclass == to || isReachable(superclass, to, visited)) return true; } return false; } diff --git a/compiler/testData/diagnostics/tests/cyclicHierarchy/classIndirectlyInheritsNested.kt b/compiler/testData/diagnostics/tests/cyclicHierarchy/classIndirectlyInheritsNested.kt new file mode 100644 index 00000000000..695b17efc6f --- /dev/null +++ b/compiler/testData/diagnostics/tests/cyclicHierarchy/classIndirectlyInheritsNested.kt @@ -0,0 +1,4 @@ +open class C : D() { + open class CC +} +open class D : C.CC() diff --git a/compiler/testData/diagnostics/tests/cyclicHierarchy/classInheritsNested.kt b/compiler/testData/diagnostics/tests/cyclicHierarchy/classInheritsNested.kt new file mode 100644 index 00000000000..45cd12d9f85 --- /dev/null +++ b/compiler/testData/diagnostics/tests/cyclicHierarchy/classInheritsNested.kt @@ -0,0 +1,3 @@ +open class E : E.EE() { + open class EE +} diff --git a/compiler/testData/diagnostics/tests/cyclicHierarchy/kotlinJavaNestedCycle.kt b/compiler/testData/diagnostics/tests/cyclicHierarchy/kotlinJavaNestedCycle.kt new file mode 100644 index 00000000000..0d587c7913e --- /dev/null +++ b/compiler/testData/diagnostics/tests/cyclicHierarchy/kotlinJavaNestedCycle.kt @@ -0,0 +1,21 @@ +// FILE: ExceptionTracker.kt + +trait ExceptionTracker : LockBasedStorageManager.ExceptionHandlingStrategy { +} + +// FILE: StorageManager.kt + +trait StorageManager : ExceptionTracker { + fun foo() +} + +// FILE: LockBasedStorageManager.java + +class LockBasedStorageManager extends StorageManager { + interface ExceptionHandlingStrategy { + void bar(); + } + + @Override + void foo() {} +} diff --git a/compiler/testData/diagnostics/tests/cyclicHierarchy/twoClassesWithNestedCycle.kt b/compiler/testData/diagnostics/tests/cyclicHierarchy/twoClassesWithNestedCycle.kt new file mode 100644 index 00000000000..0d07585e2c4 --- /dev/null +++ b/compiler/testData/diagnostics/tests/cyclicHierarchy/twoClassesWithNestedCycle.kt @@ -0,0 +1,6 @@ +open class A : B.BB() { + open class AA +} +open class B : A.AA() { + open class BB +} diff --git a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java index 608fc8248bc..056c34b859f 100644 --- a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java @@ -1823,6 +1823,16 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest { JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.TestsPackage", new File("compiler/testData/diagnostics/tests/cyclicHierarchy"), Pattern.compile("^(.+)\\.kt$"), true); } + @TestMetadata("classIndirectlyInheritsNested.kt") + public void testClassIndirectlyInheritsNested() throws Exception { + doTest("compiler/testData/diagnostics/tests/cyclicHierarchy/classIndirectlyInheritsNested.kt"); + } + + @TestMetadata("classInheritsNested.kt") + public void testClassInheritsNested() throws Exception { + doTest("compiler/testData/diagnostics/tests/cyclicHierarchy/classInheritsNested.kt"); + } + @TestMetadata("cyclicHierarchy.kt") public void testCyclicHierarchy() throws Exception { doTest("compiler/testData/diagnostics/tests/cyclicHierarchy/cyclicHierarchy.kt"); @@ -1843,11 +1853,21 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest { doTest("compiler/testData/diagnostics/tests/cyclicHierarchy/kotlinJavaKotlinCycle.kt"); } + @TestMetadata("kotlinJavaNestedCycle.kt") + public void testKotlinJavaNestedCycle() throws Exception { + doTest("compiler/testData/diagnostics/tests/cyclicHierarchy/kotlinJavaNestedCycle.kt"); + } + @TestMetadata("kt303.kt") public void testKt303() throws Exception { doTest("compiler/testData/diagnostics/tests/cyclicHierarchy/kt303.kt"); } + @TestMetadata("twoClassesWithNestedCycle.kt") + public void testTwoClassesWithNestedCycle() throws Exception { + doTest("compiler/testData/diagnostics/tests/cyclicHierarchy/twoClassesWithNestedCycle.kt"); + } + } @TestMetadata("compiler/testData/diagnostics/tests/dataClasses")