[boost] Variant/Any and the never-empty guarantee

From: Steven Watanabe (steven_at_[hidden])
Date: 2007-01-26 12:27:10

AMDG

Mathias Gaunard wrote:
> I am currently writing an object container similar to boost.any but with 
> an allocator template parameter. That allocator is of a custom type that 
> allows efficient stack allocation like boost.variant.
>
> While looking at variant's documentation, I became aware of the problem 
> with operator= and the never-empty guarantee.
> http://boost.org/doc/html/variant/design.html#variant.design.never-empty
>
> However, I think none of the solutions is really satisfying, especially 
> in my case where the allocator is generic and provided by the user and 
> where the types that are to be put in the container aren't known until 
> runtime.
> Lots of versions would need to be made, indicating whether one or 
> another option is nothrow for the object or the allocator, providing 
> fallback allocators etc.
>
> Do you think it is reasonable to simply require a nothrow move 
> constructor for the types which are to be put into my container?
> I cannot think of a case where move constructors would really want to 
> throw, and swap functions which are similar are usually no-throw.
>   
I don't think it would be a good idea to require this
True, move functions should not throw, but they
might not even exist. You could define a trait
template<class T> struct has_nothrow_move;
defaulting to boost::has_nothrow_copy<T>
and create the object on the heap when this returns false.

> There is also the 'false hopes' solution, which is said to not be 
> allowed by the standard.
> But realistically, that solution shouldn't cause any problem, since I 
> always see moving as a simple bitwise copy ; unless the object 
> references itself, which would be solved by moving it back anyway.
>   
As mentioned, that solution causes can cause an unresolvable race condition.
For example this class could not be used in a variant then.

class C {
public:
    explicit C(int i = 0) : n(i) {
       register_object(this);
    }
    ~C() {
       unregister_object(this);
    }
    static void increment_all() {
       std::for_each(objects.begin(), objects.end(), ++*_1);
    }
    C& operator++() {
       boost::mutex::scoped_lock l(m2);
       ++n;
       return(*this);
    }
private:
    static std::set<C*> objects;
    static boost::mutex m1;
    void register_object(C* c) {
       boost::mutex::scoped_lock l(m1);
       objects.insert(c);
    }
    void unregister_object(C* c) {
       boost::mutex::scoped_lock l(m1);
       objects.erase(c);
    }
    boost::mutex m2;
    int n;
};

Even single-threaded this can cause problems if the old object
stores a pointer to itself somewhere and removes it from the
destructor.  This happens if the constructor of the new object.
uses the pointer to the old object if the old object still exists.
Continuing the example above

class UsesC {
public:
    UsesC() : i(0) {
    }
    UsesC(const UsesC& other) : i(other.i) {
        C::increment_all();
    }
private:
    int i;
};
C c;
boost::variant<C, UsesC> v(c);
v = UsesC();
// v.get<UsesC>().i == 1 !!!

The call to increment_all clobbers the
value that was just constructed.

> I welcome any critics, ideas and advices about moving, exceptions, and 
> bitwise copy.

In Christ,
Steven Watanabe