-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[cxx-interop] Support ObjC protocols in C++ interop #77025
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
Conversation
@swift-ci please smoke test |
// RUN: %target-swift-frontend -typecheck %t/use-objc-types.swift -typecheck -module-name UseObjCTy -emit-clang-header-path %t/UseObjCTyExposeOnly.h -I %t -enable-experimental-cxx-interop -clang-header-expose-decls=has-expose-attr | ||
|
||
// RUN: %FileCheck %s < %t/UseObjCTyExposeOnly.h |
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.
Hmm, is this removal intentional?
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.
Yeah, we tested that the generated header has everything we need both when we use the attributes and when we expose everything that is public. I had the impression that we are migrating away from the attributes and also found that it is probably an overkill to run this test twice only to test those attributes (and we have different test for them). That being said, I am happy to add them back if you think it is still valuable to have these tests.
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.
Ah that's alright, thanks for explaining!
6c144a6
to
12754f0
Compare
@swift-ci please smoke test |
This patch introduces handling of ObjC protocols similar to how ObjC classes work. Since this only works in ObjC++, all declarations containing ObjC protocols will be protected by the __OBJC__ macro. This patch results in some `_bridgeObjC` methods being exposed, we might end up hiding those in the future, but there is no harm having them in the interop header for the interim period. rdar://136757913
12754f0
to
0e03d34
Compare
@swift-ci please smoke test |
@@ -24,6 +24,16 @@ -(ObjCKlass * _Nonnull) init:(int)x; | |||
-(int)getValue; | |||
@end | |||
|
|||
@protocol ObjCProtocol |
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 test does not run by the CI, unfortunately. It is disabled. But I enabled it locally and it does pass for me. I do not want to reenable this as part of this PR because I don't think the original failure was ever fixed (on a platform different from what I am running on).
This patch introduces handling of ObjC protocols similar to how ObjC classes work. Since this only works in ObjC++, all declarations containing ObjC protocols will be protected by the OBJC macro.
This patch results in some
_bridgeObjC
methods being exposed, we might end up hiding those in the future, but there is no harm having them in the interop header for the interim period.rdar://136757913