Revert "Further loosen selector conflict checks"

This reverts commit 517a5d0.

Author: beccadax

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"
@@ -1717,34 +1716,14 @@ 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) {
+/// 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;
-  }))
-    return false;
-
-  return true;
+  });
 }
 
 void NominalTypeDecl::recordObjCMethod(AbstractFunctionDecl *method,
@@ -1764,7 +1743,7 @@ void NominalTypeDecl::recordObjCMethod(AbstractFunctionDecl *method,
   if (auto *sf = method->getParentSourceFile()) {
     if (vec.empty()) {
       sf->ObjCMethodList.push_back(method);
-    } else if (shouldDiagnoseConflict(this, method, vec)) {
+    } else if (!isa<ProtocolDecl>(this) || !isAnAsyncAlternative(method, vec)) {
       // We have a conflict.
       sf->ObjCMethodConflicts.insert({ this, selector, isInstanceMethod });
     }

lib/Sema/TypeCheckDeclObjC.cpp

@@ -2269,7 +2269,7 @@ namespace {
       // location within that source file.
       SourceFile *lhsSF = lhs->getDeclContext()->getParentSourceFile();
       SourceFile *rhsSF = rhs->getDeclContext()->getParentSourceFile();
-      if (lhsSF && lhsSF == rhsSF) {
+      if (lhsSF == rhsSF) {
         // If only one location is valid, the valid location comes first.
         if (lhs->getLoc().isValid() != rhs->getLoc().isValid()) {
           return lhs->getLoc().isValid();

test/ClangImporter/Inputs/custom-modules/module.map

@@ -271,8 +271,3 @@ module CommonName {
   header "CommonName.h"
   export *
 }
-
-module objc_init_redundant {
-  header "objc_init_redundant.h"
-  export *
-}

test/ClangImporter/Inputs/custom-modules/objc_init_redundant.h

@@ -1,5 +1,5 @@
-// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk -I %S/Inputs/custom-modules) -import-underlying-module -import-objc-header %S/Inputs/objc_init_redundant_bridging.h -emit-sil %s -verify
-// RUN: not %target-swift-frontend(mock-sdk: %clang-importer-sdk -I %S/Inputs/custom-modules) -import-underlying-module -import-objc-header %S/Inputs/objc_init_redundant_bridging.h -emit-sil %s > %t.log 2>&1
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk -I %S/Inputs/custom-modules) -emit-sil %s -verify
+// RUN: not %target-swift-frontend(mock-sdk: %clang-importer-sdk -I %S/Inputs/custom-modules) -emit-sil %s > %t.log 2>&1
 // RUN: %FileCheck %s < %t.log
 
 // REQUIRES: objc_interop
@@ -19,12 +19,3 @@ extension NSObject {
 // CHECK: ObjectiveC.NSObjectProtocol:{{.*}}note: method 'class()' declared here
 }
 
-// rdar://96470068 - Don't want conflict diagnostics in the same module
-extension MyObject {
-  @objc func implementedInSwift() {}
-}
-
-// ...or the bridging header
-extension MyBridgedObject {
-  @objc func implementedInSwift() {}
-}

test/ClangImporter/Inputs/objc_init_redundant_bridging.h

@@ -58,4 +58,5 @@ extension Delegate {
 
 extension Delegate {
   @objc public func makeRequest(fromSwift: Request, completionHandler: (() -> Void)?) {}
+  // expected-warning@-1 {{method 'makeRequest(fromSwift:completionHandler:)' with Objective-C selector 'makeRequestFromSwift:completionHandler:' conflicts with method 'makeRequest(fromSwift:)' with the same Objective-C selector; this is an error in Swift 6}}
 }