Further loosen selector conflict checks

Some mixed-language projects import Objective-C headers through their umbrella or bridging header that declarate things that are actually implemented in Swift. This isn’t something we really supported or had tests for, but it happens in practice and we can’t break them.

Carve out a second exception to method conflict checking for when all of the conflicting methods are imported ObjC methods in either the same module or a bridging header.

Fixes rdar://96470068.

Author: beccadax

lib/AST/NameLookup.cpp

@@ -36,6 +36,7 @@
 #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"
@@ -1716,14 +1717,34 @@ 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) {
+/// 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,
@@ -1743,7 +1764,7 @@ void NominalTypeDecl::recordObjCMethod(AbstractFunctionDecl *method,
   if (auto *sf = method->getParentSourceFile()) {
     if (vec.empty()) {
       sf->ObjCMethodList.push_back(method);
-    } else if (!isa<ProtocolDecl>(this) || !isAnAsyncAlternative(method, vec)) {
+    } else if (shouldDiagnoseConflict(this, 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 == rhsSF) {
+      if (lhsSF && 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,3 +271,8 @@ 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

@@ -0,0 +1,7 @@
+#import <Foundation.h>
+
+@interface MyObject : NSObject
+
+- (void)implementedInSwift;
+
+@end

test/ClangImporter/Inputs/objc_init_redundant_bridging.h

@@ -0,0 +1,7 @@
+#import <Foundation.h>
+
+@interface MyBridgedObject : NSObject
+
+- (void)implementedInSwift;
+
+@end

test/ClangImporter/objc_init_redundant.swift

@@ -1,5 +1,5 @@
-// 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: %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: %FileCheck %s < %t.log
 
 // REQUIRES: objc_interop
@@ -19,3 +19,12 @@ 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() {}
+}