Support SwiftPM and add Swift bindings for v6#2958
Conversation
Rot127
left a comment
There was a problem hiding this comment.
Thank you for the new attempt. It looks not bad at all!
To merge this though we need to add all the generator scripts and testing so we can continue to maintain it as easy as possible.
If those require to install the Swift toolchain, so be it.
But it must be maintainable from Linux (for CI, Linux users and Windows/WSL users).
Only the APIs in bindings/swift need to be tested, and I'm using them in my prototype.
That is not enough I am afraid:
- We won't be able to see for each PR if it breaks something. Or if we have forgotten to update the bindings if a PR adds something new.
- I really tried hard over the last years to increase the test coverage a lot. So we have a high certainty that Capstone works as intended. I really don't want to give up on this philosophy. Especially because we write C here and Capstone is such a "infrastructure-kinda" tool. It should just work with very few annoyances. So I would make it a hard requirement that the Switt bindings are tested.
| # CS_ARCH_EVM, ///< Ethereum architecture | ||
| # CS_ARCH_MOS65XX, ///< MOS65XX architecture (including MOS6502) | ||
| # CS_ARCH_WASM, ///< WebAssembly architecture | ||
| # CS_ARCH_BPF, ///< Berkeley Packet Filter architecture (including eBPF) |
There was a problem hiding this comment.
Some of them are missing, do you know why?
There was a problem hiding this comment.
Yes because I've been doing it manually so far, these just rename or precise semantics of the API to make them behave a bit nicer but they're not necessary for the bindings to be available.
There was a problem hiding this comment.
All Capstone.apinotes bindings are automatically generated by Swift.
How would be generate them here?
It needs a script in bindings/swift so we can run it before every release or if a PR cahgnes the API, so we can update the bindings.
There was a problem hiding this comment.
They're not generated manually. As in during the build the Swift compiler will ask Clang to build capstone and will automatically surface types and functions.
There's full two-interoperability so there's no meed for a script.
There was a problem hiding this comment.
Oh wait I see how what I wrote is confusing, my bad. Hopefully my comment on the other thread clarified it?
There was a problem hiding this comment.
So the Swift build will generate these .apinotes on the fly? Then we wouldn't need to add them here, or do I miss something?
There was a problem hiding this comment.
No, here's an example with cs_ac_type:
When Swift builds capstone, it provides a cs_ac_type structure with a single rawValue: UInt32 field and a number of globals for the enumerators, such as let CS_AC_READ = cs_ac_type(rawValue: 1).
The .apinotes file tells the Swift compiler that:
cs_ac_typeshould be renamed toAccessSetcs_ac_typeis a flags type, which adds a whole bunch of convenience methods such asvar access: AccessSet; access.contains(AccessSet.read); access.union(otherAccess)etc.CS_AC_INVALIDshould be renamed toAccessSet.invalid(which you can shorten to .invalid when the type is known in context).CS_AC_READshould be renamed toAccessSet.read(same as above).- etc.
When a user tries to use cs_ac_type, CS_AC_READ or such, the compiler tells them it was renamed and offers an automatic fix that renames it.
It also helps in structuring the generated documentation, now instead of CS_AC_READ being in the global namespace, it's placed under AccessSet.
This is especially good with architecture enums and structures:
Another example with cs_op_type:
cs_op_typeis renamed toCapstoneOpKindcs_op_typeis a proper enum:- it generates as a Swift enum, rather than a wrapper structure
- any globals of this type are nested under the type's namespace
cs_op_typeis a closed enum. This allows Swift to do exhaustiveness checking inswitchstatements.
Let me know if that makes sense?
There was a problem hiding this comment.
I see. Is there any reason to strip the cs_ prefix for Swift?
I don't know how they do namespace management, but in my experience it is better to not remove the name prefixes, since it can come to name collisions.
There was a problem hiding this comment.
Namespaces are by-module, any definition can be accessed explicitly via capstone::SomeTypeOrFunction.
In Swift prefixes are either removed or spelled out when ambiguous. In terms of ABIs the symbol names are still the C header ones.
There was a problem hiding this comment.
In Swift prefixes are either removed or spelled out when ambiguous. In terms of ABIs the symbol names are still the C header ones.
I see. Makes sense.
|
I'll fix that next week, got a pretty busy weekend! |
Your checklist for this pull request
Detailed description
I'm depending on capstone v6 for a macOS app prototype, as such I need Swift bindings and integration into SwiftPM.
While there is already another PR that did so, it seemed extremely intrusive to capstone, attempting to force it into SwiftPM's default convention rather than configuring SwiftPM appropriately.
Here are my changes to support building capstone with SwiftPM:
Package.swift, thecapstonetarget builds capstone.incfile, they can be manually added toexcludeto avoid them but I didn't feel like doing that yet.module.modulemap, this is needed by Clang as part of Xcode's build systemHere are my changes to add (nice, safer) Swift bindings:
Package.swift, theCapstoneKittarget wraps capstone where possible.Capstone.apinotes, a sidecar file that provides Clang with additional information and renames to make the automatically generated Swift bindings more "Swifty". It's very incomplete and some parts could likely be script-generated. See Clang's documentation. See Swift's extensions.bindings/swift/CapstoneKit.swiftwith a few work-in-progress wrapper typesTest plan
All
Capstone.apinotesbindings are automatically generated by Swift.There is no need to separately test them.
Only the APIs in
bindings/swiftneed to be tested, and I'm using them in my prototype.