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

Add basic atomic types #6732

Closed
wants to merge 1 commit into from
Closed

Conversation

Aatch
Copy link
Contributor

@Aatch Aatch commented May 25, 2013

This pull request is more of an RFC than a finished implementation.

It adds some basic atomic types, with an interface modelled off of C++11's atomic types.

It also adds free functions that provide a slightly nicer interface for atomic operations, though they are unsafe because there isn't a way to be generic over "word-sized" types.

See also #5042

@brson
Copy link
Contributor

brson commented May 25, 2013

Nit: can you please put spaces in your type declarations, like foo: bar?

bors added a commit that referenced this pull request May 25, 2013
This pull request is more of an RFC than a finished implementation.

It adds some basic atomic types, with an interface modelled off of C++11's atomic types.

It also adds free functions that provide a slightly nicer interface for atomic operations, though they are unsafe because there isn't a way to be generic over "word-sized" types.

See also #5042
@brson
Copy link
Contributor

brson commented May 25, 2013

This looks good to me. I guess I shouldn't have r+ed yet because I would like you to modify the formatting of the types. Maybe you can do a follow up.

I'm not sure about the AtomicPtr type. As written it is wildly unsafe because if you call take then the value is destroyed it will (in theory) try to destroy a null pointer. In the current implementation this may work because the drop glue checks for null, but this isn't guaranteed by Rust semantics.

An AtomicOption may be more appropriate for that purpose.

@brson
Copy link
Contributor

brson commented May 25, 2013

AtomicPtr is also going to try to run dtors without synchronizing memory.

@Aatch
Copy link
Contributor Author

Aatch commented May 25, 2013

Yeah, I was really unsure about the AtomicPtr, since it seemed to be turning into some kind of half baked Cell implementation.

I may change it to be explicitly unsafe and use *mut pointers instead of ~-pointers which also avoids the issue of destructors at the expense of aliasing. On the flip side, it does make a compare-and-swap operation make more sense, semantically. Atomic operations like this are pretty low level anyway, so I'm comfortable with that tradeoff.

@bors bors closed this May 25, 2013
@Aatch Aatch mentioned this pull request May 26, 2013
bors added a commit that referenced this pull request May 29, 2013
This is a follow up to #6732. Makes everything a little more sound.

r? @brson
flip1995 pushed a commit to flip1995/rust that referenced this pull request Mar 11, 2021
…Manishearth

Fix redundant closure with macros

changelog: Fix redundant_closure FPs with macros

Fixes rust-lang#6732
Fixes rust-lang#6850
Fixes rust-lang#4354 (addresses the error message confusion)
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

Successfully merging this pull request may close these issues.

3 participants