From 525ddf9eb03825efe492ed1225d50de3b55af0cc Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Thu, 12 Feb 2015 18:40:53 +0300 Subject: [PATCH] Prevent circular secondary constructors delegation --- .../jetbrains/kotlin/diagnostics/Errors.java | 4 ++ .../rendering/DefaultErrorMessages.java | 2 + .../kotlin/resolve/BodyResolver.java | 57 +++++++++++++++++++ .../cyclicDelegationCalls.kt | 39 +++++++++++++ .../cyclicDelegationCalls.txt | 45 +++++++++++++++ .../checkers/JetDiagnosticsTestGenerated.java | 6 ++ 6 files changed, 153 insertions(+) create mode 100644 compiler/testData/diagnostics/tests/secondaryConstructors/cyclicDelegationCalls.kt create mode 100644 compiler/testData/diagnostics/tests/secondaryConstructors/cyclicDelegationCalls.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index 152dc3eba27..ace75d397d5 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -153,6 +153,10 @@ public interface Errors { DiagnosticFactory0 NULLABLE_SUPERTYPE = DiagnosticFactory0.create(ERROR, NULLABLE_TYPE); DiagnosticFactory0 DYNAMIC_SUPERTYPE = DiagnosticFactory0.create(ERROR); + // Secondary constructors + + DiagnosticFactory0 CYCLIC_CONSTRUCTOR_DELEGATION_CALL = DiagnosticFactory0.create(ERROR); + // Trait-specific DiagnosticFactory0 ABSTRACT_MODIFIER_IN_TRAIT = DiagnosticFactory0 diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java index 3e9976615c0..9555d449272 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -413,6 +413,8 @@ public class DefaultErrorMessages { MAP.put(FINAL_SUPERTYPE, "This type is final, so it cannot be inherited from"); MAP.put(SINGLETON_IN_SUPERTYPE, "Cannot inherit from a singleton"); + MAP.put(CYCLIC_CONSTRUCTOR_DELEGATION_CALL, "There's a cycle in the delegation calls chain"); + MAP.put(ILLEGAL_SELECTOR, "Expression ''{0}'' cannot be a selector (occur after a dot)", STRING); MAP.put(NO_TAIL_CALLS_FOUND, "A function is marked as tail-recursive but no tail calls are found"); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/BodyResolver.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/BodyResolver.java index 8207fb1d078..debd47b93b9 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/BodyResolver.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/BodyResolver.java @@ -127,6 +127,11 @@ public class BodyResolver { for (Map.Entry entry : c.getSecondaryConstructors().entrySet()) { resolveSecondaryConstructorBody(c, entry.getKey(), entry.getValue()); } + if (c.getSecondaryConstructors().isEmpty()) return; + Set visitedConstructors = Sets.newHashSet(); + for (Map.Entry entry : c.getSecondaryConstructors().entrySet()) { + checkCyclicConstructorDelegationCall(entry.getValue(), visitedConstructors); + } } private void resolveSecondaryConstructorBody( @@ -174,6 +179,58 @@ public class BodyResolver { } } + private void checkCyclicConstructorDelegationCall( + @NotNull ConstructorDescriptor constructorDescriptor, + @NotNull Set visitedConstructors + ) { + if (visitedConstructors.contains(constructorDescriptor)) return; + + // if visit constructor that is already in current chain + // such constructor is on cycle + Set visitedInCurrentChain = Sets.newHashSet(); + ConstructorDescriptor currentConstructorDescriptor = constructorDescriptor; + while (true) { + visitedInCurrentChain.add(currentConstructorDescriptor); + ConstructorDescriptor delegatedConstructorDescriptor = getDelegatedConstructor(currentConstructorDescriptor); + if (delegatedConstructorDescriptor == null) break; + + // if next delegation call is super or primary constructor or already visited + if (!constructorDescriptor.getContainingDeclaration().equals(delegatedConstructorDescriptor.getContainingDeclaration()) || + delegatedConstructorDescriptor.isPrimary() || + visitedConstructors.contains(delegatedConstructorDescriptor)) { + break; + } + + if (visitedInCurrentChain.contains(delegatedConstructorDescriptor)) { + reportEachConstructorOnCycle(delegatedConstructorDescriptor); + break; + } + currentConstructorDescriptor = delegatedConstructorDescriptor; + } + visitedConstructors.addAll(visitedInCurrentChain); + } + + private void reportEachConstructorOnCycle(@NotNull ConstructorDescriptor startConstructor) { + ConstructorDescriptor currentConstructor = startConstructor; + do { + PsiElement constructorToReport = DescriptorToSourceUtils.descriptorToDeclaration(currentConstructor); + if (constructorToReport != null) { + JetConstructorDelegationCall call = ((JetSecondaryConstructor) constructorToReport).getDelegationCall(); + assert call != null : "resolved call can't be null"; + trace.report(CYCLIC_CONSTRUCTOR_DELEGATION_CALL.on(call)); + } + + currentConstructor = getDelegatedConstructor(currentConstructor); + assert currentConstructor != null : "Delegated constructor should not be null in cycle"; + } while (startConstructor != currentConstructor); + } + + @Nullable + private ConstructorDescriptor getDelegatedConstructor(@NotNull ConstructorDescriptor constructor) { + ResolvedCall call = trace.get(CONSTRUCTOR_RESOLVED_DELEGATION_CALL, constructor); + return call == null ? null : call.getResultingDescriptor(); + } + public void resolveBodies(@NotNull BodiesResolveContext c) { resolveBehaviorDeclarationBodies(c); controlFlowAnalyzer.process(c); diff --git a/compiler/testData/diagnostics/tests/secondaryConstructors/cyclicDelegationCalls.kt b/compiler/testData/diagnostics/tests/secondaryConstructors/cyclicDelegationCalls.kt new file mode 100644 index 00000000000..bdcd142cfe7 --- /dev/null +++ b/compiler/testData/diagnostics/tests/secondaryConstructors/cyclicDelegationCalls.kt @@ -0,0 +1,39 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER +class A1 { + constructor(): this() {} +} + +class A2(x: Byte) { + constructor(x1: Int): this(x1, 1) {} + constructor(x1: Int, x2: Int): this(x1, x2, 2) {} + constructor(x1: Int, x2: Int, x3: Int): this(x1) {} + + // delegating to previously declared cycle + constructor(x1: Double): this(1) {} + + + // delegating to cycle declared after + constructor(x1: String): this(1L) {} + + constructor(x1: Long): this(x1, 1L) {} + constructor(x1: Long, x2: Long): this(x1, x2, 2L) {} + constructor(x1: Long, x2: Long, x3: Long): this(x1) {} + + // no cycle, just call to primary constuctor + constructor(x1: Double, x2: Double): this(x1, x2, 1.0) {} + constructor(x1: Double, x2: Double, x3: Double): this(x1, x2, x3, 1.0) {} + constructor(x1: Double, x2: Double, x3: Double, x4: Double): this(1.toByte()) {} + + constructor(): this("x", "y") {} + + constructor(x1: String, x2: String): this(x1, x2, "") {} + constructor(x1: String, x2: String, x3: String): this(x1, x2) {} +} + +open class B(x: Byte) +class A : B { + // no cycle, just call to super constuctor + constructor(x1: Double, x2: Double): this(x1, x2, 1.0) {} + constructor(x1: Double, x2: Double, x3: Double): this(x1, x2, x3, 1.0) {} + constructor(x1: Double, x2: Double, x3: Double, x4: Double): super(1.toByte()) {} +} diff --git a/compiler/testData/diagnostics/tests/secondaryConstructors/cyclicDelegationCalls.txt b/compiler/testData/diagnostics/tests/secondaryConstructors/cyclicDelegationCalls.txt new file mode 100644 index 00000000000..16cc50d8cd1 --- /dev/null +++ b/compiler/testData/diagnostics/tests/secondaryConstructors/cyclicDelegationCalls.txt @@ -0,0 +1,45 @@ +package + +internal final class A : B { + public constructor A(/*0*/ x1: kotlin.Double, /*1*/ x2: kotlin.Double) + public constructor A(/*0*/ x1: kotlin.Double, /*1*/ x2: kotlin.Double, /*2*/ x3: kotlin.Double) + public constructor A(/*0*/ x1: kotlin.Double, /*1*/ x2: kotlin.Double, /*2*/ x3: kotlin.Double, /*3*/ x4: kotlin.Double) + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +internal final class A1 { + public constructor A1() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +internal final class A2 { + public constructor A2() + public constructor A2(/*0*/ x: kotlin.Byte) + public constructor A2(/*0*/ x1: kotlin.Double) + public constructor A2(/*0*/ x1: kotlin.Double, /*1*/ x2: kotlin.Double) + public constructor A2(/*0*/ x1: kotlin.Double, /*1*/ x2: kotlin.Double, /*2*/ x3: kotlin.Double) + public constructor A2(/*0*/ x1: kotlin.Double, /*1*/ x2: kotlin.Double, /*2*/ x3: kotlin.Double, /*3*/ x4: kotlin.Double) + public constructor A2(/*0*/ x1: kotlin.Int) + public constructor A2(/*0*/ x1: kotlin.Int, /*1*/ x2: kotlin.Int) + public constructor A2(/*0*/ x1: kotlin.Int, /*1*/ x2: kotlin.Int, /*2*/ x3: kotlin.Int) + public constructor A2(/*0*/ x1: kotlin.Long) + public constructor A2(/*0*/ x1: kotlin.Long, /*1*/ x2: kotlin.Long) + public constructor A2(/*0*/ x1: kotlin.Long, /*1*/ x2: kotlin.Long, /*2*/ x3: kotlin.Long) + public constructor A2(/*0*/ x1: kotlin.String) + public constructor A2(/*0*/ x1: kotlin.String, /*1*/ x2: kotlin.String) + public constructor A2(/*0*/ x1: kotlin.String, /*1*/ x2: kotlin.String, /*2*/ x3: kotlin.String) + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +internal open class B { + public constructor B(/*0*/ x: kotlin.Byte) + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java index 1789a198b98..ede8a982d8c 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java @@ -10415,6 +10415,12 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest { doTest(fileName); } + @TestMetadata("cyclicDelegationCalls.kt") + public void testCyclicDelegationCalls() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/secondaryConstructors/cyclicDelegationCalls.kt"); + doTest(fileName); + } + @TestMetadata("generics.kt") public void testGenerics() throws Exception { String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/secondaryConstructors/generics.kt");