Begin checking cdecl implementations

This commit diagnoses cdecl implementations with no matching imported declaration, and also runs them through the ObjCImplementationChecker. Actually testing that the ObjCImplementationChecker diagnoses various failure conditions correctly will be added in a subsequent commit.

Author: beccadax

include/swift/AST/DiagnosticsClangImporter.def

@@ -134,8 +134,9 @@ WARNING(api_pattern_attr_ignored, none,
 
 ERROR(objc_implementation_two_impls, none,
       "duplicate implementation of Objective-C %select{|category %0 on }0"
-      "class %1",
-      (Identifier, ValueDecl *))
+      "%kind1",
+      (Identifier, Decl *))
+
 NOTE(previous_objc_implementation, none,
      "previously implemented by extension here", ())
 

include/swift/AST/DiagnosticsSema.def

@@ -1729,6 +1729,10 @@ ERROR(attr_objc_implementation_category_not_found,none,
       "could not find category %0 on Objective-C class %1; make sure your "
       "umbrella or bridging header imports the header that declares it",
       (Identifier, ValueDecl*))
+ERROR(attr_objc_implementation_func_not_found,none,
+      "could not find imported function '%0' matching %kind1; make sure your "
+      "umbrella or bridging header imports the header that declares it",
+      (StringRef, ValueDecl*))
 NOTE(attr_objc_implementation_fixit_remove_category_name,none,
      "remove arguments to implement the main '@interface' for this class",
      ())

include/swift/AST/TypeCheckRequests.h

@@ -4463,7 +4463,7 @@ class SemanticBriefCommentRequest
 /// This is done on all of a class's implementations at once to improve diagnostics.
 class TypeCheckObjCImplementationRequest
     : public SimpleRequest<TypeCheckObjCImplementationRequest,
