feat: growable buffer move_to method

[ianna]

• growable buffer move_to method implementation and test
• growable buffer clear method bug fix
• more thorough checks in the tests

[codecov]

Codecov Report

Merging #2178 (1096065) into main (a91d96b) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

[jpivarski]

I've always been a little confused about what std::move does. It seems pretty clear (in this example and others) that it copies data.

What's the difference between this new move_to function and the old concatenate_to function? Don't they both copy the panel data into an externally provided buffer and delete the panels?

I'm fine with making these changes (you can merge it when you're happy with it). I'm just asking.

[jpivarski]

This looks like a bug-fix. Is it?

[agoose77]

I've always been a little confused about what std::move does. It seems pretty clear (in this example and others) that it copies data.

I can chime in here! std::move just indicates to the compiler that the argument can be moved from, i.e. a consumer of the std::move(...) could avoid a copy by moving from the argument. So, it doesn't always eliminate a copy, depending upon how it is consumed. It's just a signal of lifetime intent IIRC.

[ianna]

This looks like a bug-fix. Is it?

Yes, it is.

[ianna]

I've always been a little confused about what std::move does. It seems pretty clear (in this example and others) that it copies data.

What's the difference between this new move_to function and the old concatenate_to function? Don't they both copy the panel data into an externally provided buffer and delete the panels?

The concatenate_to does not delete the panels. One has to call clean to do that. We can change it, I think. Then there will be just one method.

I'm fine with making these changes (you can merge it when you're happy with it). I'm just asking.

[jpivarski]

It would be nice if there were only one method. That way, developers don't have to guess which one to use.

Separating the functions of "copy the data out into a contiguous buffer" and "remove the panels" would keep more use-cases open. Maybe there's reason to clear the panels without copying out the data (user initiates a reset), and maybe there's a reason to copy the data out and keep filling (the original "snapshot" idea).

If clear is already a method, maybe move_to could take a boolean argument, specifying whether to clear after copying? Then, with move_to reproducing concatenate_to's functionality, it should then become concatenate_to, since we've already given out instructions on how to copy out the data, and they use the concatenate_to spelling.