Merge pull request #62980 from xymus/recover-opaque-types

[Serialization] Recover in opaque type deserialization logic on XRef errors

Author: xymus

lib/Serialization/Deserialization.cpp

@@ -158,6 +158,8 @@ void InvalidRecordKindError::anchor() {}
 const char UnsafeDeserializationError::ID = '\0';
 void UnsafeDeserializationError::anchor() {}
 
+static llvm::Error consumeErrorIfXRefNonLoadedModule(llvm::Error &&error);
+
 /// Skips a single record in the bitstream.
 ///
 /// Destroys the stream position if the next entry is not a record.
@@ -1360,7 +1362,12 @@ ModuleFile::getSubstitutionMapChecked(serialization::SubstitutionMapID id) {
   SmallVector<Type, 4> replacementTypes;
   replacementTypes.reserve(replacementTypeIDs.size());
   for (auto typeID : replacementTypeIDs) {
-    replacementTypes.push_back(getType(typeID));
+    auto typeOrError = getTypeChecked(typeID);
+    if (!typeOrError) {
+      consumeError(typeOrError.takeError());
+      continue;
+    }
+    replacementTypes.push_back(typeOrError.get());
   }
 
   // Read the conformances.
@@ -3634,7 +3641,10 @@ class DeclDeserializer {
     if (declOrOffset.isComplete())
       return declOrOffset;
 
-    const auto resultType = MF.getType(resultInterfaceTypeID);
+    auto resultTypeOrError = MF.getTypeChecked(resultInterfaceTypeID);
+    if (!resultTypeOrError)
+      return resultTypeOrError.takeError();
+    const auto resultType = resultTypeOrError.get();
     if (declOrOffset.isComplete())
       return declOrOffset;
 
@@ -3707,9 +3717,13 @@ class DeclDeserializer {
                               std::move(needsNewVTableEntry));
 
     if (opaqueReturnTypeID) {
+      auto declOrError = MF.getDeclChecked(opaqueReturnTypeID);
+      if (!declOrError)
+        return declOrError.takeError();
+
       ctx.evaluator.cacheOutput(
           OpaqueResultTypeRequest{fn},
-          cast<OpaqueTypeDecl>(MF.getDecl(opaqueReturnTypeID)));
+          cast<OpaqueTypeDecl>(declOrError.get()));
     }
 
     if (!isAccessor)
@@ -3850,26 +3864,31 @@ class DeclDeserializer {
       opaqueDecl->setGenericSignature(GenericSignature());
     if (underlyingTypeSubsID) {
       auto subMapOrError = MF.getSubstitutionMapChecked(underlyingTypeSubsID);
-      if (!subMapOrError)
-        return subMapOrError.takeError();
-
-      // Check whether there are any conditionally available substitutions.
-      // If there are, it means that "unique" we just read is a universally
-      // available substitution.
-      SmallVector<OpaqueTypeDecl::ConditionallyAvailableSubstitutions *>
-          limitedAvailability;
+      if (!subMapOrError) {
+        // If the underlying type references internal details, ignore it.
+        auto unconsumedError =
+          consumeErrorIfXRefNonLoadedModule(subMapOrError.takeError());
+        if (unconsumedError)
+          return std::move(unconsumedError);
+      } else {
+        // Check whether there are any conditionally available substitutions.
+        // If there are, it means that "unique" we just read is a universally
+        // available substitution.
+        SmallVector<OpaqueTypeDecl::ConditionallyAvailableSubstitutions *>
+            limitedAvailability;
 
-      deserializeConditionalSubstitutions(limitedAvailability);
+        deserializeConditionalSubstitutions(limitedAvailability);
 
-      if (limitedAvailability.empty()) {
-        opaqueDecl->setUniqueUnderlyingTypeSubstitutions(subMapOrError.get());
-      } else {
-        limitedAvailability.push_back(
-            OpaqueTypeDecl::ConditionallyAvailableSubstitutions::get(
-                ctx, {{VersionRange::empty(), /*unavailability=*/false}},
-                subMapOrError.get()));
+        if (limitedAvailability.empty()) {
+          opaqueDecl->setUniqueUnderlyingTypeSubstitutions(subMapOrError.get());
+        } else {
+          limitedAvailability.push_back(
+              OpaqueTypeDecl::ConditionallyAvailableSubstitutions::get(
+                  ctx, {{VersionRange::empty(), /*unavailability=*/false}},
+                  subMapOrError.get()));
 
-        opaqueDecl->setConditionallyAvailableSubstitutions(limitedAvailability);
+          opaqueDecl->setConditionallyAvailableSubstitutions(limitedAvailability);
+        }
       }
     }
     return opaqueDecl;

test/Serialization/Recovery/implementation-only-opaque-type.swift

@@ -0,0 +1,48 @@
+// Test deserialization when the underlying type of an opaque type
+// depends on an implementation-only import.
+
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+
+// RUN: %target-swift-frontend -emit-module -enable-library-evolution \
+// RUN:   -emit-module-path=%t/BaseLib.swiftmodule %t/BaseLib.swift
+// RUN: %target-swift-frontend -emit-module -enable-library-evolution \
+// RUN:   -emit-module-path=%t/HiddenLib.swiftmodule %t/HiddenLib.swift -I %t
+// RUN: %target-swift-frontend -emit-module -enable-library-evolution \
+// RUN:   -emit-module-path=%t/Lib.swiftmodule %t/Lib.swift -I %t
+
+// RUN: %target-swift-frontend -I %t -emit-ir %t/Client.swift
+
+//--- BaseLib.swift
+
+public protocol Proto { }
+
+//--- HiddenLib.swift
+
+import BaseLib
+
+public struct HiddenType : Proto {
+    public init() {}
+}
+
+//--- Lib.swift
+
+import BaseLib
+@_implementationOnly import HiddenLib
+
+@available(SwiftStdlib 5.1, *) // for the `some` keyword.
+public struct PublicStruct {
+    public init() {}
+    public func foo() -> some Proto {
+        return HiddenType()
+    }
+}
+
+//--- Client.swift
+
+import Lib
+
+if #available(SwiftStdlib 5.1, *) {
+  let s = PublicStruct()
+  let r = s.foo()
+}