Skip to content

[cxx-interop] Fix issue where multiple records in a module containing the same meth… #60338

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 2, 2022
Merged

[cxx-interop] Fix issue where multiple records in a module containing the same meth… #60338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fbda09e
Fix issue where multiple records in a module containing the same meth…
Huddie Aug 1, 2022
1242e66
Fix test
Huddie Aug 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions lib/ClangImporter/ImportDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2110,16 +2110,18 @@ namespace {
for (auto m : decl->decls()) {
if (auto method = dyn_cast<clang::CXXMethodDecl>(m)) {
if (method->getDeclName().isIdentifier()) {
if (Impl.cxxMethods.find(method->getName()) ==
Impl.cxxMethods.end()) {
Impl.cxxMethods[method->getName()] = {};
auto contextMap = Impl.cxxMethods.find(method->getDeclContext());
if (contextMap == Impl.cxxMethods.end() ||
contextMap->second.find(method->getName()) ==
contextMap->second.end()) {
Impl.cxxMethods[method->getDeclContext()][method->getName()] = {};
}
if (method->isConst()) {
// Add to const set
Impl.cxxMethods[method->getName()].first.insert(method);
Impl.cxxMethods[method->getDeclContext()][method->getName()].first.insert(method);
} else {
// Add to mutable set
Impl.cxxMethods[method->getName()].second.insert(method);
Impl.cxxMethods[method->getDeclContext()][method->getName()].second.insert(method);
}
}
}
Expand Down Expand Up @@ -2258,8 +2260,11 @@ namespace {
}

if (cxxMethod->getDeclName().isIdentifier()) {
auto &mutableFuncPtrs = Impl.cxxMethods[cxxMethod->getName()].second;
if(mutableFuncPtrs.contains(cxxMethod)) {
auto &mutableFuncPtrs =
Impl.cxxMethods[cxxMethod->getDeclContext()]
[cxxMethod->getName()]
.second;
if (mutableFuncPtrs.contains(cxxMethod)) {
result->addMemberToLookupTable(member);
}
}
Expand Down Expand Up @@ -2977,7 +2982,7 @@ namespace {
// In such a case append a suffix ("Mutating") to the mutable version
// of the method when importing to swift
if(decl->getDeclName().isIdentifier()) {
const auto &cxxMethodPair = Impl.cxxMethods[decl->getName()];
const auto &cxxMethodPair = Impl.cxxMethods[decl->getDeclContext()][decl->getName()];
const auto &constFuncPtrs = cxxMethodPair.first;
const auto &mutFuncPtrs = cxxMethodPair.second;

Expand Down
4 changes: 2 additions & 2 deletions lib/ClangImporter/ImporterImpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -615,10 +615,10 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation

/// Keep track of cxx function names, params etc in order to
/// allow for de-duping functions that differ strictly on "constness".
llvm::DenseMap<llvm::StringRef,
llvm::DenseMap<const clang::DeclContext *, llvm::DenseMap<llvm::StringRef,
std::pair<
llvm::DenseSet<clang::FunctionDecl *>,
llvm::DenseSet<clang::FunctionDecl *>>>
llvm::DenseSet<clang::FunctionDecl *>>>>
cxxMethods;

// Cache for already-specialized function templates and any thunks they may
Expand Down
6 changes: 6 additions & 0 deletions test/Interop/Cxx/class/method/Inputs/ambiguous_methods.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,10 @@ struct HasAmbiguousMethods {
int mutableMethodsCalledCount = 0;
};

struct HasAmbiguousMethods2 {
int increment(int a) const {
return a + 1;
}
};

#endif // TEST_INTEROP_CXX_CLASS_AMBIGUOUS_METHOD_METHODS_H
4 changes: 4 additions & 0 deletions test/Interop/Cxx/class/method/ambiguous-methods-module-interface.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@

// CHECK: func numberOfMutableMethodsCalled() -> Int32
// CHECK: mutating func numberOfMutableMethodsCalledMutating() -> Int32

// CHECK: struct HasAmbiguousMethods2
// CHECK: func increment(_ a: Int32) -> Int32
// CHECK-NOT: mutating func incrementMutating(_ a: Int32) -> Int32
36 changes: 18 additions & 18 deletions test/Interop/Cxx/ergonomics/implicit-computed-properties-module-interface.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
// CHECK: struct VoidGetter {
// CHECK-NOT: var
// CHECK-NEXT: init()
// CHECK-NEXT: mutating func getXMutating()
// CHECK-NEXT: mutating func setXMutating(_: Int32)
// CHECK-NEXT: mutating func getX()
// CHECK-NEXT: mutating func setX(_: Int32)
// CHECK-NEXT: }

// CHECK: struct VoidSetterNoName {
Expand All @@ -16,13 +16,13 @@
// CHECK: struct IllegalIntReturnSetter {
// CHECK-NOT: var
// CHECK-NEXT: init()
// CHECK-NEXT: mutating func setXMutating(_: Int32) -> Int32
// CHECK-NEXT: mutating func setX(_: Int32) -> Int32
// CHECK-NEXT: }

// CHECK: struct TwoParameterSetter {
// CHECK-NOT: var
// CHECK-NEXT: init()
// CHECK-NEXT: mutating func setXMutating(_: Int32, _: Int32)
// CHECK-NEXT: mutating func setX(_: Int32, _: Int32)
// CHECK-NEXT: }

// CHECK: struct NoNameSetter {
Expand Down Expand Up @@ -88,15 +88,15 @@

// CHECK: struct NotypeSetter {
// CHECK-NEXT: init()
// CHECK-NEXT: mutating func setXMutating()
// CHECK-NEXT: mutating func setX()
// CHECK-NEXT: }

// CHECK: struct IntGetterSetter {
// CHECK-NEXT: init()
// CHECK-NEXT: init(val: Int32)
// CHECK-NEXT: var x: Int32
// CHECK-NEXT: func getX() -> Int32
// CHECK-NEXT: mutating func setXMutating(_ v: Int32)
// CHECK-NEXT: mutating func setX(_ v: Int32)
// CHECK-NEXT: var val: Int32
// CHECK-NEXT: }

Expand All @@ -120,7 +120,7 @@
// CHECK: struct GetterHasArg {
// CHECK-NEXT: init()
// CHECK-NEXT: func getX(_ v: Int32) -> Int32
// CHECK-NEXT: mutating func setXMutating(_ v: Int32)
// CHECK-NEXT: mutating func setX(_ v: Int32)
// CHECK-NEXT: }

// CHECK: struct GetterSetterIsUpper {
Expand Down Expand Up @@ -172,8 +172,8 @@
// CHECK-NEXT: init()
// CHECK-NEXT: init(val: Int32)
// CHECK-NEXT: var x: Int32 { mutating get set }
// CHECK-NEXT: mutating func getXMutating() -> Int32
// CHECK-NEXT: mutating func setXMutating(_ v: Int32)
// CHECK-NEXT: mutating func getX() -> Int32
// CHECK-NEXT: mutating func setX(_ v: Int32)
// CHECK-NEXT: var val: Int32
// CHECK-NEXT: }

Expand All @@ -191,7 +191,7 @@
// CHECK-NEXT: init()
// CHECK-NEXT: var x: Int32
// CHECK-NEXT: func getX() -> Int32
// CHECK-NEXT: mutating func setXMutating(_ a: Int32, _ b: Int32)
// CHECK-NEXT: mutating func setX(_ a: Int32, _ b: Int32)
// CHECK-NEXT: }

// CHECK: struct NonTrivial {
Expand All @@ -204,33 +204,33 @@
// CHECK-NEXT: init()
// CHECK-NEXT: init(value: Int32)
// CHECK-NEXT: var x: UnsafeMutablePointer<Int32>? { mutating get set }
// CHECK-NEXT: mutating func getXMutating() -> UnsafeMutablePointer<Int32>!
// CHECK-NEXT: mutating func setXMutating(_ v: UnsafeMutablePointer<Int32>!)
// CHECK-NEXT: mutating func getX() -> UnsafeMutablePointer<Int32>!
// CHECK-NEXT: mutating func setX(_ v: UnsafeMutablePointer<Int32>!)
// CHECK-NEXT: var value: Int32
// CHECK-NEXT: }

// CHECK: struct RefGetterSetter {
// CHECK-NEXT: init()
// CHECK-NEXT: init(value: Int32)
// CHECK-NEXT: mutating func getXMutating() -> UnsafePointer<Int32>
// CHECK-NEXT: mutating func setXMutating(_ v: Int32)
// CHECK-NEXT: mutating func getX() -> UnsafePointer<Int32>
// CHECK-NEXT: mutating func setX(_ v: Int32)
// CHECK-NEXT: var value: Int32
// CHECK-NEXT: }

// CHECK: struct NonTrivialGetterSetter {
// CHECK-NEXT: init()
// CHECK-NEXT: init(value: NonTrivial)
// CHECK-NEXT: var x: NonTrivial { mutating get set }
// CHECK-NEXT: mutating func getXMutating() -> NonTrivial
// CHECK-NEXT: mutating func setXMutating(_ v: NonTrivial)
// CHECK-NEXT: mutating func getX() -> NonTrivial
// CHECK-NEXT: mutating func setX(_ v: NonTrivial)
// CHECK-NEXT: var value: NonTrivial
// CHECK-NEXT: }

// CHECK: struct DifferentTypes {
// CHECK-NEXT: init()
// CHECK-NEXT: init(value: NonTrivial)
// CHECK-NEXT: mutating func getXMutating() -> NonTrivial
// CHECK-NEXT: mutating func setXMutating(_ v: Int32)
// CHECK-NEXT: mutating func getX() -> NonTrivial
// CHECK-NEXT: mutating func setX(_ v: Int32)
// CHECK-NEXT: var value: NonTrivial
// CHECK-NEXT: }

Expand Down