Merge pull request #60315 from beccadax/revert-selector-conflict-5.7

Author: beccadax

include/swift/AST/ModuleLoader.h

@@ -226,11 +226,11 @@ class ModuleLoader {
   virtual void loadExtensions(NominalTypeDecl *nominal,
                               unsigned previousGeneration) { }
 
-  /// Load the methods within the given type that produce
+  /// Load the methods within the given class that produce
   /// Objective-C class or instance methods with the given selector.
   ///
-  /// \param typeDecl The type in which we are searching for @objc methods.
-  /// The search only considers this type and its extensions; not any
+  /// \param classDecl The class in which we are searching for @objc methods.
+  /// The search only considers this class 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(
-                 NominalTypeDecl *typeDecl,
+                 ClassDecl *classDecl,
                  ObjCSelector selector,
                  bool isInstanceMethod,
                  unsigned previousGeneration,

include/swift/AST/SourceFile.h

@@ -17,7 +17,6 @@
 #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"
 
@@ -244,20 +243,11 @@ class SourceFile final : public FileUnit {
   std::vector<ObjCUnsatisfiedOptReq> ObjCUnsatisfiedOptReqs;
 
   /// A selector that is used by two different declarations in the same class.
-  struct ObjCMethodConflict {
-    NominalTypeDecl *typeDecl;
-    ObjCSelector selector;
-    bool isInstanceMethod;
-
-    ObjCMethodConflict(NominalTypeDecl *typeDecl, ObjCSelector selector,
-                       bool isInstanceMethod)
-        : typeDecl(typeDecl), selector(selector),
-          isInstanceMethod(isInstanceMethod)
-    {}
-  };
+  /// Fields: typeDecl, selector, isInstanceMethod.
+  using ObjCMethodConflict = std::tuple<NominalTypeDecl *, ObjCSelector, bool>;
 
   /// List of Objective-C member conflicts we have found during type checking.
-  llvm::SetVector<ObjCMethodConflict> ObjCMethodConflicts;
+  std::vector<ObjCMethodConflict> ObjCMethodConflicts;
 
   /// List of attributes added by access notes, used to emit remarks for valid
   /// ones.
@@ -646,30 +636,4 @@ 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

include/swift/ClangImporter/ClangImporter.h

@@ -309,7 +309,7 @@ class ClangImporter final : public ClangModuleLoader {
                               unsigned previousGeneration) override;
 
   virtual void loadObjCMethods(
-                 NominalTypeDecl *typeDecl,
+                 ClassDecl *classDecl,
                  ObjCSelector selector,
                  bool isInstanceMethod,
                  unsigned previousGeneration,

include/swift/Sema/SourceLoader.h

@@ -85,7 +85,7 @@ class SourceLoader : public ModuleLoader {
                               unsigned previousGeneration) override;
 
   virtual void loadObjCMethods(
-                 NominalTypeDecl *typeDecl,
+                 ClassDecl *classDecl,
                  ObjCSelector selector,
                  bool isInstanceMethod,
                  unsigned previousGeneration,

include/swift/Serialization/SerializedModuleLoader.h

@@ -190,7 +190,7 @@ class SerializedModuleLoaderBase : public ModuleLoader {
                               unsigned previousGeneration) override;
 
   virtual void loadObjCMethods(
-                 NominalTypeDecl *typeDecl,
+                 ClassDecl *classDecl,
                  ObjCSelector selector,
                  bool isInstanceMethod,
                  unsigned previousGeneration,

lib/AST/ASTContext.cpp

@@ -1925,12 +1925,24 @@ 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(tyDecl, selector, isInstanceMethod,
+    loader->loadObjCMethods(classDecl, selector, isInstanceMethod,
                             previousGeneration, methods);
   }
 }

lib/AST/NameLookup.cpp

@@ -36,7 +36,6 @@
 #include "swift/Basic/Statistic.h"
 #include "swift/ClangImporter/ClangImporterRequests.h"
 #include "swift/Parse/Lexer.h"
-#include "swift/Strings.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/DenseMap.h"
@@ -1311,24 +1310,7 @@ 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
@@ -1686,7 +1668,7 @@ bool NominalTypeDecl::createObjCMethodLookup() {
   assert(!ObjCMethodLookup && "Already have an Objective-C member table");
 
   // Most types cannot have ObjC methods.
-  if (!(isa<ClassDecl>(this) || isa<ProtocolDecl>(this)))
+  if (!(isa<ClassDecl>(this)))
     return false;
 
   auto &ctx = getASTContext();
@@ -1717,36 +1699,6 @@ NominalTypeDecl::lookupDirect(ObjCSelector selector, bool isInstance) {
   return stored.Methods;
 }
 
-/// If there is an apparent conflict between \p newDecl and one of the methods
-/// in \p vec, should we diagnose it?
-static bool
-shouldDiagnoseConflict(NominalTypeDecl *ty, AbstractFunctionDecl *newDecl,
-                       llvm::TinyPtrVector<AbstractFunctionDecl *> &vec) {
-  // Are all conflicting methods imported from ObjC and in our ObjC half or a
-  // bridging header? Some code bases implement ObjC methods in Swift even
-  // though it's not exactly supported.
-  auto newDeclModuleName = newDecl->getModuleContext()->getName();
-  if (llvm::all_of(vec, [&](AbstractFunctionDecl *oldDecl) {
-    if (!oldDecl->hasClangNode())
-      return false;
-    auto oldDeclModuleName = oldDecl->getModuleContext()->getName();
-    return oldDeclModuleName == newDeclModuleName
-               || oldDeclModuleName.str() == CLANG_HEADER_MODULE_NAME;
-  }))
-    return false;
-
-  // If we're looking at protocol requirements, is the new method an async
-  // alternative of any existing method, or vice versa?
-  if (isa<ProtocolDecl>(ty) &&
-      llvm::any_of(vec, [&](AbstractFunctionDecl *oldDecl) {
-    return newDecl->getAsyncAlternative(/*isKnownObjC=*/true) == oldDecl
-              || oldDecl->getAsyncAlternative(/*isKnownObjC=*/true) == newDecl;
-  }))
-    return false;
-
-  return true;
-}
-
 void NominalTypeDecl::recordObjCMethod(AbstractFunctionDecl *method,
                                        ObjCSelector selector) {
   if (!ObjCMethodLookup && !createObjCMethodLookup())
@@ -1762,11 +1714,12 @@ void NominalTypeDecl::recordObjCMethod(AbstractFunctionDecl *method,
     return;
 
   if (auto *sf = method->getParentSourceFile()) {
-    if (vec.empty()) {
-      sf->ObjCMethodList.push_back(method);
-    } else if (shouldDiagnoseConflict(this, method, vec)) {
+    if (vec.size() == 1) {
       // We have a conflict.
-      sf->ObjCMethodConflicts.insert({ this, selector, isInstanceMethod });
+      sf->ObjCMethodConflicts.push_back(std::make_tuple(this, selector,
+                                                        isInstanceMethod));
+    } if (vec.empty()) {
+      sf->ObjCMethodList.push_back(method);
     }
   }
 

lib/ClangImporter/ClangImporter.cpp

@@ -3435,16 +3435,11 @@ void ClangImporter::loadExtensions(NominalTypeDecl *nominal,
 }
 
 void ClangImporter::loadObjCMethods(
-       NominalTypeDecl *typeDecl,
+       ClassDecl *classDecl,
        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)

lib/PrintAsClang/DeclAndTypePrinter.cpp

@@ -2019,31 +2019,12 @@ 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>() &&
-         !isAsyncAlternativeOfOtherDecl(VD);
+         !VD->getAttrs().hasAttribute<ImplementationOnlyAttr>();
 }
 
 void DeclAndTypePrinter::print(const Decl *D) {

lib/PrintAsClang/PrintAsClang.cpp

@@ -80,6 +80,7 @@ static void writePrologue(raw_ostream &out, ASTContext &ctx,
          "# include <swift/objc-prologue.h>\n"
          "#endif\n"
          "\n"
+         "#pragma clang diagnostic ignored \"-Wduplicate-method-match\"\n"
          "#pragma clang diagnostic ignored \"-Wauto-import\"\n";
   emitObjCConditional(out,
                       [&] { out << "#include <Foundation/Foundation.h>\n"; });