Sema: Fix source compatibility break from relaxed witness matching rules

This is fix for a source compat regression from:

commit 790625a
Author: Doug Gregor <[email protected]>
Date:   Mon Mar 19 15:29:32 2018 -0700

    Allow a witness's noescape parameter to match a requirement's escaping parameter

The regression is not severe but its easy enough to fix.

With the above change, it was possible for an optional requirement that did
not have a witness in Swift 4.1 to pick up a witness in Swift 4.2, because
the escaping/noescape mismatch prevented it from being considered in Swift 4.1.

If the new witness was not sufficiently visible, this caused a source
compatibility regression.

Work around this by discarding the witness if its not sufficiently
visible. In -swift-version 5, the hack expires, and we revert to the
stricter, more consistent behavior.

Fixes <rdar://problem/39614880>.

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: {})
+}