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

ptr::swap docs needs clarfication #44479

Closed
Gankra opened this issue Sep 10, 2017 · 7 comments
Closed

ptr::swap docs needs clarfication #44479

Gankra opened this issue Sep 10, 2017 · 7 comments
Assignees
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. P-medium Medium priority

Comments

@Gankra
Copy link
Contributor

Gankra commented Sep 10, 2017

The docs state that overlap is permitted, which is intended to cover the lhs==rhs case, but don't specify what that means for partial overlap. For instance:

let mut x = [0u32, 1, 2]; 
let a = &mut x[0] as *mut _ as *mut[u32; 2]; 
let b = &mut x[1] as *mut _ as *mut[u32; 2]; 
ptr::swap(a, b);

// Today: [1, 0, 1]
println!("{:?}", x); 

Reasonable implementations are forced to produce either 0 or 2 for the middle value. We should definitely at least spec that that happens. If we want to be Extra Helpful, we can specify that 0 is used, because there's basically no reason to change this behaviour, or for a different impl to diverge.

Something like "if the LHS and RHS overlap, then the overlapping region of memory will be derived from the LHS", along with the above example to clarify.

A nice thing about specifying it is that, because it's otherwise symmetric, you can swap your lhs/rhs to choose exactly which half you want to "win".

@steveklabnik steveklabnik added I-needs-decision Issue: In need of a decision. I-nominated T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Sep 11, 2017
@steveklabnik
Copy link
Member

Nominating for @rust-lang/libs discussion about what behavior to spec; then @rust-lang/docs is happy to doc whatever they decide.

@dtolnay
Copy link
Member

dtolnay commented Sep 11, 2017

I am okay with the current behavior. Agree the doc should explain what happens.

@alexcrichton
Copy link
Member

Discussed during libs triage the conclusion was "let's do what C++ does", there weren't any particular strong opinions.

@alexcrichton alexcrichton removed I-needs-decision Issue: In need of a decision. I-nominated labels Sep 13, 2017
@steveklabnik steveklabnik added A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools and removed T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Sep 13, 2017
@frewsxcv
Copy link
Member

Seems like this might be resolved by #43964?

@Gankra
Copy link
Contributor Author

Gankra commented Sep 17, 2017

No that's where the issue was raised. I didn't want to block the PR on a pre-existing issue.

@TimNN TimNN added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Sep 17, 2017
@steveklabnik steveklabnik added the P-medium Medium priority label Oct 31, 2017
@frewsxcv
Copy link
Member

frewsxcv commented Dec 3, 2017

i'd be happy to take care of this

Discussed during libs triage the conclusion was "let's do what C++ does", there weren't any particular strong opinions.

to clarify, does C++ do the 'LHS winning' behavior that @gankro mentioned in the original description of this issue? and if so, does C++ specify this behavior somewhere?

@frewsxcv
Copy link
Member

frewsxcv commented Dec 3, 2017

opened a PR for this, would appreciate if one of y'all could take a look: #46483

frewsxcv added a commit to frewsxcv/rust that referenced this issue Dec 6, 2017
…Sushi

Document behavior of `ptr::swap` with overlapping regions of memory.

Fixes rust-lang#44479.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. P-medium Medium priority
Projects
None yet
Development

No branches or pull requests

6 participants