From 74c2cb7820c6ec8fc1547d0aec5686c8c710d403 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 8 May 2015 17:14:57 +0000 Subject: [PATCH] Sema: more robust diagnostic for circular inheritance The code would generate different diagnosics depending on the cycle having length 1 or longer. The length 1 case was broken if the path had a prefix that wasn't part of the cycle, eg if we have C : A, A : A and visit C first. Swift SVN r28317 --- lib/Sema/TypeCheckDecl.cpp | 27 ++++++++++--------- test/decl/class/circular_inheritance.swift | 3 +++ test/decl/protocol/protocols.swift | 3 +++ .../0394-void.swift | 2 +- .../0876-x.swift | 2 +- .../0896-void.swift | 2 +- .../1973-void.swift | 2 +- .../2206-void.swift | 2 +- 8 files changed, 25 insertions(+), 18 deletions(-) rename validation-test/{compiler_crashers => compiler_crashers_fixed}/0394-void.swift (79%) rename validation-test/{compiler_crashers => compiler_crashers_fixed}/0876-x.swift (86%) rename validation-test/{compiler_crashers => compiler_crashers_fixed}/0896-void.swift (80%) rename validation-test/{compiler_crashers => compiler_crashers_fixed}/1973-void.swift (87%) rename validation-test/{compiler_crashers => compiler_crashers_fixed}/2206-void.swift (85%) diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index fd3bd224573d8..8397855d2a566 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -616,26 +616,27 @@ static void checkCircularity(TypeChecker &tc, T *decl, case CircularityCheck::Checking: { // We're already checking this protocol, which means we have a cycle. - - // The type directly references itself. - if (path.size() == 1) { - tc.diagnose((*path.begin())->getLoc(), + + // The beginning of the path might not be part of the cycle, so find + // where the cycle starts. + auto cycleStart = path.end() - 1; + while (*cycleStart != decl) { + assert(cycleStart != path.begin() && "Missing cycle start?"); + --cycleStart; + } + + // If the path length is 1 the type directly references itself. + if (path.end() - cycleStart == 1) { + tc.diagnose(path.back()->getLoc(), circularDiag, - (*path.begin())->getName().str()); - + path.back()->getName().str()); + decl->setInvalid(); decl->overwriteType(ErrorType::get(tc.Context)); breakInheritanceCycle(decl); break; } - // Find the beginning of the cycle within the full path. - auto cycleStart = path.end()-2; - while (*cycleStart != decl) { - assert(cycleStart != path.begin() && "Missing cycle start?"); - --cycleStart; - } - // Form the textual path illustrating the cycle. llvm::SmallString<128> pathStr; for (auto i = cycleStart, iEnd = path.end(); i != iEnd; ++i) { diff --git a/test/decl/class/circular_inheritance.swift b/test/decl/class/circular_inheritance.swift index 614724b13ecaf..8ad0a23333f62 100644 --- a/test/decl/class/circular_inheritance.swift +++ b/test/decl/class/circular_inheritance.swift @@ -6,3 +6,6 @@ class A : C { } // expected-note{{class 'A' declared here}} class TrivialCycle : TrivialCycle {} // expected-error{{circular class inheritance TrivialCycle}} protocol P : P {} // expected-error{{circular protocol inheritance P}} + +class Isomorphism : Automorphism { } +class Automorphism : Automorphism { } // expected-error{{circular class inheritance Automorphism}} diff --git a/test/decl/protocol/protocols.swift b/test/decl/protocol/protocols.swift index 437594c5f0e33..b5141a71a36ac 100644 --- a/test/decl/protocol/protocols.swift +++ b/test/decl/protocol/protocols.swift @@ -81,6 +81,9 @@ protocol CircleMiddle : CircleStart { func circle_middle() } // expected-error{{ protocol CircleStart : CircleEnd { func circle_start() } // expected-note{{protocol 'CircleStart' declared here}} protocol CircleEnd : CircleMiddle { func circle_end()} // expected-note{{protocol 'CircleEnd' declared here}} +protocol CircleEntry : CircleTrivial { } +protocol CircleTrivial : CircleTrivial { } // expected-error{{circular protocol inheritance CircleTrivial}} + struct Circle { func circle_start() {} func circle_middle() {} diff --git a/validation-test/compiler_crashers/0394-void.swift b/validation-test/compiler_crashers_fixed/0394-void.swift similarity index 79% rename from validation-test/compiler_crashers/0394-void.swift rename to validation-test/compiler_crashers_fixed/0394-void.swift index ae7c7317adce3..61c1b4e5aaa29 100644 --- a/validation-test/compiler_crashers/0394-void.swift +++ b/validation-test/compiler_crashers_fixed/0394-void.swift @@ -1,4 +1,4 @@ -// RUN: not --crash %target-swift-frontend %s -parse +// RUN: not %target-swift-frontend %s -parse // Distributed under the terms of the MIT license // Test case submitted to project by https://github.com/practicalswift (practicalswift) diff --git a/validation-test/compiler_crashers/0876-x.swift b/validation-test/compiler_crashers_fixed/0876-x.swift similarity index 86% rename from validation-test/compiler_crashers/0876-x.swift rename to validation-test/compiler_crashers_fixed/0876-x.swift index b055590922bc1..97b0c091bf798 100644 --- a/validation-test/compiler_crashers/0876-x.swift +++ b/validation-test/compiler_crashers_fixed/0876-x.swift @@ -1,4 +1,4 @@ -// RUN: not --crash %target-swift-frontend %s -parse +// RUN: not %target-swift-frontend %s -parse // Distributed under the terms of the MIT license // Test case submitted to project by https://github.com/practicalswift (practicalswift) diff --git a/validation-test/compiler_crashers/0896-void.swift b/validation-test/compiler_crashers_fixed/0896-void.swift similarity index 80% rename from validation-test/compiler_crashers/0896-void.swift rename to validation-test/compiler_crashers_fixed/0896-void.swift index a47ea0dfb5177..9a162dca790dd 100644 --- a/validation-test/compiler_crashers/0896-void.swift +++ b/validation-test/compiler_crashers_fixed/0896-void.swift @@ -1,4 +1,4 @@ -// RUN: not --crash %target-swift-frontend %s -parse +// RUN: not %target-swift-frontend %s -parse // Distributed under the terms of the MIT license // Test case submitted to project by https://github.com/practicalswift (practicalswift) diff --git a/validation-test/compiler_crashers/1973-void.swift b/validation-test/compiler_crashers_fixed/1973-void.swift similarity index 87% rename from validation-test/compiler_crashers/1973-void.swift rename to validation-test/compiler_crashers_fixed/1973-void.swift index 1341dfb113a4f..a0b095043c5ac 100644 --- a/validation-test/compiler_crashers/1973-void.swift +++ b/validation-test/compiler_crashers_fixed/1973-void.swift @@ -1,4 +1,4 @@ -// RUN: not --crash %target-swift-frontend %s -parse +// RUN: not %target-swift-frontend %s -parse // Distributed under the terms of the MIT license // Test case submitted to project by https://github.com/practicalswift (practicalswift) diff --git a/validation-test/compiler_crashers/2206-void.swift b/validation-test/compiler_crashers_fixed/2206-void.swift similarity index 85% rename from validation-test/compiler_crashers/2206-void.swift rename to validation-test/compiler_crashers_fixed/2206-void.swift index 8c0f8d00287c7..fce2b2c93e71f 100644 --- a/validation-test/compiler_crashers/2206-void.swift +++ b/validation-test/compiler_crashers_fixed/2206-void.swift @@ -1,4 +1,4 @@ -// RUN: not --crash %target-swift-frontend %s -parse +// RUN: not %target-swift-frontend %s -parse // Distributed under the terms of the MIT license // Test case submitted to project by https://github.com/practicalswift (practicalswift)