-
Notifications
You must be signed in to change notification settings - Fork 10.5k
🍒[5.9][Concurrency] Adopt stable keyword consuming instead of __owned #65575
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
Conversation
@swift-ci please test |
Depends on #65455 since that introduces new methods; so we want to do the same to the new ones IF we are to merge these new methods. |
add test for warnings emitted when missing consuming attribute use -emit-sil in mock SDK tests for better coverage
116d98e
to
4cad4f6
Compare
@swift-ci please test |
Github is having issues; |
@swift-ci please test |
Just as a note:
This isn't what the PR is improving. It's just using the |
I mean this specifically was causing bad diagnostics telling people to implement " |
Thanks for review! |
Description:
This change is ABI stable with regards to the mangled name of the enqueue method.
We confirmed this by manually comparing mangled names of a method with __owned and later a consuming parameter -- please double check this though @kavon as you'd know more about this than myself.
This also improves diagnostics around implementing move-only job requirement, since the diagnostics offered by _owned are wrong: they don't show the attribute leading people to add func enqueue( job: ExecutorJob) which is incorrect because they MUST add consuming.
This PR includes tests that simulate an SDK with either the __owned or consuming versions and implementations with the other spelling, to make sure this works as expected.
Risk: Low, the mangling and ABI is the same.
Review by: @kavon @DougGregor
Testing: CI testing; manually verified mangling of the changed method
Original PR: #65571
Radar: rdar://108467550