Memcpy is backwards

[mkpankov]

I think C order is more consistent because the common usage pattern is this:

    char buffer[256];
    int result = produce_stuff_to_allocated_buffer (buffer, input1, input2);

With that order, everything that gets written to is on the left (result and buffer). Swapping for “more natural” left-to-right order means writeable names are further from each other. I personally think it would make such usage pattern less readable.

Of course, this might be an anti-pattern in Rust - I’m not qualified to tell how such a function would be idiomatically implemented in Rust.

[cgaebel]

I personally think the change was without merit. I’ve never gotten the argument order wrong on ptr::copy or ptr::copy_nonoverlapped before this change. It’s “obvious” to people familiar with systems programming. The ordering on the parameters of memcpy have been around longer than I’ve been alive!

[aturon]

It was absolutely a mistake to push this through without an RFC, and I apologize for signing off on it. It wound up causing some painful fallout, and it’s clear that many have strong opinions about the order here that weren’t taken into account. Of course, this kind of thing won’t happen in the future, since we won’t be able to make changes to stable APIs.

That said, I’m in much the same boat as @nikomatsakis: it’s not clear to me whether it’s worth another round of breakage at this point – something we want to reserve for emergencies only. But I definitely do see the argument that “muscle memory” from memcpy could easily get you into trouble here.

One possibility to avoid outright breakage would be to deprecate the functions, and introduce differently-named replacements. (copy used to be called copy_memory until a short while ago, anyway.) That would give people more time and help in making an adjustment, and we can do a deprecation cleanup before 1.0 ships.

Note, however, that if we do revert the change to ptr operations, that would reintroduce inconsistency with IO operations, and changing all of the relevant functions, even through deprecation, would be pretty massive. (That’s why the ptr functions were changed in the first place). We could, of course, decide internal consistency across ptr and IO isn’t so important.

[cgaebel]

Wanting consistency between ptr and io seems very weird to me, especially when the alternative is consistency between ptr and how it's been done in C for years.

[petrochenkov]

I don’t care much about the specific parameter ordering (the current one is acceptable), but I’m sure in one thing - you shouldn’t break things again by reverting the controversial commit.

[bfops]

Reposting my 1.6 American cents from the Github discussion:

Maybe this is the worst of both worlds, but what if the intrinsics are left to be C ordering, and the higher Rusty wrappers are given the Rust ordering? It could definitely be confusing if you switched between the two, but in my head, “intrinsic” says “I don’t control what this function looks like, I just link to it”.

That said, it would really only make sense if std::intrinsics had a “specifically LLVM intrinsics” section, and wasn’t all viewed as “rustc intrinsics”. In that case, I’d probably also be in favor of renaming them to match their LLVM counterparts.

[gkoz]

I’m guessing ptr::replace and mem::replace would need to be harmonized as well?

[CloudiDust]

@gkoz, not necessary.

I see it this way, generally, we say “copy/move A to B”, so for copying/moving functions, the correct order should be src, dst. (When we do assignments, the data flows right to left, but when we call a function, the arguments are evaluated left to right, so IMHO it is C that gets the order wrong, but C is such an important precedent that many, including me, are used to the dst, src order when dealing with the memcpy family of functions.)

However, “replace” is different. We say “replace B with A”, and the order is naturally dst, src.

[MTRust]

I don’t know, src, dst seems like the natural order of these parameters. I know C does it the other way around, but since Rust tries to fix many of C’s problems, isn’t it in place to also fix the ordering of this?

We already do things in the opposite order than C in many ways (ie variable declarations) as it is more natural, why make an exception here?

I’m for leaving it as it is.

[iopq]

Off topic: This is why I prefer keyword arguments.

[bfops]

Ooh +1. This is a great use case for keyword arguments. They do kind of exist to solve the exact problem of “hey we have a function whose parameter ordering is moot and could be accidentally used wrong”.

[rkjnsn]

It does make some amount of sense that the thing to which the verb applies comes first (you are copying src but you are replacing dst), but I still worry that this will be confusing.

[insaneinside]

We might also view the C convention as resembling an assignment, e.g.:

const int C = 5;
int x;
memcpy(&x, &C, sizeof(int));

might be interpreted as

int x = 5;

So in my mind there’s some doubt as to the validity of the arguments claiming it’s more natural to have the source argument first; if that’s the case, then we should use

5 = int x;

right? (This argument is reductio ad absurdum; let’s stay focused.)

[comex]

I think assignments are more important than you suggest. In C, a = b is similar to memcpy(&a, &b, sizeof(a)), so it makes sense that the variables should be in the same order.

For the same reason, in assembly, I prefer the order mov dst, src, which almost every architecture uses but x86, where Intel syntax does it that way but the arguably-more-common AT&T syntax flips the arguments around. I guess the existence of the latter is proof that src, dst feels more natural to many.

(I don’t understand the point about argument evaluation order. In C it’s undefined, and in other languages it’s pretty uncommon to write calls where it matters…)

[pczarn]

Showing which variables are mutated in the code is more important than showing where the input comes from. You can effortlessly scan groups of statements when the destination is visually aligned in the first column. The dst, src order is more helpful most of the time. This is also the reason why I dislike AT&T assembly syntax. I agree with @mkpankov.

[CloudiDust]

Yes, and we can totally argue for also reversing the arguments of functions like std::io::copy so everything is dst, src. The interpretations can go both ways and be both valid.

[CloudiDust]

Oops, I forgot that C had no defined eval order (while many other languages specify left-to-right).

[carllerche]

What is done is done. While the original change sucked (it was a fun time debugging), beta has landed, it is flagged as a stable API and the order of the arguments is a silly reason to break the API in beta (as it was in alpha, but we can’t turn back time).

Just let it be.

[liigo]

My solution:

Add back ptr::memcopy(dest, src, size) (with the same C parameters’ order), and deprecate ptr::copy for now.

(I don’t think Rust’s ptr::copy should consistent with C’s memcpy: they are independent functions with significant different names. ptr::copy(src, dest, size) may be add back later if needed. ptr::memcopy, not ptr::memcpy, will exists always. It’s up to you/users which function to use to copy memory.)

[bascule]

Like @carllerche I think it doesn’t make any sense to break backwards compatibility with the beta. The damage is done, time to move on. Trying to undo it would make things worse (as I am guessing that many things have already migrated over to the new ordering convention)

Backing out this change after beta shipped would, I believe, further instill fears that beta isn’t as stable as originally promised.