Deprecate atomic compare_and_swap method #79261
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
r? @Amanieu |
faern
force-pushed
the
deprecate-compare-and-swap
branch
from
bfff1a6 to
df2ef1d
Compare
the8472
reviewed
Nov 21, 2020
the8472
reviewed
Nov 21, 2020
jyn514
added
the
T-libs-api
|
@faern can you add |
|
@jyn514 Aliases added! I also improved the documentation regarding what the |
faern
force-pushed
the
deprecate-compare-and-swap
branch
2 times, most recently
from
bfc618f to
50b2ade
Compare
|
Is there anything I can do to help this progress? Are we waiting for the libs team to discuss this or is it blocked on something else? |
|
Oops, this slipped off my TODO list. I still to double-check with @rust-lang/libs since we are deprecating a function that is quite widely used. @rfcbot fcp merge |
|
Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
rfcbot
added
proposed-final-comment-period
|
I'm fine with the deprecation itself, but I continue to think it would be better to batch deprecations somehow, instead of doing them piecemeal. Where my main motivation for batching is to reduce churn. |
rfcbot
added
the
final-comment-period
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
rfcbot
removed
the
proposed-final-comment-period
|
@BurntSushi I guess it would be possible to set the deprecation version to a future version. Maybe always set it to Rust |
|
@faern Yeah, the library team hasn't discussed it I don't think, so I'm not requesting any change of course for this deprecation. That is a nifty idea though. |
rfcbot
added
finished-final-comment-period
|
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. The RFC will be merged soon. |
|
@bors r+ |
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Dec 22, 2020
…=Amanieu Deprecate atomic compare_and_swap method Finish implementing [RFC 1443](https://github.com/rust-lang/rfcs/blob/master/text/1443-extended-compare-and-swap.md) (rust-lang/rfcs#1443). It was decided to deprecate `compare_and_swap` [back in Rust 1.12 already](rust-lang#31767 (comment)). I can't find any info about that decision being reverted. My understanding is just that it has been forgotten. If there has been a decision on keeping `compare_and_swap` then it's hard to find, and even if this PR does not go through it can act as a place where people can find out about the decision being reverted. Atomic operations are hard to understand, very hard. And it does not help that there are multiple similar methods to do compare and swap with. They are so similar that for a reader it might be hard to understand the difference. This PR aims to make that simpler by finally deprecating `compare_and_swap` which is essentially just a more limited version of `compare_exchange`. The documentation is also updated (according to the RFC text) to explain the differences a bit better. Even if we decide to not deprecate `compare_and_swap`. I still think the documentation for the atomic operations should be improved to better describe their differences and similarities. And the documentation can be written nicer than the PR currently proposes, but I wanted to start somewhere. Most of it is just copied from the RFC. The documentation for `compare_exchange` and `compare_exchange_weak` indeed describe how they work! The problem is that they are more complex and harder to understand than `compare_and_swap`. So for someone who does not fully grasp this they might fall back to using `compare_and_swap`. Making the documentation outline the similarities and differences might build a bridge for people so they can cross over to the more powerful and sometimes more efficient operations. The conversions I do to avoid the `std` internal deprecation errors are very straight forward `compare_and_swap -> compare_exchange` changes where the orderings are just using the mapping in the new documentation. Only in one place did I use `compare_exchange_weak`. This can probably be improved further. But the goal here was not for those operations to be perfect. Just to not get worse and to allow the deprecation to happen.
|
☀️ Test successful - checks-actions |
This was referenced Dec 24, 2020
bors bot
added a commit
to crossbeam-rs/crossbeam
that referenced
this pull request
Dec 24, 2020
617: Replace deprecated compare_and_swap with compare_exchange r=jeehoonkang a=taiki-e `compare_and_swap` is deprecated in 1.50. (rust-lang/rust#79261) This patch replaces the uses of `compare_and_swap` with `compare_exchange`. See also the document about `compare_and_swap` -> `compare_exchange(_weak)` migration: https://doc.rust-lang.org/nightly/core/sync/atomic/struct.AtomicUsize.html#migrating-to-compare_exchange-and-compare_exchange_weak Co-authored-by: Taiki Endo <te316e89@gmail.com>
bors bot
added a commit
to crossbeam-rs/crossbeam
that referenced
this pull request
Dec 24, 2020
619: Deprecate AtomicCell::compare_and_swap r=jeehoonkang a=taiki-e The standard library deprecated `compare_and_swap` in favor of `compare_exchange(_weak)` in 1.50. (rust-lang/rust#79261) Given why std `compare_and_swap` was deprecated, it probably makes sense to do the same with `AtomicCell::compare_and_swap`. Closes #618 Co-authored-by: Taiki Endo <te316e89@gmail.com>
This was referenced Dec 24, 2020
RalfJung
reviewed
Dec 29, 2020
| /// # Migrating to `compare_exchange` and `compare_exchange_weak` | ||
| /// | ||
| /// `compare_and_swap` is equivalent to `compare_exchange` with the following mapping for | ||
| /// memory orderings: |
There was a problem hiding this comment.
It's not equivalent though, the return type is different...
See #80486.
5 tasks
netbsd-srcmastr
pushed a commit
to NetBSD/pkgsrc
that referenced
this pull request
Apr 19, 2021
Pkgsrc changes:
* Adjust patches, re-compute line offsets, fix capitalization.
* Remove i686/FreeBSD support, no longer provided upstream.
* Bump bootstraps to 1.49.0.
* Change USE_TOOLS from bsdtar to gtar.
* Reduce diffs to pkgsrc-wip package patches.
* Allow rust.BUILD_TARGET to override automatic choice of target.
* Add an i586/NetBSD (pentium) bootstrap variant (needs testing),
not yet added as bootstrap since 1.49 doesn't have that variant.
Upstream changes:
Version 1.50.0 (2021-02-11)
============================
Language
-----------------------
- [You can now use `const` values for `x` in `[x; N]` array
expressions.][79270] This has been technically possible since
1.38.0, as it was unintentionally stabilized.
- [Assignments to `ManuallyDrop<T>` union fields are now considered
safe.][78068]
Compiler
-----------------------
- [Added tier 3\* support for the `armv5te-unknown-linux-uclibceabi`
target.][78142]
- [Added tier 3 support for the `aarch64-apple-ios-macabi` target.][77484]
- [The `x86_64-unknown-freebsd` is now built with the full toolset.][79484]
\* Refer to Rust's [platform support page][forge-platform-support] for more
information on Rust's tiered platform support.
Libraries
-----------------------
- [`proc_macro::Punct` now implements `PartialEq<char>`.][78636]
- [`ops::{Index, IndexMut}` are now implemented for fixed sized
arrays of any length.][74989]
- [On Unix platforms, the `std::fs::File` type now has a "niche"
of `-1`.][74699] This value cannot be a valid file descriptor,
and now means `Option<File>` takes up the same amount of space
as `File`.
Stabilized APIs
---------------
- [`bool::then`]
- [`btree_map::Entry::or_insert_with_key`]
- [`f32::clamp`]
- [`f64::clamp`]
- [`hash_map::Entry::or_insert_with_key`]
- [`Ord::clamp`]
- [`RefCell::take`]
- [`slice::fill`]
- [`UnsafeCell::get_mut`]
The following previously stable methods are now `const`.
- [`IpAddr::is_ipv4`]
- [`IpAddr::is_ipv6`]
- [`Layout::size`]
- [`Layout::align`]
- [`Layout::from_size_align`]
- `pow` for all integer types.
- `checked_pow` for all integer types.
- `saturating_pow` for all integer types.
- `wrapping_pow` for all integer types.
- `next_power_of_two` for all unsigned integer types.
- `checked_power_of_two` for all unsigned integer types.
Cargo
-----------------------
- [Added the `[build.rustc-workspace-wrapper]` option.][cargo/8976]
This option sets a wrapper to execute instead of `rustc`, for
workspace members only.
- [`cargo:rerun-if-changed` will now, if provided a directory, scan the entire
contents of that directory for changes.][cargo/8973]
- [Added the `--workspace` flag to the `cargo update` command.][cargo/8725]
Misc
----
- [The search results tab and the help button are focusable with
keyboard in rustdoc.][79896]
- [Running tests will now print the total time taken to execute.][75752]
Compatibility Notes
-------------------
- [The `compare_and_swap` method on atomics has been deprecated.][79261]
It's recommended to use the `compare_exchange` and
`compare_exchange_weak` methods instead.
- [Changes in how `TokenStream`s are checked have fixed some cases
where you could write unhygenic `macro_rules!` macros.][79472]
- [`#![test]` as an inner attribute is now considered unstable like
other inner macro attributes, and reports an error by default
through the `soft_unstable` lint.][79003]
- [Overriding a `forbid` lint at the same level that it was set is
now a hard error.][78864]
- [Dropped support for all cloudabi targets.][78439]
- [You can no longer intercept `panic!` calls by supplying your
own macro.][78343] It's recommended to use the `#[panic_handler]`
attribute to provide your own implementation.
- [Semi-colons after item statements (e.g. `struct Foo {};`) now
produce a warning.][78296]
[74989]: rust-lang/rust#74989
[79261]: rust-lang/rust#79261
[79896]: rust-lang/rust#79896
[79484]: rust-lang/rust#79484
[79472]: rust-lang/rust#79472
[79270]: rust-lang/rust#79270
[79003]: rust-lang/rust#79003
[78864]: rust-lang/rust#78864
[78636]: rust-lang/rust#78636
[78439]: rust-lang/rust#78439
[78343]: rust-lang/rust#78343
[78296]: rust-lang/rust#78296
[78068]: rust-lang/rust#78068
[75752]: rust-lang/rust#75752
[74699]: rust-lang/rust#74699
[78142]: rust-lang/rust#78142
[77484]: rust-lang/rust#77484
[cargo/8976]: rust-lang/cargo#8976
[cargo/8973]: rust-lang/cargo#8973
[cargo/8725]: rust-lang/cargo#8725
[`IpAddr::is_ipv4`]: https://doc.rust-lang.org/stable/std/net/enum.IpAddr.html#method.is_ipv4
[`IpAddr::is_ipv6`]: https://doc.rust-lang.org/stable/std/net/enum.IpAddr.html#method.is_ipv6
[`Layout::align`]: https://doc.rust-lang.org/stable/std/alloc/struct.Layout.html#method.align
[`Layout::from_size_align`]: https://doc.rust-lang.org/stable/std/alloc/struct.Layout.html#method.from_size_align
[`Layout::size`]: https://doc.rust-lang.org/stable/std/alloc/struct.Layout.html#method.size
[`Ord::clamp`]: https://doc.rust-lang.org/stable/std/cmp/trait.Ord.html#method.clamp
[`RefCell::take`]: https://doc.rust-lang.org/stable/std/cell/struct.RefCell.html#method.take
[`UnsafeCell::get_mut`]: https://doc.rust-lang.org/stable/std/cell/struct.UnsafeCell.html#method.get_mut
[`bool::then`]: https://doc.rust-lang.org/stable/std/primitive.bool.html#method.then
[`btree_map::Entry::or_insert_with_key`]: https://doc.rust-lang.org/stable/std/collections/btree_map/enum.Entry.html#method.or_insert_with_key
[`f32::clamp`]: https://doc.rust-lang.org/stable/std/primitive.f32.html#method.clamp
[`f64::clamp`]: https://doc.rust-lang.org/stable/std/primitive.f64.html#method.clamp
[`hash_map::Entry::or_insert_with_key`]: https://doc.rust-lang.org/stable/std/collections/hash_map/enum.Entry.html#method.or_insert_with_key
[`slice::fill`]: https://doc.rust-lang.org/stable/std/primitive.slice.html#method.fill
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Finish implementing RFC 1443 (rust-lang/rfcs#1443).
It was decided to deprecate
compare_and_swapback in Rust 1.12 already. I can't find any info about that decision being reverted. My understanding is just that it has been forgotten. If there has been a decision on keepingcompare_and_swapthen it's hard to find, and even if this PR does not go through it can act as a place where people can find out about the decision being reverted.Atomic operations are hard to understand, very hard. And it does not help that there are multiple similar methods to do compare and swap with. They are so similar that for a reader it might be hard to understand the difference. This PR aims to make that simpler by finally deprecating
compare_and_swapwhich is essentially just a more limited version ofcompare_exchange. The documentation is also updated (according to the RFC text) to explain the differences a bit better.Even if we decide to not deprecate
compare_and_swap. I still think the documentation for the atomic operations should be improved to better describe their differences and similarities. And the documentation can be written nicer than the PR currently proposes, but I wanted to start somewhere. Most of it is just copied from the RFC.The documentation for
compare_exchangeandcompare_exchange_weakindeed describe how they work! The problem is that they are more complex and harder to understand thancompare_and_swap. So for someone who does not fully grasp this they might fall back to usingcompare_and_swap. Making the documentation outline the similarities and differences might build a bridge for people so they can cross over to the more powerful and sometimes more efficient operations.The conversions I do to avoid the
stdinternal deprecation errors are very straight forwardcompare_and_swap -> compare_exchangechanges where the orderings are just using the mapping in the new documentation. Only in one place did I usecompare_exchange_weak. This can probably be improved further. But the goal here was not for those operations to be perfect. Just to not get worse and to allow the deprecation to happen.