Skip to content

Commit

Permalink
[cxx-interop] Fix issue where multiple records in a module containing…
Browse files Browse the repository at this point in the history 
… the same meth… (#60338)
  • Loading branch information
@Huddie
Huddie committed Aug 2, 2022
1 parent 2304bb9 commit 379fc1f
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 28 deletions.
21 changes: 13 additions & 8 deletions lib/ClangImporter/ImportDecl.cpp
View file
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
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
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
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
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

0 comments on commit 379fc1f

Please sign in to comment.