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

AMDG On 4/29/19 12:52 PM, Antony Polukhin wrote:

пн, 29 апр. 2019 г. в 21:08, Steven Watanabe :

...

On 4/29/19 12:04 PM, Antony Polukhin wrote:

...

пн, 29 апр. 2019 г. в 20:44, Steven Watanabe via Boost :

...

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