Merge pull request #59479 from beccadax/you-completion-me

beccadax · web-flow · commit a4a6d0943a33 · 2022-06-17T09:58:44.000-07:00
Allow @objc protocols to declare async alternatives
diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h
@@ -6757,7 +6757,7 @@ class AbstractFunctionDecl : public GenericContext, public ValueDecl {
   bool hasDynamicSelfResult() const;
 
   /// The async function marked as the alternative to this function, if any.
-  AbstractFunctionDecl *getAsyncAlternative() const;
+  AbstractFunctionDecl *getAsyncAlternative(bool isKnownObjC = false) const;
 
   /// If \p asyncAlternative is set, then compare its parameters to this
   /// (presumed synchronous) function's parameters to find the index of the
diff --git a/include/swift/AST/ModuleLoader.h b/include/swift/AST/ModuleLoader.h
@@ -226,11 +226,11 @@ class ModuleLoader {
   virtual void loadExtensions(NominalTypeDecl *nominal,
                               unsigned previousGeneration) { }
 
-  /// Load the methods within the given class that produce
+  /// Load the methods within the given type that produce
   /// Objective-C class or instance methods with the given selector.
   ///
-  /// \param classDecl The class in which we are searching for @objc methods.
-  /// The search only considers this class and its extensions; not any
+  /// \param typeDecl The type in which we are searching for @objc methods.
+  /// The search only considers this type and its extensions; not any
   /// superclasses.
   ///
   /// \param selector The selector to search for.
@@ -246,7 +246,7 @@ class ModuleLoader {
   /// selector and are instance/class methods as requested. This list will be
   /// extended with any methods found in subsequent generations.
   virtual void loadObjCMethods(
-                 ClassDecl *classDecl,
+                 NominalTypeDecl *typeDecl,
                  ObjCSelector selector,
                  bool isInstanceMethod,
                  unsigned previousGeneration,
diff --git a/include/swift/AST/SourceFile.h b/include/swift/AST/SourceFile.h
@@ -17,6 +17,7 @@
 #include "swift/AST/Import.h"
 #include "swift/AST/SynthesizedFileUnit.h"
 #include "swift/Basic/Debug.h"
+#include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
 
@@ -243,11 +244,20 @@ class SourceFile final : public FileUnit {
   std::vector<ObjCUnsatisfiedOptReq> ObjCUnsatisfiedOptReqs;
 
   /// A selector that is used by two different declarations in the same class.
-  /// Fields: typeDecl, selector, isInstanceMethod.
-  using ObjCMethodConflict = std::tuple<NominalTypeDecl *, ObjCSelector, bool>;
+  struct ObjCMethodConflict {
+    NominalTypeDecl *typeDecl;
+    ObjCSelector selector;
+    bool isInstanceMethod;
+
+    ObjCMethodConflict(NominalTypeDecl *typeDecl, ObjCSelector selector,
+                       bool isInstanceMethod)
+        : typeDecl(typeDecl), selector(selector),
+          isInstanceMethod(isInstanceMethod)
+    {}
+  };
 
   /// List of Objective-C member conflicts we have found during type checking.
-  std::vector<ObjCMethodConflict> ObjCMethodConflicts;
+  llvm::SetVector<ObjCMethodConflict> ObjCMethodConflicts;
 
   /// List of attributes added by access notes, used to emit remarks for valid
   /// ones.
@@ -636,4 +646,30 @@ inline void simple_display(llvm::raw_ostream &out, const SourceFile *SF) {
 }
 } // end namespace swift
 
+namespace llvm {
+
+template<>
+struct DenseMapInfo<swift::SourceFile::ObjCMethodConflict> {
+  using ObjCMethodConflict = swift::SourceFile::ObjCMethodConflict;
+
+  static inline ObjCMethodConflict getEmptyKey() {
+    return ObjCMethodConflict(nullptr, {}, false);
+  }
+  static inline ObjCMethodConflict getTombstoneKey() {
+    return ObjCMethodConflict(nullptr, {}, true);
+  }
+  static inline unsigned getHashValue(ObjCMethodConflict a) {
+    return hash_combine(hash_value(a.typeDecl),
+                  DenseMapInfo<swift::ObjCSelector>::getHashValue(a.selector),
+                  hash_value(a.isInstanceMethod));
+  }
+  static bool isEqual(ObjCMethodConflict a, ObjCMethodConflict b) {
+    return a.typeDecl == b.typeDecl && a.selector == b.selector &&
+           a.isInstanceMethod == b.isInstanceMethod;
+  }
+};
+
+}
+
+
 #endif
diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h
@@ -3441,7 +3441,8 @@ class ConditionalRequirementsRequest
 
 class RenamedDeclRequest
     : public SimpleRequest<RenamedDeclRequest,
-                           ValueDecl *(const ValueDecl *, const AvailableAttr *),
+                           ValueDecl *(const ValueDecl *, const AvailableAttr *,
+                                       bool isKnownObjC),
                            RequestFlags::Cached> {
 public:
   using SimpleRequest::SimpleRequest;
@@ -3450,7 +3451,7 @@ class RenamedDeclRequest
   friend SimpleRequest;
 
   ValueDecl *evaluate(Evaluator &evaluator, const ValueDecl *attached,
-                      const AvailableAttr *attr) const;
+                      const AvailableAttr *attr, bool isKnownObjC) const;
 
 public:
   bool isCached() const { return true; }
diff --git a/include/swift/AST/TypeCheckerTypeIDZone.def b/include/swift/AST/TypeCheckerTypeIDZone.def
@@ -390,7 +390,7 @@ SWIFT_REQUEST(TypeChecker, GetImplicitSendableRequest,
               ProtocolConformance *(NominalTypeDecl *),
               Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, RenamedDeclRequest,
-              ValueDecl *(const ValueDecl *),
+              ValueDecl *(const ValueDecl *, const AvailableAttr *, bool),
               Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, ClosureEffectsRequest,
               FunctionType::ExtInfo(ClosureExpr *),
diff --git a/include/swift/ClangImporter/ClangImporter.h b/include/swift/ClangImporter/ClangImporter.h
@@ -309,7 +309,7 @@ class ClangImporter final : public ClangModuleLoader {
                               unsigned previousGeneration) override;
 
   virtual void loadObjCMethods(
-                 ClassDecl *classDecl,
+                 NominalTypeDecl *typeDecl,
                  ObjCSelector selector,
                  bool isInstanceMethod,
                  unsigned previousGeneration,
diff --git a/include/swift/Sema/SourceLoader.h b/include/swift/Sema/SourceLoader.h
@@ -85,7 +85,7 @@ class SourceLoader : public ModuleLoader {
                               unsigned previousGeneration) override;
 
   virtual void loadObjCMethods(
-                 ClassDecl *classDecl,
+                 NominalTypeDecl *typeDecl,
                  ObjCSelector selector,
                  bool isInstanceMethod,
                  unsigned previousGeneration,
diff --git a/include/swift/Serialization/SerializedModuleLoader.h b/include/swift/Serialization/SerializedModuleLoader.h
@@ -190,7 +190,7 @@ class SerializedModuleLoaderBase : public ModuleLoader {
                               unsigned previousGeneration) override;
 
   virtual void loadObjCMethods(
-                 ClassDecl *classDecl,
+                 NominalTypeDecl *typeDecl,
                  ObjCSelector selector,
                  bool isInstanceMethod,
                  unsigned previousGeneration,
diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp
@@ -1919,24 +1919,12 @@ void ASTContext::loadObjCMethods(
   PrettyStackTraceSelector stackTraceSelector("looking for", selector);
   PrettyStackTraceDecl stackTraceDecl("...in", tyDecl);
 
-  // @objc protocols cannot have @objc extension members, so if we've recorded
-  // everything in the protocol definition, we've recorded everything. And we
-  // only ever use the ObjCSelector version of `NominalTypeDecl::lookupDirect()`
-  // on protocols in primary file typechecking, so we don't care about protocols
-  // that need to be loaded from files.
-  // TODO: Rework `ModuleLoader::loadObjCMethods()` to support protocols too if
-  //       selector-based `NominalTypeDecl::lookupDirect()` ever needs to work
-  //       in more situations.
-  ClassDecl *classDecl = dyn_cast<ClassDecl>(tyDecl);
-  if (!classDecl)
-    return;
-
   for (auto &loader : getImpl().ModuleLoaders) {
     // Ignore the Clang importer if we've been asked for Swift-only results.
     if (swiftOnly && loader.get() == getClangModuleLoader())
       continue;
 
-    loader->loadObjCMethods(classDecl, selector, isInstanceMethod,
+    loader->loadObjCMethods(tyDecl, selector, isInstanceMethod,
                             previousGeneration, methods);
   }
 }
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -7627,7 +7627,8 @@ bool AbstractFunctionDecl::hasDynamicSelfResult() const {
   return isa<ConstructorDecl>(this);
 }
 
-AbstractFunctionDecl *AbstractFunctionDecl::getAsyncAlternative() const {
+AbstractFunctionDecl *
+AbstractFunctionDecl::getAsyncAlternative(bool isKnownObjC) const {
   // Async functions can't have async alternatives
   if (hasAsync())
     return nullptr;
@@ -7651,7 +7652,8 @@ AbstractFunctionDecl *AbstractFunctionDecl::getAsyncAlternative() const {
   }
 
   auto *renamedDecl = evaluateOrDefault(
-      getASTContext().evaluator, RenamedDeclRequest{this, avAttr}, nullptr);
+      getASTContext().evaluator, RenamedDeclRequest{this, avAttr, isKnownObjC},
+      nullptr);
   auto *alternative = dyn_cast_or_null<AbstractFunctionDecl>(renamedDecl);
   if (!alternative || !alternative->hasAsync())
     return nullptr;
diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp
@@ -1310,7 +1310,24 @@ class swift::ObjCMethodLookupTable
         : public llvm::DenseMap<std::pair<ObjCSelector, char>,
                                 StoredObjCMethods>,
           public ASTAllocated<ObjCMethodLookupTable>
-{};
+{
+  SWIFT_DEBUG_DUMP {
+    llvm::errs() << "ObjCMethodLookupTable:\n";
+    for (auto pair : *this) {
+      auto selector = pair.getFirst().first;
+      auto isInstanceMethod = pair.getFirst().second;
+      auto &methods = pair.getSecond();
+
+      llvm::errs() << "  \"" << (isInstanceMethod ? "-" : "+") << selector
+                   << "\":\n";
+      for (auto method : methods.Methods) {
+        llvm::errs() << "  - \"";
+        method->dumpRef(llvm::errs());
+        llvm::errs() << "\"\n";
+      }
+    }
+  }
+};
 
 MemberLookupTable::MemberLookupTable(ASTContext &ctx) {
   // Register a cleanup with the ASTContext to call the lookup table
@@ -1699,6 +1716,16 @@ NominalTypeDecl::lookupDirect(ObjCSelector selector, bool isInstance) {
   return stored.Methods;
 }
 
+/// Is the new method an async alternative of any existing method, or vice
+/// versa?
+static bool isAnAsyncAlternative(AbstractFunctionDecl *newDecl,
+                                 llvm::TinyPtrVector<AbstractFunctionDecl *> &vec) {
+  return llvm::any_of(vec, [&](AbstractFunctionDecl *oldDecl) {
+    return newDecl->getAsyncAlternative(/*isKnownObjC=*/true) == oldDecl
+              || oldDecl->getAsyncAlternative(/*isKnownObjC=*/true) == newDecl;
+  });
+}
+
 void NominalTypeDecl::recordObjCMethod(AbstractFunctionDecl *method,
                                        ObjCSelector selector) {
   if (!ObjCMethodLookup && !createObjCMethodLookup())
@@ -1714,12 +1741,11 @@ void NominalTypeDecl::recordObjCMethod(AbstractFunctionDecl *method,
     return;
 
   if (auto *sf = method->getParentSourceFile()) {
-    if (vec.size() == 1) {
-      // We have a conflict.
-      sf->ObjCMethodConflicts.push_back(std::make_tuple(this, selector,
-                                                        isInstanceMethod));
-    } if (vec.empty()) {
+    if (vec.empty()) {
       sf->ObjCMethodList.push_back(method);
+    } else if (!isa<ProtocolDecl>(this) || !isAnAsyncAlternative(method, vec)) {
+      // We have a conflict.
+      sf->ObjCMethodConflicts.insert({ this, selector, isInstanceMethod });
     }
   }
 
diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp
@@ -3569,11 +3569,16 @@ void ClangImporter::loadExtensions(NominalTypeDecl *nominal,
 }
 
 void ClangImporter::loadObjCMethods(
-       ClassDecl *classDecl,
+       NominalTypeDecl *typeDecl,
        ObjCSelector selector,
        bool isInstanceMethod,
        unsigned previousGeneration,
        llvm::TinyPtrVector<AbstractFunctionDecl *> &methods) {
+  // TODO: We don't currently need to load methods from imported ObjC protocols.
+  auto classDecl = dyn_cast<ClassDecl>(typeDecl);
+  if (!classDecl)
+    return;
+
   const auto *objcClass =
       dyn_cast_or_null<clang::ObjCInterfaceDecl>(classDecl->getClangDecl());
   if (!objcClass)
diff --git a/lib/PrintAsClang/DeclAndTypePrinter.cpp b/lib/PrintAsClang/DeclAndTypePrinter.cpp
@@ -1124,7 +1124,8 @@ class DeclAndTypePrinter::Implementation
     assert(!AvAttr->Rename.empty());
 
     auto *renamedDecl = evaluateOrDefault(
-        getASTContext().evaluator, RenamedDeclRequest{D, AvAttr}, nullptr);
+        getASTContext().evaluator, RenamedDeclRequest{D, AvAttr, false},
+        nullptr);
     if (renamedDecl) {
       assert(shouldInclude(renamedDecl) &&
              "ObjC printer logic mismatch with renamed decl");
@@ -2112,12 +2113,31 @@ auto DeclAndTypePrinter::getImpl() -> Implementation {
   return Implementation(os, *this, outputLang);
 }
 
+static bool isAsyncAlternativeOfOtherDecl(const ValueDecl *VD) {
+  auto AFD = dyn_cast<AbstractFunctionDecl>(VD);
+  if (!AFD || !AFD->isAsyncContext() || !AFD->getObjCSelector())
+    return false;
+
+  auto type = AFD->getDeclContext()->getSelfNominalTypeDecl();
+  if (!type)
+    return false;
+  auto others = type->lookupDirect(AFD->getObjCSelector(),
+                                   AFD->isInstanceMember());
+
+  for (auto other : others)
+    if (other->getAsyncAlternative() == AFD)
+      return true;
+
+  return false;
+}
+
 bool DeclAndTypePrinter::shouldInclude(const ValueDecl *VD) {
   return !VD->isInvalid() &&
          (outputLang == OutputLanguageMode::Cxx
               ? cxx_translation::isVisibleToCxx(VD, minRequiredAccess)
               : isVisibleToObjC(VD, minRequiredAccess)) &&
-         !VD->getAttrs().hasAttribute<ImplementationOnlyAttr>();
+         !VD->getAttrs().hasAttribute<ImplementationOnlyAttr>() &&
+         !isAsyncAlternativeOfOtherDecl(VD);
 }
 
 void DeclAndTypePrinter::print(const Decl *D) {
diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp
@@ -6294,7 +6294,8 @@ static bool parametersMatch(const AbstractFunctionDecl *a,
 
 ValueDecl *RenamedDeclRequest::evaluate(Evaluator &evaluator,
                                         const ValueDecl *attached,
-                                        const AvailableAttr *attr) const {
+                                        const AvailableAttr *attr,
+                                        bool isKnownObjC) const {
   if (!attached || !attr)
     return nullptr;
 
@@ -6325,7 +6326,7 @@ ValueDecl *RenamedDeclRequest::evaluate(Evaluator &evaluator,
   auto minAccess = AccessLevel::Private;
   if (attached->getModuleContext()->isExternallyConsumed())
     minAccess = AccessLevel::Public;
-  bool attachedIsObjcVisible =
+  bool attachedIsObjcVisible = isKnownObjC ||
       objc_translation::isVisibleToObjC(attached, minAccess);
 
   SmallVector<ValueDecl *, 4> lookupResults;
diff --git a/lib/Sema/TypeCheckDeclObjC.cpp b/lib/Sema/TypeCheckDeclObjC.cpp
@@ -2422,11 +2422,8 @@ bool swift::diagnoseUnintendedObjCMethodOverrides(SourceFile &sf) {
 /// Retrieve the source file for the given Objective-C member conflict.
 static TinyPtrVector<AbstractFunctionDecl *>
 getObjCMethodConflictDecls(const SourceFile::ObjCMethodConflict &conflict) {
-  NominalTypeDecl *typeDecl = std::get<0>(conflict);
-  ObjCSelector selector = std::get<1>(conflict);
-  bool isInstanceMethod = std::get<2>(conflict);
-
-  return typeDecl->lookupDirect(selector, isInstanceMethod);
+  return conflict.typeDecl->lookupDirect(conflict.selector,
+                                         conflict.isInstanceMethod);
 }
 
 static ObjCAttr *getObjCAttrIfFromAccessNote(ValueDecl *VD) {
@@ -2452,7 +2449,8 @@ bool swift::diagnoseObjCMethodConflicts(SourceFile &sf) {
   // Sort the set of conflicts so we get a deterministic order for
   // diagnostics. We use the first conflicting declaration in each set to
   // perform the sort.
-  auto localConflicts = sf.ObjCMethodConflicts;
+  llvm::SmallVector<SourceFile::ObjCMethodConflict, 4> localConflicts;
+  llvm::copy(sf.ObjCMethodConflicts, std::back_inserter(localConflicts));
   std::sort(localConflicts.begin(), localConflicts.end(),
             [&](const SourceFile::ObjCMethodConflict &lhs,
                 const SourceFile::ObjCMethodConflict &rhs) {
@@ -2463,9 +2461,6 @@ bool swift::diagnoseObjCMethodConflicts(SourceFile &sf) {
   // Diagnose each conflict.
   bool anyConflicts = false;
   for (const auto &conflict : localConflicts) {
-    NominalTypeDecl *tyDecl = std::get<0>(conflict);
-    ObjCSelector selector = std::get<1>(conflict);
-
     auto methods = getObjCMethodConflictDecls(conflict);
 
     // Erase any invalid or stub declarations. We don't want to complain about
@@ -2545,10 +2540,10 @@ bool swift::diagnoseObjCMethodConflicts(SourceFile &sf) {
                                                 : diag::objc_redecl,
                                      diagInfo.first, diagInfo.second,
                                      origDiagInfo.first, origDiagInfo.second,
-                                     selector);
+                                     conflict.selector);
 
       // Protocols weren't checked for selector conflicts in 5.0.
-      diag.warnUntilSwiftVersionIf(!isa<ClassDecl>(tyDecl), 6);
+      diag.warnUntilSwiftVersionIf(!isa<ClassDecl>(conflict.typeDecl), 6);
 
       auto objcAttr = getObjCAttrIfFromAccessNote(conflictingDecl);
       swift::softenIfAccessNote(conflictingDecl, objcAttr, diag);
diff --git a/lib/Serialization/ModuleFile.cpp b/lib/Serialization/ModuleFile.cpp
diff --git a/lib/Serialization/ModuleFile.h b/lib/Serialization/ModuleFile.h
diff --git a/lib/Serialization/ModuleFormat.h b/lib/Serialization/ModuleFormat.h
diff --git a/lib/Serialization/Serialization.cpp b/lib/Serialization/Serialization.cpp
diff --git a/lib/Serialization/SerializedModuleLoader.cpp b/lib/Serialization/SerializedModuleLoader.cpp
diff --git a/test/ClangImporter/objc_async_conformance.swift b/test/ClangImporter/objc_async_conformance.swift
diff --git a/test/PrintAsObjC/protocols.swift b/test/PrintAsObjC/protocols.swift
