Memcpy is backwards

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

[MTRust]

The thing is, the only reason we might want the dst, src ordering is to be consistent with C, however we’re already not consistent with C in A LOT of places, because it was determined that it is not worth preserving the legacy of C in a new language that tries to fix C’s problems and I think that this is just another case trying to do that.

From my point of view, it is pretty clear, when thinking about copying something, the normal way of doing this is thinking about WHAT you want to copy first, because you first need something to copy and only then thinking about WHERE to copy it.

C does this the other way around, but C does a lot of strange stuff.

Another thing raised here, that I agree with, is that the thing has already been done and you’re probably going to cause more damage by reverting the change than leaving it as is, possibly even to the early reputation of Rust as a reliable platform, if we now break something we already promised we won’t.

There is going to be a lot of tempation to fix APIs with flaws that are going to be exposed by being put under usage no one here tried yet, but we must ressist the temptation or run the risk of hammering adoption.

[comex]

The thing is, the only reason we might want the dst, src ordering is to be consistent with C

No, that’s inaccurate. A few people in this thread (including me) have claimed dst, src is more natural; we seem to be outnumbered by those believing the opposite, which is fine by me, but consistency with C is not the only reason to choose it.

[cgaebel]

I don’t see a compelling reason to make memcpy have the same argument order as io::fs::copy. Those are two clearly different functions, that have done clearly different things for ages. There’s not even in the same top-level module in std!

The argument that “in English, we say 'move this from here to here”, doesn’t hold up under scrutiny. Do we say “move this from here to here, but n of them?” No. We say “move n things from src, to dst”, but no one is arguing that we should make that the order. That would be silly.

C++'s std::copy has its parameter order because it lives in std::algorithm, and should be compatible with the other functions there. It wasn’t intended as a memcpy replacement. It was intended to replace a loop with assignment. They’re different in C++. In fact, it’s best practice to call memcpy when you want a memcpy.

When I came to rust, I didn’t ask myself “What’s the function for copying bytes from one location to another?”. I thought to myself “What’s memcpy in rust?”. I suspect I’m not alone. The parameter order is not “one of C’s great mistakes”. Google around. Very, very few people are confused by this, and no one is unhappy with the status quo. Compare it to the results for “C NULL pointer”.

I call memcpy infinitely more than io::fs::copy, but you don’t see me arguing that io::fs::copy should change its parameter order. Keep the arguments in the same order as memcpy, and stop surprising people actually using rust for systems programming.

[mkpankov]

I guess the key question is whether people disagree with the substance of the decision (i.e., internal consistency is less important than consistency with memcpy, or perhaps that I/O APIs ought to have been changed instead) or just the silent, breaking nature of it. I personally tend to think the change was right on the merits—consistency is better than not, and I don’t care as to which ordering we use—but it ought to have been (at minimum) advertised more widely. Still, I’m somewhat on the fence.

I disagree with substance of the decision and I feel it’s inconsistent with good, useful mental model. The model is “what is being assigned/written to is on the left”. It is distinct from std::io::copy and from cp src dst. These both “counterexamples” come from IO world, and I don’t think IO and memory are to be confused. Not even speaking of that ptr::copy is used much more in a systems programming language.

One more reason to distinguish memory and IO operations is this: with memory copying, it is primitive copying, that is, byte-to-byte copying w/o any extra work going on. With cp, new file is created (with underlying kernel operations like inode allocation), and its contents are copied from the original file’s contents.

I think original order for ptr::copy should be restored ((dst, src,...)) and a targeted lint should be used to prevent another round of silent breakage.

Update: clarify “IO” example.

[rkjnsn]

I think we should definitely leave it the way it is now. Changing it original without an RFC was probably a mistake, but changing it again now that the beta has been released would be a much greater one, sending a much worse message. I think the bar for such a change should be very high. In this case, it’s not correcting a bug or limitation in the API, and there doesn’t seem to any consensus about what order is preferred, let alone one strong enough to warrant such a breaking change, now.

Even without the issue of breaking code unnecessarily after beta, I am sympathetic to the argument that the first argument to a function should be the object to which the verb applies. In this case, that would be the thing to be copied.

[iopq]

then make your own wrapper for it that does it the old way - your wrapper will NEVER break because you maintain it yourself

and you can have it in the order YOU want

[mkpankov]

This type of argument is symmetrical - why don’t you make a wrapper that does it the new way?

[system]

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.