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

Comments/prior art on Atomic<T> #7

Open
LukasKalbertodt opened this issue Sep 2, 2020 · 1 comment
Open

Comments/prior art on Atomic<T> #7

LukasKalbertodt opened this issue Sep 2, 2020 · 1 comment

Comments

@LukasKalbertodt
Copy link
Member

I was not sure where best to comment this RFC draft. I just wanted to throw a library of mine into the discussion: atomig, which main feature is Atomic<T>.

In atomig's case, the trait bounds on T are a bit more specific and deliberate than in the RFC draft. The docs should explain all the traits fairly well, so I won't repeat myself here.

But one important aspect of having more specific traits: some methods, like fetch_add and fetch_or are bounded by AtomInteger and AtomLogic traits respectively, which in turn are only implemented for types on which integer operations and logical operations make sense, respectively. With the current draft, one could have an Atomic<f32> and call fetch_add on it, but get garbage back, as the atomic add operation is an integer addition. Again, I think the docs should explain this nicely.

Just wanted to throw that out here.

CC @jswrenn

@jswrenn
Copy link
Member

jswrenn commented Sep 2, 2020

With the current draft, one could have an Atomic and call fetch_add on it, but get garbage back, as the atomic add operation is an integer addition.

I don't define fetch_add on Atomic<T> for precisely this reason. I envision basically the solution you describe to accomodate the type-specific methods!

The Atomic<T> extension is the least fleshed out of the hypothetical future possibilities—there's only just enough there to describe the basic pattern. I'll be sure we keep your prior art in mind when we develop Atomic<T> further.

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