[AST] Explicitly track the validation state of Decls.

Previously some decls (TypeAliasDecl and ExtensionDecl) had bits
explicitly marking whether they've been validated, while other decls
just deduced this from hasInterfaceType. The doing the latter doesn't
work when the interface type can be computed before doing full
validation (such as protocols and associatedtypes, which have trivial
interface types), and so an explicit bit is adopted for all decls.

include/swift/AST/Decl.h

@@ -238,11 +238,15 @@ class alignas(1 << DeclAlignInBits) Decl {
     /// \brief Whether this declaration is currently being validated.
     unsigned BeingValidated : 1;
 
+    /// \brief Whether we have started validating the declaration; this *isn't*
+    /// reset after finishing it.
+    unsigned ValidationStarted : 1;
+
     /// \brief Whether this declaration was added to the surrounding
     /// DeclContext of an active #if config clause.
     unsigned EscapedFromIfConfig : 1;
   };
-  enum { NumDeclBits = 12 };
+  enum { NumDeclBits = 13 };
   static_assert(NumDeclBits <= 32, "fits in an unsigned");
 
   class PatternBindingDeclBitfields {
@@ -423,13 +427,8 @@ class alignas(1 << DeclAlignInBits) Decl {
   class TypeAliasDeclBitfields {
     friend class TypeAliasDecl;
     unsigned : NumGenericTypeDeclBits;
-
-    /// Whether we have completed validation of the typealias.
-    /// This is necessary because unlike other declarations, a
-    /// typealias will not get an interface type right away.
-    unsigned HasCompletedValidation : 1;
   };
-  enum { NumTypeAliasDeclBits = NumGenericTypeDeclBits + 1 };
+  enum { NumTypeAliasDeclBits = NumGenericTypeDeclBits };
   static_assert(NumTypeAliasDeclBits <= 32, "fits in an unsigned");
 
   class NominalTypeDeclBitFields {
@@ -577,9 +576,6 @@ class alignas(1 << DeclAlignInBits) Decl {
     /// FIXME: Is this too fine-grained?
     unsigned CheckedInheritanceClause : 1;
 
-    /// Whether this extension has already been validated.
-    unsigned Validated : 1;
-
     /// An encoding of the default and maximum access level for this extension.
     ///
     /// This is encoded as (1 << (maxAccess-1)) | (1 << (defaultAccess-1)),
@@ -590,7 +586,7 @@ class alignas(1 << DeclAlignInBits) Decl {
     /// Whether there is are lazily-loaded conformances for this extension.
     unsigned HasLazyConformances : 1;
   };
-  enum { NumExtensionDeclBits = NumDeclBits + 6 };
+  enum { NumExtensionDeclBits = NumDeclBits + 5 };
   static_assert(NumExtensionDeclBits <= 32, "fits in an unsigned");
 
   class IfConfigDeclBitfields {
@@ -659,6 +655,7 @@ protected:
     DeclBits.FromClang = false;
     DeclBits.EarlyAttrValidation = false;
     DeclBits.BeingValidated = false;
+    DeclBits.ValidationStarted = false;
     DeclBits.EscapedFromIfConfig = false;
   }
 
@@ -815,7 +812,18 @@ public:
   void setIsBeingValidated(bool ibv = true) {
     assert(DeclBits.BeingValidated != ibv);
     DeclBits.BeingValidated = ibv;
+    if (ibv) {
+      DeclBits.ValidationStarted = true;
     }
+  }
+
+  bool hasValidationStarted() const { return DeclBits.ValidationStarted; }
+
+  /// Manually indicate that validation has started for the declaration.
+  ///
+  /// This is implied by setIsBeingValidated(true) (i.e. starting validation)
+  /// and so rarely needs to be called directly.
+  void setValidationStarted() { DeclBits.ValidationStarted = true; }
 
   bool escapedFromIfConfig() const {
     return DeclBits.EscapedFromIfConfig;
@@ -1644,19 +1652,9 @@ public:
 
   void setInherited(MutableArrayRef<TypeLoc> i) { Inherited = i; }
 
-  /// Whether we started validating this extension.
-  bool validated() const {
-    return ExtensionDeclBits.Validated;
-  }
-
-  /// Set whether we have validated this extension.
-  void setValidated(bool validated = true) {
-    ExtensionDeclBits.Validated = validated;
-  }
-
   /// Whether we have fully checked the extension.
   bool hasValidSignature() const {
-    return validated() && !isBeingValidated();
+    return hasValidationStarted() && !isBeingValidated();
   }
 
   /// Whether we already type-checked the inheritance clause.
@@ -2442,14 +2440,6 @@ public:
   /// aliases.
   void setUnderlyingType(Type type);
 
-  bool hasCompletedValidation() const {
-    return TypeAliasDeclBits.HasCompletedValidation;
-  }
-
-  void setHasCompletedValidation() {
-    TypeAliasDeclBits.HasCompletedValidation = 1;
-  }
-
   /// For generic typealiases, return the unbound generic type.
   UnboundGenericType *getUnboundGenericType() const;
 

lib/AST/Decl.cpp

@@ -808,7 +808,6 @@ ExtensionDecl::ExtensionDecl(SourceLoc extensionLoc,
     ExtendedType(extendedType),
     Inherited(inherited)
 {
-  ExtensionDeclBits.Validated = false;
   ExtensionDeclBits.CheckedInheritanceClause = false;
   ExtensionDeclBits.DefaultAndMaxAccessLevel = 0;
   ExtensionDeclBits.HasLazyConformances = false;
@@ -2261,10 +2260,7 @@ TypeAliasDecl::TypeAliasDecl(SourceLoc TypeAliasLoc, SourceLoc EqualLoc,
                              Identifier Name, SourceLoc NameLoc,
                              GenericParamList *GenericParams, DeclContext *DC)
   : GenericTypeDecl(DeclKind::TypeAlias, DC, Name, NameLoc, {}, GenericParams),
-    TypeAliasLoc(TypeAliasLoc),
+    TypeAliasLoc(TypeAliasLoc), EqualLoc(EqualLoc) {}
-    EqualLoc(EqualLoc) {
-  TypeAliasDeclBits.HasCompletedValidation = false;
-}
 
 SourceRange TypeAliasDecl::getSourceRange() const {
   if (UnderlyingTy.hasLocation())
@@ -2273,7 +2269,7 @@ SourceRange TypeAliasDecl::getSourceRange() const {
 }
 
 void TypeAliasDecl::setUnderlyingType(Type underlying) {
-  setHasCompletedValidation();
+  setValidationStarted();
 
   // lldb creates global typealiases containing archetypes
   // sometimes...

lib/ClangImporter/ImportDecl.cpp

@@ -3868,7 +3868,7 @@ namespace {
       SmallVector<TypeLoc, 4> inheritedTypes;
       importObjCProtocols(result, decl->getReferencedProtocols(),
                           inheritedTypes);
-      result->setValidated();
+      result->setValidationStarted();
       result->setInherited(Impl.SwiftContext.AllocateCopy(inheritedTypes));
       result->setCheckedInheritanceClause();
       result->setMemberLoader(&Impl, 0);
@@ -7197,7 +7197,7 @@ ClangImporter::Implementation::importDeclContextOf(
   auto swiftTyLoc = TypeLoc::withoutLoc(nominal->getDeclaredType());
   auto ext = ExtensionDecl::create(SwiftContext, SourceLoc(), swiftTyLoc, {},
                                    getClangModuleForDecl(decl), nullptr);
-  ext->setValidated();
+  ext->setValidationStarted();
   ext->setCheckedInheritanceClause();
   ext->setMemberLoader(this, reinterpret_cast<uintptr_t>(declSubmodule));
 

lib/ClangImporter/ImporterImpl.h

@@ -1105,6 +1105,8 @@ public:
     D->setAccessibility(access);
     if (auto ASD = dyn_cast<AbstractStorageDecl>(D))
       ASD->setSetterAccessibility(access);
+    // All imported decls are constructed fully validated.
+    D->setValidationStarted();
     return D;
   }
 

lib/Sema/ITCDecl.cpp

@@ -311,7 +311,7 @@ bool IterativeTypeChecker::isResolveTypeDeclSatisfied(TypeDecl *typeDecl) {
     } else if (auto ext = dyn_cast<ExtensionDecl>(dc)) {
       if (ext->isBeingValidated())
         return true;
-      if (ext->validated())
+      if (ext->hasValidationStarted())
         return false;
     } else {
       break;
@@ -329,7 +329,7 @@ void IterativeTypeChecker::processResolveTypeDecl(
   if (auto typeAliasDecl = dyn_cast<TypeAliasDecl>(typeDecl)) {
     if (typeAliasDecl->getDeclContext()->isModuleScopeContext() &&
         typeAliasDecl->getGenericParams() == nullptr) {
-      typeAliasDecl->setHasCompletedValidation();
+      typeAliasDecl->setValidationStarted();
 
       TypeResolutionOptions options;
       if (typeAliasDecl->getFormalAccess() <= Accessibility::FilePrivate)

lib/Sema/TypeCheckDecl.cpp

@@ -979,21 +979,14 @@ static bool contextAllowsPatternBindingWithoutVariables(DeclContext *dc) {
   return true;
 }
 
-/// Validate the given pattern binding declaration.
+/// Validate the \c entryNumber'th entry in \c binding.
-static void validatePatternBindingDecl(TypeChecker &tc,
+static void validatePatternBindingEntry(TypeChecker &tc,
                                         PatternBindingDecl *binding,
                                         unsigned entryNumber) {
   // If the pattern already has a type, we're done.
-  if (binding->getPattern(entryNumber)->hasType() ||
+  if (binding->getPattern(entryNumber)->hasType())
-      binding->isBeingValidated())
     return;
 
-  binding->setIsBeingValidated();
-
-  // On any path out of this function, make sure to mark the binding as done
-  // being type checked.
-  SWIFT_DEFER { binding->setIsBeingValidated(false); };
-
   // Resolve the pattern.
   auto *pattern = tc.resolvePattern(binding->getPattern(entryNumber),
                                     binding->getDeclContext(),
@@ -1078,6 +1071,19 @@ static void validatePatternBindingDecl(TypeChecker &tc,
       tc.checkTypeModifyingDeclAttributes(var);
 }
 
+/// Validate the entries in the given pattern binding declaration.
+static void validatePatternBindingEntries(TypeChecker &tc,
+                                          PatternBindingDecl *binding) {
+  if (binding->hasValidationStarted())
+    return;
+
+  binding->setIsBeingValidated();
+  SWIFT_DEFER { binding->setIsBeingValidated(false); };
+
+  for (unsigned i = 0, e = binding->getNumPatternEntries(); i != e; ++i)
+    validatePatternBindingEntry(tc, binding, i);
+}
+
 void swift::makeFinal(ASTContext &ctx, ValueDecl *D) {
   if (D && !D->isFinal()) {
     D->getAttrs().add(new (ctx) FinalAttr(/*IsImplicit=*/true));
@@ -3430,7 +3436,7 @@ void TypeChecker::validateDecl(PrecedenceGroupDecl *PGD) {
   checkDeclAttributesEarly(PGD);
   checkDeclAttributes(PGD);
 
-  if (PGD->isInvalid() || PGD->isBeingValidated())
+  if (PGD->isInvalid() || PGD->hasValidationStarted())
     return;
   PGD->setIsBeingValidated();
   SWIFT_DEFER { PGD->setIsBeingValidated(false); };
@@ -3752,8 +3758,7 @@ public:
 
   void visitPatternBindingDecl(PatternBindingDecl *PBD) {
     // Check all the pattern/init pairs in the PBD.
-    for (unsigned i = 0, e = PBD->getNumPatternEntries(); i != e; ++i)
+    validatePatternBindingEntries(TC, PBD);
-      validatePatternBindingDecl(TC, PBD, i);
 
     if (PBD->isBeingValidated())
       return;
@@ -6907,8 +6912,12 @@ void TypeChecker::validateDecl(ValueDecl *D) {
   // Handling validation failure due to re-entrancy is left
   // up to the caller, who must call hasValidSignature() to
   // check that validateDecl() returned a fully-formed decl.
-  if (D->isBeingValidated())
+  if (D->hasValidationStarted()) {
+    // If this isn't reentrant (i.e. D has already been validated), the
+    // signature better be valid.
+    assert(D->isBeingValidated() || D->hasValidSignature());
     return;
+  }
 
   PrettyStackTraceDecl StackTrace("validating", D);
 
@@ -6951,9 +6960,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
     llvm_unreachable("handled above");
 
   case DeclKind::AssociatedType: {
-    if (D->hasInterfaceType())
-      return;
-
     auto assocType = cast<AssociatedTypeDecl>(D);
 
     assocType->setIsBeingValidated();
@@ -6977,14 +6983,7 @@ void TypeChecker::validateDecl(ValueDecl *D) {
   }
 
   case DeclKind::TypeAlias: {
-    // Type aliases may not have an underlying type yet.
     auto typeAlias = cast<TypeAliasDecl>(D);
-
-    if (typeAlias->hasCompletedValidation())
-      return;
-
-    typeAlias->setHasCompletedValidation();
-
     // Check generic parameters, if needed.
     typeAlias->setIsBeingValidated();
     SWIFT_DEFER { typeAlias->setIsBeingValidated(false); };
@@ -7016,8 +7015,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
   case DeclKind::Struct:
   case DeclKind::Class: {
     auto nominal = cast<NominalTypeDecl>(D);
-    if (nominal->hasInterfaceType())
-      return;
     nominal->computeType();
 
     // Check generic parameters, if needed.
@@ -7055,8 +7052,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
 
   case DeclKind::Protocol: {
     auto proto = cast<ProtocolDecl>(D);
-    if (proto->hasInterfaceType())
-      return;
     proto->computeType();
 
     // Validate the generic type signature, which is just <Self : P>.
@@ -7109,8 +7104,7 @@ void TypeChecker::validateDecl(ValueDecl *D) {
           diagnose(VD, diag::pattern_used_in_type, VD->getName());
 
         } else {
-          for (unsigned i = 0, e = PBD->getNumPatternEntries(); i != e; ++i)
+          validatePatternBindingEntries(*this, PBD);
-            validatePatternBindingDecl(*this, PBD, i);
         }
 
         auto parentPattern = VD->getParentPattern();
@@ -7220,23 +7214,17 @@ void TypeChecker::validateDecl(ValueDecl *D) {
   }
       
   case DeclKind::Func: {
-    if (D->hasInterfaceType())
-      return;
     typeCheckDecl(D, true);
     break;
   }
 
   case DeclKind::Subscript:
   case DeclKind::Constructor:
-    if (D->hasInterfaceType())
-      return;
     typeCheckDecl(D, true);
     break;
 
   case DeclKind::Destructor:
   case DeclKind::EnumElement: {
-    if (D->hasInterfaceType())
-      return;
     if (auto container = dyn_cast<NominalTypeDecl>(D->getDeclContext())) {
       validateDecl(container);
       typeCheckDecl(D, true);
@@ -7429,12 +7417,11 @@ GenericParamList *cloneGenericParams(ASTContext &ctx,
 } // namespace swift
 
 void TypeChecker::validateExtension(ExtensionDecl *ext) {
-  // If we already validated this extension, there's nothing more to do.
+  // If we're currently validating, or have already validated this extension,
-  if (ext->validated())
+  // there's nothing more to do now.
+  if (ext->hasValidationStarted())
     return;
 
-  ext->setValidated();
-
   ext->setIsBeingValidated();
   SWIFT_DEFER { ext->setIsBeingValidated(false); };
 

lib/Serialization/Deserialization.cpp

@@ -3471,7 +3471,6 @@ Decl *ModuleFile::getDecl(DeclID DID, Optional<DeclContext *> ForcedContext) {
                                        DeclTypeCursor.GetCurrentBitNo()));
 
     nominal->addExtension(extension);
-    extension->setValidated(true);
 
     break;
   }
@@ -3533,7 +3532,9 @@ Decl *ModuleFile::getDecl(DeclID DID, Optional<DeclContext *> ForcedContext) {
   if (DAttrs)
     declOrOffset.get()->getAttrs().setRawAttributeChain(DAttrs);
 
-  return declOrOffset;
+  auto decl = declOrOffset.get();
+  decl->setValidationStarted();
+  return decl;
 }
 
 /// Translate from the Serialization function type repr enum values to the AST