Catch failures earlier caused by premature requests for extended nominal

Author: David Ungar

include/swift/AST/Decl.h

@@ -1677,7 +1677,11 @@ class ExtensionDecl final : public GenericContext, public Decl,
   TypeRepr *ExtendedTypeRepr;
 
   /// The nominal type being extended.
-  NominalTypeDecl *ExtendedNominal = nullptr;
+  ///
+  /// The bit indicates whether binding has been attempted. The pointer can be
+  /// null if either no binding was attempted or if binding could not find  the
+  /// extended nominal.
+  llvm::PointerIntPair<NominalTypeDecl *, 1, bool> ExtendedNominal;
 
   MutableArrayRef<TypeLoc> Inherited;
 
@@ -1736,6 +1740,12 @@ class ExtensionDecl final : public GenericContext, public Decl,
   SourceRange getBraces() const { return Braces; }
   void setBraces(SourceRange braces) { Braces = braces; }
 
+  bool hasBeenBound() const { return ExtendedNominal.getInt(); }
+
+  void setExtendedNominal(NominalTypeDecl *n) {
+    ExtendedNominal.setPointerAndInt(n, true);
+  }
+
   /// Retrieve the type being extended.
   ///
   /// Only use this entry point when the complete type, as spelled in the source,
@@ -1744,8 +1754,21 @@ class ExtensionDecl final : public GenericContext, public Decl,
   Type getExtendedType() const;
 
   /// Retrieve the nominal type declaration that is being extended.
+  /// Will  trip an assertion if the declaration has not already been computed.
+  /// In order to fail fast when type checking work is attempted
+  /// before extension binding has taken place.
+
   NominalTypeDecl *getExtendedNominal() const;
 
+  /// Compute the nominal type declaration that is being extended.
+  NominalTypeDecl *computeExtendedNominal() const;
+
+  /// \c hasBeenBound means nothing if this extension can never been bound
+  /// because it is not at the top level.
+  bool canNeverBeBound() const;
+
+  bool hasValidParent() const;
+
   /// Determine whether this extension has already been bound to a nominal
   /// type declaration.
   bool alreadyBoundToNominal() const { return NextExtension.getInt(); }

include/swift/AST/DeclContext.h

@@ -461,6 +461,10 @@ class alignas(1 << DeclContextAlignInBits) DeclContext {
   /// AnyObject dynamic lookup.
   bool mayContainMembersAccessedByDynamicLookup() const;
 
+  /// Extensions are only allowed at the level in a file
+  /// FIXME: do this for Protocols, too someday
+  bool canBeParentOfExtension() const;
+
   /// Returns true if lookups within this context could affect downstream files.
   ///
   /// \param functionsAreNonCascading If true, functions are considered non-

lib/AST/Decl.cpp

@@ -1076,9 +1076,25 @@ ExtensionDecl::takeConformanceLoaderSlow() {
 }
 
 NominalTypeDecl *ExtensionDecl::getExtendedNominal() const {
+  assert((hasBeenBound() || canNeverBeBound()) &&
+         "Extension must have already been bound (by bindExtensions)");
+  return ExtendedNominal.getPointer();
+}
+
+NominalTypeDecl *ExtensionDecl::computeExtendedNominal() const {
   ASTContext &ctx = getASTContext();
-  return evaluateOrDefault(ctx.evaluator,
-    ExtendedNominalRequest{const_cast<ExtensionDecl *>(this)}, nullptr);
+  return evaluateOrDefault(
+      ctx.evaluator, ExtendedNominalRequest{const_cast<ExtensionDecl *>(this)},
+      nullptr);
+}
+
+bool ExtensionDecl::canNeverBeBound() const {
+  // \c bindExtensions() only looks at valid parents for extensions.
+  return !hasValidParent();
+}
+
+bool ExtensionDecl::hasValidParent() const {
+  return getDeclContext()->canBeParentOfExtension();
 }
 
 bool ExtensionDecl::isConstrainedExtension() const {

lib/AST/DeclContext.cpp

@@ -478,6 +478,10 @@ bool DeclContext::mayContainMembersAccessedByDynamicLookup() const {
   return false;
 }
 
+bool DeclContext::canBeParentOfExtension() const {
+  return isa<SourceFile>(this);
+}
+
 bool DeclContext::walkContext(ASTWalker &Walker) {
   switch (getContextKind()) {
   case DeclContextKind::Module:

lib/AST/NameLookupRequests.cpp

@@ -74,17 +74,19 @@ Optional<NominalTypeDecl *> ExtendedNominalRequest::getCachedResult() const {
   // Note: if we fail to compute any nominal declaration, it's considered
   // a cache miss. This allows us to recompute the extended nominal types
   // during extension binding.
+  // This recomputation is also what allows you to extend types defined inside
+  // other extensions, regardless of source file order. See \c bindExtensions(),
+  // which uses a worklist algorithm that attempts to bind everything until
+  // fixed point.
   auto ext = std::get<0>(getStorage());
-  if (ext->ExtendedNominal)
-    return ext->ExtendedNominal;
-
-  return None;
+  if (!ext->hasBeenBound() || !ext->getExtendedNominal())
+    return None;
+  return ext->getExtendedNominal();
 }
 
 void ExtendedNominalRequest::cacheResult(NominalTypeDecl *value) const {
   auto ext = std::get<0>(getStorage());
-  if (value)
-    ext->ExtendedNominal = value;
+  ext->setExtendedNominal(value);
 }
 
 //----------------------------------------------------------------------------//

lib/IDE/SyntaxModel.cpp

@@ -792,6 +792,9 @@ bool ModelASTWalker::walkToDeclPre(Decl *D) {
     pushStructureNode(SN, NTD);
 
   } else if (auto *ED = dyn_cast<ExtensionDecl>(D)) {
+    // Normally bindExtension() would take care of computing the extended
+    // nominal. It must be done before asking for generic parameters.
+    ED->computeExtendedNominal();
     SyntaxStructureNode SN;
     setDecl(SN, D);
     SN.Kind = SyntaxStructureKind::Extension;

lib/Sema/TypeCheckDecl.cpp

@@ -3058,7 +3058,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
     // Produce any diagnostics for the extended type.
     auto extType = ED->getExtendedType();
 
-    auto nominal = ED->getExtendedNominal();
+    auto nominal = ED->computeExtendedNominal();
     if (nominal == nullptr) {
       const bool wasAlreadyInvalid = ED->isInvalid();
       ED->setInvalid();

lib/Sema/TypeChecker.cpp

@@ -229,7 +229,9 @@ static void bindExtensions(SourceFile &SF) {
   // Utility function to try and resolve the extended type without diagnosing.
   // If we succeed, we go ahead and bind the extension. Otherwise, return false.
   auto tryBindExtension = [&](ExtensionDecl *ext) -> bool {
-    if (auto nominal = ext->getExtendedNominal()) {
+    assert(!ext->canNeverBeBound() &&
+           "Only extensions that can ever be bound get here.");
+    if (auto nominal = ext->computeExtendedNominal()) {
       bindExtensionToNominal(ext, nominal);
       return true;
     }
@@ -615,7 +617,7 @@ void swift::typeCheckCompletionDecl(Decl *D) {
   DiagnosticSuppression suppression(Ctx.Diags);
   (void)createTypeChecker(Ctx);
   if (auto ext = dyn_cast<ExtensionDecl>(D)) {
-    if (auto *nominal = ext->getExtendedNominal()) {
+    if (auto *nominal = ext->computeExtendedNominal()) {
       // FIXME(InterfaceTypeRequest): Remove this.
       (void)nominal->getInterfaceType();
     }