Break some cycles

Computing the generic signature changes the way that cycles appear in the compiler.  For now, break these cycles.  We should investigate each place where hasComputedGenericSignature() is used in service of breaking cycles.  See rdar://55263708

Author: CodaFi

lib/AST/GenericSignatureBuilder.cpp

@@ -5296,6 +5296,12 @@ class GenericSignatureBuilder::InferRequirementsWalker : public TypeWalker {
     auto decl = ty->getAnyNominal();
     if (!decl) return Action::Continue;
 
+    // FIXME: The GSB and the request evaluator both detect a cycle here if we
+    // force a recursive generic signature.  We should look into moving cycle
+    // detection into the generic signature request(s) - see rdar://55263708
+    if (!decl->hasComputedGenericSignature())
+      return Action::Continue;
+    
     auto genericSig = decl->getGenericSignature();
     if (!genericSig)
       return Action::Continue;

lib/AST/ProtocolConformance.cpp

@@ -547,17 +547,9 @@ void NormalProtocolConformance::differenceAndStoreConditionalRequirements()
     return;
   }
 
-  auto extensionSig = ext->getGenericSignature();
-  if (!extensionSig) {
-    if (auto lazyResolver = ctxt.getLazyResolver()) {
-      lazyResolver->resolveExtension(ext);
-      extensionSig = ext->getGenericSignature();
-    }
-  }
-
   // The type is generic, but the extension doesn't have a signature yet, so
   // we might be in a recursive validation situation.
