[Serialization] Drop enums if a case payload can't be deserialized.

Layout for an enum depends very intimately on its cases---both their
existence and what their payload types are. That means there's no way
to "partly" recover from failure to deserialize an individual case's
payload type, the way we can partly recover from failing to
deserialize an initializer in a class. Add deserialization recovery
to enums by validating all of their payload types up front, and
dropping the enum if we can't import all of the cases.

This is the first time where we're trying to do deserialization
recovery for a /type/, and that could have many more ripple effects
than for a var/func/subscript/init. A better answer here might be to
still import the enum but mark it as unavailable, but in that case
we'd have to make sure to propagate that unavailability to anything
that /used/ the enum as well. (In Swift, availability is checked based
on use of the name, so if someone manages to refer to an enum using
inferred types we'd be in trouble.)

There is one case here that's not covered: if an enum case has a
payload that references a type declaration nested within the enum, but
then that nested type /itself/ can't be loaded for some reason, we
have no way to check that up front, because we can't even try to load
the nested type without loading its parent DeclContext (the enum). I
can't think of an easy solution for this right now.

(In the future, we'll be able to support dropping a single case for
resilient enums. But we're not there right now.)

rdar://problem/31920901

Author: jrose-apple

include/swift/Serialization/ModuleFormat.h

@@ -54,7 +54,7 @@ const uint16_t VERSION_MAJOR = 0;
 /// in source control, you should also update the comment to briefly
 /// describe what change you made. The content of this comment isn't important;
 /// it just ensures a conflict if two people change the module format.
-const uint16_t VERSION_MINOR = 345; // Last change: list of dependency types
+const uint16_t VERSION_MINOR = 346; // Last change: dependency types for enums
 
 using DeclID = PointerEmbeddedInt<unsigned, 31>;
 using DeclIDField = BCFixed<31>;
@@ -802,7 +802,8 @@ namespace decls_block {
     TypeIDField,            // raw type
     AccessibilityKindField, // accessibility
     BCVBR<4>,               // number of conformances
-    BCArray<TypeIDField>    // inherited types
+    BCVBR<4>,               // number of inherited types
+    BCArray<TypeIDField>    // inherited types, followed by dependency types
     // Trailed by the generic parameters (if any), the members record, and
     // finally conformance info (if any).
   >;

lib/Serialization/Deserialization.cpp

@@ -3265,24 +3265,35 @@ ModuleFile::getDeclChecked(DeclID DID, Optional<DeclContext *> ForcedContext) {
     GenericEnvironmentID genericEnvID;
     TypeID rawTypeID;
     uint8_t rawAccessLevel;
-    unsigned numConformances;
-    ArrayRef<uint64_t> rawInheritedIDs;
+    unsigned numConformances, numInheritedTypes;
+    ArrayRef<uint64_t> rawInheritedAndDependencyIDs;
 
     decls_block::EnumLayout::readRecord(scratch, nameID, contextID,
                                         isImplicit, genericEnvID, rawTypeID,
                                         rawAccessLevel,
-                                        numConformances, rawInheritedIDs);
+                                        numConformances, numInheritedTypes,
+                                        rawInheritedAndDependencyIDs);
 
     auto DC = getDeclContext(contextID);
     if (declOrOffset.isComplete())
       return declOrOffset;
 
+    Identifier name = getIdentifier(nameID);
+    for (TypeID dependencyID :
+           rawInheritedAndDependencyIDs.slice(numInheritedTypes)) {
+      auto dependency = getTypeChecked(dependencyID);
+      if (!dependency) {
+        return llvm::make_error<TypeError>(
+            name, takeErrorInfo(dependency.takeError()));
+      }
+    }
+
     auto genericParams = maybeReadGenericParams(DC);
     if (declOrOffset.isComplete())
       return declOrOffset;
 
-    auto theEnum = createDecl<EnumDecl>(SourceLoc(), getIdentifier(nameID),
-                                        SourceLoc(), None, genericParams, DC);
+    auto theEnum = createDecl<EnumDecl>(SourceLoc(), name, SourceLoc(), None,
+                                        genericParams, DC);
 
     declOrOffset = theEnum;
 
@@ -3301,7 +3312,8 @@ ModuleFile::getDeclChecked(DeclID DID, Optional<DeclContext *> ForcedContext) {
 
     theEnum->computeType();
 
-    handleInherited(theEnum, rawInheritedIDs);
+    handleInherited(theEnum,
+                    rawInheritedAndDependencyIDs.slice(0, numInheritedTypes));
 
     theEnum->setMemberLoader(this, DeclTypeCursor.GetCurrentBitNo());
     skipRecord(DeclTypeCursor, decls_block::MEMBERS);

lib/Serialization/Serialization.cpp

@@ -2315,12 +2315,46 @@ void Serializer::writeForeignErrorConvention(const ForeignErrorConvention &fec){
                                            resultTypeID);
 }
 
-static SmallVector<Type, 4> collectDependenciesFromType(CanType ty) {
-  llvm::SmallSetVector<Type, 4> result;
-  ty.visit([&](CanType next) {
-    if (auto *nominal = next->getAnyNominal())
-      result.insert(nominal->getDeclaredInterfaceType());
+/// Returns true if the declaration of \p decl depends on \p problemContext
+/// based on lexical nesting.
+///
+/// - \p decl is \p problemContext
+/// - \p decl is declared within \p problemContext
+/// - \p decl is declared in an extension of a type that depends on
+///   \p problemContext
+static bool contextDependsOn(const NominalTypeDecl *decl,
+                             const DeclContext *problemContext) {
+  const DeclContext *dc = decl;
+  do {
+    if (dc == problemContext)
+      return true;
+    if (auto *extension = dyn_cast<ExtensionDecl>(dc))
+      dc = extension->getAsNominalTypeOrNominalTypeExtensionContext();
+    else
+      dc = dc->getParent();
+  } while (!dc->isModuleScopeContext());
+  return false;
+}
+
+static void collectDependenciesFromType(llvm::SmallSetVector<Type, 4> &seen,
+                                        Type ty,
+                                        const DeclContext *excluding) {
+  ty.visit([&](Type next) {
+    auto *nominal = next->getAnyNominal();
+    if (!nominal)
+      return;
+    // FIXME: The child context case is still important for enums. It's
+    // possible an enum element has a payload that references a type declaration
+    // nested within the enum that can't be imported (for whatever reason).
+    if (contextDependsOn(nominal, excluding))
+      return;
+    seen.insert(nominal->getDeclaredInterfaceType());
   });
+}
+
+static SmallVector<Type, 4> collectDependenciesFromType(Type ty) {
+  llvm::SmallSetVector<Type, 4> result;
+  collectDependenciesFromType(result, ty, /*excluding*/nullptr);
   return result.takeVector();
 }
 
@@ -2668,9 +2702,20 @@ void Serializer::writeDecl(const Decl *D) {
                           ConformanceLookupKind::All,
                           nullptr, /*sorted=*/true);
 
-    SmallVector<TypeID, 4> inheritedTypes;
+    SmallVector<TypeID, 4> inheritedAndDependencyTypes;
     for (auto inherited : theEnum->getInherited())
-      inheritedTypes.push_back(addTypeRef(inherited.getType()));
+      inheritedAndDependencyTypes.push_back(addTypeRef(inherited.getType()));
+
+    llvm::SmallSetVector<Type, 4> dependencyTypes;
+    for (const EnumElementDecl *nextElt : theEnum->getAllElements()) {
+      if (!nextElt->hasAssociatedValues())
+        continue;
+      collectDependenciesFromType(dependencyTypes,
+                                  nextElt->getArgumentInterfaceType(),
+                                  /*excluding*/theEnum);
+    }
+    for (Type ty : dependencyTypes)
+      inheritedAndDependencyTypes.push_back(addTypeRef(ty));
 
     uint8_t rawAccessLevel =
       getRawStableAccessibility(theEnum->getFormalAccess());
@@ -2685,7 +2730,8 @@ void Serializer::writeDecl(const Decl *D) {
                             addTypeRef(theEnum->getRawType()),
                             rawAccessLevel,
                             conformances.size(),
-                            inheritedTypes);
+                            theEnum->getInherited().size(),
+                            inheritedAndDependencyTypes);
 
     writeGenericParams(theEnum->getGenericParams());
     writeMembers(theEnum->getMembers(), false);

test/Serialization/Recovery/typedefs-in-enums.swift

@@ -0,0 +1,81 @@
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: %target-swift-frontend -emit-sil -o - -emit-module-path %t/Lib.swiftmodule -module-name Lib -I %S/Inputs/custom-modules -disable-objc-attr-requires-foundation-module %s > /dev/null
+
+// RUN: %target-swift-ide-test -source-filename=x -print-module -module-to-print Lib -I %t -I %S/Inputs/custom-modules | %FileCheck %s
+
+// RUN: %target-swift-ide-test -source-filename=x -print-module -module-to-print Lib -I %t -I %S/Inputs/custom-modules -Xcc -DBAD | %FileCheck -check-prefix CHECK-RECOVERY %s
+
+// RUN: %target-swift-frontend -typecheck -I %t -I %S/Inputs/custom-modules -Xcc -DBAD -DTEST %s -verify
+
+#if TEST
+
+import Typedefs
+import Lib
+
+func use(_: OkayEnum) {}
+// FIXME: Better to import the enum and make it unavailable.
+func use(_: BadEnum) {} // expected-error {{use of undeclared type 'BadEnum'}}
+
+func test() {
+  _ = producesOkayEnum()
+  _ = producesBadEnum() // expected-error {{use of unresolved identifier 'producesBadEnum'}}
+}
+
+#else // TEST
+
+import Typedefs
+
+public enum BadEnum {
+  case noPayload
+  case perfectlyOkayPayload(Int)
+  case problematic(Any, WrappedInt)
+  case alsoOkay(Any, Any, Any)
+}
+// CHECK-LABEL: enum BadEnum {
+// CHECK-RECOVERY-NOT: enum BadEnum
+
+public enum OkayEnum {
+  case noPayload
+  case plainOldAlias(Any, UnwrappedInt)
+  case other(Int)
+}
+// CHECK-LABEL: enum OkayEnum {
+// CHECK-NEXT:   case noPayload
+// CHECK-NEXT:   case plainOldAlias(Any, UnwrappedInt)
+// CHECK-NEXT:   case other(Int)
+// CHECK-NEXT: }
+// CHECK-RECOVERY-LABEL: enum OkayEnum {
+// CHECK-RECOVERY-NEXT:   case noPayload
+// CHECK-RECOVERY-NEXT:   case plainOldAlias(Any, Int32)
+// CHECK-RECOVERY-NEXT:   case other(Int)
+// CHECK-RECOVERY-NEXT: }
+
+public enum OkayEnumWithSelfRefs {
+  public struct Nested {}
+  indirect case selfRef(OkayEnumWithSelfRefs)
+  case nested(Nested)
+}
+// CHECK-LABEL: enum OkayEnumWithSelfRefs {
+// CHECK-NEXT:   struct Nested {
+// CHECK-NEXT:     init()
+// CHECK-NEXT:   }
+// CHECK-NEXT:   indirect case selfRef(OkayEnumWithSelfRefs)
+// CHECK-NEXT:   case nested(OkayEnumWithSelfRefs.Nested)
+// CHECK-NEXT: }
+// CHECK-RECOVERY-LABEL: enum OkayEnumWithSelfRefs {
+// CHECK-RECOVERY-NEXT:   struct Nested {
+// CHECK-RECOVERY-NEXT:     init()
+// CHECK-RECOVERY-NEXT:   }
+// CHECK-RECOVERY-NEXT:   indirect case selfRef(OkayEnumWithSelfRefs)
+// CHECK-RECOVERY-NEXT:   case nested(OkayEnumWithSelfRefs.Nested)
+// CHECK-RECOVERY-NEXT: }
+
+public func producesBadEnum() -> BadEnum { return .noPayload }
+// CHECK-LABEL: func producesBadEnum() -> BadEnum
+// CHECK-RECOVERY-NOT: func producesBadEnum() -> BadEnum
+
+public func producesOkayEnum() -> OkayEnum { return .noPayload }
+// CHECK-LABEL: func producesOkayEnum() -> OkayEnum
+// CHECK-RECOVERY-LABEL: func producesOkayEnum() -> OkayEnum
+
+#endif // TEST