-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[cxx-interop] Cast "data" to the correct type in _swift_dispatch_data_apply. #34266
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
[cxx-interop] Cast "data" to the correct type in _swift_dispatch_data_apply. #34266
Conversation
The implicit conversions are OK in C but C++ will error. To make this header valid in both C and C++ we should just always cast.
@@ -215,8 +215,8 @@ static inline unsigned int | |||
_swift_dispatch_data_apply( | |||
__swift_shims_dispatch_data_t data, | |||
__swift_shims_dispatch_data_applier SWIFT_DISPATCH_NOESCAPE applier) { | |||
return dispatch_data_apply(data, ^bool(dispatch_data_t data, size_t off, const void *loc, size_t size){ | |||
return applier(data, off, loc, size); | |||
return dispatch_data_apply((dispatch_data_t)data, ^bool(dispatch_data_t data, size_t off, const void *loc, size_t size){ |
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.
An alternative solution would be to update dispatch_data_apply
to take a __swift_shims_dispatch_data_t
. That would modify a public function to accept a private type so, I figured this solution would be better.
@swift-ci test |
Build failed |
Java error. Unrelated. @swift-ci please test OS X Platform. |
@swift-ci please test OS X. |
Thanks, @varungandhi-apple. I'll commit this tomorrow unless anyone else wants to review. |
It's possible that this [1] build failure was caused by this patch. A (full) OS X test did succeed for this patch before it landed, though. Should I revert or wait until the next build also fails (or doesn't fail). I'll plan on the latter unless I hear otherwise. [1] = https://ci.swift.org/job/oss-swift-incremental-RA-osx/12891/ |
I think the failure:
is unrelated to this patch. I think we've seen it sometimes non-deterministically, I suspect it is an incremental build issue. |
I think you're right. That makes the most sense. I just saw all the other commits were all lldb stuff and this was a failure while building the stdlib. Thanks for looking into it :) |
The implicit conversions are OK in C but C++ will error. To make this header valid in both C and C++ we should just always cast.
This and swiftlang/swift-corelibs-foundation#2895 fix the Linux builds for #32721.