Merge pull request #17738 from slavapestov/optional-witness-visibility-hackaround-4.2

Sema: Fix source compatibility break from relaxed witness matching rules [4.2]

Author: slavapestov

lib/Sema/TypeCheckProtocol.cpp

@@ -2763,6 +2763,21 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
 
     case CheckKind::Access:
     case CheckKind::AccessOfSetter: {
+      // Swift 4.2 relaxed some rules for protocol witness matching.
+      //
+      // This meant that it was possible for an optional protocol requirement
+      // to have a witness where previously in Swift 4.1 it did not.
+      //
+      // Since witnesses must be as visible as the protocol, this caused a
+      // source compatibility break if the witness was not sufficiently
+      // visible.
+      //
+      // Work around this by discarding the witness if its not sufficiently
+      // visible.
+      if (!TC.Context.isSwiftVersionAtLeast(5))
+        if (requirement->getAttrs().hasAttribute<OptionalAttr>())
+          return ResolveWitnessResult::Missing;
+
       // Avoid relying on the lifetime of 'this'.
       const DeclContext *DC = this->DC;
       diagnoseOrDefer(requirement, false,

test/Compatibility/optional_visibility.swift

@@ -0,0 +1,13 @@
+// RUN: %target-typecheck-verify-swift -enable-objc-interop -swift-version 4
+
+@objc protocol Opt {
+  @objc optional func f(callback: @escaping () -> ())
+}
+
+class Conforms : Opt {
+  private func f(callback: () -> ()) {} // expected-note {{'f' declared here}}
+}
+
+func g(x: Conforms) {
+  _ = x.f(callback: {}) // expected-error {{'f' is inaccessible due to 'private' protection level}}
+}

test/decl/protocol/req/optional_visibility.swift

@@ -0,0 +1,15 @@
+// RUN: %target-typecheck-verify-swift -enable-objc-interop -swift-version 5
+
+@objc protocol Opt {
+  @objc optional func f(callback: @escaping () -> ())
+}
+
+class Conforms : Opt {
+  private func f(callback: () -> ()) {}
+  // expected-error@-1 {{method 'f(callback:)' must be declared internal because it matches a requirement in internal protocol 'Opt'}}
+  // expected-note@-2 {{mark the instance method as 'internal' to satisfy the requirement}}
+}
+
+func g(x: Conforms) {
+  _ = x.f(callback: {})
+}