Sema: Fix order dependency in @objc inference from witnessed protocol requirement

If we haven't validated the declaration yet, the 'witnesses @objc
requirement' check would immediately fail. Move the validateDecl()
call to matchWitness() to fix this.

Fixes <rdar://problem/49482328>, <https://bugs.swift.org/browse/SR-10257>.

Author: slavapestov

lib/Sema/TypeCheckProtocol.cpp

@@ -426,7 +426,6 @@ static ValueDecl *getStandinForAccessor(AbstractStorageDecl *witnessStorage,
 
 RequirementMatch
 swift::matchWitness(
-             TypeChecker &tc,
              DeclContext *dc, ValueDecl *req, ValueDecl *witness,
              llvm::function_ref<
                      std::tuple<Optional<RequirementMatch>, Type, Type>(void)> 
@@ -442,10 +441,17 @@ swift::matchWitness(
   if (req->getKind() != witness->getKind())
     return RequirementMatch(witness, MatchKind::KindConflict);
 
+  // If the witness has not been validated yet, do so now.
+  if (!witness->hasValidSignature()) {
+    auto &ctx = dc->getASTContext();
+    ctx.getLazyResolver()->resolveDeclSignature(witness);
+  }
+
   // If the witness is invalid, record that and stop now.
   if (witness->isInvalid())
     return RequirementMatch(witness, MatchKind::WitnessInvalid);
 
+  // If we're currently validating the witness, bail out.
   if (!witness->hasValidSignature())
     return RequirementMatch(witness, MatchKind::Circularity);
 
@@ -948,7 +954,7 @@ swift::matchWitness(TypeChecker &tc,
     return result;
   };
 
-  return matchWitness(tc, dc, req, witness, setup, matchTypes, finalize);
+  return matchWitness(dc, req, witness, setup, matchTypes, finalize);
 }
 
 static bool
@@ -1151,9 +1157,6 @@ bool WitnessChecker::findBestWitness(
         continue;
       }
 
-      if (!witness->hasValidSignature())
-        TC.validateDecl(witness);
-
       auto match = matchWitness(TC, ReqEnvironmentCache, Proto, conformance, DC,
                                 requirement, witness);
       if (match.isViable()) {

lib/Sema/TypeCheckProtocol.h

@@ -897,7 +897,6 @@ class AssociatedTypeInference {
 ///
 /// \returns the result of performing the match.
 RequirementMatch matchWitness(
-             TypeChecker &tc,
              DeclContext *dc, ValueDecl *req, ValueDecl *witness,
              llvm::function_ref<
                      std::tuple<Optional<RequirementMatch>, Type, Type>(void)>

lib/Sema/TypeCheckProtocolInference.cpp

@@ -756,7 +756,7 @@ AssociatedTypeInference::inferTypeWitnessesViaValueWitness(ValueDecl *req,
   // Match the witness. If we don't succeed, throw away the inference
   // information.
   // FIXME: A renamed match might be useful to retain for the failure case.
-  if (matchWitness(tc, dc, req, witness, setup, matchTypes, finalize)
+  if (matchWitness(dc, req, witness, setup, matchTypes, finalize)
           .Kind != MatchKind::ExactMatch) {
     inferred.Inferred.clear();
   }

test/decl/protocol/conforms/objc_from_witness_corner_case.swift

@@ -0,0 +1,31 @@
+// RUN: %target-swift-frontend -disable-objc-attr-requires-foundation-module -print-ast %s | %FileCheck %s
+
+// REQUIRES: objc_interop
+
+// This bug required an elaborate setup where isObjC() was checked prior
+// to validateDecl() getting called on a declaration. In this case, we
+// did not infer @objc from witnessed protocol requirements as required.
+//
+// https://bugs.swift.org/browse/SR-10257
+
+@objc public protocol P {
+  @objc optional func f()
+}
+
+public class Other {
+  // This triggers a walk over all nominals in the file, collecting
+  // @objc members into the dynamic dispatch lookup table.
+  let a = (Base() as AnyObject).g()
+}
+
+@objc public class Base : P {
+  @objc public func g() -> Int { return 0 }
+}
+
+public class D : Base {
+  // This method witnesses P.f() and so it should be @objc.
+  //
+  // CHECK-LABEL: @objc public func f()
+  public func f() {}
+}
+