Add a couple of workarounds for Swift on Windows - #1414
Conversation
Sorry, something went wrong.
The C++ Interop efforts in Swift currently have some limitations. In particular, it cannot support trivial types with non-trivial destructors. As a workaround, provide a copy constructor which can be used by the Swift interop while using the regular semantics for all other cases. A second issue arises in the handling of futures. Unfortunately, it is not currently possible to pass an indirect block parameter which prevents the construction of a callback. Workaround this by providing an inline shim to use a direct parameter (i.e. indirect value through a pointer) which then allows a callback to be formed. Both of these items are being tracked upstream but seem to be potentially sufficient to enable the use of Swift for using the C++ SDK for desktop scenarios.
| #if FIREBASE_PLATFORM_WINDOWS | ||
| namespace firebase { | ||
| namespace auth { | ||
| Auth::Auth(const Auth &) noexcept = default; |
There was a problem hiding this comment.
I'm uncomfortable having this copy constructor broadly available for all Windows developers. Could you wrap this and all of the other changes in an #if for the Swift compatibility that you can use in your build? e.g. FIREBASE_SWIFT_WINDOWS=1
Sorry, something went wrong.
There was a problem hiding this comment.
That wouldn't work. That would mean that the distribution of firebase would not be usable as the copy-constructor would not be available when used for Swift.
Note that the header actually indicates that the copy constructor is not declared (and thus the compiler will synthesize one - except because it is a movable type, it will prefer to move it) when the interop is not enabled. This means that this is only going to add the distribution size but will be discarded by the linker as static linking is employed. As a result the copy constructor is not available for Windows developers, they would need to define __swift__ for it to be made available.
Sorry, something went wrong.
There was a problem hiding this comment.
OK, I'm following. I'll take another look at this after our next release goes out.
Sorry, something went wrong.
✅ Integration test succeeded!Requested by @jonsimantov on commit refs/pull/1414/merge |
Sorry, something went wrong.
jonsimantov
left a comment
There was a problem hiding this comment.
This looks good to merge, though we will need to confirm after merge that it doesn't break our C++ packaging tests and Unity SDK build (in which case we'd need to roll it back).
Sorry, something went wrong.
🍞 Dismissed stale approval on external PR.
|
@jonsimantov - anything else left before we can merge this? |
Sorry, something went wrong.
🍞 Dismissed stale approval on external PR.
jonsimantov
left a comment
There was a problem hiding this comment.
Looks good - let's go ahead and get this in. I do want to make sure this doesn't cause any weirdness with our public release, but I'll take a look next time we do one and roll this back if it needs any changes at that point.
Sorry, something went wrong.
🍞 Dismissed stale approval on external PR.
|
bumping (with apologies) in case this can be merged, so people can use it... |
Sorry, something went wrong.
|
You should have permission to merge since I approved it, but if not, let me know and I'll do it. |
Sorry, something went wrong.
|
Could this be merged? I would also like to be able to use firebase-cpp from Swift… |
Sorry, something went wrong.
|
Any progress on this? |
Sorry, something went wrong.
|
I don't believe that I have permission to merge. |
Sorry, something went wrong.
Reviewers
1 more reviewerAt least 1 approving review is required to merge this pull request.
The C++ Interop efforts in Swift currently have some limitations. In particular, it cannot support trivial types with non-trivial destructors. As a workaround, provide a copy constructor which can be used by the Swift interop while using the regular semantics for all other cases.
A second issue arises in the handling of futures. Unfortunately, it is not currently possible to pass an indirect block parameter which prevents the construction of a callback. Workaround this by providing an inline shim to use a direct parameter (i.e. indirect value through a pointer) which then allows a callback to be formed.
Both of these items are being tracked upstream but seem to be potentially sufficient to enable the use of Swift for using the C++ SDK for desktop scenarios.