[cxx-interop] Fix spurious ambiguous member lookup

include/swift/ClangImporter/ClangImporterRequests.h

@@ -135,9 +135,11 @@ class CXXNamespaceMemberLookup
 struct ClangRecordMemberLookupDescriptor final {
   NominalTypeDecl *recordDecl;
   DeclName name;
+  bool inherited;
 
-  ClangRecordMemberLookupDescriptor(NominalTypeDecl *recordDecl, DeclName name)
-      : recordDecl(recordDecl), name(name) {
+  ClangRecordMemberLookupDescriptor(NominalTypeDecl *recordDecl, DeclName name,
+                                    bool inherited = false)
+      : recordDecl(recordDecl), name(name), inherited(inherited) {
     assert(isa<clang::RecordDecl>(recordDecl->getClangDecl()));
   }
 

lib/ClangImporter/ClangImporter.cpp

@@ -63,6 +63,7 @@
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LangStandard.h"
 #include "clang/Basic/Module.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/CAS/CASOptions.h"
 #include "clang/CAS/IncludeTree.h"
@@ -6144,6 +6145,7 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
     Evaluator &evaluator, ClangRecordMemberLookupDescriptor desc) const {
   NominalTypeDecl *recordDecl = desc.recordDecl;
   DeclName name = desc.name;
+  bool inherited = desc.inherited;
 
   auto &ctx = recordDecl->getASTContext();
   auto allResults = evaluateOrDefault(
@@ -6197,9 +6199,11 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
           continue;
 
         // Add Clang members that are imported lazily.
-        auto baseResults = evaluateOrDefault(
-            ctx.evaluator,
-            ClangRecordMemberLookup({cast<NominalTypeDecl>(import), name}), {});
+        auto baseResults =
+            evaluateOrDefault(ctx.evaluator,
+                              ClangRecordMemberLookup(
+                                  {cast<NominalTypeDecl>(import), name, true}),
+                              {});
         // Add members that are synthesized eagerly, such as subscripts.
         for (auto member :
              cast<NominalTypeDecl>(import)->getCurrentMembersWithoutLoading()) {
@@ -6218,6 +6222,21 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
           // as that would cause an ambiguous lookup.
           if (foundNameArities.count(getArity(foundInBase)))
             continue;
+
+          // Do not importBaseMemberDecl() if this is a recursive lookup into
+          // some class's superclass. importBaseMemberDecl() caches synthesized
+          // members, which does not work if we call it on its result, e.g.:
+          //
+          //    importBaseMemberDecl(importBaseMemberDecl(foundInBase,
+          //    recorDecl), recordDecl)
+          //
+          // Instead, we simply pass on the imported decl (foundInBase) as is,
+          // so that only the top-most request calls importBaseMemberDecl().
+          if (inherited) {
+            result.push_back(foundInBase);
+            continue;
+          }
+
           if (auto newDecl = clangModuleLoader->importBaseMemberDecl(
                   foundInBase, recordDecl)) {
             result.push_back(newDecl);

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

@@ -0,0 +1,13 @@
+#pragma once
+
+struct Base1 {
+  int methodBase(void) const { return 1; }
+};
+
+struct IBase1 : Base1 {
+  int methodIBase(void) const { return 11; }
+};
+
+struct IIBase1 : IBase1 {
+  int methodIIBase(void) const { return 111; }
+};

test/Interop/Cxx/class/inheritance/Inputs/module.modulemap

@@ -43,3 +43,8 @@ module FunctionsObjC {
   requires cplusplus
   requires objc
 }
+
+module InheritedLookup {
+  header "inherited-lookup.h"
+  requires cplusplus
+}

test/Interop/Cxx/class/inheritance/fields-irgen.swift

@@ -9,6 +9,6 @@ func testGetX() -> CInt {
 
 let _ = testGetX()
 
-// CHECK: define {{.*}}linkonce_odr{{.*}} i32 @{{.*}}__synthesizedBaseCall___synthesizedBaseGetterAccessor{{.*}}(ptr {{.*}} %[[THIS_PTR:.*]])
+// CHECK: define {{.*}}linkonce_odr{{.*}} i32 @{{(.*)(30CopyTrackedDerivedDerivedClass33__synthesizedBaseGetterAccessor_x|__synthesizedBaseGetterAccessor_x@CopyTrackedDerivedDerivedClass)(.*)}}(ptr {{.*}} %[[THIS_PTR:.*]])
 // CHECK: %[[ADD_PTR:.*]] = getelementptr inbounds i8, ptr %{{.*}}, i{{32|64}} 4
-// CHECK: call{{.*}} i32 @{{.*}}__synthesizedBaseGetterAccessor{{.*}}(ptr {{.*}} %[[ADD_PTR]])
+// CHECK: %[[X:.*]] = getelementptr inbounds %class.CopyTrackedBaseClass, ptr %[[ADD_PTR]], i32 0, i32 0

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

@@ -0,0 +1,16 @@
+// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -cxx-interoperability-mode=default)
+//
+// REQUIRES: executable_test
+import InheritedLookup
+import StdlibUnittest
+
+var InheritedMemberTestSuite = TestSuite("Test if inherited lookup works")
+
+InheritedMemberTestSuite.test("IIBase1::method() resolves to grandparent") {
+  let iibase1 = IIBase1()
+  expectEqual(iibase1.methodBase(), 1)
+  expectEqual(iibase1.methodIBase(), 11)
+  expectEqual(iibase1.methodIIBase(), 111)
+}
+
+runAllTests()

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

@@ -0,0 +1,24 @@
+// RUN: %target-typecheck-verify-swift -verify-ignore-unknown -I %S/Inputs -cxx-interoperability-mode=default
+import InheritedLookup
+
+extension IIBase1 {
+    func ext() {
+        // NOTE: we deliberately look up a missing member above because doing so
+        // forces multiple ClangRecordMemberLookup requests, which should be
+        // idempotent (though this hasn't always been the case, because bugs).
+        missing() // expected-error {{cannot find 'missing' in scope}}
+
+        // For instance, a non-idempotent ClangRecordMemberLookup would cause
+        // the following to appear ambiguous:
+        methodBase()
+        methodIBase()
+        methodIIBase()
+    }
+}
+
+func f(v: IIBase1) {
+    v.missing() // expected-error {{'IIBase1' has no member 'missing'}}
+    v.methodBase()
+    v.methodIBase()
+    v.methodIIBase()
+}

test/Interop/Cxx/class/inheritance/subscript-irgen.swift

@@ -9,6 +9,6 @@ func testGetX() -> CInt {
 
 let _ = testGetX()
 
-// CHECK: define {{.*}}linkonce_odr{{.*}} i32 @{{.*}}__synthesizedBaseCall___synthesizedBaseCall_operatorSubscript{{.*}}(ptr {{.*}} %[[THIS_PTR:.*]], i32 {{.*}})
+// CHECK: define {{.*}}linkonce_odr{{.*}} i32 @{{(.*)(30CopyTrackedDerivedDerivedClass39__synthesizedBaseCall_operatorSubscript|__synthesizedBaseCall_operatorSubscript@CopyTrackedDerivedDerivedClass)(.*)}}(ptr {{.*}} %[[THIS_PTR:.*]], i32 {{.*}})
 // CHECK: %[[ADD_PTR:.*]] = getelementptr inbounds i8, ptr %{{.*}}, i{{32|64}} 4
-// CHECK: call{{.*}} i32 @{{.*}}__synthesizedBaseCall_operatorSubscript{{.*}}(ptr {{.*}} %[[ADD_PTR]], i32 {{.*}})
+// CHECK: %[[CALL:.*]] = call {{.*}} i32 @{{.*}}CopyTrackedBaseClass{{.*}}(ptr {{.*}} %[[ADD_PTR]], i32 {{.*}})

test/Interop/Cxx/class/move-only/inherited-field-access-irgen.swift

@@ -17,14 +17,10 @@ func testSetX(_ x: CInt) {
 
 testSetX(2)
 
-// CHECK: define {{.*}}linkonce_odr{{.*}} ptr @{{.*}}__synthesizedBaseCall___synthesizedBaseGetterAccessor{{.*}}
-
-// CHECK: define {{.*}}linkonce_odr{{.*}} ptr @{{.*}}__synthesizedBaseCall___synthesizedBaseSetterAccessor{{.*}}
-
-// CHECK: define {{.*}}linkonce_odr{{.*}} ptr @{{.*}}__synthesizedBaseGetterAccessor{{.*}}
+// CHECK: define {{.*}}linkonce_odr{{.*}} ptr @{{(.*)(31NonCopyableHolderDerivedDerived*33__synthesizedBaseGetterAccessor_x|__synthesizedBaseGetterAccessor_x@NonCopyableHolderDerivedDerived)(.*)}}
 // CHECK: %[[VPTR:.*]] = getelementptr inbounds %struct.NonCopyableHolder
 // CHECK: ret ptr %[[VPTR]]
 
-// CHECK: define {{.*}}linkonce_odr{{.*}} ptr @{{.*}}__synthesizedBaseSetterAccessor{{.*}}
+// CHECK: define {{.*}}linkonce_odr{{.*}} ptr @{{(.*)(31NonCopyableHolderDerivedDerived33__synthesizedBaseSetterAccessor_x|__synthesizedBaseSetterAccessor_x@NonCopyableHolderDerivedDerived)(.*)}}
 // CHECK: %[[VPTRS:.*]] = getelementptr inbounds %struct.NonCopyableHolder
 // CHECK: ret ptr %[[VPTRS]]

test/Interop/Cxx/class/move-only/inherited-field-access-silgen.swift

@@ -26,8 +26,8 @@ testSetX(2)
 
 // CHECK: sil shared [transparent] @$sSo024NonCopyableHolderDerivedD0V1xSo0aB0Vvlu : $@convention(method) (@{{(in_)?}}guaranteed NonCopyableHolderDerivedDerived) -> UnsafePointer<NonCopyable>
 // CHECK: {{.*}}(%[[SELF_VAL:.*]] : ${{(\*)?}}NonCopyableHolderDerivedDerived):
-// CHECK: function_ref @{{.*}}__synthesizedBaseCall___synthesizedBaseGetterAccessor{{.*}} : $@convention(cxx_method) (@in_guaranteed NonCopyableHolderDerivedDerived) -> UnsafePointer<NonCopyable>
+// CHECK: function_ref @{{(.*)(31NonCopyableHolderDerivedDerived33__synthesizedBaseGetterAccessor_x|__synthesizedBaseGetterAccessor_x@NonCopyableHolderDerivedDerived)(.*)}} : $@convention(cxx_method) (@in_guaranteed NonCopyableHolderDerivedDerived) -> UnsafePointer<NonCopyable>
 // CHECK-NEXT: apply %{{.*}}
 
 // CHECK: sil shared [transparent] @$sSo024NonCopyableHolderDerivedD0V1xSo0aB0Vvau : $@convention(method) (@inout NonCopyableHolderDerivedDerived) -> UnsafeMutablePointer<NonCopyable>
-// CHECK: function_ref @{{.*}}__synthesizedBaseCall___synthesizedBaseSetterAccessor{{.*}} : $@convention(cxx_method) (@inout NonCopyableHolderDerivedDerived) -> UnsafeMutablePointer<NonCopyable>
+// CHECK: function_ref @{{(.*)(31NonCopyableHolderDerivedDerived33__synthesizedBaseSetterAccessor_x|__synthesizedBaseSetterAccessor_x@NonCopyableHolderDerivedDerived)(.*)}} : $@convention(cxx_method) (@inout NonCopyableHolderDerivedDerived) -> UnsafeMutablePointer<NonCopyable>

test/Interop/Cxx/foreign-reference/function-inheritance-irgen.swift

@@ -10,6 +10,6 @@ func testGetX() -> CInt {
 let _ = testGetX()
 
 
-// CHECK: define {{.*}}linkonce_odr{{.*}} i32 @{{.*}}__synthesizedBaseCall___synthesizedBaseCall_{{.*}}(ptr {{.*}} %[[THIS_PTR:.*]])
+// CHECK: define {{.*}}linkonce_odr{{.*}} i32 @{{(.*)(30CopyTrackedDerivedDerivedClass26__synthesizedBaseCall_getX|__synthesizedBaseCall_getX@CopyTrackedDerivedDerivedClass)(.*)}}(ptr {{.*}} %[[THIS_PTR:.*]])
 // CHECK: %[[ADD_PTR:.*]] = getelementptr inbounds i8, ptr %{{.*}}, i{{32|64}} 4
 // CHECK: call{{.*}} i32 @{{.*}}(ptr {{.*}} %[[ADD_PTR]])