Boost.Variant's boost::recursive_wrapper missing noexcept specifier on move constructor

[peterhuene@…]

Using a sequence container of recursive variant, like std::vector<recursive_variant_>, results in the variant type not being movable.

Example:

#include <boost/variant.hpp>
#include <vector>

using namespace std;
using namespace boost;

struct foo
{
	foo() = default;
	foo(foo&&) noexcept = default;
	foo(foo const&) = delete;
};

int main() {
	typedef make_recursive_variant<
		foo,
		vector<recursive_variant_>
	>::type variant_type;

	vector<variant_type> value;
	value.emplace_back();
	variant_type other = std::move(value);
}

This errors at the move assignment because variant_type is not movable. The reason it is not movable is because boost::recursive_wrapper's move constructor is not marked as noexcept, which is a requirement for vector to enable move semantics.

Currently boost::recursive_wrapper's move constructor is not marked noexcept because it allocates a new T when being moved.

I propose that boost::recursive_wrapper be changed to store a shared_ptr<T> instead of T* so that, when moved, we move the shared_ptr, which is a noexcept operation. For the overload that constructs by T&&, we can leave as not specifying noexcept and construct with:

_p(new T(detail::variant::move(operand)))

The copy semantics of boost::recursive_wrapper can be left as-is.

[peterhuene@…]

Actually, upon more thought, there's no reason to use shared_ptr<T> here at all.

boost::recursive_wrapper(boost::recursive_wrapper&& operand) noexcept

This simply needs to take ownership of operand's _p so that operand doesn't delete it. No shared_ptr required (silly suggestion).

[Steven Watanabe]

I'm pretty sure that we discussed this implementation of move construction on the developer's list a couple of years ago, and decided that it was incorrect, because it introduces an additional moved-from state.

[peterhuene@…]

Is invoking T's move constructor when the wrapper is moved really more important than maintaining move semantics for vector<recursive_variant_> and, by extension, the entire variant type?

I get that the wrapper is supposed to be as transparent as possible to the user of the recursive variant (i.e. T's move constructor would normally be invoked on move), but, in my opinion, the intent of move semantics is still being adhered to if _p is moved instead of a new T allocated and then moved into.

To put it another way: I think forcing copy semantics for recursive variants with sequence containers (for the sake of complete wrapper transparency?) to be unreasonably burdensome for users expecting their variants to be efficiently movable.

[Antony Polukhin]

This issue has been discussed a lot. After a very long discussion it was decided to leave recursive_wrapper as is.

Personaly I dislike the existing approach, but agree that it must stay as is.

The only way that I could help you, is to show some possible workarounds. Here's a workaround for std::vectors that support incomplete/forward declared types (fastest possible solution):

#include <boost/variant.hpp>
#include <vector>

using namespace std;
using namespace boost;

struct foo
{
	foo() = default;
	foo(foo&&) noexcept = default;
	foo(foo const&) = delete;
};

struct variant_type: boost::variant<foo, vector<variant_type> > {
    typedef boost::variant<foo, vector<variant_type> > base_t;
    
    variant_type() = default;
    
    template <class T>
    variant_type(T&& v)
        : base_t(std::forward<T>(v))
    {}
    
};


int main() {
	vector<variant_type> value;
	value.emplace_back();
	variant_type other = std::move(value);
}

This example must work on popular STL implementations.