Skip to content
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

Add NS_ENUM support, for Swift #701

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

cormacrelf
Copy link
Contributor

@cormacrelf cormacrelf commented Jul 6, 2021

See NS_ENUM docs. See this excerpt from docs.md for motivation:

Normally, the ergonomics of C enums in Swift is very poor; you can't use the variants directly and pass them to function accepting the typedef name, because Swift sees the variants each as just raw integers and the typedef name as a newtype. So you essentially have to redefine the enum in Swift, get its .rawValue and convert it to the C enum type, and convert back in the other direction.

With NS_ENUM/CF_ENUM, you can import e.g. #[repr(i32)] pub enum A { Abc, Def } in Swift and use it as A.abc as you would with a Swift enum.

Config is like so:

sys_includes = ["CoreFoundation/CoreFoundation.h"]
[enum]
swift_enum_macro = "CF_ENUM"

Output is like so:

typedef CF_ENUM(uint64_t, A) {
  a1 = 0,
  a2 = 2,
  a3,
  a4 = 5,
};

It clobbers cpp_compat's effect on enums, which is normally to add a C++11 fixed type in an ifdef (enum Name: uint32_t { ... }). This is because CF_ENUM/NS_ENUM will already do that; the difference is it works in Objective-C as well, and maybe they do some other annotation magic to get Swift to recognise enums.

Edit: I'll fix clippy later and also there's a failing test (test_body) but I think that was present before I made the changes so I'll check that too.

src/bindgen/ir/ty.rs Outdated Show resolved Hide resolved
@cormacrelf
Copy link
Contributor Author

@emilio I think all the issues are resolved, thanks for the reviews, please have another look when you have a chance :)

// technically you're not going to get #[repr(double)] because that's not valid rust,
// but let's check anyway
.map_or(false, |prim| prim.can_be_enum_fixed_type());
.is_some();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this is not what you intended. This is effectively self.repr.ty.is_some() so it should be simplified to that probably.

src/bindgen/ir/enumeration.rs Show resolved Hide resolved
@@ -669,12 +669,6 @@ impl EnumConfig {
}
self.private_default_tagged_enum_constructor
}
pub(crate) fn swift_enum_macro(&self, annotations: &AnnotationSet) -> Option<String> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

To be clear, I wasn't particularly against the annotation (though it probably isn't terribly useful and we can add it back if ever needed). I just thought that storing the "resolved" value shouldn't be necessary, since we store the annotations already.

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.

None yet

2 participants