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