Teach SuperclassDeclRequest to always diagnose circularity.

Rather than relying on clients to cope with the potential for circular
inheritance of superclass declarations, teach SuperclassDeclRequest to
establish whether circular inheritance has occurred and produce "null"
in such cases. This allows other clients to avoid having to think about

To benefit from this, have SuperclassTypeRequest evaluate
SuperclassDeclRequest first and, if null, produce a Type(). This
ensures that we don't get into an inconsistent situation where there
is a superclass type but no superclass declaration.

Author: DougGregor

include/swift/AST/NameLookupRequests.h

@@ -160,6 +160,10 @@ class SuperclassDeclRequest :
   evaluate(Evaluator &evaluator, NominalTypeDecl *subject) const;
 
 public:
+  // Cycle handling
+  void diagnoseCycle(DiagnosticEngine &diags) const;
+  void noteCycleStep(DiagnosticEngine &diags) const;
+
   // Caching
   bool isCached() const { return true; }
   Optional<ClassDecl *> getCachedResult() const;

lib/AST/Decl.cpp

@@ -4276,7 +4276,7 @@ bool ClassDecl::isIncompatibleWithWeakReferences() const {
 
 bool ClassDecl::inheritsSuperclassInitializers() const {
   // If there's no superclass, there's nothing to inherit.
-  if (!getSuperclass())
+  if (!getSuperclassDecl())
     return false;
 
   auto &ctx = getASTContext();

lib/AST/NameLookup.cpp

@@ -2290,12 +2290,33 @@ SuperclassDeclRequest::evaluate(Evaluator &evaluator,
                                   inheritedTypes, modulesFound, anyObject);
 
     // Look for a class declaration.
+    ClassDecl *superclass = nullptr;
     for (auto inheritedNominal : inheritedNominalTypes) {
-      if (auto classDecl = dyn_cast<ClassDecl>(inheritedNominal))
-        return classDecl;
+      if (auto classDecl = dyn_cast<ClassDecl>(inheritedNominal)) {
+        superclass = classDecl;
+        break;
+      }
     }
-  }
 
+    // If we found a superclass, ensure that we don't have a circular
+    // inheritance hierarchy by evaluating its superclass. This forces the
+    // diagnostic at this point and then suppresses the superclass failure.
+    if (superclass) {
+      auto result = Ctx.evaluator(SuperclassDeclRequest{superclass});
+      bool hadCycle = false;
+      if (auto err = result.takeError()) {
+        llvm::handleAllErrors(std::move(err),
+          [&hadCycle](const CyclicalRequestError<SuperclassDeclRequest> &E) {
+          hadCycle = true;
+          });
+
+        if (hadCycle)
+          return nullptr;
+      }
+
+      return superclass;
+    }
+  }
 
   return nullptr;
 }

lib/AST/NameLookupRequests.cpp

