Skip to content

[Serialization] Keep indirect conformances knowledge with safety #65267

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
merged 1 commit into from
Apr 19, 2023
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: 5 additions & 5 deletions lib/Serialization/Serialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3124,14 +3124,11 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
if (hasSafeMembers)
return true;

// We can mark the extension unsafe only if it has no public
// We can mark the extension unsafe only if it has no public
// conformances.
auto protocols = ext->getLocalProtocols(
ConformanceLookupKind::OnlyExplicit);
bool hasSafeConformances = std::any_of(protocols.begin(),
protocols.end(),
isDeserializationSafe);
if (hasSafeConformances)
if (!protocols.empty())
return true;

// Truly empty extensions are safe, it may happen in swiftinterfaces.
Expand All @@ -3141,6 +3138,9 @@ 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
52 changes: 52 additions & 0 deletions test/Serialization/Safety/indirect-conformance.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// RUN: %empty-directory(%t)
// RUN: split-file %s %t

// REQUIRES: asserts

//--- Lib.swift

// RUN: %target-swift-frontend -emit-module %t/Lib.swift \
// RUN: -enable-library-evolution -swift-version 5 \
// RUN: -emit-module-path %t/Lib.swiftmodule \
// RUN: -emit-module-interface-path %t/Lib.swiftinterface

// RUN: cat %t/Lib.swiftinterface | %FileCheck %t/Lib.swift

public protocol PublicProtocol : AnyObject {}
// CHECK: public protocol PublicProtocol : AnyObject

protocol InternalProtocol: PublicProtocol {}
// CHECK-NOT: InternalProtocol

public class IndirectConformant {
public init() {}
}
extension IndirectConformant: InternalProtocol {}
// CHECK: extension Lib.IndirectConformant : Lib.PublicProtocol {}

//--- Client.swift

/// Works without safety.
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t

/// Works with safety.
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
// RUN: -enable-deserialization-safety \
// RUN: -Xllvm -debug-only=Serialization 2>&1 \
// RUN: | %FileCheck %t/Client.swift

/// Works with swiftinterface.
// RUN: rm %t/Lib.swiftmodule
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t

import Lib

func requireConformanceToPublicProtocol(_ a: PublicProtocol) {}
requireConformanceToPublicProtocol(IndirectConformant())

/// 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'
6 changes: 3 additions & 3 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, unsafe: 'InternalProto'
// SAFETY-INTERNAL: Serialization safety, safe: 'InternalProto'
// NO-SAFETY-INTERNAL: Serialization safety, safe: 'InternalProto'
private protocol PrivateProto {}
// SAFETY-PRIVATE: Serialization safety, unsafe: 'PrivateProto'
// SAFETY-PRIVATE: Serialization safety, safe: 'PrivateProto'
fileprivate protocol FileprivateProto {}
// SAFETY-PRIVATE: Serialization safety, unsafe: 'FileprivateProto'
// SAFETY-PRIVATE: Serialization safety, safe: 'FileprivateProto'

internal struct InternalStruct : PublicProto {
// SAFETY-INTERNAL: Serialization safety, unsafe: 'InternalStruct'
Expand Down
12 changes: 6 additions & 6 deletions test/Serialization/Safety/unsafe-extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ extension ExtendedPublic : PublicProto {

/// Internal
internal protocol InternalProto {}
// SAFETY-INTERNAL: Serialization safety, unsafe: 'InternalProto'
// SAFETY-INTERNAL: Serialization safety, safe: 'InternalProto'
// NO-SAFETY-INTERNAL: Serialization safety, safe: 'InternalProto'
internal struct ExtendedInternal {}
// SAFETY-INTERNAL: Serialization safety, unsafe: 'ExtendedInternal'
Expand All @@ -63,7 +63,7 @@ extension ExtendedInternal : InternalProto {}

/// Private
private protocol PrivateProto {}
// SAFETY-PRIVATE: Serialization safety, unsafe: 'PrivateProto'
// SAFETY-PRIVATE: Serialization safety, safe: '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, unsafe: 'FileprivateProto'
// SAFETY-PRIVATE: Serialization safety, safe: '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, unsafe: 'extension ExtendedPublic'
// SAFETY-INTERNAL: Serialization safety, safe: 'extension ExtendedPublic'
// NO-SAFETY-INTERNAL: Serialization safety, safe: 'extension ExtendedPublic'
}
extension ExtendedPublic : PrivateProto {
// SAFETY-PRIVATE: Serialization safety, unsafe: 'extension ExtendedPublic'
// SAFETY-PRIVATE: Serialization safety, safe: 'extension ExtendedPublic'
}
extension ExtendedPublic : FileprivateProto {
// SAFETY-PRIVATE: Serialization safety, unsafe: 'extension ExtendedPublic'
// SAFETY-PRIVATE: Serialization safety, safe: 'extension ExtendedPublic'
}

extension ExtendedPublic {
Expand Down