Change compare_exchange to return a Result<T, T>

[Amanieu]

As per the discussion in #31767

I also changed the feature name from extended_compare_and_swap to compare_exchange.

r? @alexcrichton

[alexcrichton]

Thanks @Amanieu! From a technical perspective the change looks good to me, just a question of APIs now! I'm tagging this with T-libs to ensure it comes up during triage.

So I originally thought that this may make the classical "compare exchange loop" a little less ergnomic as getting the value out would require unwrap_or_else or something like that, but when I write them down...

current:

loop {
    let new = f(current);
    let old = foo.compare_exchange(current, new, ord1, ord2);
    if old == current {
        break
    }
    current = old;
}

new:

loop {
    let new = f(current);
    match foo.compare_exchange(current, new, ord1, ord2) {
        Ok(..) => break,
        Err(old) => current = old,
    }
}

I gotta say using match here seems quite appealing to me!

To get a handle on the ergonomics though it may be worthwhile to port a project like crossbeam to using this API instead of the old one (just as a smoke test)

[alexcrichton]

The libs team discussed this during triage yesterday and the conclusion was that this seems like a good change to have! It was brought up though that we may not want to change the feature names as that's kinda unnecessary breakage, would you be ok changing back?

[Amanieu]

Done

[alexcrichton]

@bors: r+ 2964f1873e54b42cc3994ba8882d2486305c32c3

[bors]

⌛ Testing commit 2964f18 with merge dedfe4a...

[bors]

💔 Test failed - auto-mac-64-opt

[Amanieu]

Fixed

[eddyb]

@alexcrichton This will have to wait until #32080 is merged, btw.

[bors]

☔ The latest upstream changes (presumably #32080) made this pull request unmergeable. Please resolve the merge conflicts.

[Amanieu]

I think this should be fixed now

[alexcrichton]

@bors: r+ 6f0a1307741e7d1d297aeffd2ec15eb905e1d487

[bors]

⌛ Testing commit 6f0a130 with merge 424af07...

[alexcrichton]

@bors: retry force clean

[bors]

⌛ Testing commit 6f0a130 with merge 8021fa5...

[bors]

💔 Test failed - auto-mac-64-nopt-t

[Amanieu]

Fixed.

Note that it seems atomic intrinsics currently don't work on non-integer types, this should be properly fixed at some point...

[alexcrichton]

@bors: r+ 421fed1

[bors]

⌛ Testing commit 421fed1 with merge 58ca226...

[bors]

💔 Test failed - auto-win-msvc-64-opt-rustbuild

[alexcrichton]

@bors: retry

On Sat, Mar 19, 2016 at 12:11 PM, bors [email protected] wrote:

[image: 💔] Test failed - auto-win-msvc-64-opt-rustbuild
http://buildbot.rust-lang.org/builders/auto-win-msvc-64-opt-rustbuild/builds/419

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#32244 (comment)

[bors]

⌛ Testing commit 421fed1 with merge 8eeb506...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-64-x-armhf, auto-linux-64-x-armsf, auto-linux-64-x-freebsd, auto-linux-64-x-netbsd, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-mac-ios-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt, auto-win-msvc-64-opt-rustbuild