Repository navigation
compare_and_swap "migration" documentation should explain how to get return value #80486
Description
The compare_and_swap docs explain how to switch the order to compare_exchange, but that is not the only difference: the return type also changed. The docs don't say anything about how to best get the old behavior back with the new method. Many concurrent algorithms are written based on getting the old value back, so there should be a convenient way to get that behavior.
Cc @faern
Activity
RalfJung commentedon Dec 29, 2020
FWIW, the underlying primitive in LLVM that backs
compare_exchangereturns(T, bool)-- which IMO makes much more sense thanResult<T, T>. But that ship has long sailed...RalfJung commentedon Dec 29, 2020
Looks like
unwrap_or_else(|x| x)is one option. It's not pretty but it works.m-ou-se commentedon Dec 29, 2020
related: #79315
lolgesten commentedon Feb 23, 2021
I'm probably dense, but I don't understand how I am supposed to change my code.
This is what I had, but reading the
compare_exchangedocumentation, I don't understand what I'm supposed to do now.I understand the motivation for the deprecation, but I feel sad it's done at the cost of a more complex API. For my use case, I simply want atomic access to a bool, and now I'm forced to get my head around the double
Orderingparameters to get slightly better performance on ARM (a platform I don't use) + a double boolean Result return type.Wouldn't it be possible to not deprecate
compare_and_swapfor people like me, who just want the simple use case?RalfJung commentedon Feb 23, 2021
@lolgesten without commenting on the rest of what you said, this is how you could change your code:
This translation can be applied systematically.
However, I assume what you really want to express here is "if the CAS happened successfully", which would be more clearly expressed as
or
Arguably, this code is more clear than your original code, since it is easier to see that it checks for "successful CAS".
lolgesten commentedon Feb 23, 2021
@RalfJung thanks! This is what I was suspecting I should do.
In terms of improving the documentation. The table added in the doc for
compare_and_swaphas the column names "Original", "Success", "Failure". Maybe that could be improved to more clearly indicate the columns are forcompare_and_swapandcompare_exchange arg1andcompare_exchange arg2.And also
It's not clear to me whether the doc about
failurehere is equivalent toResult::Err(boolean). The docs could maybe be clearer about whatResult::OkvsResult:Errmeans.faern commentedon Feb 23, 2021
I agree that the "Original" column could be better named. But the column names "Success" and "Failure" are named after the arguments to
compare_exchange. I think that makes sense.arg1andarg2would be strange since the first two arguments tocompare_exchangeiscurrentandnew. But yes, the text could elaborate a bit more and show one example of a migration maybe.RalfJung commentedon Mar 27, 2021
Yes. :-)
hkBst commentedon Jan 30, 2025
@RalfJung do you still feel that the compare_exchange docs need to mention some way of unpacking the Result to a value? In your own example you seemed to prefer
if let Ok() =andif .. .is_ok().RalfJung commentedon Jan 30, 2025
I think the docs should explain how to do "mindless migration", yes, even if the result might not be idiomatic. I have opened a PR for that: #136300.
Rollup merge of rust-lang#136300 - RalfJung:compare-and-swap, r=joboet
Unrolled build for rust-lang#136300
Rollup merge of #136300 - RalfJung:compare-and-swap, r=joboet
Rollup merge of #136300 - RalfJung:compare-and-swap, r=joboet
Rollup merge of rust-lang#136300 - RalfJung:compare-and-swap, r=joboet