Memcpy is backwards

[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.

[eridius]

If I had sole power to make the decision about parameter order without any repercussions for existing code, I’m not sure whether I’d pick src, dst or dst, src. Both have their merits. Although I think I’m leaning towards the src, dst ordering.

That said, since we shipped the beta with this change, I think that’s a very strong argument in favor of leaving it alone. There’s no overwhelming benefit to changing it, so we should avoid breaking everybody all over again.

[P1start]

As someone who has not coded in C very much, I think the new order is much better: in English, we say “move this from here to there” most of the time. I probably would have got it wrong if I had tried to use it before this change. Rust is aiming for a similar area as C/C++, so I think it makes sense to consider this change as just another improvement over C. I certainly don’t think we should use the dst, src order just because C does - one of the biggest advantages of Rust is how it improves over C/C++ instead of just sticking to how they do things.

However, we certainly need a very prominent message on its documentation page explaining the difference in argument order. I’m sure this will be a very common pitfall for C/C++ devs trying out Rust.

[petrochenkov]

C++ is not very consistent in this area, for example std::copy (link), which is a common replacement for memcpy in “safe” C++, uses “src, dst” ordering. So, “dst first, src second” isn’t imprinted irreversibly in the brain of a C++ programmer and I don’t think this mistake will be especially common.

[eridius]

Even C breaks this convention. bcopy() is defined as

void bcopy(const void *src, void *dst, size_t len);

So even in pure C I always double-check parameter order for this stuff. So I don’t think think we need to be overly concerned with warning C programmers about src, dst order.

[apoelstra]

On Wed, Apr 08, 2015 at 05:10:21PM +0000, mahkoh wrote:

[cgaebel]

     bcopy() was deprecated in IEEE Std 1003.1-2001 (``POSIX.1'') and removed in IEEE Std 1003.1-2008
     (``POSIX.1'').

It was deprecated because everyone used memcpy. This is a good argument for the memcpy parameter order.

[eridius]

This is a good argument for the memcpy parameter order.

It’s a good argument for having a consistent parameter order. It says nothing about whether Rust should adopt C parameter order for these methods.

[TechnoMancer]

I suggest that the solution to this problem is to use keyword arguments. In addition an error lint should be associated with the functions that errors if you call them without using keyword arguments. This means that any order can be used and the calls are not confusing as they spell out the source and destination explicitly.

[DanielFath]

@TechnoMancer Someone suggested keyword arguments earlier, however the problem with them is that means, that changing parameter name now becomes a breaking change.

[mkpankov]

Honestly, I don’t want to specify src = ..., dst = ... at every function invocation.

[DanielFath]

If it is anything like Python’s keywords, it’s optional, however then changing order AND changing name of input params is breaking change.

[iox]

I personally like the (src, dst) ordering, and would be happy to keep it (it reminds me of the POSIX ‘cp’ command). But I don’t think we should worry too much about reverting it here. While I agree that in ordinary circumstances we should not break stable APIs, this is not an ordinary circumstance: this was a changed pushed through without an RFC, at the last second before beta began. If compelling reasons emerge to change it back, we should absolutely do so. The language isn’t released yet, and breaking changes should be expected to occur every now and then (even to “stable” APIs if circumstances warrant it).