dsharlet / array

C++ multidimensional arrays in the spirit of the STL
Apache License 2.0
198 stars 15 forks source link

Move constructor no longer copies by default when using a custom allocator #88

Closed woodychow closed 1 year ago

woodychow commented 1 year ago

It only copies when the allocator is not propagate_on_container_move_assignment.

Fixes issue#47

dsharlet commented 1 year ago

Thanks for iterating with me, this is looking pretty good. One last nit I'd like to do is understand whether this fixes #47 or not. I think it does, but after you merge it, I will write a test that verifies if it does or not. Note that that issue is about the lifetime/number of instantiations of the allocator itself, rather than the lifetime of the elements of the array, so it's a bit different than any existing test coverage I think.

woodychow commented 1 year ago

No problem. Thanks for reviewing my change.

I don't think my change will fix #47 as the new allocator is going to be default constructed first. To avoid this, I believe you need template specialization with propagate_on_container_move_assignment, and do alloc_(std::move(other.alloc_)) before entering the constructor body.

dsharlet commented 1 year ago

I was wondering why the Travis build didn't run, Travis disabled our account for a negative account balance... on a free account. Simply signing in to travis seems to have fixed it, so I'll just merge this and then test it as part of looking at the issues I mentioned anyways.

woodychow commented 1 year ago

Cool. Thanks very much!