Merge pull request #60081 from beccadax/earth-import-conflict

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() {}
+}

test/Concurrency/objc_async_overload.swift

@@ -58,5 +58,4 @@ 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}}
 }