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

Atomic* should not be copyable #8380

Closed
brson opened this issue Aug 7, 2013 · 4 comments
Closed

Atomic* should not be copyable #8380

brson opened this issue Aug 7, 2013 · 4 comments
Labels
P-medium Medium priority

Comments

@brson
Copy link
Contributor

brson commented Aug 7, 2013

Most of the types in unstable::atomics are implicitly copyable. This results in some confusing situations and is also incorrect since it's doing non-atomic copies.

@Aatch
Copy link
Contributor

Aatch commented Aug 7, 2013

I don't think an "atomic copy" is a thing? Since all the atomic operations essentially just work on the self pointer, copying an atomic type results in an entire new value, in a new memory location. I'm pretty sure that an "atomic copy" is impossible since it requires a read and then store to 2 separate memory locations. The only thing I can think of is the lack of an atomic load.

I'm also not sure that being implicitly copyable is an issue, other the above point, and would prefer to keep this way to be honest. There is no semantic difference between using the Atomic* types and the methods on them and using the atomic intrinsics directly pointers to the values you want to manipulate. The only real advantage of the atomic types is a nicer API. The way it currently is means that the atomic types are identical (except for AtomicBool) to their non-atomic counterparts.

@brson
Copy link
Contributor Author

brson commented Aug 7, 2013

It's doing a copy of the value at a memory location that is only accessed through atomics without doing an atomic load.

It's not true that they are identical to non-atomic types. They encapsulate a memory location to which they only allow access via atomics - except for this non-atomic load.

@bblum
Copy link
Contributor

bblum commented Aug 8, 2013

clone() would make sense, I think.

@catamorphism
Copy link
Contributor

Nominating

@brson brson closed this as completed in eabdc8c Nov 8, 2013
flip1995 pushed a commit to flip1995/rust that referenced this issue Feb 24, 2022
Don't lint `needless_borrow` in method receiver positions

fixes rust-lang#8408
fixes rust-lang#8407
fixes rust-lang#8391
fixes rust-lang#8367
fixes rust-lang#8380

This is a temporary fix for `needless_borrow`. The proper fix is included in rust-lang#8355.

This should probably be merged into rustc before beta branches on Friday. This issue has been reported six or seven times in the past couple of weeks.

changelog: Fix various issues with `needless_borrow` n´. Note to changelog writer: those issues might have been introduced in this release cycle, so this might not matter in the changelog.
flip1995 pushed a commit to flip1995/rust that referenced this issue Jun 30, 2022
Add lint `explicit_auto_deref` take 2

fixes: rust-lang#234
fixes: rust-lang#8367
fixes: rust-lang#8380

Still things to do:

* ~~This currently only lints `&*<expr>` when it doesn't trigger `needless_borrow`.~~
* ~~This requires a borrow after a deref to trigger. So `*<expr>` changing `&&T` to `&T` won't be caught.~~
* The `deref` and `deref_mut` trait methods aren't linted.
* Neither ~~field accesses~~, nor method receivers are linted.
* ~~This probably shouldn't lint reborrowing.~~
* Full slicing to deref should probably be handled here as well. e.g. `&vec[..]` when just `&vec` would do

changelog: new lint `explicit_auto_deref`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-medium Medium priority
Projects
None yet
Development

No branches or pull requests

4 participants