[Swift Bindings] Ensure projection tooling generate bindings that compile for selected frameworks #2900
Conversation
kotlarmilos
requested review from
dalexsoto,
rolfbjarne,
stephen-hawley,
mcumming,
matouskozak and
jkurdek
There was a problem hiding this comment.
I think it would be a good idea to zero out _payload after freeing it, which would avoid any accidental access of freed memory:
https://gist.github.com/kotlarmilos/907cde8ce6af5f2730f8f33e432a503e#file-swift-storekit-cs-L105
https://gist.github.com/kotlarmilos/907cde8ce6af5f2730f8f33e432a503e#file-swift-storekit-cs-L112
There was a problem hiding this comment.
I think that gracefully handling unknown types will be tricky. The complexity is in the fact that given an argument we need to make an ABI decision (indirect result, opaque pointer, etc). We wont be able to make those decision if we dont find the type in the TypeDatabase.
If compilation is our goal for now maybe we can do the following approach. Do AnyType subtitution in wrappers signatures - this should work as this does not touch ABI. And then just not generate pinvokes when argument is not known.
src/Swift.Bindings/src/Emitter/StringCSharpEmitter/Handler/MethodHandler.cs
Outdated
Show resolved
Hide resolved
src/Swift.Bindings/src/Emitter/StringCSharpEmitter/Handler/MethodHandler.cs
Outdated
Show resolved
Hide resolved
| var returnType = MethodDecl.CSSignature.First().CSTypeIdentifier.Name; | ||
| var argument = MethodDecl.CSSignature.First(); | ||
| var returnType = argument.CSTypeIdentifier.Name; | ||
| var typeRecord = MarshallingHelpers.GetType(argument, TypeDatabase.Registrar); |
There was a problem hiding this comment.
I feel that we now operate on two abstraction layers in this call. We have high-level MethodRequiresIndirectResult which calls type database and low level MarshallingHelpers.GetType which can lead to confusing behaviour as discussed in previous comment.
There was a problem hiding this comment.
I feel that some marshalling should be handled before emitting. However, I would prefer implementing more support before structuring it.
When replacing unsupported types with AnyType placeholders, method conflicts might occur (same name and signature). To address this, I completely skipped generating wrappers to ensure compilation succeeds. The next step would be to reduce these methods during marshalling and emit them with a NotImplementedException body.
I think this is a good start -- it is safe to assume that |
|
Thanks for prompt review. If there is any additional feedback later, we will address it as a follow-up. |
Description
This PR:
Generated bindings that compile for StoreKit2: https://gist.github.com/kotlarmilos/907cde8ce6af5f2730f8f33e432a503e
Changes
This PR handles unknown types gracefully by introducing
AnyTypeas a Swift placeholder type. Additionally, it simplifying the projection tooling UX by removing unnecessary args and unifies theSwiftandSwift.Runtimenamespaces into a single directoryThis PR does not change the layout of projections.
Validation
Both official and PR pipelines now include a bindings compilation check for frameworks to ensure that generated code can compile.