Add a couple of workarounds for Swift on Windows #1414
Conversation
compnerd
force-pushed
the
swift
branch
3 times, most recently
from
9b6c99c
to
51e693e
Compare
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
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.
There was a problem hiding this comment.
OK, I'm following. I'll take another look at this after our next release goes out.
github-actions
bot
added
the
tests: in-progress
✅ Integration test succeeded!Requested by @jonsimantov on commit refs/pull/1414/merge |
firebase-workflow-trigger
bot
removed
the
tests: in-progress
github-actions
bot
added
tests: succeeded
github-actions
bot
dismissed
jonsimantov’s stale review
🍞 Dismissed stale approval on external PR.
|
@jonsimantov - anything else left before we can merge this? |
github-actions
bot
dismissed
jonsimantov’s stale review
🍞 Dismissed stale approval on external PR.
github-actions
bot
added
tests: in-progress
firebase-workflow-trigger
bot
removed
the
tests: in-progress
github-actions
bot
added
tests: in-progress
firebase-workflow-trigger
bot
removed
the
tests: in-progress
github-actions
bot
added
tests: succeeded
github-actions
bot
dismissed
jonsimantov’s stale review
🍞 Dismissed stale approval on external PR.
github-actions
bot
added
tests: in-progress
firebase-workflow-trigger
bot
removed
the
tests: in-progress
|
bumping (with apologies) in case this can be merged, so people can use it... |
|
You should have permission to merge since I approved it, but if not, let me know and I'll do it. |
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.