Sema: Improve circularity checks

The previous patches regressed a test where we used to diagnose
(poorly) a circular associated type, like so:

  associatedtype e: e

With the error "inheritance from non-protocol, non-class type 'e'".

This error went away, because we end up not setting the interface
type of the associated type early enough. Instead, we return an
ErrorType from resolveTypeInContext() and diagnose nothing.

With this patch, emit a diagnostic at the point where the ErrorType
first appears.

Also, remove the isRecursive() bit from AssociatedTypeDecl, and
remove isBeingTypeChecked() which duplicates a bit with the same
name in Decl.

Author: slavapestov

include/swift/AST/Decl.h

@@ -515,13 +515,8 @@ class alignas(1 << DeclAlignInBits) Decl {
   class AssociatedTypeDeclBitfields {
     friend class AssociatedTypeDecl;
     unsigned : NumTypeDeclBits;
-
-    unsigned Recursive : 1;
-
-    /// Whether or not this declaration is currently being type-checked.
-    unsigned BeingTypeChecked : 1;
   };
-  enum { NumAssociatedTypeDeclBits = NumTypeDeclBits + 2 };
+  enum { NumAssociatedTypeDeclBits = NumTypeDeclBits };
   static_assert(NumAssociatedTypeDeclBits <= 32, "fits in an unsigned");
 
   class ImportDeclBitfields {
@@ -2521,17 +2516,6 @@ class AssociatedTypeDecl : public AbstractTypeParamDecl {
   SourceLoc getStartLoc() const { return KeywordLoc; }
   SourceRange getSourceRange() const;
 
-  void setIsRecursive();
-  bool isRecursive() { return AssociatedTypeDeclBits.Recursive; }
-
-  /// Whether the declaration is currently being validated.
-  bool isBeingTypeChecked() { return AssociatedTypeDeclBits.BeingTypeChecked; }
-
-  /// Toggle whether or not the declaration is being validated.
-  void setIsBeingTypeChecked(bool ibt = true) {
-    AssociatedTypeDeclBits.BeingTypeChecked = ibt;
-  }
-
   static bool classof(const Decl *D) {
     return D->getKind() == DeclKind::AssociatedType;
   }

include/swift/AST/DiagnosticsSema.def

@@ -1543,6 +1543,8 @@ ERROR(requires_generic_param_made_equal_to_concrete,none,
 ERROR(requires_superclass_conflict,none,
       "generic parameter %0 cannot be a subclass of both %1 and %2",
       (Identifier, Type, Type))
+ERROR(recursive_type_reference,none,
+      "type %0 references itself", (Identifier))
 ERROR(recursive_requirement_reference,none,
       "type may not reference itself as a requirement",())
 ERROR(recursive_same_type_constraint,none,

lib/AST/ArchetypeBuilder.cpp

@@ -1429,18 +1429,9 @@ bool ArchetypeBuilder::addAbstractTypeParamRequirements(
   auto markRecursive = [&](AssociatedTypeDecl *assocType,
                            ProtocolDecl *proto,
                            SourceLoc loc) {
-    if (!pa->isRecursive() && !assocType->isRecursive()) {
+    if (!pa->isRecursive() && !assocType->isInvalid()) {
       Diags.diagnose(assocType->getLoc(),
                      diag::recursive_requirement_reference);
-        
-      // Mark all associatedtypes in this protocol as recursive (and error-type)
-      // to avoid later crashes dealing with this invalid protocol in other
-      // contexts.
-      auto containingProto =
-        assocType->getDeclContext()->getAsProtocolOrProtocolExtensionContext();
-      for (auto member : containingProto->getMembers())
-        if (auto assocType = dyn_cast<AssociatedTypeDecl>(member))
-          assocType->setIsRecursive();
     }
     pa->setIsRecursive();
 

lib/AST/Decl.cpp

@@ -2270,9 +2270,7 @@ AssociatedTypeDecl::AssociatedTypeDecl(DeclContext *dc, SourceLoc keywordLoc,
                                        TypeLoc defaultDefinition)
   : AbstractTypeParamDecl(DeclKind::AssociatedType, dc, name, nameLoc),
     KeywordLoc(keywordLoc), DefaultDefinition(defaultDefinition)
-{
-  AssociatedTypeDeclBits.Recursive = 0;
-}
+{ }
 
 AssociatedTypeDecl::AssociatedTypeDecl(DeclContext *dc, SourceLoc keywordLoc,
                                        Identifier name, SourceLoc nameLoc,
@@ -2283,7 +2281,6 @@ AssociatedTypeDecl::AssociatedTypeDecl(DeclContext *dc, SourceLoc keywordLoc,
     ResolverContextData(resolverData)
 {
   assert(Resolver && "missing resolver");
-  AssociatedTypeDeclBits.Recursive = 0;
 }
 
 void AssociatedTypeDecl::computeType() {
@@ -2314,10 +2311,6 @@ SourceRange AssociatedTypeDecl::getSourceRange() const {
   return SourceRange(KeywordLoc, endLoc);
 }
 
-void AssociatedTypeDecl::setIsRecursive() {
-  AssociatedTypeDeclBits.Recursive = true;
-}
-
 EnumDecl::EnumDecl(SourceLoc EnumLoc,
                      Identifier Name, SourceLoc NameLoc,
                      MutableArrayRef<TypeLoc> Inherited,

lib/Sema/TypeCheckDecl.cpp

@@ -6877,11 +6877,6 @@ void TypeChecker::validateDecl(ValueDecl *D, bool resolveTypeParams) {
   case DeclKind::AssociatedType: {
     auto typeParam = cast<AbstractTypeParamDecl>(D);
     auto assocType = dyn_cast<AssociatedTypeDecl>(typeParam);
-    if (assocType && assocType->isRecursive()) {
-      D->setInvalid();
-      break;
-    }
-
     DeclContext *DC = typeParam->getDeclContext();
 
     if (assocType && !assocType->hasInterfaceType())

lib/Sema/TypeCheckType.cpp

@@ -595,7 +595,9 @@ Type TypeChecker::applyUnboundGenericArguments(
     auto genericSig = decl->getGenericSignature();
     if (!decl->hasInterfaceType() ||
         decl->isValidatingGenericSignature()) {
-      diagnose(loc, diag::recursive_requirement_reference);
+      diagnose(loc, diag::recursive_type_reference,
+               decl->getName());
+      diagnose(noteLoc, diag::type_declared_here);
       return nullptr;
     }
 
@@ -662,8 +664,12 @@ static Type resolveTypeDecl(TypeChecker &TC, TypeDecl *typeDecl, SourceLoc loc,
     TC.validateDecl(typeDecl);
 
     // FIXME: More principled handling of circularity.
-    if (!typeDecl->hasInterfaceType())
+    if (!typeDecl->hasInterfaceType()) {
+      TC.diagnose(loc, diag::recursive_type_reference,
+                  typeDecl->getName());
+      TC.diagnose(typeDecl->getLoc(), diag::type_declared_here);
       return ErrorType::get(TC.Context);
+    }
   }
 
   // Resolve the type declaration to a specific type. How this occurs

test/Generics/generic_types.swift

@@ -226,7 +226,9 @@ var y: X5<X4, Int> // expected-error{{'X5' requires the types 'X4.AssocP' (aka '
 
 // Recursive generic signature validation.
 class Top {}
-class Bottom<T : Bottom<Top>> {} // expected-error {{type may not reference itself as a requirement}}
+class Bottom<T : Bottom<Top>> {}
+// expected-error@-1 {{type 'Bottom' references itself}}
+// expected-note@-2 {{type declared here}}
 
 // Invalid inheritance clause
 

test/NameBinding/scope_map_lookup.swift

@@ -24,7 +24,9 @@ protocol P1 {
 
 // Protocols involving associated types.
 protocol AProtocol {
-  associatedtype e : e  // expected-error {{inheritance from non-protocol, non-class type 'Self.e'}}
+  associatedtype e : e
+  // expected-error@-1 {{type 'e' references itself}}
+  // expected-note@-2 {{type declared here}}
 }
 
 // Extensions.

test/Sema/circular_decl_checking.swift

@@ -44,7 +44,9 @@ var TopLevelVar: TopLevelVar? { return nil } // expected-error 2 {{use of undecl
 
 
 protocol AProtocol {
-  associatedtype e : e  // expected-error {{inheritance from non-protocol, non-class type 'Self.e'}}
+  associatedtype e : e
+  // expected-error@-1 {{type 'e' references itself}}
+  // expected-note@-2 {{type declared here}}
 }
 
 

test/decl/protocol/req/recursion.swift

@@ -23,7 +23,9 @@ protocol Y {
 public protocol P {
   associatedtype T
 }
-public struct S<A: P> where A.T == S<A> {} // expected-error{{type may not reference itself as a requirement}}
+public struct S<A: P> where A.T == S<A> {}
+// expected-error@-1{{type 'S' references itself}}
+// expected-note@-2{{type declared here}}
 
 protocol I {
   init() // expected-note 2{{protocol requires initializer 'init()' with type '()'}}
@@ -33,8 +35,9 @@ protocol PI {
   associatedtype T : I
 }
 
-struct SI<A: PI> : I where A : I, A.T == SI<A> { // expected-error{{type may not reference itself as a requirement}}
-}
+struct SI<A: PI> : I where A : I, A.T == SI<A> {}
+// expected-error@-1{{type 'SI' references itself}}
+// expected-note@-2{{type declared here}}
 
 // Used to hit infinite recursion
 struct S4<A: PI> : I where A : I {

validation-test/compiler_crashers/28393-swift-type-transform.swift renamed to validation-test/compiler_crashers_fixed/28393-swift-type-transform.swift

@@ -5,6 +5,6 @@
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
-// RUN: not --crash %target-swift-frontend %s -typecheck
+// RUN: not %target-swift-frontend %s -typecheck
 // REQUIRES: asserts
 enum S:A{let d=b}protocol A{associatedtype b:A=S

validation-test/compiler_crashers/28438-swift-typebase-getcanonicaltype.swift renamed to validation-test/compiler_crashers_fixed/28438-swift-typebase-getcanonicaltype.swift

@@ -5,6 +5,6 @@
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
-// RUN: not --crash %target-swift-frontend %s -typecheck
+// RUN: not %target-swift-frontend %s -typecheck
 // REQUIRES: asserts
 protocol A{typealias e}struct B:A{var f=e