From f818a5d72a595c90bf2f3672960081ed495f6f7e Mon Sep 17 00:00:00 2001 From: John Hui Date: Wed, 11 Dec 2024 17:47:17 -0800 Subject: [PATCH 1/9] Add inherited member lookup test case --- .../inheritance/Inputs/inherited-lookup.h | 9 +++++++++ .../class/inheritance/Inputs/module.modulemap | 5 +++++ .../inherited-lookup-executable.swift | 14 +++++++++++++ .../inherited-lookup-typechecker.swift | 20 +++++++++++++++++++ 4 files changed, 48 insertions(+) create mode 100644 test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h create mode 100644 test/Interop/Cxx/class/inheritance/inherited-lookup-executable.swift create mode 100644 test/Interop/Cxx/class/inheritance/inherited-lookup-typechecker.swift diff --git a/test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h b/test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h new file mode 100644 index 0000000000000..0b1e8b25110f5 --- /dev/null +++ b/test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h @@ -0,0 +1,9 @@ +#pragma once + +struct Base1 { + int method(void) const { return 1; } +}; + +struct IBase1 : Base1 { }; + +struct IIBase1 : IBase1 { }; diff --git a/test/Interop/Cxx/class/inheritance/Inputs/module.modulemap b/test/Interop/Cxx/class/inheritance/Inputs/module.modulemap index 0bf1c65a553a9..45736330ddf5e 100644 --- a/test/Interop/Cxx/class/inheritance/Inputs/module.modulemap +++ b/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 +} diff --git a/test/Interop/Cxx/class/inheritance/inherited-lookup-executable.swift b/test/Interop/Cxx/class/inheritance/inherited-lookup-executable.swift new file mode 100644 index 0000000000000..bf72b751d2dbe --- /dev/null +++ b/test/Interop/Cxx/class/inheritance/inherited-lookup-executable.swift @@ -0,0 +1,14 @@ +// 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.method(), 1) +} + +runAllTests() diff --git a/test/Interop/Cxx/class/inheritance/inherited-lookup-typechecker.swift b/test/Interop/Cxx/class/inheritance/inherited-lookup-typechecker.swift new file mode 100644 index 0000000000000..cb7a3b1cde4bb --- /dev/null +++ b/test/Interop/Cxx/class/inheritance/inherited-lookup-typechecker.swift @@ -0,0 +1,20 @@ +// 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: + method() + } +} + +func f(v: IIBase1) { + v.missing() // expected-error {{'IIBase1' has no member 'missing'}} + v.method() +} From 8d832302a3449ccff67f4c3d42096ac7804d9645 Mon Sep 17 00:00:00 2001 From: John Hui Date: Wed, 11 Dec 2024 18:32:04 -0800 Subject: [PATCH 2/9] Fix spurious ambiguous member lookup --- .../ClangImporter/ClangImporterRequests.h | 13 +++++++++++- lib/ClangImporter/ClangImporter.cpp | 21 +++++++++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/include/swift/ClangImporter/ClangImporterRequests.h b/include/swift/ClangImporter/ClangImporterRequests.h index d5f56a9365d23..ad47df9aa632f 100644 --- a/include/swift/ClangImporter/ClangImporterRequests.h +++ b/include/swift/ClangImporter/ClangImporterRequests.h @@ -24,6 +24,7 @@ #include "swift/Basic/Statistic.h" #include "swift/ClangImporter/ClangImporter.h" #include "clang/AST/Type.h" +#include "clang/Basic/Specifiers.h" #include "llvm/ADT/Hashing.h" #include "llvm/ADT/TinyPtrVector.h" @@ -135,12 +136,22 @@ class CXXNamespaceMemberLookup struct ClangRecordMemberLookupDescriptor final { NominalTypeDecl *recordDecl; DeclName name; + clang::AccessSpecifier inheritance; + // Base case lookup on (most) derived class ClangRecordMemberLookupDescriptor(NominalTypeDecl *recordDecl, DeclName name) - : recordDecl(recordDecl), name(name) { + : recordDecl(recordDecl), name(name), inheritance(clang::AS_none) { assert(isa(recordDecl->getClangDecl())); } + // Recursive lookup on inherited classes + ClangRecordMemberLookupDescriptor(NominalTypeDecl *recordDecl, DeclName name, + clang::AccessSpecifier inheritance) + : recordDecl(recordDecl), name(name), inheritance(inheritance) { + assert(isa(recordDecl->getClangDecl())); + assert(inheritance != clang::AS_none && "class inheritance should be specified"); + } + friend llvm::hash_code hash_value(const ClangRecordMemberLookupDescriptor &desc) { return llvm::hash_combine(desc.name, desc.recordDecl); diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index aa3716a43dc2c..56e43df0bac72 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -64,6 +64,7 @@ #include "clang/Basic/LangStandard.h" #include "clang/Basic/Module.h" #include "clang/Basic/TargetInfo.h" +#include "clang/Basic/Specifiers.h" #include "clang/CAS/CASOptions.h" #include "clang/CAS/IncludeTree.h" #include "clang/CodeGen/ObjectFilePCHContainerOperations.h" @@ -6144,6 +6145,7 @@ TinyPtrVector ClangRecordMemberLookup::evaluate( Evaluator &evaluator, ClangRecordMemberLookupDescriptor desc) const { NominalTypeDecl *recordDecl = desc.recordDecl; DeclName name = desc.name; + clang::AccessSpecifier inheritance = desc.inheritance; auto &ctx = recordDecl->getASTContext(); auto allResults = evaluateOrDefault( @@ -6180,7 +6182,8 @@ TinyPtrVector ClangRecordMemberLookup::evaluate( foundNameArities.insert(getArity(valueDecl)); for (auto base : cxxRecord->bases()) { - if (base.getAccessSpecifier() != clang::AccessSpecifier::AS_public) + clang::AccessSpecifier baseAccess = base.getAccessSpecifier(); + if (baseAccess != clang::AccessSpecifier::AS_public) continue; clang::QualType baseType = base.getType(); @@ -6199,7 +6202,7 @@ TinyPtrVector ClangRecordMemberLookup::evaluate( // Add Clang members that are imported lazily. auto baseResults = evaluateOrDefault( ctx.evaluator, - ClangRecordMemberLookup({cast(import), name}), {}); + ClangRecordMemberLookup({cast(import), name, baseAccess}), {}); // Add members that are synthesized eagerly, such as subscripts. for (auto member : cast(import)->getCurrentMembersWithoutLoading()) { @@ -6218,6 +6221,20 @@ TinyPtrVector 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 (inheritance != clang::AS_none) { + result.push_back(foundInBase); + continue; + } + if (auto newDecl = clangModuleLoader->importBaseMemberDecl( foundInBase, recordDecl)) { result.push_back(newDecl); From 0fe808caed67eb59fad2c9c3da9725902445dcaa Mon Sep 17 00:00:00 2001 From: John Hui Date: Thu, 12 Dec 2024 10:44:16 -0800 Subject: [PATCH 3/9] Formatting --- include/swift/ClangImporter/ClangImporterRequests.h | 5 +++-- lib/ClangImporter/ClangImporter.cpp | 9 ++++++--- .../Cxx/class/inheritance/Inputs/inherited-lookup.h | 4 ++-- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/include/swift/ClangImporter/ClangImporterRequests.h b/include/swift/ClangImporter/ClangImporterRequests.h index ad47df9aa632f..76a0ae42700e9 100644 --- a/include/swift/ClangImporter/ClangImporterRequests.h +++ b/include/swift/ClangImporter/ClangImporterRequests.h @@ -146,10 +146,11 @@ struct ClangRecordMemberLookupDescriptor final { // Recursive lookup on inherited classes ClangRecordMemberLookupDescriptor(NominalTypeDecl *recordDecl, DeclName name, - clang::AccessSpecifier inheritance) + clang::AccessSpecifier inheritance) : recordDecl(recordDecl), name(name), inheritance(inheritance) { assert(isa(recordDecl->getClangDecl())); - assert(inheritance != clang::AS_none && "class inheritance should be specified"); + assert(inheritance != clang::AS_none && + "class inheritance should be specified"); } friend llvm::hash_code diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index 56e43df0bac72..dae8eebd51535 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -63,8 +63,8 @@ #include "clang/Basic/IdentifierTable.h" #include "clang/Basic/LangStandard.h" #include "clang/Basic/Module.h" -#include "clang/Basic/TargetInfo.h" #include "clang/Basic/Specifiers.h" +#include "clang/Basic/TargetInfo.h" #include "clang/CAS/CASOptions.h" #include "clang/CAS/IncludeTree.h" #include "clang/CodeGen/ObjectFilePCHContainerOperations.h" @@ -6202,7 +6202,9 @@ TinyPtrVector ClangRecordMemberLookup::evaluate( // Add Clang members that are imported lazily. auto baseResults = evaluateOrDefault( ctx.evaluator, - ClangRecordMemberLookup({cast(import), name, baseAccess}), {}); + ClangRecordMemberLookup( + {cast(import), name, baseAccess}), + {}); // Add members that are synthesized eagerly, such as subscripts. for (auto member : cast(import)->getCurrentMembersWithoutLoading()) { @@ -6226,7 +6228,8 @@ TinyPtrVector ClangRecordMemberLookup::evaluate( // 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) + // 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(). diff --git a/test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h b/test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h index 0b1e8b25110f5..fcb9dee528496 100644 --- a/test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h +++ b/test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h @@ -4,6 +4,6 @@ struct Base1 { int method(void) const { return 1; } }; -struct IBase1 : Base1 { }; +struct IBase1 : Base1 {}; -struct IIBase1 : IBase1 { }; +struct IIBase1 : IBase1 {}; From 4d14901803a6fb0fa1969752de32b7ab2a3046ab Mon Sep 17 00:00:00 2001 From: John Hui Date: Thu, 12 Dec 2024 15:25:05 -0800 Subject: [PATCH 4/9] Fix test cases Also include a little more of the mangled symbol, for more precise testing + better readability of the `// CHECK: ` directives (though they are still pretty unreadable) --- test/Interop/Cxx/class/inheritance/fields-irgen.swift | 4 ++-- test/Interop/Cxx/class/inheritance/subscript-irgen.swift | 4 ++-- .../class/move-only/inherited-field-access-irgen.swift | 8 ++------ .../class/move-only/inherited-field-access-silgen.swift | 4 ++-- .../foreign-reference/function-inheritance-irgen.swift | 2 +- 5 files changed, 9 insertions(+), 13 deletions(-) diff --git a/test/Interop/Cxx/class/inheritance/fields-irgen.swift b/test/Interop/Cxx/class/inheritance/fields-irgen.swift index 21e81cea7337d..2ce539a53d811 100644 --- a/test/Interop/Cxx/class/inheritance/fields-irgen.swift +++ b/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 @{{.*}}30CopyTrackedDerivedDerivedClass{{.*}}33__synthesizedBaseGetterAccessor_x{{.*}}(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 diff --git a/test/Interop/Cxx/class/inheritance/subscript-irgen.swift b/test/Interop/Cxx/class/inheritance/subscript-irgen.swift index 0778085dbe9b0..5b479ab35118b 100644 --- a/test/Interop/Cxx/class/inheritance/subscript-irgen.swift +++ b/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 @{{.*}}30CopyTrackedDerivedDerivedClass{{.*}}39__synthesizedBaseCall_operatorSubscript{{.*}}(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 @{{.*}}20CopyTrackedBaseClass{{.*}}(ptr {{.*}} %[[ADD_PTR]], i32 {{.*}}) diff --git a/test/Interop/Cxx/class/move-only/inherited-field-access-irgen.swift b/test/Interop/Cxx/class/move-only/inherited-field-access-irgen.swift index 77d89f906d4f2..6a4e0e452938e 100644 --- a/test/Interop/Cxx/class/move-only/inherited-field-access-irgen.swift +++ b/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{{.*}} // CHECK: %[[VPTR:.*]] = getelementptr inbounds %struct.NonCopyableHolder // CHECK: ret ptr %[[VPTR]] -// CHECK: define {{.*}}linkonce_odr{{.*}} ptr @{{.*}}__synthesizedBaseSetterAccessor{{.*}} +// CHECK: define {{.*}}linkonce_odr{{.*}} ptr @{{.*}}31NonCopyableHolderDerivedDerived{{.*}}33__synthesizedBaseSetterAccessor_x{{.*}} // CHECK: %[[VPTRS:.*]] = getelementptr inbounds %struct.NonCopyableHolder // CHECK: ret ptr %[[VPTRS]] diff --git a/test/Interop/Cxx/class/move-only/inherited-field-access-silgen.swift b/test/Interop/Cxx/class/move-only/inherited-field-access-silgen.swift index d69dbd39fe9d6..62809df711a98 100644 --- a/test/Interop/Cxx/class/move-only/inherited-field-access-silgen.swift +++ b/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 // CHECK: {{.*}}(%[[SELF_VAL:.*]] : ${{(\*)?}}NonCopyableHolderDerivedDerived): -// CHECK: function_ref @{{.*}}__synthesizedBaseCall___synthesizedBaseGetterAccessor{{.*}} : $@convention(cxx_method) (@in_guaranteed NonCopyableHolderDerivedDerived) -> UnsafePointer +// CHECK: function_ref @{{.*}}31NonCopyableHolderDerivedDerived{{.*}}33__synthesizedBaseGetterAccessor_x{{.*}} : $@convention(cxx_method) (@in_guaranteed NonCopyableHolderDerivedDerived) -> UnsafePointer // CHECK-NEXT: apply %{{.*}} // CHECK: sil shared [transparent] @$sSo024NonCopyableHolderDerivedD0V1xSo0aB0Vvau : $@convention(method) (@inout NonCopyableHolderDerivedDerived) -> UnsafeMutablePointer -// CHECK: function_ref @{{.*}}__synthesizedBaseCall___synthesizedBaseSetterAccessor{{.*}} : $@convention(cxx_method) (@inout NonCopyableHolderDerivedDerived) -> UnsafeMutablePointer +// CHECK: function_ref @{{.*}}31NonCopyableHolderDerivedDerived{{.*}}33__synthesizedBaseSetterAccessor_x{{.*}} : $@convention(cxx_method) (@inout NonCopyableHolderDerivedDerived) -> UnsafeMutablePointer diff --git a/test/Interop/Cxx/foreign-reference/function-inheritance-irgen.swift b/test/Interop/Cxx/foreign-reference/function-inheritance-irgen.swift index 732f06acf99fa..43c048b15382b 100644 --- a/test/Interop/Cxx/foreign-reference/function-inheritance-irgen.swift +++ b/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 @{{.*}}30CopyTrackedDerivedDerivedClass{{.*}}26__synthesizedBaseCall_getX{{.*}}(ptr {{.*}} %[[THIS_PTR:.*]]) // CHECK: %[[ADD_PTR:.*]] = getelementptr inbounds i8, ptr %{{.*}}, i{{32|64}} 4 // CHECK: call{{.*}} i32 @{{.*}}(ptr {{.*}} %[[ADD_PTR]]) From c50a16a53c0be3b3961bd5aeca571721bce412d3 Mon Sep 17 00:00:00 2001 From: John Hui Date: Thu, 12 Dec 2024 15:30:34 -0800 Subject: [PATCH 5/9] Simplify implementation to something self-explanatory --- .../swift/ClangImporter/ClangImporterRequests.h | 15 +++------------ lib/ClangImporter/ClangImporter.cpp | 17 ++++++++--------- 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/include/swift/ClangImporter/ClangImporterRequests.h b/include/swift/ClangImporter/ClangImporterRequests.h index 76a0ae42700e9..d2d78a2181351 100644 --- a/include/swift/ClangImporter/ClangImporterRequests.h +++ b/include/swift/ClangImporter/ClangImporterRequests.h @@ -136,21 +136,12 @@ class CXXNamespaceMemberLookup struct ClangRecordMemberLookupDescriptor final { NominalTypeDecl *recordDecl; DeclName name; - clang::AccessSpecifier inheritance; + bool inherited; - // Base case lookup on (most) derived class - ClangRecordMemberLookupDescriptor(NominalTypeDecl *recordDecl, DeclName name) - : recordDecl(recordDecl), name(name), inheritance(clang::AS_none) { - assert(isa(recordDecl->getClangDecl())); - } - - // Recursive lookup on inherited classes ClangRecordMemberLookupDescriptor(NominalTypeDecl *recordDecl, DeclName name, - clang::AccessSpecifier inheritance) - : recordDecl(recordDecl), name(name), inheritance(inheritance) { + bool inherited = false) + : recordDecl(recordDecl), name(name), inherited(inherited) { assert(isa(recordDecl->getClangDecl())); - assert(inheritance != clang::AS_none && - "class inheritance should be specified"); } friend llvm::hash_code diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index dae8eebd51535..2edfa8ea6f438 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -6145,7 +6145,7 @@ TinyPtrVector ClangRecordMemberLookup::evaluate( Evaluator &evaluator, ClangRecordMemberLookupDescriptor desc) const { NominalTypeDecl *recordDecl = desc.recordDecl; DeclName name = desc.name; - clang::AccessSpecifier inheritance = desc.inheritance; + bool inherited = desc.inherited; auto &ctx = recordDecl->getASTContext(); auto allResults = evaluateOrDefault( @@ -6182,8 +6182,7 @@ TinyPtrVector ClangRecordMemberLookup::evaluate( foundNameArities.insert(getArity(valueDecl)); for (auto base : cxxRecord->bases()) { - clang::AccessSpecifier baseAccess = base.getAccessSpecifier(); - if (baseAccess != clang::AccessSpecifier::AS_public) + if (base.getAccessSpecifier() != clang::AccessSpecifier::AS_public) continue; clang::QualType baseType = base.getType(); @@ -6200,11 +6199,11 @@ TinyPtrVector ClangRecordMemberLookup::evaluate( continue; // Add Clang members that are imported lazily. - auto baseResults = evaluateOrDefault( - ctx.evaluator, - ClangRecordMemberLookup( - {cast(import), name, baseAccess}), - {}); + auto baseResults = + evaluateOrDefault(ctx.evaluator, + ClangRecordMemberLookup( + {cast(import), name, true}), + {}); // Add members that are synthesized eagerly, such as subscripts. for (auto member : cast(import)->getCurrentMembersWithoutLoading()) { @@ -6233,7 +6232,7 @@ TinyPtrVector ClangRecordMemberLookup::evaluate( // // Instead, we simply pass on the imported decl (foundInBase) as is, // so that only the top-most request calls importBaseMemberDecl(). - if (inheritance != clang::AS_none) { + if (inherited) { result.push_back(foundInBase); continue; } From 23769c574e0eac165c85f574f0731da88d4417a5 Mon Sep 17 00:00:00 2001 From: John Hui Date: Thu, 12 Dec 2024 15:38:30 -0800 Subject: [PATCH 6/9] Get rid of redundant #include --- include/swift/ClangImporter/ClangImporterRequests.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/swift/ClangImporter/ClangImporterRequests.h b/include/swift/ClangImporter/ClangImporterRequests.h index d2d78a2181351..4b7e0a445b4ef 100644 --- a/include/swift/ClangImporter/ClangImporterRequests.h +++ b/include/swift/ClangImporter/ClangImporterRequests.h @@ -24,7 +24,6 @@ #include "swift/Basic/Statistic.h" #include "swift/ClangImporter/ClangImporter.h" #include "clang/AST/Type.h" -#include "clang/Basic/Specifiers.h" #include "llvm/ADT/Hashing.h" #include "llvm/ADT/TinyPtrVector.h" From b61e6840c0d4abb61c5b8111c31df4156fe5d2f5 Mon Sep 17 00:00:00 2001 From: John Hui Date: Mon, 16 Dec 2024 10:07:34 -0800 Subject: [PATCH 7/9] Fix mangling to account for MSVC --- test/Interop/Cxx/class/inheritance/fields-irgen.swift | 2 +- test/Interop/Cxx/class/inheritance/subscript-irgen.swift | 4 ++-- .../Cxx/class/move-only/inherited-field-access-irgen.swift | 4 ++-- .../Cxx/class/move-only/inherited-field-access-silgen.swift | 4 ++-- .../Cxx/foreign-reference/function-inheritance-irgen.swift | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/test/Interop/Cxx/class/inheritance/fields-irgen.swift b/test/Interop/Cxx/class/inheritance/fields-irgen.swift index 2ce539a53d811..b18a39da3ce7d 100644 --- a/test/Interop/Cxx/class/inheritance/fields-irgen.swift +++ b/test/Interop/Cxx/class/inheritance/fields-irgen.swift @@ -9,6 +9,6 @@ func testGetX() -> CInt { let _ = testGetX() -// CHECK: define {{.*}}linkonce_odr{{.*}} i32 @{{.*}}30CopyTrackedDerivedDerivedClass{{.*}}33__synthesizedBaseGetterAccessor_x{{.*}}(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: %[[X:.*]] = getelementptr inbounds %class.CopyTrackedBaseClass, ptr %[[ADD_PTR]], i32 0, i32 0 diff --git a/test/Interop/Cxx/class/inheritance/subscript-irgen.swift b/test/Interop/Cxx/class/inheritance/subscript-irgen.swift index 5b479ab35118b..762fb1262033f 100644 --- a/test/Interop/Cxx/class/inheritance/subscript-irgen.swift +++ b/test/Interop/Cxx/class/inheritance/subscript-irgen.swift @@ -9,6 +9,6 @@ func testGetX() -> CInt { let _ = testGetX() -// CHECK: define {{.*}}linkonce_odr{{.*}} i32 @{{.*}}30CopyTrackedDerivedDerivedClass{{.*}}39__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:.*]] = call {{.*}} i32 @{{.*}}20CopyTrackedBaseClass{{.*}}(ptr {{.*}} %[[ADD_PTR]], i32 {{.*}}) +// CHECK: %[[CALL:.*]] = call {{.*}} i32 @{{.*}}CopyTrackedBaseClass{{.*}}(ptr {{.*}} %[[ADD_PTR]], i32 {{.*}}) diff --git a/test/Interop/Cxx/class/move-only/inherited-field-access-irgen.swift b/test/Interop/Cxx/class/move-only/inherited-field-access-irgen.swift index 6a4e0e452938e..75643f9261883 100644 --- a/test/Interop/Cxx/class/move-only/inherited-field-access-irgen.swift +++ b/test/Interop/Cxx/class/move-only/inherited-field-access-irgen.swift @@ -17,10 +17,10 @@ func testSetX(_ x: CInt) { testSetX(2) -// CHECK: define {{.*}}linkonce_odr{{.*}} ptr @{{.*}}31NonCopyableHolderDerivedDerived{{.*}}33__synthesizedBaseGetterAccessor_x{{.*}} +// 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 @{{.*}}31NonCopyableHolderDerivedDerived{{.*}}33__synthesizedBaseSetterAccessor_x{{.*}} +// CHECK: define {{.*}}linkonce_odr{{.*}} ptr @{{(.*)(31NonCopyableHolderDerivedDerived33__synthesizedBaseSetterAccessor_x|__synthesizedBaseSetterAccessor_x@NonCopyableHolderDerivedDerived)(.*)}} // CHECK: %[[VPTRS:.*]] = getelementptr inbounds %struct.NonCopyableHolder // CHECK: ret ptr %[[VPTRS]] diff --git a/test/Interop/Cxx/class/move-only/inherited-field-access-silgen.swift b/test/Interop/Cxx/class/move-only/inherited-field-access-silgen.swift index 62809df711a98..5539e89b16777 100644 --- a/test/Interop/Cxx/class/move-only/inherited-field-access-silgen.swift +++ b/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 // CHECK: {{.*}}(%[[SELF_VAL:.*]] : ${{(\*)?}}NonCopyableHolderDerivedDerived): -// CHECK: function_ref @{{.*}}31NonCopyableHolderDerivedDerived{{.*}}33__synthesizedBaseGetterAccessor_x{{.*}} : $@convention(cxx_method) (@in_guaranteed NonCopyableHolderDerivedDerived) -> UnsafePointer +// CHECK: function_ref @{{(.*)(31NonCopyableHolderDerivedDerived33__synthesizedBaseGetterAccessor_x|__synthesizedBaseGetterAccessor_x@NonCopyableHolderDerivedDerived)(.*)}} : $@convention(cxx_method) (@in_guaranteed NonCopyableHolderDerivedDerived) -> UnsafePointer // CHECK-NEXT: apply %{{.*}} // CHECK: sil shared [transparent] @$sSo024NonCopyableHolderDerivedD0V1xSo0aB0Vvau : $@convention(method) (@inout NonCopyableHolderDerivedDerived) -> UnsafeMutablePointer -// CHECK: function_ref @{{.*}}31NonCopyableHolderDerivedDerived{{.*}}33__synthesizedBaseSetterAccessor_x{{.*}} : $@convention(cxx_method) (@inout NonCopyableHolderDerivedDerived) -> UnsafeMutablePointer +// CHECK: function_ref @{{(.*)(31NonCopyableHolderDerivedDerived33__synthesizedBaseSetterAccessor_x|__synthesizedBaseSetterAccessor_x@NonCopyableHolderDerivedDerived)(.*)}} : $@convention(cxx_method) (@inout NonCopyableHolderDerivedDerived) -> UnsafeMutablePointer diff --git a/test/Interop/Cxx/foreign-reference/function-inheritance-irgen.swift b/test/Interop/Cxx/foreign-reference/function-inheritance-irgen.swift index 43c048b15382b..c0100971221ad 100644 --- a/test/Interop/Cxx/foreign-reference/function-inheritance-irgen.swift +++ b/test/Interop/Cxx/foreign-reference/function-inheritance-irgen.swift @@ -10,6 +10,6 @@ func testGetX() -> CInt { let _ = testGetX() -// CHECK: define {{.*}}linkonce_odr{{.*}} i32 @{{.*}}30CopyTrackedDerivedDerivedClass{{.*}}26__synthesizedBaseCall_getX{{.*}}(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]]) From bb0d6bbd63a211be2a4d3baa399bace097cd0dfe Mon Sep 17 00:00:00 2001 From: John Hui Date: Mon, 16 Dec 2024 17:00:07 -0800 Subject: [PATCH 8/9] Add IBase method to inherited-lookup test --- .../Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h | 6 ++++-- .../Cxx/class/inheritance/inherited-lookup-executable.swift | 3 ++- .../class/inheritance/inherited-lookup-typechecker.swift | 6 ++++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h b/test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h index fcb9dee528496..98099974b54ac 100644 --- a/test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h +++ b/test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h @@ -1,9 +1,11 @@ #pragma once struct Base1 { - int method(void) const { return 1; } + int methodBase(void) const { return 1; } }; -struct IBase1 : Base1 {}; +struct IBase1 : Base1 { + int methodIBase(void) const { return 11; } +}; struct IIBase1 : IBase1 {}; diff --git a/test/Interop/Cxx/class/inheritance/inherited-lookup-executable.swift b/test/Interop/Cxx/class/inheritance/inherited-lookup-executable.swift index bf72b751d2dbe..351db770aa868 100644 --- a/test/Interop/Cxx/class/inheritance/inherited-lookup-executable.swift +++ b/test/Interop/Cxx/class/inheritance/inherited-lookup-executable.swift @@ -8,7 +8,8 @@ var InheritedMemberTestSuite = TestSuite("Test if inherited lookup works") InheritedMemberTestSuite.test("IIBase1::method() resolves to grandparent") { let iibase1 = IIBase1() - expectEqual(iibase1.method(), 1) + expectEqual(iibase1.methodBase(), 1) + expectEqual(iibase1.methodIBase(), 11) } runAllTests() diff --git a/test/Interop/Cxx/class/inheritance/inherited-lookup-typechecker.swift b/test/Interop/Cxx/class/inheritance/inherited-lookup-typechecker.swift index cb7a3b1cde4bb..a479eac62b52e 100644 --- a/test/Interop/Cxx/class/inheritance/inherited-lookup-typechecker.swift +++ b/test/Interop/Cxx/class/inheritance/inherited-lookup-typechecker.swift @@ -10,11 +10,13 @@ extension IIBase1 { // For instance, a non-idempotent ClangRecordMemberLookup would cause // the following to appear ambiguous: - method() + methodBase() + methodIBase() } } func f(v: IIBase1) { v.missing() // expected-error {{'IIBase1' has no member 'missing'}} - v.method() + v.methodBase() + v.methodIBase() } From 379852678c9dfa97bb1316bc47edd2fb5c40a12d Mon Sep 17 00:00:00 2001 From: John Hui Date: Mon, 16 Dec 2024 17:01:19 -0800 Subject: [PATCH 9/9] Add IIBase method too --- test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h | 4 +++- .../Cxx/class/inheritance/inherited-lookup-executable.swift | 1 + .../Cxx/class/inheritance/inherited-lookup-typechecker.swift | 2 ++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h b/test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h index 98099974b54ac..443d5078cc4eb 100644 --- a/test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h +++ b/test/Interop/Cxx/class/inheritance/Inputs/inherited-lookup.h @@ -8,4 +8,6 @@ struct IBase1 : Base1 { int methodIBase(void) const { return 11; } }; -struct IIBase1 : IBase1 {}; +struct IIBase1 : IBase1 { + int methodIIBase(void) const { return 111; } +}; diff --git a/test/Interop/Cxx/class/inheritance/inherited-lookup-executable.swift b/test/Interop/Cxx/class/inheritance/inherited-lookup-executable.swift index 351db770aa868..7ab797c0eb7a3 100644 --- a/test/Interop/Cxx/class/inheritance/inherited-lookup-executable.swift +++ b/test/Interop/Cxx/class/inheritance/inherited-lookup-executable.swift @@ -10,6 +10,7 @@ InheritedMemberTestSuite.test("IIBase1::method() resolves to grandparent") { let iibase1 = IIBase1() expectEqual(iibase1.methodBase(), 1) expectEqual(iibase1.methodIBase(), 11) + expectEqual(iibase1.methodIIBase(), 111) } runAllTests() diff --git a/test/Interop/Cxx/class/inheritance/inherited-lookup-typechecker.swift b/test/Interop/Cxx/class/inheritance/inherited-lookup-typechecker.swift index a479eac62b52e..5e59a54494e0d 100644 --- a/test/Interop/Cxx/class/inheritance/inherited-lookup-typechecker.swift +++ b/test/Interop/Cxx/class/inheritance/inherited-lookup-typechecker.swift @@ -12,6 +12,7 @@ extension IIBase1 { // the following to appear ambiguous: methodBase() methodIBase() + methodIIBase() } } @@ -19,4 +20,5 @@ func f(v: IIBase1) { v.missing() // expected-error {{'IIBase1' has no member 'missing'}} v.methodBase() v.methodIBase() + v.methodIIBase() }