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

Subject: Re: [boost] [variant] Heads-up!
From: Steven Watanabe (watanabesj_at_[hidden])
Date: 2019-04-29 19:27:09

AMDG

On 4/29/19 12:52 PM, Antony Polukhin wrote:
> пн, 29 апр. 2019 г. в 21:08, Steven Watanabe <watanabesj_at_[hidden]>:
>>
>> 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.
> 

I don't agree with this characterization of theoretical
vs. practical.  If invariants are not enforced strictly
they are essentially worthless--unless you only want
your program to mostly work and don't care about bugs,
that is.

> However, from the practical point of view :
> * noexcept(false) move constructor degrades performance of variant

Sacrificing correctness for performance is an unacceptable tradeoff.

> * move constructor that does a dynamic memory allocation is a surprise
> for the majority of users
> 

It may be a surprise, but it is something that you already
need to be aware of, due to the fact that move can
call the copy-constructor (or copy-assignment operator)
if there is no move constructor defined.

>>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.
> 

So we're demonstrating an incorrect solution to
show people how to solve problems correctly?

>>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.
> 

That's mostly irrelevant.  The standard library provides
much stronger guarantees for all of its types.

> 3 against 1.
> 

Counting arguments like this is nonsensical.

> 
> 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.
> 

That sounds like the worst of all possible worlds.

In Christ,
Steven Watanabe