WIP: Added atomic bitflags generation

f-gagel commented

2024-01-06 19:12:43 -06:00

(Migrated from github.com)

The idea is to enable the macro to create a wrapper type for atomic flags, using an attribute as opt in. All the main flags logic will still be handled by the regular flags, with the atomic flags only acting as an adapter to some atomic int type.

bitflags! {
    #[atomic_attr(derive(Default))]
    #[atomic(MyAtomicFlags)]
    pub struct MyFlags: u32 {
        const A = 1;
        const B = 2;
    }
}

fn main() {
    let flags = MyAtomicFlags::default();
    flags.fetch_insert(MyFlags::A, Ordering::Relaxed);
}

Some open questions to me are:

Is the attribute sufficient as an opt in or should a feature flag be added?
Is there a nicer way to declare the atomic flags type and especially its attributes?

The idea is to enable the macro to create a wrapper type for atomic flags, using an attribute as opt in. All the main flags logic will still be handled by the regular flags, with the atomic flags only acting as an adapter to some atomic int type. ```rust bitflags! { #[atomic_attr(derive(Default))] #[atomic(MyAtomicFlags)] pub struct MyFlags: u32 { const A = 1; const B = 2; } } fn main() { let flags = MyAtomicFlags::default(); flags.fetch_insert(MyFlags::A, Ordering::Relaxed); } ``` Some open questions to me are: - Is the attribute sufficient as an opt in or should a feature flag be added? - Is there a nicer way to declare the atomic flags type and especially its attributes?

KodrAus commented

2024-01-16 16:27:46 -06:00

(Migrated from github.com)

Hi @f-gagel 👋 Thanks for the PR! I've tried to avoid introducing any custom attributes to bitflags to keep the implementation from becoming too complex.

I had a look at whether we could already do this with a custom Bits implementation and we can't really, because you'll want to operate on non-atomic integers, but store the results atomically.

Depending on your use-case, you could declare your bitflags type over a regular integer type, and just load/store it from a shared atomic integer as necessary. That would neatly separate storage concerns from the flags type itself, and let you do multiple bitwise operations on the flags value before needing to use atomics to store them.

Hi @f-gagel :wave: Thanks for the PR! I've tried to avoid introducing any custom attributes to `bitflags` to keep the implementation from becoming too complex. I had a look at whether we could already do this with a custom `Bits` implementation and we can't really, because you'll want to operate on non-atomic integers, but store the results atomically. Depending on your use-case, you could declare your bitflags type over a regular integer type, and just load/store it from a shared atomic integer as necessary. That would neatly separate storage concerns from the flags type itself, and let you do multiple bitwise operations on the flags value before needing to use atomics to store them.

f-gagel commented

2024-02-03 13:34:35 -06:00

(Migrated from github.com)

Hi. Thanks for the feedback!

I understand the concern about keeping the macro straightforward. For the atomic storage would a generic wrapper type Atomic<Flags> be an option? My goal with this is to not just enable load and store but also fetch_* operations for flags or would that be outside the scope of the bitflags crate?

Hi. Thanks for the feedback! I understand the concern about keeping the macro straightforward. For the atomic storage would a generic wrapper type `Atomic<Flags>` be an option? My goal with this is to not just enable load and store but also fetch_* operations for flags or would that be outside the scope of the bitflags crate?

kkysen commented

2024-03-04 15:13:28 -06:00

(Migrated from github.com)

Hi @f-gagel, I've been able to accomplish something similar with atomig by #[derive(Atom, AtomLogic)]ing. It doesn't result in exactly the same API as this PR, but it's pretty similar. You can do Atomic<Flags> and things like .fetch_or.

Hi @f-gagel, I've been able to accomplish something similar with [`atomig`](https://docs.rs/atomig) by `#[derive(Atom, AtomLogic)]`ing. It doesn't result in exactly the same API as this PR, but it's pretty similar. You can do `Atomic<Flags>` and things like `.fetch_or`.

KodrAus commented

2024-03-19 22:41:38 -05:00

(Migrated from github.com)

Thanks @kkysen 👍 That seems like a good way to go here.

I think this isn't something we'll be able to support in bitflags directly due to the additional feature complexity so will go ahead and close this one, but I appreciate the time you've spent putting an implementation together.

Thanks @kkysen :+1: That seems like a good way to go here. I think this isn't something we'll be able to support in `bitflags` directly due to the additional feature complexity so will go ahead and close this one, but I appreciate the time you've spent putting an implementation together.

f-gagel commented

2024-04-03 15:49:59 -05:00

(Migrated from github.com)

Hi, I understand that you want to avoid that extra complexity. Thank you for your consideration.
@kkysen atomig works really nicely for this case. Thanks for pointing it out.

To anyone looking for how to get my preferred API, here's the neccessary glue: https://gist.github.com/f-gagel/7cb44e6ce27022cf2991b11d27044610

Hi, I understand that you want to avoid that extra complexity. Thank you for your consideration. @kkysen `atomig` works really nicely for this case. Thanks for pointing it out. To anyone looking for how to get my preferred API, here's the neccessary glue: https://gist.github.com/f-gagel/7cb44e6ce27022cf2991b11d27044610

❤️ 1

Pull request closed

Please reopen this pull request to perform a merge.

Rows
Columns

WIP: Added atomic bitflags generation #391

Pull request closed