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 [T]::swap_with_slice #44031

Merged
merged 1 commit into from
Aug 25, 2017
Merged

Add [T]::swap_with_slice #44031

merged 1 commit into from
Aug 25, 2017

Conversation

scottmcm
Copy link
Member

The safe version of a method from ptr, like [T]::copy_from_slice is.

Tracking issue: #44030

The safe version of a method from ptr, like [T]::copy_from_slice
@rust-highfive
Copy link
Collaborator

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member

Seems like a nifty API to me, thanks!

@rust-lang/libs, any thoughts on naming here?

I'd sort of be in favor of swap_slice as opposed to swap_with_slice?

@alexcrichton alexcrichton assigned alexcrichton and unassigned brson Aug 22, 2017
@scottmcm
Copy link
Member Author

I chose swap_with_slice here for differentiation with [T]::swap(usize, usize) and for similarity with copy_from_slice (which isn't copy_slice), but I'm happy to change it to whatever.

@aturon
Copy link
Member

aturon commented Aug 22, 2017

I'm ambivalent on the naming, but slightly favor @scottmcm's proposed naming.

@alexcrichton
Copy link
Member

IIRC copy_from_slice was selected to ensure the direction of the copy was clear, although with a swap it's less important as the direction shouldn't matter? I don't necessarily dislike swap_with_slice, I just wasn't sure where with came from!

@scottmcm
Copy link
Member Author

Oh, right, clone_from_slice is Clone::clone_from + "this takes a slice". That does seem to argue for swap_slice = mem::swap + "this takes a slice". The preposition isn't really needed.

I guess this with is sortof like SliceExt::ends_with or intrinsics::add_with_overflow, but very different from the withs in Vec::with_capacity or Entry::or_insert_with...

@alexcrichton
Copy link
Member

Eh and to be clear I'm not opposed to the name at all, just wanted to pontificate for a bit before merging.

@aidanhs aidanhs added the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Aug 23, 2017
@alexcrichton
Copy link
Member

Ok well sounds like there aren't many other thoughts, so let's just decide this before stabilization

@bors: r+

@bors
Copy link
Contributor

bors commented Aug 24, 2017

📌 Commit c4cb2d1 has been approved by alexcrichton

@alexcrichton alexcrichton added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Aug 24, 2017
@bors
Copy link
Contributor

bors commented Aug 25, 2017

⌛ Testing commit c4cb2d1 with merge ba65645...

bors added a commit that referenced this pull request Aug 25, 2017
Add [T]::swap_with_slice

The safe version of a method from `ptr`, like `[T]::copy_from_slice` is.

Tracking issue: #44030
@bors
Copy link
Contributor

bors commented Aug 25, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing ba65645 to master...

@bors bors merged commit c4cb2d1 into rust-lang:master Aug 25, 2017
@scottmcm scottmcm deleted the swap_with_slice branch August 31, 2017 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants