Boost logo

Boost :

Subject: Re: [boost] [variant] Heads-up!
From: pbristow_at_[hidden]
Date: 2019-05-01 14:16:50

-----Original Message-----
From: Boost <boost-bounces_at_[hidden]> On Behalf Of Steven Watanabe via
Sent: 1 May 2019 03:17
To: boost_at_[hidden]
Cc: Steven Watanabe <watanabesj_at_[hidden]>
Subject: Re: [boost] [variant] Heads-up!


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!

Here's my summary of why this is a terrible idea:

- Boost.Variant provides the never-empty guarantee.
  You may disagree about whether this guarantee is
  useful, but it is far too late to remove a feature
  of an old, established component that is advertised
  quite loudly in the documentation.

- This change breaks the never-empty guarantee because
  it allows a variant to be empty. Even if the changes
  to variant::empty were reverted, it still cannot
  be treated as a change solely to recursive_wrapper.
  A non-empty variant containing an empty recursive_wrapper
  would not conceptually violate variant's never empty
  guarantee. Unfortunately, this view is not viable
  due to variant's special handling of recursive_wrapper.

- I find the view that there's no problem with causing
  undefined behavior that cannot be detected at compile
  time in existing code because nobody could possibly
  be depending on it, to be incredibly myopic.

- The argument that the never-empty guarantee only applies
  to exception-safety and not to move is specious, because
  that's not how invariants work.

- The old discussion of this topic concluded with a
  decision to leave recursive_wrapper alone and introduce
  a new type with the new behavior (among other things).
  This new change appears to completely ignore the consensus
  established at that time, which is one that I agree with.
  Nothing has changed since then, and no new arguments
  are adduced.


It is certainly against Boost tradition of avoiding breaking changes where

Allowing this behaviour with a new macro (and a skull'n'crossbones warning in
the Boost macros documentation) might just be acceptable, but requiring a new
NO_ macro to get the previous behaviour doesn't get my support FWIW.


Paul A. Bristow
Prizet Farmhouse
Kendal, Cumbria

Boost list run by bdawes at, gregod at, cpdaniel at, john at