8000 [Clang][Sema] Allow qualified type names in `swift_name` attribute by egorzhdan · Pull Request #10899 · swiftlang/llvm-project · GitHub
[go: up one dir, main page]

Skip to content

[Clang][Sema] Allow qualified type names in swift_name attribute #10899

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
@egorzhdan egorzhdan commented Jun 25, 2025

Upstream PR: llvm#145947

This will allow adding __attribute__((swift_name("MyNamespace.MyType.my_method()"))) on standalone C++ functions to make Swift import them as member functions of a type that is nested in a C++ namespace.

rdar://138934888

@egorzhdan egorzhdan requested a review from a team as a code owner June 25, 2025 17:46
@egorzhdan egorzhdan force-pushed the egorzhdan/20240723-allow-qual-swift-name branch 2 times, most recently from 425f4d4 to bcb7d65 Compare June 25, 2025 17:57
@egorzhdan
Copy link
Author

@swift-ci please test

Copy link
@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

// ContextName might be qualified, e.g. 'MyNamespace.MyStruct'.
SmallVector<StringRef, 1> ContextNameComponents;
ContextName.split(ContextNameComponents, '.');
for (auto Component : ContextNameComponents) {
Copy link

Choose a reason for hiding this comment

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

nit: use std::all_of or llvm::all_of here?

Copy link
Author

Choose a reason for hiding this comment

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

Sounds good, done!

@egorzhdan egorzhdan force-pushed the egorzhdan/20240723-allow-qual-swift-name branch from bcb7d65 to a75b134 Compare June 25, 2025 18:43
@egorzhdan
Copy link
Author

@swift-ci please test

@egorzhdan
Copy link
Author

@swift-ci please test macOS

8000

Copy link
@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.

LGTM!

I wonder if at some point we will have an ask to support raw identifier literals, but probably that is a problem for another day 🥲


void nestedStruct_method(MyNS::NestedStruct) SWIFT_NAME("MyNS.NestedStruct.method(self:)");
void nestedStruct_methodConstRef(const MyNS::NestedStruct&) SWIFT_NAME("MyNS.NestedStruct.methodConstRef(self:)");
void nestedStruct_invalidContext1(MyNS::NestedStruct) SWIFT_NAME(".MyNS.NestedStruct.invalidContext1(self:)"); // expected-warning {{'swift_name' attribute has invalid identifier for the context name}}

Choose a reason for hiding this comment

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

Maybe we could test double dots (..) as well? Although I do expect that to be rejected as expected.

Copy link
A 8000 uthor

Choose a reason for hiding this comment

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

Added this test case and a couple more, thanks

This will allow adding `__attribute__((swift_name("MyNamespace.MyType.my_method()")))` on standalone C++ functions to make Swift import them as member functions of a type that is nested in a C++ namespace.

rdar://138934888
@egorzhdan egorzhdan force-pushed the egorzhdan/20240723-allow-qual-swift-name branch from a75b134 to 2df5b2b Compare June 26, 2025 11:22
@egorzhdan
Copy link
Author

@swift-ci please test

@egorzhdan
Copy link
Author

swiftlang/swift#82496

@swift-ci please test macOS

@egorzhdan
Copy link
Author

swiftlang/swift#82496

@swift-ci please test Windows

@egorzhdan egorzhdan merged commit d25ca79 into stable/20240723 Jun 27, 2025
3 checks passed
@egorzhdan egorzhdan deleted the egorzhdan/20240723-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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0