Skip to content

[cxx-interop] Allow import-as-member for types in namespaces #82496

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.

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 1 commit into from
Jun 27, 2025

Conversation

egorzhdan
Copy link
Contributor

This adds support for swift_name attribute being used with C++ types that are declared within namespaces, e.g.

__attribute__((swift_name("MyNamespace.MyType.my_method()")))

Previously import-as-member would only accept a top-level unqualified type name.

This relies on swiftlang/llvm-project#10899.

rdar://138934888

@egorzhdan egorzhdan added the c++ interop Feature: Interoperability with C++ label Jun 25, 2025
@egorzhdan
Copy link
Contributor Author

swiftlang/llvm-project#10899

@swift-ci please smoke test

Copy link
Contributor

@j-hui j-hui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

: lookup(SerializedSwiftName(nameComponent),
std::make_pair(ContextKind::TranslationUnit, StringRef()));
bool entryFound = false;
for (auto entry : entries) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: use llvm::find_if

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to refactor that locally, but that means we have to do the casting multiple times, both within the find_if and outside of it. I personally prefer a simple loop here, but I can change it if you feel strongly.

@egorzhdan
Copy link
Contributor Author

swiftlang/llvm-project#10899

@swift-ci please smoke test macOS

Copy link
Contributor

@Xazax-hun Xazax-hun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG! A minor suggestion to improve the tests but it is completely optional.

NamespacesTestSuite.test("Struct in a deep namespace") {
let s = MyNS.MyDeepNS.DeepNestedStruct()
expectEqual(456, s.method())
expectEqual(456, s.methodConstRef())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If each method added a distinct number to the value we could be 100% sure we end up calling the right method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, let's do that.

This adds support for `swift_name` attribute being used with C++ types that are declared within namespaces, e.g.
```
__attribute__((swift_name("MyNamespace.MyType.my_method()")))
```

Previously import-as-member would only accept a top-level unqualified type name.

rdar://138934888
@egorzhdan egorzhdan force-pushed the egorzhdan/allow-qual-swift-name branch from 392cdd9 to e95f6a3 Compare June 26, 2025 11:45
@egorzhdan
Copy link
Contributor Author

swiftlang/llvm-project#10899

@swift-ci please smoke test

@egorzhdan
Copy link
Contributor Author

swiftlang/llvm-project#10899

@swift-ci please test Windows

@egorzhdan egorzhdan requested a review from DougGregor June 26, 2025 17:43
@egorzhdan egorzhdan merged commit f8664ad into main Jun 27, 2025
3 checks passed
@egorzhdan egorzhdan deleted the egorzhdan/allow-qual-swift-name branch June 27, 2025 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants