-
Notifications
You must be signed in to change notification settings - Fork 341
[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
base: stable/20240723
Are you sure you want to change the base?
[Clang][Sema] Allow qualified type names in swift_name
attribute
#10899
Conversation
425f4d4
to
bcb7d65
Compare
@swift-ci please test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
clang/lib/Sema/SemaSwift.cpp
Outdated
// ContextName might be qualified, e.g. 'MyNamespace.MyStruct'. | ||
SmallVector<StringRef, 1> ContextNameComponents; | ||
ContextName.split(ContextNameComponents, '.'); | ||
for (auto Component : ContextNameComponents) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, done!
bcb7d65
to
a75b134
Compare
@swift-ci please test |
@swift-ci please test macOS |
There was a problem hiding this 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}} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
a75b134
to
2df5b2b
Compare
@swift-ci please test |
@swift-ci please test macOS |
@swift-ci please test Windows |
This patch will need to go upstream.
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