Append potential logic error observation

Using a vector after emptying it with append, is possible, but probably a logic error. Would it be better for append to take ownership of its argument?

It takes an &mut, which does signal it's modifying other. Changing it now would break existing code. Changing it would also prevent reusing its memory (optimization technique).

Not taking ownership lets the caller reuse the allocation, and lets an exclusive borrower perform the operation ergonomically/efficiently. The chance of a logic error is somewhat mitigated by Rust's mut binding requirements and calling conventions as well. (If you own the Vec, you have to declare it mut and pass a &mut. If you don't, you have to have a &mut Vec<_> and not a &mut [_], i.e., being able to change the length was probably intentional / is probably communicated by the API.)

In any case, too late to change it now (it has to stick around forever for backwards compatibility, and it'd be too much churn for not enough reason to deprecate it).


An ownership-taking alternative already exists by the way.

fn example<T>(sink: &mut Vec<T>, source: Vec<T>) {
    sink.extend(source);
    // errors
    // let _ = source;
}

(Incidentally, "safety" has UB-related meaning in Rust that doesn't cover potential logic errors like this topic.)

3 Likes

Well, a different method could do it, it would need a good name. If you want to reuse an empty vector, it should be easy to just init one. In my project, I have some structs that have an append method, so I can take ownership of the argument at that level.

If anyone who can, wants to improve the title, I have no objection.

1 Like

extend() does exactly this.

fn main() {
    let mut one = vec![1,2,3];
    let two = vec![4,5,6];
    
    one.extend(two);
}

OK, extend does it.

No, why?

Actually, no. Extend is generic (it takes any iterator), so this works:

one.extend(two.drain(..));

I guess this sort of usage pattern can't be prevented unless the RHS is concretely Vec<T> (ie., Self), at which point there's no benefit to such a by-value method.

It seems easy to me to use a vector thinking it is not empty, so leaving the vector around, when you could easily init a new one, seems risky to me.

If extend takes ownership of the argument, which I think it does, its out of scope from the caller. The caller will not use it again.

Did you check my example code above? It looks like you are not right.

But what if you do want to re-use it? Shouldn't that be allowed? It's a common pattern, and I experienced exactly zero such bugs during the 15+ years I've been using any language with dynamic arrays.

1 Like

I don't think transferring ownership (move) of an object in Rust is ever used just to avoid confusion, e.g., when the object may be unlikely to be needed any longer, or is empty. At least I can't think of any cases of this. I only see ownership transfer being used when truly necessary to avoid undefined behavior.

That's not really true; the ownership model is often used for describing semantics (eg. MutexGuard unlocks on drop, and database transaction handlers can rollback or commit automatically). But the "vector is empty after appending" is not one of these cases.

1 Like