@@ -45,6 +45,19 @@ SourceLoc InheritedDeclsReferencedRequest::getNearestLoc() const {
 //----------------------------------------------------------------------------//
 // Superclass declaration computation.
 //----------------------------------------------------------------------------//
+void SuperclassDeclRequest::diagnoseCycle(DiagnosticEngine &diags) const {
+  // FIXME: Improve this diagnostic.
+  auto nominalDecl = std::get<0>(getStorage());
+  diags.diagnose(nominalDecl, diag::circular_class_inheritance,
+                 nominalDecl->getName());
+}
+
+void SuperclassDeclRequest::noteCycleStep(DiagnosticEngine &diags) const {
+  auto decl = std::get<0>(getStorage());
+  diags.diagnose(decl, diag::kind_declname_declared_here,
+                 decl->getDescriptiveKind(), decl->getName());
+}
+
 Optional<ClassDecl *> SuperclassDeclRequest::getCachedResult() const {
   auto nominalDecl = std::get<0>(getStorage());
 

lib/SILGen/SILGen.cpp

@@ -1288,8 +1288,8 @@ bool SILGenModule::requiresIVarDestroyer(ClassDecl *cd) {
   // Only needed if we have non-trivial ivars, we're not a root class, and
   // the superclass is not @objc.
   return (hasNonTrivialIVars(cd) &&
-          cd->getSuperclass() &&
-          !cd->getSuperclass()->getClassOrBoundGenericClass()->hasClangNode());
+          cd->getSuperclassDecl() &&
+          !cd->getSuperclassDecl()->hasClangNode());
 }
 
 /// TODO: This needs a better name.

lib/Sema/CodeSynthesis.cpp

@@ -1076,12 +1076,11 @@ InheritsSuperclassInitializersRequest::evaluate(Evaluator &eval,
   if (decl->getAttrs().hasAttribute<InheritsConvenienceInitializersAttr>())
     return true;
 
-  auto superclass = decl->getSuperclass();
-  assert(superclass);
+  auto superclassDecl = decl->getSuperclassDecl();
+  assert(superclassDecl);
 
   // If the superclass has known-missing designated initializers, inheriting
   // is unsafe.
-  auto *superclassDecl = superclass->getClassOrBoundGenericClass();
   if (superclassDecl->getModuleContext() != decl->getParentModule() &&
       superclassDecl->hasMissingDesignatedInitializers())
     return false;
@@ -1357,7 +1356,7 @@ HasDefaultInitRequest::evaluate(Evaluator &evaluator,
   // Don't synthesize a default for a subclass, it will attempt to inherit its
   // initializers from its superclass.
   if (auto *cd = dyn_cast<ClassDecl>(decl))
-    if (cd->getSuperclass())
+    if (cd->getSuperclassDecl())
       return false;
 
   // If the user has already defined a designated initializer, then don't

lib/Sema/TypeCheckRequestFunctions.cpp

@@ -93,6 +93,12 @@ SuperclassTypeRequest::evaluate(Evaluator &evaluator,
       return proto->getGenericSignature()
           ->getSuperclassBound(proto->getSelfInterfaceType());
     }
+
+    if (!proto->getSuperclassDecl())
+      return Type();
+  } else if (auto classDecl = dyn_cast<ClassDecl>(nominalDecl)) {
+    if (!classDecl->getSuperclassDecl())
+      return Type();
   }
 
   for (unsigned int idx : indices(nominalDecl->getInherited())) {

test/decl/class/circular_inheritance.swift

@@ -3,12 +3,12 @@
 // RUN: %target-typecheck-verify-swift
 // RUN: not %target-swift-frontend -typecheck -debug-cycles %s -build-request-dependency-graph -output-request-graphviz %t.dot -stats-output-dir %t/stats-dir 2> %t.cycles
 
-class Left // expected-error {{circular reference}} expected-note {{through reference here}}
+class Left // expected-error {{'Left' inherits from itself}} expected-note {{through reference here}}
     : Right.Hand { // expected-note {{through reference here}}
   class Hand {}
 }
 
-class Right // expected-note 2 {{through reference here}}
+class Right // expected-note {{through reference here}} expected-note{{class 'Right' declared here}}
   : Left.Hand { // expected-note {{through reference here}}
   class Hand {}
 }
@@ -23,20 +23,19 @@ protocol P : P {} // expected-error {{protocol 'P' refines itself}}
 class Isomorphism : Automorphism { }
 class Automorphism : Automorphism { } // expected-error{{'Automorphism' inherits from itself}}
 
-// FIXME: Useless error
-let _ = A() // expected-error{{'A' cannot be constructed because it has no accessible initializers}}
+let _ = A()
 
 class Outer {
   class Inner : Outer {}
 }
 
-class Outer2 // expected-error {{circular reference}} expected-note {{through reference here}}
+class Outer2 // expected-error {{'Outer2' inherits from itself}} expected-note {{through reference here}}
     : Outer2.Inner { // expected-note {{through reference here}}
 
   class Inner {}
 }
 
-class Outer3 // expected-error {{circular reference}} expected-note {{through reference here}}
+class Outer3 // expected-error {{'Outer3' inherits from itself}} expected-note {{through reference here}}
     : Outer3.Inner<Int> { // expected-note {{through reference here}}
   class Inner<T> {}
 }
@@ -54,27 +53,21 @@ class Outer3 // expected-error {{circular reference}} expected-note {{through re
 
 protocol Initable {
   init()
-  // expected-note@-1 {{protocol requires initializer 'init()' with type '()'; do you want to add a stub?}}
-  // expected-note@-2 {{did you mean 'init'?}}
 }
 
 protocol Shape : Circle {}
 
 class Circle : Initable & Circle {}
 // expected-error@-1 {{'Circle' inherits from itself}}
-// expected-error@-2 {{type 'Circle' does not conform to protocol 'Initable'}}
+// expected-error@-2 {{initializer requirement 'init()' can only be satisfied by a 'required' initializer in non-final class 'Circle'}}
 
 func crash() {
-  Circle()
-  // expected-error@-1 {{'Circle' cannot be constructed because it has no accessible initializers}}
+  _ = Circle()
 }
 
 // FIXME: We shouldn't emit the redundant "circular reference" diagnostics here.
 class WithDesignatedInit : WithDesignatedInit {
   // expected-error@-1 {{'WithDesignatedInit' inherits from itself}}
-  // expected-error@-2 {{circular reference}}
-  // expected-note@-3 {{through reference here}}
-  // expected-note@-4 2 {{through reference here}}
 
-  init(x: Int) {} // expected-error {{circular reference}}
+  init(x: Int) {}
 }