Merge pull request #9730 from jrose-apple/4.0-deserialization-recovery-for-enums

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

Author: tkremenek

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 = 344; // Last change: ctor newly required flag
+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/ModuleFile.cpp

@@ -1330,7 +1330,14 @@ void ModuleFile::lookupValue(DeclName name,
     auto iter = OperatorMethodDecls->find(name.getBaseName());
     if (iter != OperatorMethodDecls->end()) {
       for (auto item : *iter) {
-        auto VD = cast<ValueDecl>(getDecl(item.second));
+        Expected<Decl *> declOrError = getDeclChecked(item.second);
+        if (!declOrError) {
+          if (!getContext().LangOpts.EnableDeserializationRecovery)
+            fatal(declOrError.takeError());
+          llvm::consumeError(declOrError.takeError());
+          continue;
+        }
+        auto VD = cast<ValueDecl>(declOrError.get());
         results.push_back(VD);
       }
     }

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,94 @@
+// 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'}}
+
+  // Force a lookup of the ==
+  _ = Optional(OkayEnum.noPayload).map { $0 == .noPayload }
+}
+
+#else // TEST
+
+import Typedefs
+
+public enum BadEnum {
+  case noPayload
+  case perfectlyOkayPayload(Int)
+  case problematic(Any, WrappedInt)
+  case alsoOkay(Any, Any, Any)
+
+  static public func ==(a: BadEnum, b: BadEnum) -> Bool {
+    return false
+  }
+}
+// CHECK-LABEL: enum BadEnum {
+// CHECK-RECOVERY-NOT: enum BadEnum
+
+public enum OkayEnum {
+  case noPayload
+  case plainOldAlias(Any, UnwrappedInt)
+  case other(Int)
+
+  static public func ==(a: OkayEnum, b: OkayEnum) -> Bool {
+    return false
+  }
+}
+// CHECK-LABEL: enum OkayEnum {
+// CHECK-NEXT:   case noPayload
+// CHECK-NEXT:   case plainOldAlias(Any, UnwrappedInt)
+// CHECK-NEXT:   case other(Int)
+// CHECK-NEXT:   static func ==(a: OkayEnum, b: OkayEnum) -> Bool
+// 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:   static func ==(a: OkayEnum, b: OkayEnum) -> Bool
+// 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