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 intrinsics should take unsafe pointers not borrowed pointers #6314

Closed
brson opened this issue May 7, 2013 · 10 comments
Closed

atomic intrinsics should take unsafe pointers not borrowed pointers #6314

brson opened this issue May 7, 2013 · 10 comments
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@brson
Copy link
Contributor

brson commented May 7, 2013

These are very unsafe operations. We should not be pretending that they operate on safe pointers.

@mnemotic
Copy link

@brson I'd like to take a stab at this.

@Aatch
Copy link
Contributor

Aatch commented Jun 30, 2013

@mnemotic I did most of the current work on atomics. This change should just require changing typeck/mod.rs to use raw pointers instead of region pointers. You'll also need to update the intrinsics in unstable/intrinsics.rs

Finally, if your willing, I have a few issues open for finishing the atomic support, this just means adding the rest of the operations to the types in unstable/atomics.rs.

I'm aatch on IRC too, so hit me up if you have any questions.

@mnemotic
Copy link

@Aatch Thanks for the support. Re: other issues; I've looked at #7421 and #7422. Are there others? I mean to look into those after I tackle this, but I'm not committing to it (yet).

@bblum
Copy link
Contributor

bblum commented Jul 7, 2013

I think I disagree with this. There's nothing about atomic ops that break the type system; they are only dangerous in conjunction with unsafe-shared-mutable-state. So you'll already have to have unsafe blocks if you're doing unsafe atomic ops.

@auroranockert
Copy link
Contributor

@bblum But what are the use-cases for atomics without shared mutable state? Couldn't you just use normal integers then?

@bblum
Copy link
Contributor

bblum commented Jul 14, 2013

It is true that they are not good for much when used in otherwise-safe code. But that doesn't make them unsafe in their own right. Sort of like how creating a *T is fine, but dereferencing it is unsafe.

I suppose they are just the intrinsics, not the interface of the unstable::atomics library, so it does not matter as much as I initially thought, since pretty much nobody except unstable::atomics should be using them directly.

@bblum
Copy link
Contributor

bblum commented Jul 14, 2013

Think about it like this: An operation is unsafe iff the compiler cannot verify that an arbitrary use of the operation will not break type safety or memory safety. If we can reason about the operation to the extent that we're sure that no use of it will crash, there's no reason for it to be unsafe.

@emberian
Copy link
Member

emberian commented Aug 8, 2013

I actually agree with bblum here. There's no reason to make atomic ops unsafe if you already need unsafe code to do unsafe things with them. They're perfectly safe and well-defined.

@pnkfelix
Copy link
Member

Nominating for P-backcompat-libs.

@pnkfelix
Copy link
Member

brson pointed out to me that the intrinsics are not part of the backcompat api, so this never would be P-backcompat-foo.

At this point brson also thinks that we do not need to change to *T; the arguments here for leaving as is have convinced him that this bug can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

No branches or pull requests

7 participants