From: Douglas Gregor (gregod_at_[hidden])
Date: 2002-06-28 09:38:54
On Friday 28 June 2002 06:03 am, Itay Maman wrote:
> It seems that the variant issue is very popular these days ... :)
>
> The last couple of weeks I've been implementing a variant class
> based on the ideas (which were, in large part, contributed by Doug
> Gregor) discussed here in "Proposal --- a type safe union"
>
> The main advantage of this variant implementation is the flexibility of
> its visitor mechanism, which accepts 'standard' function objects. Having
> this flexibility at hand, the implementation of other key-features (e.g:
> recursive types) became fairly simple.
>
> The source code (with two short programgs) is available at
> http://groups.yahoo.com/group/boost/files/variant_b.tar.gz
Very nice. It's quite a bit shorter than I had expected. I have a few specific
comments on the interface and implementation, but overall I'm very happy with
it.
Interface comments:
1) get_ptr: first of all, is this meant to be part of the public interface? If
so, I think it might be more intuitive to use explicit specialization to get
the type instead of exposing 'Type_to_type' to the user, e.g., use
v.get_ptr<T>() instead of v.get_ptr(Type_to_type<T>()). The alternative
(which should probably exist anyway) is to have a variant_cast that acts like
any_cast.
2) The default constructor is dangerous - it says that the value has tid 0,
but doesn't set any value. I think the default constructor should
default-construct the first type (that way, it is impossible to have a
variant that has no value in it).
3) Is the 'assign' member function meant to be public? It's in the public
interface, but it does not handle variant-to-variant copies like the
assignment operator or constructors do.
4) The Variant class takes a typelist parameter. We discussed this a bit, and
there were three options:
a) Always take a typelist parameter (e.g., Variant<mpl::list<t1, t2, t3> >)
b) Always take types as separate parameters (e.g., Variant<t1, t2, t3>)
c) Allow syntax from both a & b
Here's my take on the relative merits of each interface (feel free to
correct, expand, annotate, etc. Whatever we choose we should document well).
a) Pros:
i) Typelist can be of unlimited length
ii) Easy to use variant when a metaprogram determines the list of types
Cons:
i) Requires users to learn typelists (even for simple uses)
b) Pros:
i) Simple syntax for simple uses
ii) Typelists are an implementation detail
Cons:
i) Number of types in a variant is limited
ii) Harder to use variant when a metaprogram determines the list of types
(need to be able to 'unpack' a typelist into a bunch of types passed to
variant)
iii) Error messages will include all those extra template parameters that
nobody cares about
c) Pros:
i) Everything from a and b :)
Cons:
i) Can we always tell what's a typelist and what isn't? Does something
like boost::tuple, which is both a typelist and a data structure, confuse the
variant class?
ii) Overloading might confuse users (though overloading to this degree is
generally understood by C++ programmers)
iii) Error messages will include all those extra template parameters that
nobody cares about
My proposed solution: First of all, I put a _lot_ of weight on the 'simple
syntax for simple uses' argument. I think users can live long, happy
programming lives without ever seeing a typelist. However, I know that there
are cases where typelists of unknown length will be used in variants, so at
some point we need to support that. For this reason, I think we should go
with 'c' and allow both syntaxes, but in a restricted manner. The template
signature would look something like this:
template<typename TypelistOrT1,
typename InvalidOrT2 = unused,
typename InvalidOrT3 = unused,
...
typename InvalidOrT4 = unused>
class variant;
So this says that a syntax like 'variant<mpl::list<int, float, double> >' is
okay, as is 'variant<int, float, double>', but an ambiguous instance like
'variant<tuple<int, float>, double>' is resolved by considering the tuple
argument as a normal type argument and not a typelist. Essentially, the
second template parameter (InvalidOrT2) is how we determine if the first
parameter is a typelist or a variant: if InvalidOrT2 is 'unused', then the
first template parameter must be a typelist and we ignore all other
parameters. If 'InvalidOrT2' is anything else, then the first template
parameter is a normal type, and we should construct the typelist from all of
the template parameters. This solves the first 'con' of option 'c', because
we don't have to know if a type is a typelist or a normal type, or both;
instead, we use context information. Granted, this makes something like
'variant<int>' illegal, but I don't see any harm in making useless,
inefficient code illegal.
Implementation comments:
1) Great Stack_Holder::swap :)
2) Why the const_cast in assign_variant_to_variant? I'd think it should keep
the source of the copy const, always, but non-const values are always used
for the copy?
3) In Assign_helper::set_item, the value passed in should probably be a passed
as a reference-to-const instead of a copy.
4) Is there any benefit to the use of copy/destroy function pointers instead
of a cascading 'if' built with a metarecursive function? The function
pointers impose an extra 8-byte overhead in the variant -- is this overhead
justified by an efficiency boost? The same question applies to the visitor
table.
Doug