Remove resolveExtension and validateExtension

Push the relevant work this was doing down into the callers.  Most of them didn't want to validate the entire extension and force its generic parameters, they just wanted to validate the nominal type.

We must eagerly validate the generic signature of extensions, though.  This avoids a class of cycles where non-generic members of an extension with a generic signaure will call through to getGenericSignatureOfContext and force the generic signature anyways.  When this calls through to protocol witness matching, the signature can be recursively computed.

Author: CodaFi

include/swift/AST/LazyResolver.h

@@ -60,12 +60,6 @@ class LazyResolver {
   /// consistency and provides the value a type.
   virtual void resolveDeclSignature(ValueDecl *VD) = 0;
 
-  /// Resolve the type of an extension.
-  ///
-  /// This can be called to ensure that the members of an extension can be
-  /// considered to be members of the extended type.
-  virtual void resolveExtension(ExtensionDecl *ext) = 0;
-
   /// Resolve any implicitly-declared constructors within the given nominal.
   virtual void resolveImplicitConstructors(NominalTypeDecl *nominal) = 0;
 

lib/AST/ConformanceLookupTable.cpp

@@ -798,8 +798,8 @@ ConformanceLookupTable::getConformance(NominalTypeDecl *nominal,
   // Everything about this conformance is nailed down, so we can now ensure that
   // the extension is fully resolved.
   if (auto resolver = nominal->getASTContext().getLazyResolver()) {
-    if (auto ED = dyn_cast<ExtensionDecl>(conformingDC)) {
-      resolver->resolveExtension(ED);
+    if (auto ext = dyn_cast<ExtensionDecl>(conformingDC)) {
+      resolver->resolveDeclSignature(ext->getExtendedNominal());
     } else {
       resolver->resolveDeclSignature(cast<NominalTypeDecl>(conformingDC));
     }

lib/AST/Type.cpp

@@ -1098,8 +1098,8 @@ CanType TypeBase::computeCanonicalType() {
       assert(resolver && "Need to resolve generic parameter depth");
       if (auto decl =
             gpDecl->getDeclContext()->getInnermostDeclarationDeclContext())
-        if (auto valueDecl = dyn_cast<ValueDecl>(decl))
-          resolver->resolveDeclSignature(valueDecl);
+        if (auto valueDecl = decl->getAsGenericContext())
+          (void)valueDecl->getGenericSignature();
     }
 
     assert(gpDecl->getDepth() != GenericTypeParamDecl::InvalidDepth &&

lib/Sema/IDETypeCheckingRequests.cpp

@@ -56,9 +56,7 @@ static bool isExtensionAppliedInternal(const DeclContext *DC, Type BaseTy,
   if (!ED->isConstrainedExtension())
     return true;
 
-  TypeChecker *TC = &TypeChecker::createForContext((DC->getASTContext()));
-  TC->validateExtension(const_cast<ExtensionDecl *>(ED));
-
+  (void)TypeChecker::createForContext(DC->getASTContext());
   GenericSignature *genericSig = ED->getGenericSignature();
   SubstitutionMap substMap = BaseTy->getContextSubstitutionMap(
       DC->getParentModule(), ED->getExtendedNominal());

lib/Sema/TypeCheckDecl.cpp

@@ -3086,9 +3086,14 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
       return;
     }
 
-    TC.validateExtension(ED);
+    // Validate the nominal type declaration being extended.
+    TC.validateDecl(nominal);
+    // Don't bother computing the generic signature if the extended nominal
+    // type didn't pass validation so we don't crash.
+    if (!nominal->isInvalid())
+      (void)ED->getGenericSignature();
+    ED->setValidationToChecked();
 
-    extType = ED->getExtendedType();
     if (extType && !extType->hasError()) {
       // The first condition catches syntactic forms like
       //     protocol A & B { ... } // may be protocols or typealiases
@@ -3757,7 +3762,20 @@ void TypeChecker::validateDecl(ValueDecl *D) {
     if (!nominal->hasValidSignature())
       return;
   } else if (auto ext = dyn_cast<ExtensionDecl>(dc)) {
-    validateExtension(ext);
+    // If we're currently validating, or have already validated this extension,
+    // there's nothing more to do now.
+    if (!ext->hasValidationStarted()) {
+      DeclValidationRAII IBV(ext);
+
+      if (auto *nominal = ext->getExtendedNominal()) {
+        // Validate the nominal type declaration being extended.
+        validateDecl(nominal);
+        
+        // Eagerly validate the generic signature of the extension.
+        if (!nominal->isInvalid())
+          (void)ext->getGenericSignature();
+      }
+    }
     if (!ext->hasValidSignature())
       return;
   }
@@ -3819,7 +3837,7 @@ void TypeChecker::validateDecl(ValueDecl *D) {
 
   case DeclKind::OpaqueType: {
     auto opaque = cast<OpaqueTypeDecl>(D);
-    DeclValidationRAII IBV(opaque);
+    opaque->setValidationToChecked();
     break;
   }
 
@@ -3828,11 +3846,7 @@ void TypeChecker::validateDecl(ValueDecl *D) {
   case DeclKind::Class: {
     auto nominal = cast<NominalTypeDecl>(D);
     nominal->computeType();
-
-    // Check generic parameters, if needed.
-    DeclValidationRAII IBV(nominal);
-    (void)nominal->getGenericSignature();
-    nominal->setSignatureIsValidated();
+    nominal->setValidationToChecked();
 
     if (auto *ED = dyn_cast<EnumDecl>(nominal)) {
       // @objc enums use their raw values as the value representation, so we
@@ -3848,11 +3862,7 @@ void TypeChecker::validateDecl(ValueDecl *D) {
     auto proto = cast<ProtocolDecl>(D);
     if (!proto->hasInterfaceType())
       proto->computeType();
-
-    // Validate the generic type signature, which is just <Self : P>.
-    DeclValidationRAII IBV(proto);
-    (void)proto->getGenericSignature();
-    proto->setSignatureIsValidated();
+    proto->setValidationToChecked();
 
     break;
   }
@@ -4349,26 +4359,6 @@ ExtendedTypeRequest::evaluate(Evaluator &eval, ExtensionDecl *ext) const {
   return extendedType;
 }
 
-void TypeChecker::validateExtension(ExtensionDecl *ext) {
-  // If we're currently validating, or have already validated this extension,
-  // there's nothing more to do now.
-  if (ext->hasValidationStarted())
-    return;
-
-  DeclValidationRAII IBV(ext);
-
-  if (auto *nominal = ext->getExtendedNominal()) {
-    // Validate the nominal type declaration being extended.
-    validateDecl(nominal);
-    
-    // FIXME: validateExtension is going to disappear soon.  In the mean time
-    // don't bother computing the generic signature if the extended nominal type
-    // didn't pass validation so we don't crash.
-    if (!nominal->isInvalid())
-      (void)ext->getGenericSignature();
-  }
-}
-
 /// Build a default initializer string for the given pattern.
 ///
 /// This string is suitable for display in diagnostics.

lib/Sema/TypeCheckProtocolInference.cpp

@@ -188,22 +188,27 @@ AssociatedTypeInference::inferTypeWitnessesViaValueWitnesses(
     // Invalid case.
     if (extendedNominal == nullptr)
       return true;
-
+    
+    // Validate the nominal type being extended.
+    tc.validateDecl(extendedNominal);
+    if (extendedNominal->isInvalid())
+      return true;
+    
     // Assume unconstrained concrete extensions we found witnesses in are
     // always viable.
     if (!isa<ProtocolDecl>(extendedNominal))
       return !extension->isConstrainedExtension();
-
-    // Build a generic signature.
-    tc.validateExtension(extension);
-
+    
     // 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;
 
+    // Build a generic signature.
+    auto *extensionSig = extension->getGenericSignature();
+    
     // The condition here is a bit more fickle than
     // `isExtensionApplied`. That check would prematurely reject
     // extensions like `P where AssocType == T` if we're relying on a
@@ -212,8 +217,7 @@ AssociatedTypeInference::inferTypeWitnessesViaValueWitnesses(
     // because those have to be explicitly declared on the type somewhere
     // so won't be affected by whatever answer inference comes up with.
     auto selfTy = extension->getSelfInterfaceType();
-    for (const Requirement &reqt
-         : extension->getGenericSignature()->getRequirements()) {
+    for (const Requirement &reqt : extensionSig->getRequirements()) {
       switch (reqt.getKind()) {
       case RequirementKind::Conformance:
       case RequirementKind::Superclass:

lib/Sema/TypeChecker.cpp

@@ -614,10 +614,14 @@ void swift::typeCheckCompletionDecl(Decl *D) {
   DiagnosticSuppression suppression(Ctx.Diags);
   TypeChecker &TC = createTypeChecker(Ctx);
 
-  if (auto ext = dyn_cast<ExtensionDecl>(D))
-    TC.validateExtension(ext);
-  else
+  if (auto ext = dyn_cast<ExtensionDecl>(D)) {
+    if (auto *nominal = ext->getExtendedNominal()) {
+      // Validate the nominal type declaration being extended.
+      TC.validateDecl(nominal);
+    }
+  } else {
     TC.validateDecl(cast<ValueDecl>(D));
+  }
 }
 
 void swift::typeCheckPatternBinding(PatternBindingDecl *PBD,

lib/Sema/TypeChecker.h

@@ -1015,10 +1015,6 @@ class TypeChecker final : public LazyResolver {
     validateDecl(VD);
   }
 
-  virtual void resolveExtension(ExtensionDecl *ext) override {
-    validateExtension(ext);
-  }
-
   virtual void resolveImplicitConstructors(NominalTypeDecl *nominal) override {
     addImplicitConstructors(nominal);
   }