Skip to content
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

The AtomInteger trait is not needed for Atomic::fetch_update #3

Closed
bruceg opened this issue Apr 14, 2021 · 3 comments
Closed

The AtomInteger trait is not needed for Atomic::fetch_update #3

bruceg opened this issue Apr 14, 2021 · 3 comments

Comments

@bruceg
Copy link
Contributor

bruceg commented Apr 14, 2021

The function Atomic::fetch_update is only available for types that implement AtomInteger. However, it doesn't actually do any implicit integer operations like, say, fn fetch_add does — it explicitly leaves the update to a Rust closure that is restricted to safe operations on the data types. As such, the restriction is not needed, allowing fetch_update to be used on enum types, for example.

Here's my example that does not work with atomig as it is now:

#[derive(Atom, Copy)]
#[repr(u8)]
enum Enum {
    Off,
    On,
}

fn foo() {
    let t: Atomic<Enum> = Atomic::new(Enum::Off);
    t.fetch_update(Ordering::Relaxed, Ordering::Relaxed, |t0| Some(match t0 {
        Enum::Off => Enum::On,
        Enum::On => Enum::Off,
    }));
}
@LukasKalbertodt
Copy link
Owner

Somehow I managed to unwatch my own repository 😲 I just now stumbled upon your comment by pure chance. Sorry for the delay!

The problem here is that fetch_update was not implemented for non-integer atomics in std for some time: rust-lang/rust#78639
This seems to be a simple oversight though. AtomicBool::fetch_update and AtomicPtr::fetch_update were already added and their stabilization hits stable with 1.53, in roughly three weeks. I will wait till then to properly fix this issue. I will let you know once I release the fix as 0.3.

@LukasKalbertodt
Copy link
Owner

LukasKalbertodt commented Jun 18, 2021

I just released 0.3 which has fetch_update for all T: Atom available, not only for T: AtomInteger.

(Oh and don't mind the failing CI, that's just GitHub Actions not running Rust 1.53 yet.)

@bruceg
Copy link
Contributor Author

bruceg commented Jun 22, 2021

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants