Use an extra shims header to remove _silgen_name from Dispatch. #5854
Conversation
|
@swift-ci Please test macOS |
|
Build failed |
jrose-apple
force-pushed
the
excise-silgen_name-from-Dispatch
branch
from
e79cafe to
df4fbef
Compare
|
@swift-ci Please test macOS |
|
@swift-ci Please ASan test |
|
@swift-ci Please smoke test Linux |
|
Build failed |
|
Filed rdar://problem/29340689 for the odd compiler-rt failure. @swift-ci Please test macOS |
jrose-apple
force-pushed
the
excise-silgen_name-from-Dispatch
branch
from
df4fbef to
0d11c1e
Compare
jrose-apple
changed the title
|
@swift-ci Please test macOS |
|
do we need an equivalent change to swift-corelibs-libdispatch for the Linux overlay ? I see _silgen_name in src/swift/ files |
| return _dispatch_data_destructor_munmap; | ||
| } | ||
|
|
||
| #define SWIFT_DISPATCH_SOURCE(t) \ |
There was a problem hiding this comment.
Ah, good call, will change.
| __swift_shims_dispatch_block_t block) { | ||
| dispatch_barrier_async(queue, block); | ||
| } | ||
|
|
There was a problem hiding this comment.
more a question for @mwwa : why don't we have _swift_dispatch_barrier_sync here (i.e. why can _syncBarrier use __dispatch_barrier_sync ) ?
There was a problem hiding this comment.
We don't have a sync<T> call that takes a DispatchWorkItem. The reason we have to have these shims is to preserve the identity of DispatchWorkItem/dispatch_block_t such that the runtime doesn't wrap them in thunk closures. The barrier case in sync<T> is only ever fed closures anyway, so we don't need a shim here.
|
lgtm but @mwwa should have a look too since he is the one that originally wrote this code |
|
I forgot that the Linux overlay would have _silgen_name in it too. I don't think anything there will break, but if they depend on DispatchShims.mm they might. Let's find out. @swift-ci Please test |
|
Build failed |
|
Same, LGTM. |
|
All tests passed, so I'll update the name for SWIFT_DISPATCH_SOURCE_TYPE, commit, and then do swift-corelibs-dispatch separately. |
We still have a bunch of redeclarations of Dispatch functions to avoid the automatic bridging of dispatch_data_t and dispatch_block_t, but mostly this is a vast reduction in complexity (and increase in safety).
...so we can see it in failure logs.
jrose-apple
force-pushed
the
excise-silgen_name-from-Dispatch
branch
from
0d11c1e to
1c68744
Compare
|
Full tests passed already, so just running smoke tests this time. @swift-ci Please smoke test |
jrose-apple
requested review from
ktopley-apple
and removed request for
ktopley-apple
Follow-up to #5309 for Dispatch.
We still have a bunch of redeclarations of Dispatch functions to avoid the automatic bridging of dispatch_data_t and dispatch_block_t, but mostly this is a vast reduction in complexity (and increase in safety).
I think there are still bugs, so I'm not going to call any of the Dispatch folks to review just yet.
After this gets merged, we'll only be using _silgen_name outside the core stdlib in Foundation and "Platform" (Darwin or Glibc), plus a number of things that don't actually get shipped (benchmarks and StdlibUnittest).