-                           evaluator::SideEffect(ExtensionDecl *),
+                           evaluator::SideEffect(Decl *),
                            RequestFlags::Cached> {
 public:
   using SimpleRequest::SimpleRequest;
@@ -4473,7 +4473,7 @@ class TypeCheckObjCImplementationRequest
 
   // Evaluation.
   evaluator::SideEffect
-  evaluate(Evaluator &evaluator, ExtensionDecl *ED) const;
+  evaluate(Evaluator &evaluator, Decl *D) const;
 
 public:
   // Separate caching.

lib/ClangImporter/ClangImporter.cpp

@@ -5783,28 +5783,100 @@ findContextInterfaceAndImplementation(DeclContext *dc) {
   return constructResult(interfaceDecl, implDecls, classDecl, categoryName);
 }
 
+static void lookupRelatedFuncs(AbstractFunctionDecl *func,
+                               SmallVectorImpl<ValueDecl *> &results) {
+  DeclName swiftName;
+  if (auto accessor = dyn_cast<AccessorDecl>(func))
+    swiftName = accessor->getStorage()->getName();
+  else
+    swiftName = func->getName();
+
+  if (auto ty = func->getDeclContext()->getSelfNominalTypeDecl()) {
+    ty->lookupQualified({ ty }, DeclNameRef(swiftName), func->getLoc(),
+                        NL_QualifiedDefault | NL_IgnoreAccessControl, results);
+  }
+  else {
+    auto mod = func->getDeclContext()->getParentModule();
+    mod->lookupQualified(mod, DeclNameRef(swiftName), func->getLoc(),
+                         NL_RemoveOverridden | NL_IgnoreAccessControl, results);
+  }
+}
+
+static ObjCInterfaceAndImplementation
+findFunctionInterfaceAndImplementation(AbstractFunctionDecl *func) {
+  if (!func)
+    return {};
+
+  // If this isn't either a clang import or an implementation, there's no point
+  // doing any work here.
+  if (!func->hasClangNode() && !func->isObjCImplementation())
+    return {};
+
+  OptionalEnum<AccessorKind> accessorKind;
+  if (auto accessor = dyn_cast<AccessorDecl>(func))
+    accessorKind = accessor->getAccessorKind();
+
+  StringRef clangName = func->getCDeclName();
+  if (clangName.empty())
+    return {};
+
+  SmallVector<ValueDecl *, 4> results;
+  lookupRelatedFuncs(func, results);
+
+  // Classify the `results` as either the interface or an implementation.
+  // (Multiple implementations are invalid but utterable.)
+  Decl *interface = nullptr;
+  TinyPtrVector<Decl *> impls;
+
+  for (ValueDecl *result : results) {
+    AbstractFunctionDecl *resultFunc = nullptr;
+    if (accessorKind) {
+      if (auto resultStorage = dyn_cast<AbstractStorageDecl>(result))
+        resultFunc = resultStorage->getAccessor(*accessorKind);
+    }
+    else
+      resultFunc = dyn_cast<AbstractFunctionDecl>(result);
+
+    if (!resultFunc)
+      continue;
+
+    if (resultFunc->getCDeclName() != clangName)
+      continue;
+
+    if (resultFunc->hasClangNode()) {
+      if (interface) {
+        // This clang name is overloaded. That should only happen with C++
+        // functions/methods, which aren't currently supported.
+        return {};
+      }
+      interface = result;
+    } else if (resultFunc->isObjCImplementation()) {
+      impls.push_back(result);
+    }
+  }
+
+  // If we found enough decls to construct a result, `func` should be among them
+  // somewhere.
+  assert(interface == nullptr || impls.empty() ||
+         interface == func || llvm::is_contained(impls, func));
+
+  return constructResult(interface, impls, interface,
+                         /*categoryName=*/Identifier());
+}
+
 ObjCInterfaceAndImplementation ObjCInterfaceAndImplementationRequest::
 evaluate(Evaluator &evaluator, Decl *decl) const {
-  // These have direct links to their counterparts through the
+  // Types and extensions have direct links to their counterparts through the
   // `@_objcImplementation` attribute. Let's resolve that.
   // (Also directing nulls here, where they'll early-return.)
   if (auto ty = dyn_cast_or_null<NominalTypeDecl>(decl))
     return findContextInterfaceAndImplementation(ty);
   else if (auto ext = dyn_cast<ExtensionDecl>(decl))
     return findContextInterfaceAndImplementation(ext);
+  // Abstract functions have to be matched through their @_cdecl attributes.
+  else if (auto func = dyn_cast<AbstractFunctionDecl>(decl))
+    return findFunctionInterfaceAndImplementation(func);
 
-  // Anything else is resolved by first locating the context's interface and
-  // impl, then matching it to its counterpart. (Instead of calling
-  // `findContextInterfaceAndImplementation()` directly, we'll use the request
-  // recursively to take advantage of caching.)
-  auto contextDecl = decl->getDeclContext()->getAsDecl();
-  if (!contextDecl)
-    return {};
-
-  ObjCInterfaceAndImplementationRequest req(contextDecl);
-  /*auto contextPair =*/ evaluateOrDefault(evaluator, req, {});
-
-  // TODO: Implement member matching.
   return {};
 }
 

lib/Sema/TypeCheckAttr.cpp

@@ -1554,6 +1554,14 @@ visitObjCImplementationAttr(ObjCImplementationAttr *attr) {
 
       attr->setCategoryNameInvalid();
     }
+
+    // FIXME: if (AFD->getCDeclName().empty())
+
+    if (!AFD->getImplementedObjCDecl()) {
+      diagnose(attr->getLocation(),
+               diag::attr_objc_implementation_func_not_found,
+               AFD->getCDeclName(), AFD);
+    }
   }
 }
 

lib/Sema/TypeCheckDeclObjC.cpp

@@ -2775,12 +2775,12 @@ bool swift::diagnoseObjCUnsatisfiedOptReqConflicts(SourceFile &sf) {
   return anyDiagnosed;
 }
 
