Skip to content

[Serialization] Public overrides of internal decls are ignorable by clients #63068

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
Jan 17, 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
20 changes: 16 additions & 4 deletions lib/Serialization/Serialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3345,15 +3345,27 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
///
/// This should be kept conservative. Compiler crashes are still better than
/// miscompiles.
static bool overriddenDeclAffectsABI(const ValueDecl *overridden) {
static bool overriddenDeclAffectsABI(const ValueDecl *override,
const ValueDecl *overridden) {
if (!overridden)
return false;
// There's one case where we know a declaration doesn't affect the ABI of
// There's a few cases where we know a declaration doesn't affect the ABI of
// its overrides after they've been compiled: if the declaration is '@objc'
// and 'dynamic'. In that case, all accesses to the method or property will
// go through the Objective-C method tables anyway.
if (overridden->hasClangNode() || overridden->shouldUseObjCDispatch())
return false;

// In a public-override-internal case, the override doesn't have ABI
// implications.
auto isPublic = [](const ValueDecl *VD) {
return VD->getFormalAccessScope(VD->getDeclContext(),
/*treatUsableFromInlineAsPublic*/true)
.isPublic();
};
if (isPublic(override) && !isPublic(overridden))
return false;

return true;
}

Expand Down Expand Up @@ -4044,7 +4056,7 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
fn->isImplicitlyUnwrappedOptional(),
S.addDeclRef(fn->getOperatorDecl()),
S.addDeclRef(fn->getOverriddenDecl()),
overriddenDeclAffectsABI(fn->getOverriddenDecl()),
overriddenDeclAffectsABI(fn, fn->getOverriddenDecl()),
fn->getName().getArgumentNames().size() +
fn->getName().isCompoundName(),
rawAccessLevel,
Expand Down Expand Up @@ -4136,7 +4148,7 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
uint8_t(getStableAccessorKind(fn->getAccessorKind()));

bool overriddenAffectsABI =
overriddenDeclAffectsABI(fn->getOverriddenDecl());
overriddenDeclAffectsABI(fn, fn->getOverriddenDecl());

Type ty = fn->getInterfaceType();
SmallVector<IdentifierID, 4> dependencies;
Expand Down
67 changes: 67 additions & 0 deletions test/Serialization/Safety/override-internal-func.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/// Deserialization can ignore public overrides to internal methods.

// RUN: %empty-directory(%t)
// RUN: split-file %s %t

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

/// Build against the swiftmodule.
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
// RUN: -enable-deserialization-safety

/// Build against the swiftinterface.
// RUN: rm %t/Lib.swiftmodule
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
// RUN: -enable-deserialization-safety

//--- Lib.swift

open class Base {
public func publicMethod() -> Int {
return 1
}

fileprivate func fileprivateMethod() -> Int {
return 1
}

internal func internalMethod() -> Int {
return 1
}
}

open class Derived : Base {
open override func publicMethod() -> Int {
return super.publicMethod() + 1
}

open override func fileprivateMethod() -> Int {
return super.fileprivateMethod() + 1
}

open override func internalMethod() -> Int {
return super.internalMethod() + 1
}
}

//--- Client.swift

import Lib

public class OtherFinalDerived : Derived {
public override func publicMethod() -> Int {
return super.publicMethod() + 1
}

public override func fileprivateMethod() -> Int {
return super.fileprivateMethod() + 1
}

public override func internalMethod() -> Int {
return super.internalMethod() + 1
}
}