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

std: Standardize (input, output) param orderings #23866

Merged
merged 1 commit into from
Mar 31, 2015

Conversation

alexcrichton
Copy link
Member

This functions swaps the order of arguments to a few functions that previously
took (output, input) parameters, but now take (input, output) parameters (in
that order).

The affected functions are:

  • ptr::copy
  • ptr::copy_nonoverlapping
  • slice::bytes::copy_memory
  • intrinsics::copy
  • intrinsics::copy_nonoverlapping

Closes #22890
[breaking-change]

This functions swaps the order of arguments to a few functions that previously
took (output, input) parameters, but now take (input, output) parameters (in
that order).

The affected functions are:

* ptr::copy
* ptr::copy_nonoverlapping
* slice::bytes::copy_memory
* intrinsics::copy
* intrinsics::copy_nonoverlapping

Closes rust-lang#22890
[breaking-change]
@rust-highfive
Copy link
Collaborator

r? @brson

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

@alexcrichton
Copy link
Member Author

r? @aturon

@rust-highfive rust-highfive assigned aturon and unassigned brson Mar 30, 2015
}

#[stable(feature = "rust1", since = "1.0.0")]
#[cfg(not(stage0))]
pub use intrinsics::copy;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be an off-topic, but I always wanted to ask - why copy and copy_nonoverlapping are in ptr and not in mem?
mem::copy - copy memory, totally understandable and resembles memcpy
ptr::copy - what? copy pointer?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APIs that are centered around raw pointers go in std::ptr currently while those dealing with references go in std::mem (in general). Also, the _memory suffix from ptr::copy_memory was recently dropped as there's probably not much you're copying with pointers other than the memory itself.

@aturon
Copy link
Member

aturon commented Mar 30, 2015

@bors: r+ acd48a2

Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 31, 2015
…aturon

This functions swaps the order of arguments to a few functions that previously
took (output, input) parameters, but now take (input, output) parameters (in
that order).

The affected functions are:

* ptr::copy
* ptr::copy_nonoverlapping
* slice::bytes::copy_memory
* intrinsics::copy
* intrinsics::copy_nonoverlapping

Closes rust-lang#22890
[breaking-change]
bors added a commit that referenced this pull request Mar 31, 2015
@bors bors merged commit acd48a2 into rust-lang:master Mar 31, 2015
@mahkoh
Copy link
Contributor

mahkoh commented Apr 2, 2015

Thanks for causing segfaults all over my code with this change that was obviously not in need of an RFC because it was submitted by @alexcrichton himself. This is what one has to expect from #[stable] Rust.

@alexcrichton alexcrichton deleted the switch-some-orders branch April 2, 2015 19:45
@Stebalien
Copy link
Contributor

IIRC, stable isn't really stable until 1.0 (or maybe beta?). As is, we're still in alpha. From the blog post:

Our promise is that, if you are using the stable release of Rust, you will never dread upgrading to the next release.

@mahkoh
Copy link
Contributor

mahkoh commented Apr 2, 2015

Yeah, who gives a beep about undetectable changes that are guaranteed to introduce memory unsafety all over the place downstream.

@Stebalien
Copy link
Contributor

Other people silently breaking your code is really annoying but please remain civil; this is not lkml. You're using an unstable alpha language and Alex made that change to prevent future confusion (users generally expect copy to be from-to).

@mahkoh
Copy link
Contributor

mahkoh commented Apr 2, 2015

Other people silently breaking your code is really annoying

You're conflating breaking with introducing undefined behavior that cannot be detected without manually auditing every place copy is used. Most people won't even notice that copy has been changed and won't understand why they get segfaults or how to find the source of the problem.

this is not lkml

I don't read lkml but you're making it sound like a swell place.

(users generally expect copy to be from-to)

Nonsense.

@comex
Copy link
Contributor

comex commented Apr 5, 2015

It's too late now, but I guarantee this will confuse a large number of people coming from C and expecting a memcpy replacement. Though I suppose that bridge is already crossed by std::ptr::copy taking an element count rather than byte count - I made the error of assuming the latter when I first used (the old name of) that function.

@blaenk
Copy link
Contributor

blaenk commented Apr 7, 2015

Regarding the confusion, I honestly think that if people blindly write code expecting it to be the same as in some other language then that's on them. If it were named the same then yeah I would agree that could be misleading.

@sinistersnare
Copy link
Contributor

(not trying to be vitriolic)

Was this ever discussed in an RFC? This seems like something that would have happened in one of the stabilization RFCs, and I stopped subscribing to rustc dev a couple months ago (mailbox was becoming too fat)

@pcwalton
Copy link
Contributor

pcwalton commented Apr 7, 2015

@aturon @alexcrichton Whoa, this was not the right call. I'm with the others. If i had known about this, I would be completely opposed.

@pcwalton
Copy link
Contributor

pcwalton commented Apr 7, 2015

I think this should be reverted. Submitted #24142 to revert it.

@aochagavia
Copy link
Contributor

@alexcrichton what about mem::replace? It still has output, input order. Should I open a PR to fix it?

jooert added a commit to jooert/rust-bswap that referenced this pull request Apr 29, 2015
Swap order of input and output parameters of ptr::copy_nonoverlapping
(rust-lang/rust#23866), remove usage of
std::num::Int (rust-lang/rust#23549), change
(rust-lang/rust#23860).
jooert added a commit to jooert/rust-bswap that referenced this pull request Apr 29, 2015
Swap order of input and output parameters of ptr::copy_nonoverlapping
(rust-lang/rust#23866), remove usage of
std::num::Int (rust-lang/rust#23549), change
(rust-lang/rust#23860).
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.

std::slice::bytes::copy_memory and std::io::copy take their arguments in different orders