[6.2][cxx-interop] Fix unqualified name lookup failure

Explanation: C++ interop synthesizes certain forwarding functions in an
_ObjC module. This confuses MemberImportVisibility. This patch adds
logic to work this around by keeping a mapping between the synthesized
and the original function and looks up where the synthesized functions
belong to based on the original functions' parent module.
Scope: C++ forward interop when MemberImportVisibility is enabled.
Issues: rdar://154887575
Original PRs: swiftlang#82840
Risk: Low, a narrow change makes getModuleContextForNameLookupForCxxDecl more
precise, and it is only used with MemberImportVisibility.
Testing: Added a compiler test.
Reviewers: @egorzhdan, @tshortli, @hnrklssn

Author: Xazax-hun

include/swift/AST/ClangModuleLoader.h

@@ -216,6 +216,9 @@ class ClangModuleLoader : public ModuleLoader {
                                           DeclContext *newContext,
                                           ClangInheritanceInfo inheritance) = 0;
 
+  /// Returnes the original method if \param decl is a clone from a base class
+  virtual ValueDecl *getOriginalForClonedMember(const ValueDecl *decl) = 0;
+
   /// Emits diagnostics for any declarations named name
   /// whose direct declaration context is a TU.
   virtual void diagnoseTopLevelValue(const DeclName &name) = 0;

include/swift/ClangImporter/ClangImporter.h

@@ -655,6 +655,8 @@ class ClangImporter final : public ClangModuleLoader {
   ValueDecl *importBaseMemberDecl(ValueDecl *decl, DeclContext *newContext,
                                   ClangInheritanceInfo inheritance) override;
 
+  ValueDecl *getOriginalForClonedMember(const ValueDecl *decl) override;
+
   /// Emits diagnostics for any declarations named name
   /// whose direct declaration context is a TU.
   void diagnoseTopLevelValue(const DeclName &name) override;

lib/AST/Decl.cpp

@@ -906,15 +906,29 @@ static ModuleDecl *getModuleContextForNameLookupForCxxDecl(const Decl *decl) {
   if (!ctx.LangOpts.EnableCXXInterop)
     return nullptr;
 
+  // When we clone members for base classes the cloned members have no
+  // corresponding Clang nodes. Look up the original imported declaration to
+  // figure out what Clang module does the cloned member originate from.
+  bool isClonedMember = false;
+  if (auto VD = dyn_cast<ValueDecl>(decl))
+    if (auto loader = ctx.getClangModuleLoader())
+      if (auto original = loader->getOriginalForClonedMember(VD)) {
+        isClonedMember = true;
+        decl = original;
+      }
+
   if (!decl->hasClangNode())
     return nullptr;
 
   auto parentModule = decl->getModuleContext();
 
   // We only need to look for the real parent module when the existing parent
   // is the imported header module.
-  if (!parentModule->isClangHeaderImportModule())
+  if (!parentModule->isClangHeaderImportModule()) {
+    if (isClonedMember)
+      return parentModule;
     return nullptr;
+  }
 
   auto clangModule = decl->getClangDecl()->getOwningModule();
   if (!clangModule)

lib/ClangImporter/ClangImporter.cpp

@@ -5516,10 +5516,11 @@ const clang::CXXMethodDecl *getCalledBaseCxxMethod(FuncDecl *baseMember) {
 
 // Construct a Swift method that represents the synthesized C++ method
 // that invokes the base C++ method.
-FuncDecl *synthesizeBaseFunctionDeclCall(ClangImporter &impl, ASTContext &ctx,
-                                         NominalTypeDecl *derivedStruct,
-                                         NominalTypeDecl *baseStruct,
-                                         FuncDecl *baseMember) {
+static FuncDecl *synthesizeBaseFunctionDeclCall(ClangImporter &impl,
+                                                ASTContext &ctx,
+                                                NominalTypeDecl *derivedStruct,
+                                                NominalTypeDecl *baseStruct,
+                                                FuncDecl *baseMember) {
   auto *cxxMethod = getCalledBaseCxxMethod(baseMember);
   if (!cxxMethod)
     return nullptr;
@@ -7661,11 +7662,26 @@ ValueDecl *ClangImporter::Implementation::importBaseMemberDecl(
   if (known == clonedBaseMembers.end()) {
     ValueDecl *cloned = cloneBaseMemberDecl(decl, newContext, inheritance);
     known = clonedBaseMembers.insert({key, cloned}).first;
+    clonedMembers.insert(std::make_pair(cloned, decl));
   }
 
   return known->second;
 }
 
+ValueDecl *ClangImporter::Implementation::getOriginalForClonedMember(
+    const ValueDecl *decl) {
+  // If this is a cloned decl, we don't want to reclone it
+  // Otherwise, we may end up with multiple copies of the same method
+  if (!decl->hasClangNode()) {
+    // Skip decls with a clang node as those will never be a clone
+    auto result = clonedMembers.find(decl);
+    if (result != clonedMembers.end())
+      return result->getSecond();
+  }
+
+  return nullptr;
+}
+
 size_t ClangImporter::Implementation::getImportedBaseMemberDeclArity(
     const ValueDecl *valueDecl) {
   if (auto *func = dyn_cast<FuncDecl>(valueDecl)) {
@@ -7682,6 +7698,10 @@ ClangImporter::importBaseMemberDecl(ValueDecl *decl, DeclContext *newContext,
   return Impl.importBaseMemberDecl(decl, newContext, inheritance);
 }
 
+ValueDecl *ClangImporter::getOriginalForClonedMember(const ValueDecl *decl) {
+  return Impl.getOriginalForClonedMember(decl);
+}
+
 void ClangImporter::diagnoseTopLevelValue(const DeclName &name) {
   Impl.diagnoseTopLevelValue(name);
 }

lib/ClangImporter/ImporterImpl.h

@@ -688,6 +688,9 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
   llvm::DenseMap<std::pair<ValueDecl *, DeclContext *>, ValueDecl *>
       clonedBaseMembers;
 
+  // Map all cloned methods back to the original member
+  llvm::DenseMap<ValueDecl *, ValueDecl *> clonedMembers;
+
 public:
   llvm::DenseMap<const clang::ParmVarDecl*, FuncDecl*> defaultArgGenerators;
 
@@ -696,6 +699,8 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
   ValueDecl *importBaseMemberDecl(ValueDecl *decl, DeclContext *newContext,
                                   ClangInheritanceInfo inheritance);
 
+  ValueDecl *getOriginalForClonedMember(const ValueDecl *decl);
+
   static size_t getImportedBaseMemberDeclArity(const ValueDecl *valueDecl);
 
   // Cache for already-specialized function templates and any thunks they may

test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h

@@ -16,3 +16,12 @@ struct IIOne : IOne {
 struct IIIOne : IIOne {
   int methodIII(void) const { return -111; }
 };
+
+class Base {
+public:
+  bool baseMethod() const { return true; }
+};
+
+namespace Bar {
+class Derived : public Base {};
+} // namespace Bar

test/Interop/Cxx/class/inheritance/inherited-lookup-executable.swift

@@ -1,6 +1,7 @@
 // RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -cxx-interoperability-mode=default)
 //
 // REQUIRES: executable_test
+
 import InheritedLookup
 import StdlibUnittest
 

test/Interop/Cxx/class/inheritance/inherited-lookup-memberimportvisibility.swift

@@ -0,0 +1,22 @@
+// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -cxx-interoperability-mode=default -enable-upcoming-feature MemberImportVisibility)
+//
+// REQUIRES: executable_test
+// REQUIRES: swift_feature_MemberImportVisibility
+
+import InheritedLookup
+import StdlibUnittest
+
+var InheritedMemberTestSuite = TestSuite("Test if inherited lookup works")
+
+extension Bar.Derived {
+    public func callBase() {
+        let _ = baseMethod() 
+    }
+}
+
+InheritedMemberTestSuite.test("Look up base methods from extensions") {
+  let a = Bar.Derived()
+  a.callBase()
+}
+
+runAllTests()