Skip to content

Serialization: Refine safety checks for extensions/protocols #68205

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions lib/Serialization/Serialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
14 changes: 8 additions & 6 deletions test/Serialization/Safety/indirect-conformance.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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'
8 changes: 4 additions & 4 deletions test/Serialization/Safety/unsafe-decls.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
16 changes: 8 additions & 8 deletions test/Serialization/Safety/unsafe-extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down