[gil] Review of move assignment operator for image class

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[gil] Review of move assignment operator for image class

Boost - Dev mailing list
Hi,

We're adding move semantics to some of GIL classes [1]
and we'd like to receive feedback on the move-assignment operator
for the image class [2] (copied below) that we've worked out so far
if there are any major blunders to correct or gaps to fill for C++11
compliant implementation:

image& operator=(image&& img)
{
    if (this == std::addressof(img))
        return *this;

    auto const exchange_memory = [](image& lhs, image& rhs)
    {
        lhs._memory = boost::exchange(rhs._memory, nullptr);
        lhs._align_in_bytes = boost::exchange(rhs._align_in_bytes, 0);
        lhs._allocated_bytes = boost::exchange(rhs._allocated_bytes, 0);
        lhs._view = boost::exchange(rhs._view, image::view_t{});
    };

    constexpr bool pocma =
std::allocator_traits<Alloc>::propagate_on_container_move_assignment::value;
    if (pocma)
    {
        // non-sticky allocator, can adopt the memory, fast
        destruct_pixels(this->_view);
        this->deallocate();
        this->_alloc = img._alloc;
        exchange_memory(*this, img);
    }
    else if (_alloc == img._alloc)
    {
        // allocator stuck to the rhs, but it's equivalent of ours, we
can still adopt the memory
        destruct_pixels(_view);
        this->deallocate();
        exchange_memory(*this, img);
    }
    else
    {
        // cannot propagate the allocator and cannot adopt the memory
        if (img._memory)
        {
            allocate_and_copy(img.dimensions(), img._view);
            destruct_pixels(img._view);
            img.deallocate();
            img._view = image::view_t{};
        }
        else
        {
            destruct_pixels(this->_view);
            this->deallocate();
            this->_view = view_t{};
        }
    }
    return *this;
}

One thing that is still missing is the noexcept with expression
evaluated based on something like this

template <class _Alloc>
using is_noexcept = std::conditional_t
<
    std::allocator_traits<_Alloc>::is_always_equal::value,
    _Equal_allocators,
    typename std::allocator_traits<_Alloc>::propagate_on_container_move_assignment::type
>;

but we there is no is_always_equal in C++11, so there are gaps to fill.

Any comments are appreciated.

[1] https://github.com/boostorg/gil/pull/457/
[2]  https://github.com/boostorg/gil/blob/7d8df1c105e81070e30ef68ea7abb8e39111d6e5/include/boost/gil/image.hpp#L152-L200

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [gil] Review of move assignment operator for image class

Boost - Dev mailing list
On Mon, 30 Mar 2020 at 20:38, Mateusz Loskot <[hidden email]> wrote:

>
> image& operator=(image&& img)
> {
>     if (this == std::addressof(img))
>         return *this;
>
>     auto const exchange_memory = [](image& lhs, image& rhs)
>     {
>         lhs._memory = boost::exchange(rhs._memory, nullptr);
>         lhs._align_in_bytes = boost::exchange(rhs._align_in_bytes, 0);
>         lhs._allocated_bytes = boost::exchange(rhs._allocated_bytes, 0);
>         lhs._view = boost::exchange(rhs._view, image::view_t{});
>     };
>
>     constexpr bool pocma =
> std::allocator_traits<Alloc>::propagate_on_container_move_assignment::value;
>     if (pocma)
>     {

This obviously is not good as non-constexpr will require _Alloc::operator=
but there is no `if constexpr` in C++11 to discard instantiation.

Is there any BOOST_IF_CONSTEXPR magic for such a situation?
Since we are stuck at C++11, we can't use hana::eval_if
and looks like we only can do the old-school tag dispatching.

p.s. Sorry if my initial C++ question w/o strict reference to Boost
was off-topic,
or perhaps I made a non-commentable terrible C++ blunder :)

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost