Revert regression of memory unsafe append_array (same vector into same vector).

[Ivorforce]

Follow-up of #101385

This will cause a minor performance regression. But Vector is CoW so it won't be noticeable (and probably not even observable).

See #31736.

[lawnjelly]

#31736 was a great issue, I have to commend the author.

I've since been guilty of exactly this mistake myself (in LocalVector::erase_multiple_unordered(), it's a really easy thing to overlook. I suspect there's a lot of const T & in the containers that could do with reviewing for this bug.

Incidentally, for things like append in the long run, it could be worth checking for within the vector to avoid making a copy, because the benefits to doing the "check within" were higher the larger the size of the object being copied. As you say, here, COW might mostly negate this cost luckily.

[akien-mga]

I would amend #101385 into this PR's commit, it seems weird for #101385 to add comments for 3 cases and leave it to another PR to add the comment to the 4th case.

And add more exhaustive docs while fixing a regression to prevent such regressions in the future makes sense to me semantically.

[akien-mga]

For the record this was changed in #86966 (CC @Muller-Castro).
Might be worth re-reviewing that PR and its open follow-ups with #31736 in mind.

[Ivorforce]

I would amend #101385 into this PR's commit, it seems weird for #101385 to add comments for 3 cases and leave it to another PR to add the comment to the 4th case.

And add more exhaustive docs while fixing a regression to prevent such regressions in the future makes sense to me semantically.

I did it this way because I like to have separate no-op commits (like adding docs) and behavior change commits. But I'm happy to oblige if you'd prefer to have it all in one.

Edit: Done!

[Muller-Castro]

Hello! 🤝 should I rollback the Vector<T> params?

[Ivorforce]

Hello! 🤝 should I rollback the Vector<T> params?

Hi! 👋
No, it's not necessary, except when container elements may be moved around. I think in the case of Vector this PR just about covers it, but we may have missed more instances of this bug in other containers.

[akien-mga]

Thanks!