Re: [boost] [variant] Heads-up!

Subject: Re: [boost] [variant] Heads-up!
From: Lee Clagett (forum_at_[hidden])
Date: 2019-04-29 19:13:04

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, April 29, 2019 6:52 PM, Antony Polukhin via Boost <boost_at_[hidden]> wrote:

> пн, 29 апр. 2019 г. в 21:08, Steven Watanabe watanabesj_at_[hidden]:
>
> > AMDG
> > On 4/29/19 12:04 PM, Antony Polukhin wrote:
> >
> > > пн, 29 апр. 2019 г. в 20:44, Steven Watanabe via Boost boost_at_[hidden]:
> > >
> > > > On 4/29/19 11:30 AM, Antony Polukhin via Boost wrote:
> > > >
> > > > > I've merged a very cool optimization by Nikita Kniazev into the master
> > > > > branch. From now on boost::variant does pointer stealing for recursive
> > > > > variants.
> > > > > This significantly improves the performance of the variants move constructors.
> > > > > However if you use a variant variable after the std::move for anything
> > > > > except destruction and assignment then you're getting an UB. Beware!
> > > >
> > > > boost::variant goes to great lengths to prevent
> > > > exactly this situation. You just broke it. This
> > > > change is unacceptable. Please revert it. This
> > > > optimization can be used iff. you have a way to
> > > > construct a valid object in the rhs.
> > >
> > > You can restore the old slow pre-rvalue era behavior by defining
> > > BOOST_VARIANT_NO_RECURSIVE_WRAPPER_POINTER_STEALING.
> >
> > That doesn't make it okay. Look, this optimization
> > would be fine with a different variant, one that doesn't
> > provide the never-empty guarantee. That isn't boost::variant.
> > <...>
>
> From the theoretical point of view I'm on your side. boost::variant is
> something that is never-empty.
>
> However, from the practical point of view :
>
> -   noexcept(false) move constructor degrades performance of variant
> -   move constructor that does a dynamic memory allocation is a surprise
>     for the majority of users
>
>     From the teachers point of view... Boost is a collection of high
>     quality libraries and many people look into the source codes to learn
>     new tricks and correct approaches for solving problems.
>     noexcept(false) move constructor that implicitly dynamically allocates
>     is something that I would not prefer to show anyone.
>
>     From the C++ Standard Library point of view... Well, you can only
>     assign new value or destroy a moved away variable. Everything is fine
>     here.

This was the original argument in the thread Steven Watanabe
highlighted [1]. IIRC, the problem was that boost::variant always had a
never-empty guarantee, and so the "valid but unspecified state"
specification for C++ moves was not met by the optimization.

>     3 against 1.
>
>     I see a way to restore the theoretical beauty of the variant. We can
>     just remove the empty() member function or force it to always return
>     false. So that variant is still never empty, but using a variant after
>     move is an UB.

If the one negative is adding UB to valid existing code at the next
point release, doesn't that outweigh any number of positive outcomes?
Why wouldn't this be an opt-in strategy - this property of moves has
been established for years now.

Lee

[1] https://lists.boost.org/Archives/boost/2013/02/200744.php