-
Notifications
You must be signed in to change notification settings - Fork 10.5k
rdar://problem/26424505 Import as Member: QoI errors #2650
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
rdar://problem/26424505 Import as Member: QoI errors #2650
Conversation
…API notes don't crash.
We now reject this in Clang.
This is now diagnosed on the Clang side.
… notes. Most cases fall out from swift_name validation on the Clang side dropping invalid API notes, though the validation on the Clang side is conservative and misses some cases. We still have Swift-side work to fall back to the original name here.
@jrose-apple Do you mind reviewing these changes along with apple/swift-clang#15 ? |
@swift-ci test |
Failure is expected until apple/swift-clang#15 merges. |
apple/swift-clang#15 has been merged. |
@swift-ci smoke test os x platform |
@swift-ci test linux |
If Linux CI comes back good I'll merge. |
__attribute__((swift_name("ErrorProto.mutateSomeStaticState()"))); // ok | ||
void mutateSomeInstanceState(ErrorProto_t self) __attribute__(( | ||
swift_name("ErrorProto.mutateSomeInstanceState(self:)"))); // error | ||
|
||
// Non-prototype declaration | ||
extern void IAMErrorStructHasPrototype(void) | ||
__attribute__((swift_name("ErrorStruct.hasPrototype()"))); // ok | ||
extern void IAMErrorStructNonPrototype() | ||
__attribute__((swift_name("ErrorStruct.nonPrototype()"))); // error |
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.
This is pretty much the entire thing being tested, and now the comment is wrong. If there's a test on the Clang side I'm okay with removing it.
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.
That was my thinking; now that we have better Clang-side validation and just throw the attr away if it's invalid, this wasn't really nonredundantly testing anything anymore.
Test updates necessary to keep up with the swift-clang changes in apple/swift-clang#15 . This depends on that pull request getting merged into Clang's 3.0 branch simultaneously.