Merge pull request #68205 from tshortli/conformance-deserialization-safety-refinement

Serialization: Refine safety checks for extensions/protocols

Author: tshortli

lib/Serialization/Serialization.cpp

@@ -3289,9 +3289,10 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
 
       // We can mark the extension unsafe only if it has no public
       // conformances.
-      auto protocols = ext->getLocalProtocols(
-                                        ConformanceLookupKind::OnlyExplicit);
-      if (!protocols.empty())
+      auto protocols = ext->getLocalProtocols(ConformanceLookupKind::All);
+      bool hasSafeConformances = std::any_of(protocols.begin(), protocols.end(),
+                                             isDeserializationSafe);
+      if (hasSafeConformances)
         return true;
 
       // Truly empty extensions are safe, it may happen in swiftinterfaces.
@@ -3301,9 +3302,6 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
       return false;
     }
 
-    if (isa<ProtocolDecl>(decl))
-      return true;
-
     auto value = cast<ValueDecl>(decl);
 
     // A decl is safe if formally accessible publicly.

test/Serialization/Safety/indirect-conformance.swift

@@ -12,18 +12,22 @@
 
 // RUN: cat %t/Lib.swiftinterface | %FileCheck %t/Lib.swift
 
-public protocol PublicProtocol : AnyObject {}
-// CHECK: public protocol PublicProtocol : AnyObject
+public protocol PublicProtocol {}
+// CHECK: public protocol PublicProtocol
 
 protocol InternalProtocol: PublicProtocol {}
 // CHECK-NOT: InternalProtocol
 
 public class IndirectConformant {
     public init() {}
 }
+
 extension IndirectConformant: InternalProtocol {}
 // CHECK: extension Lib.IndirectConformant : Lib.PublicProtocol {}
 
+extension String: InternalProtocol {}
+// CHECK: extension Swift.String : Lib.PublicProtocol {}
+
 //--- Client.swift
 
 /// Works without safety.
@@ -43,10 +47,8 @@ import Lib
 
 func requireConformanceToPublicProtocol(_ a: PublicProtocol) {}
 requireConformanceToPublicProtocol(IndirectConformant())
+requireConformanceToPublicProtocol("string")
 
-/// Deserialization safety should keep the original chain. We're mostly
-/// documenting the current safety implementation details here, if we can get
-/// without deserializing 'InternalProtocol' it would be even better.
 // CHECK: Deserialized: 'IndirectConformant'
 // CHECK: Deserialized: 'PublicProtocol'
-// CHECK: Deserialized: 'InternalProtocol'
+// CHECK-NOT: Deserialized: 'InternalProtocol'

test/Serialization/Safety/unsafe-decls.swift

@@ -34,12 +34,12 @@
 public protocol PublicProto {}
 // SAFETY-PRIVATE: Serialization safety, safe: 'PublicProto'
 internal protocol InternalProto {}
-// SAFETY-INTERNAL: Serialization safety, safe: 'InternalProto'
-// NO-SAFETY-INTERNAL: Serialization safety, safe: 'InternalProto'
+// SAFETY-INTERNAL: Serialization safety, unsafe: 'InternalProto'
+// NO-SAFETY-INTERNAL: Serialization safety, unsafe: 'InternalProto'
 private protocol PrivateProto {}
-// SAFETY-PRIVATE: Serialization safety, safe: 'PrivateProto'
+// SAFETY-PRIVATE: Serialization safety, unsafe: 'PrivateProto'
 fileprivate protocol FileprivateProto {}
-// SAFETY-PRIVATE: Serialization safety, safe: 'FileprivateProto'
+// SAFETY-PRIVATE: Serialization safety, unsafe: 'FileprivateProto'
 
 internal struct InternalStruct : PublicProto {
 // SAFETY-INTERNAL: Serialization safety, unsafe: 'InternalStruct'

test/Serialization/Safety/unsafe-extensions.swift

@@ -47,8 +47,8 @@ extension ExtendedPublic : PublicProto {
 
 /// Internal
 internal protocol InternalProto {}
-// SAFETY-INTERNAL: Serialization safety, safe: 'InternalProto'
-// NO-SAFETY-INTERNAL: Serialization safety, safe: 'InternalProto'
+// SAFETY-INTERNAL: Serialization safety, unsafe: 'InternalProto'
+// NO-SAFETY-INTERNAL: Serialization safety, unsafe: 'InternalProto'
 internal struct ExtendedInternal {}
 // SAFETY-INTERNAL: Serialization safety, unsafe: 'ExtendedInternal'
 // NO-SAFETY-INTERNAL: Serialization safety, safe: 'ExtendedInternal'
@@ -63,7 +63,7 @@ extension ExtendedInternal : InternalProto {}
 
 /// Private
 private protocol PrivateProto {}
-// SAFETY-PRIVATE: Serialization safety, safe: 'PrivateProto'
+// SAFETY-PRIVATE: Serialization safety, unsafe: 'PrivateProto'
 private struct ExtendedPrivate {}
 // SAFETY-PRIVATE: Serialization safety, unsafe: 'ExtendedPrivate'
 extension ExtendedPrivate {
@@ -75,7 +75,7 @@ extension ExtendedPrivate : PrivateProto {}
 
 /// Fileprivate
 private protocol FileprivateProto {}
-// SAFETY-PRIVATE: Serialization safety, safe: 'FileprivateProto'
+// SAFETY-PRIVATE: Serialization safety, unsafe: 'FileprivateProto'
 private struct ExtendedFileprivate {}
 // SAFETY-PRIVATE: Serialization safety, unsafe: 'ExtendedFileprivate'
 extension ExtendedFileprivate {
@@ -87,14 +87,14 @@ extension ExtendedFileprivate : FileprivateProto {}
 
 /// Back to public
 extension ExtendedPublic : InternalProto {
-// SAFETY-INTERNAL: Serialization safety, safe: 'extension ExtendedPublic'
-// NO-SAFETY-INTERNAL: Serialization safety, safe: 'extension ExtendedPublic'
+// SAFETY-INTERNAL: Serialization safety, unsafe: 'extension ExtendedPublic'
+// NO-SAFETY-INTERNAL: Serialization safety, unsafe: 'extension ExtendedPublic'
 }
 extension ExtendedPublic : PrivateProto {
-// SAFETY-PRIVATE: Serialization safety, safe: 'extension ExtendedPublic'
+// SAFETY-PRIVATE: Serialization safety, unsafe: 'extension ExtendedPublic'
 }
 extension ExtendedPublic : FileprivateProto {
-// SAFETY-PRIVATE: Serialization safety, safe: 'extension ExtendedPublic'
+// SAFETY-PRIVATE: Serialization safety, unsafe: 'extension ExtendedPublic'
 }
 
 extension ExtendedPublic {