[boost] Re: Variant (aka Discriminated Union) implementation

From: Itay Maman (itay_maman_at_[hidden])
Date: 2002-07-01 16:02:16

Just a few comments following a brief look at the code:

The code is still not exception-safe. Let's assume that the placement 
new() operator (variant.hpp line 308) throws. The exception is caught 
and you restore the old content of 'dest'. However, by that time 
dest.~variant() has already been invoked. The std::memcpy() cannot undo 
all possible side effects of dest.~variant().

I believe that "return *this = operand;" (line 366) should have been: 
"return *this = temp;". Anyway, the swap() member function is not safe, 
either way, since the second assignment might throw, which - again - 
puts you in trouble, since you have to undo the effects of the first 
assignment.

Visitor_adapter stores a copy of visitor. By doing so you make it pretty 
difficult for a visitor to pass back to the calling site information 
which was gathered during the visit. This makes the visitor not as 
useful as it might have been.

Anyway, I think that most users will skip the visit_with() free function 
and will use the variant::visit_with() member function, simply because 
there is no substantial reason not to use the member function. Moreover, 
I think that variant::operator() is a more natural method for applying a 
visitor to a variant.

Trying to be as objective as possible, I think that your variant's core 
behavioral aspects/implementation is not as good as the ones offered by 
   my implementation. On the other hand, its high-end interface is more 
complete (e.g: the type_switch). Additionaly, I think that we will do 
the boost community a great service if we combine them together. At the 
current state, boost members have to double reviews each new draft, 
since the same ideas pop-up at both implementations. This, effectively, 
slows down the development.

Anyway, I think we should talk about it privately. My e-mail address is: 
itay_maman_at_[hidden]

-Itay