-void TypeChecker::checkObjCImplementation(ExtensionDecl *ED) {
-  if (!ED->getImplementedObjCDecl())
+void TypeChecker::checkObjCImplementation(Decl *D) {
+  if (!D->getImplementedObjCDecl())
     return;
 
-  evaluateOrDefault(ED->getASTContext().evaluator,
-                    TypeCheckObjCImplementationRequest{ED},
+  evaluateOrDefault(D->getASTContext().evaluator,
+                    TypeCheckObjCImplementationRequest{D},
                     evaluator::SideEffect());
 }
 
@@ -2901,10 +2901,20 @@ class ObjCImplementationChecker {
   llvm::SmallDenseMap<ValueDecl *, ObjCSelector, 16> unmatchedCandidates;
 
 public:
-  ObjCImplementationChecker(ExtensionDecl *ext)
-      : diags(ext->getASTContext().Diags)
+  ObjCImplementationChecker(Decl *D)
+      : diags(D->getASTContext().Diags)
   {
-    assert(!ext->hasClangNode() && "passed interface, not impl, to checker");
+    assert(!D->hasClangNode() && "passed interface, not impl, to checker");
+
+    if (auto func = dyn_cast<AbstractFunctionDecl>(D)) {
+      addCandidate(D);
+      addRequirement(D->getImplementedObjCDecl());
+
+      return;
+    }
+
+    // Otherwise this must be an extension.
+    auto ext = cast<ExtensionDecl>(D);
 
     // Conformances are declared exclusively in the interface, so diagnose any
     // in the implementation right away.
@@ -3083,12 +3093,20 @@ class ObjCImplementationChecker {
   }
 
   static ObjCSelector getExplicitObjCName(ValueDecl *VD) {
+    if (auto cdeclAttr = VD->getAttrs().getAttribute<CDeclAttr>()) {
+      auto ident = VD->getASTContext().getIdentifier(cdeclAttr->Name);
+      return ObjCSelector(VD->getASTContext(), 0, { ident });
+    }
     if (auto objcAttr = VD->getAttrs().getAttribute<ObjCAttr>())
       return objcAttr->getName().value_or(ObjCSelector());
     return ObjCSelector();
   }
 
   static llvm::Optional<ObjCSelector> getObjCName(ValueDecl *VD) {
+    if (!VD->getCDeclName().empty()) {
+      auto ident = VD->getASTContext().getIdentifier(VD->getCDeclName());
+      return ObjCSelector(VD->getASTContext(), 0, { ident });
+    }
     return VD->getObjCRuntimeName();
   }
 
@@ -3401,8 +3419,11 @@ class ObjCImplementationChecker {
     if (req->isInstanceMember() != cand->isInstanceMember())
       return MatchOutcome::WrongStaticness;
 
-    if (cand->getDeclContext()->getImplementedObjCContext()
-          != req->getDeclContext())
+    // Check only applies to members of implementations, not implementations in
+    // their own right.
+    if (!cand->isObjCImplementation()
+          && cand->getDeclContext()->getImplementedObjCContext()
+                 != req->getDeclContext())
       return MatchOutcome::WrongCategory;
 
     if (cand->getKind() != req->getKind())
@@ -3648,8 +3669,8 @@ class ObjCImplementationChecker {
 }
 
 evaluator::SideEffect TypeCheckObjCImplementationRequest::
-evaluate(Evaluator &evaluator, ExtensionDecl *ED) const {
-  PrettyStackTraceDecl trace("checking member implementations of", ED);
+evaluate(Evaluator &evaluator, Decl *D) const {
+  PrettyStackTraceDecl trace("checking member implementations of", D);
 
   // FIXME: Because we check extension-by-extension, candidates and requirements
   // from different extensions are never compared, so we never get an
@@ -3658,7 +3679,7 @@ evaluate(Evaluator &evaluator, ExtensionDecl *ED) const {
   // candidates we considered to all unmatched requirements in the module, and
   // vice versa. The tricky bit is making sure we only diagnose for candidates
   // and requirements in our primary files!
-  ObjCImplementationChecker checker(ED);
+  ObjCImplementationChecker checker(D);
 
   checker.matchRequirements();
   checker.diagnoseUnmatchedCandidates();

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -3603,6 +3603,8 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
         reason.setAttrInvalid();
       }
     }
+
+    TypeChecker::checkObjCImplementation(FD);
   }
 
   void visitModuleDecl(ModuleDecl *) { }
@@ -4045,6 +4047,8 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
 
     checkDefaultArguments(CD->getParameters());
     checkVariadicParameters(CD->getParameters(), CD);
+
+    TypeChecker::checkObjCImplementation(CD);
   }
 
   void visitDestructorDecl(DestructorDecl *DD) {

lib/Sema/TypeChecker.h

@@ -657,8 +657,8 @@ void checkDeclCircularity(NominalTypeDecl *decl);
 /// Type check whether an extension matches its Objective-C interface, if it
 /// has one.
 ///
-/// \param ED The extension to check.
-void checkObjCImplementation(ExtensionDecl *ED);
+/// \param D The declaration to check.
+void checkObjCImplementation(Decl *D);
 
 /// Type check whether the given switch statement exhaustively covers
 /// its domain.

test/decl/ext/objc_implementation.swift

@@ -476,7 +476,7 @@ func CImplFunc2(_: Int32) {
 
 @_objcImplementation @_cdecl("CImplFuncMissing")
 func CImplFuncMissing(_: Int32) {
-  // FIXME: expected-DISABLED-error@-1 {{fnord}}
+  // expected-error@-2 {{could not find imported function 'CImplFuncMissing' matching global function 'CImplFuncMissing'; make sure your umbrella or bridging header imports the header that declares it}}
 }
 
 func usesAreNotAmbiguous(obj: ObjCClass) {