Merge pull request #63068 from xymus/serial-pub-override

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

Author: xymus

lib/Serialization/Serialization.cpp

@@ -3368,15 +3368,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;
   }
 
@@ -4067,7 +4079,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,
@@ -4159,7 +4171,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;

test/Serialization/Safety/override-internal-func.swift

@@ -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
+  }
+}