-  if (!extensionSig) {
+  if (!ext->hasComputedGenericSignature()) {
     // If the extension is invalid, it won't ever get a signature, so we
     // "succeed" with an empty result instead.
     if (ext->isInvalid()) {
@@ -569,6 +561,15 @@ void NormalProtocolConformance::differenceAndStoreConditionalRequirements()
     failure();
     return;
   }
+  
+  // FIXME: All of this will be removed when validateExtension goes away.
+  auto extensionSig = ext->getGenericSignature();
+  if (!extensionSig) {
+    if (auto lazyResolver = ctxt.getLazyResolver()) {
+      lazyResolver->resolveExtension(ext);
+      extensionSig = ext->getGenericSignature();
+    }
+  }
 
   auto canExtensionSig = extensionSig->getCanonicalSignature();
   auto canTypeSig = typeSig->getCanonicalSignature();

lib/Sema/ConstraintSystem.cpp

@@ -501,7 +501,9 @@ Type ConstraintSystem::openUnboundGenericType(UnboundGenericType *unbound,
 
   // If the unbound decl hasn't been validated yet, we have a circular
   // dependency that isn't being diagnosed properly.
-  if (!unboundDecl->getGenericSignature()) {
+  //
+  // FIXME: Delete this condition.  He's dead Jim.
+  if (!unboundDecl->hasComputedGenericSignature()) {
     TC.diagnose(unboundDecl, diag::circular_reference);
     return Type();
   }

lib/Sema/MiscDiagnostics.cpp

@@ -1478,9 +1478,13 @@ bool TypeChecker::getDefaultGenericArgumentsString(
     genericParamText << contextTy;
   };
 
-  interleave(typeDecl->getInnermostGenericParamTypes(),
-             printGenericParamSummary, [&]{ genericParamText << ", "; });
-
+  // FIXME: We can potentially be in the middle of creating a generic signature
+  // if we get here.  Break this cycle.
+  if (typeDecl->hasComputedGenericSignature()) {
+    interleave(typeDecl->getInnermostGenericParamTypes(),
+               printGenericParamSummary, [&]{ genericParamText << ", "; });
+  }
+  
   genericParamText << ">";
   return true;
 }

lib/Sema/TypeCheckDecl.cpp

@@ -3720,8 +3720,8 @@ static void validateResultType(TypeChecker &TC,
     resultTyLoc.setType(
         TC.getOrCreateOpaqueResultType(resolution, decl, opaqueTy));
   } else {
-    TC.validateType(resultTyLoc, resolution,
-                    TypeResolverContext::FunctionResult);
+    TypeChecker::validateType(TC.Context, resultTyLoc, resolution,
+                              TypeResolverContext::FunctionResult);
   }
 }
 
@@ -3862,7 +3862,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
     for (auto member : proto->getMembers()) {
       if (auto *aliasDecl = dyn_cast<TypeAliasDecl>(member)) {
         if (!aliasDecl->isGeneric()) {
-          assert(aliasDecl->getGenericSignature() == proto->getGenericSignature());
           // FIXME: Force creation of the protocol's generic environment now.
           (void) proto->getGenericEnvironment();
 

lib/Sema/TypeCheckProtocolInference.cpp

@@ -197,10 +197,11 @@ AssociatedTypeInference::inferTypeWitnessesViaValueWitnesses(
     // Build a generic signature.
     tc.validateExtension(extension);
 
-    // The extension may not have a generic signature set up yet, as a
-    // recursion breaker, in which case we can't yet confidently reject its
-    // witnesses.
-    if (!extension->getGenericSignature())
+    // FIXME: The extension may not have a generic signature set up yet as
+    // resolving signatures may trigger associated type inference.  This cycle
+    // is now detectable and we should look into untangling it
+    // - see rdar://55263708
+    if (!extension->hasComputedGenericSignature())
       return true;
 
     // The condition here is a bit more fickle than

lib/Sema/TypeCheckType.cpp

@@ -977,7 +977,7 @@ static Type resolveTypeDecl(TypeDecl *typeDecl, SourceLoc loc,
   if (!options.is(TypeResolverContext::ExtensionBinding) ||
       !isa<NominalTypeDecl>(typeDecl)) {
     // Validate the declaration.
-    if (lazyResolver)
+    if (lazyResolver && !typeDecl->hasInterfaceType())
       lazyResolver->resolveDeclSignature(typeDecl);
 
     // If we were not able to validate recursively, bail out.

test/decl/nested/protocol.swift

@@ -57,10 +57,10 @@ protocol SillyProtocol {
   class InnerClass<T> {} // expected-error {{type 'InnerClass' cannot be nested in protocol 'SillyProtocol'}}
 }
 
+// N.B. Redeclaration checks don't see this case because `protocol A` is invalid.
 enum OuterEnum {
   protocol C {} // expected-error{{protocol 'C' cannot be nested inside another declaration}}
-  // expected-note@-1{{'C' previously declared here}}
-  case C(C) // expected-error{{invalid redeclaration of 'C'}}
+  case C(C)
 }
 
 class OuterClass {
@@ -105,7 +105,9 @@ func testLookup(_ x: OuterForUFI.Inner) {
   x.req()
   x.extMethod()
 }
+
+// N.B. Lookup fails here because OuterForUFI.Inner is marked invalid.
 func testLookup<T: OuterForUFI.Inner>(_ x: T) {
-  x.req()
-  x.extMethod()
+  x.req() // expected-error {{value of type 'T' has no member 'req'}}
+  x.extMethod() // expected-error {{value of type 'T' has no member 'extMethod'}}
 }

test/decl/overload.swift

@@ -536,9 +536,10 @@ enum SR_10084_E_2 {
   }
 }
 
+// N.B. Redeclaration checks don't see this case because `protocol A` is invalid.
 enum SR_10084_E_3 {
-  protocol A {} //expected-error {{protocol 'A' cannot be nested inside another declaration}} // expected-note {{'A' previously declared here}}
-  case A // expected-error {{invalid redeclaration of 'A'}}
+  protocol A {} //expected-error {{protocol 'A' cannot be nested inside another declaration}}
+  case A
 }
 
 enum SR_10084_E_4 {
@@ -551,9 +552,10 @@ enum SR_10084_E_5 {
   case C // expected-error {{invalid redeclaration of 'C'}}
 }
 
+// N.B. Redeclaration checks don't see this case because `protocol D` is invalid.
 enum SR_10084_E_6 {
-  case D // expected-note {{'D' previously declared here}}
-  protocol D {} //expected-error {{protocol 'D' cannot be nested inside another declaration}} // expected-error {{invalid redeclaration of 'D'}}
+  case D
+  protocol D {} //expected-error {{protocol 'D' cannot be nested inside another declaration}}
 }
 
 enum SR_10084_E_7 {
@@ -604,4 +606,4 @@ enum SR_10084_E_15 {
 enum SR_10084_E_16 {
   typealias Z = Int // expected-note {{'Z' previously declared here}}
   case Z // expected-error {{invalid redeclaration of 'Z'}}
-}
+}