-
Notifications
You must be signed in to change notification settings - Fork 206
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
Swift bindings generator with simple parser and emitter #2525
Swift bindings generator with simple parser and emitter #2525
Conversation
The SwiftRuntimeLibrary provides runtime marshaling support and projections of common Swift types.
The bindings generator includes a parser, marshaller, and emitter. The parser generates module declarations by consuming an ABI JSON file. The marshaller facilitates type marshalling between C# and Swift, while the emitter is a string-based IndentedTextWriter that outputs C# source code.
This PR has been updated with a bindings generator that includes a simple parser and emitter (refer to the updated description and docs). Please note that this is an MVP for CryptoKit dev templates. |
Co-authored-by: Rolf Bjarne Kvinge <rolf@xamarin.com>
Co-authored-by: Rolf Bjarne Kvinge <rolf@xamarin.com>
This commit introduces support for the UnsafeRawPointer type. It also adds a test case for the existing CryptoKit bindings.
… into swift-bindings/runtime-library
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.
Good job! Looking good and much easier to read through than the original big PR. I put some comments but they are more questions about the future so no need to address them in this PR.
This commit introduces support for the UnsafeBufferPointer types. It also adds a test case for the CryptoKit bindings.
This PR includes projection tooling skeleton and implements CryptoKit dev templates with unsafe (buffer) pointers. We will move forward with surfacing https://developer.apple.com/documentation/cryptokit/chachapoly enum and https://developer.apple.com/documentation/cryptokit/symmetrickey struct in a subsequent PR. Please review this PR and provide feedback. |
# Swift output files | ||
*.swiftinterface* | ||
*.swiftdoc* | ||
*.swiftmodule* | ||
*.abi* | ||
*.swiftsourceinfo* | ||
*.dylib | ||
|
||
# Testing artifacts | ||
testing/ | ||
src/samples/**/*Bindings.cs |
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.
These should all go under artifacts/
. In the current experiment form, this seems okay, but prior to merging into dotnet/runtime, all outputs should be under artifacts/
.
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.
Oops. Sorry, I thought we were working in a dotnet/runtime fork. Never mind.
/// <summary> | ||
/// Represents a method declaration. | ||
/// </summary> | ||
public sealed record MethodDecl |
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.
Using record
s in public APIs hasn't been confirmed as yet. This is likely okay for the current state. Just be aware that if this is going to be publicly consumable, the API review team may push back.
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
docs/README.md
Outdated
@@ -44,6 +44,8 @@ The table below lists the Swift types and their corresponding C# types. | |||
| `Swift.UInt16` | `ushort` | | |||
| `Swift.Int8` | `sbyte` | | |||
| `Swift.UInt8` | `byte` | | |||
| `Swift.UnsafePointer` | `void*` | |
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.
I know we touched on this before - it does not feel right to drop the type information for the strongly typed pointers. Strong typing is an extra level of protection against programming bugs that we are giving away 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.
Ok, as we discussed, a thin layer would be useful 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.
Types for unsafe pointers have been added. Mutable pointers are projected as structs with generics in C# to address methods overload issues and enable future extensibility.
public unsafe readonly struct UnsafeBufferPointer | ||
{ | ||
private readonly void* _Position; | ||
public readonly nint Count; |
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.
Swift does not seem to have fields as first-class concept. Their docs only talk about properties: https://docs.swift.org/swift-book/documentation/the-swift-programming-language/properties/
I am wondering whether mapping stored properties to fields in C# is conceptually correct.
Is it a breaking change in Swift to change a stored property to a computed property in non-frozen type? I would expect that it is not a breaking change - can you confirm that?
If computed properties and stored properties are interchangeable in Swift, we should map both to properties in C#. Ie make this public nint Count { get; };
or private nint _count; public nint Count => _count;
.
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.
Good catch. This is not a breaking change in Swift. I confirmed it by consuming a library with struct that evolved from
public struct MyType {
public var myProperty: Int = 0
public init() {}
}
to
public struct MyType {
private var _myProperty: Int = 0
public var myProperty: Int {
get { _myProperty }
set { _myProperty = newValue }
}
public init() {}
}
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.
Buffer pointers are updated with private readonly nint _count; public nint Count => _count;
.
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.
IIRC, if a property is stored, you still get getters and setter generated by the compiler. Since BTfS doesn't deal with struct lowering, it maps properties (stored or otherwise) as C# properties.
This commit implements mutable pointers as generics in C# and update CryptoKit test cases. The parser is updated to process Swift generic types and projects them into C# generic types.
_count = count; | ||
} | ||
|
||
public void* BaseAddress => _position; |
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.
public void* BaseAddress => _position; | |
public T* BaseAddress => _position; |
_count = count; | ||
} | ||
|
||
public void* BaseAddress => _position; |
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.
public void* BaseAddress => _position; | |
public T* BaseAddress => _position; |
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.
All void*
in the strongly typed wrappers should be T*
.
public unsafe readonly struct UnsafePointer<T> | ||
{ | ||
private readonly void* _rawValue; | ||
public UnsafePointer(void* _rawValue) |
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.
The argument name should not have underscore. Also, should the argument name match the property name that it corresponds to UnsafePointer(void* poin_rawValue)
?
// <summary> | ||
// Represents Swift UnsafePointer in C#. | ||
// </summary> | ||
public unsafe readonly struct UnsafePointer<T> |
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.
I think it would make sense to have implicit conversions to/from T* for these types.
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.
(And to/from void*
for the non-generic variant.)
{ | ||
private readonly void* _position; | ||
private readonly void* _end; | ||
public UnsafeRawBufferPointer(void* start, nint count) |
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.
public UnsafeRawBufferPointer(void* start, nint count) | |
public UnsafeRawBufferPointer(void* baseAddress, nint count) |
Match property name?
Description
This PR introduces a toolset for generating C# bindings from Swift interface files. This is the MVP aiming to support the generation of C# bindings for CryptoKit dev templates.
The tooling consists of the following components:
The parser and emitter are modular and implement
ISwiftParser
andICSharpEmitter
interfaces. The Swift ABI parser can be replaced with a swiftinterface parser, while the string-based emitter can be replaced with an object model-based emitter.Workflow:
The unit tests verify the XML layout of the type database and various P/Invoke cases. Additionally, CryptoKit dev templates with unsafe (buffer) pointers are implemented.