Re: [boost] [variant] late "review"

From: Eric Friedman (ebf_at_[hidden])
Date: 2006-11-19 00:14:17

Hi Dave,

I assume most of your comments are intended to suggest places where I
could improve the documentation, but I will answer them directly in any
case. If you could suggest which of these points should make it back
into the documentation, I would appreciate your opinion.

David Abrahams wrote:
 > I'm just looking through the variant library docs in detail and had
 > the following commments:
 >
 > * 
http://www.boost.org/regression-logs/cs-win32_metacomm/doc/html/variant/tutorial.html
 >   shows static_visitor being used with no arguments and yet
 > 
http://www.boost.org/regression-logs/cs-win32_metacomm/doc/html/boost/static_visitor.html
 >   doesn't indicate any default for the first parameter.  Also, once
 >   you start using an argument in the tutorial, it comes with no
 >   explanation, which is confusing.

The default for the first parameter is void. Yes, I could probably
explain this in the tutorial.

 > * It's not obvious why we're deriving anything from static_visitor.
 >   It seems as though it shouldn't be necessary and that it might not
 >   be simplifying anything.

It provides the nested result_type, which is required for the
StaticVisitor concept.

More specifically, I had intended to develop a Boost.Visitor library
that would provide support for static and dynamic visitors as well as
other constructs such as type switches. There is actually some rather
old code in the sandbox:

http://boost-sandbox.cvs.sourceforge.net/boost-sandbox/boost-sandbox/boost/visitor/

http://boost-sandbox.cvs.sourceforge.net/boost-sandbox/boost-sandbox/boost/type_switch.hpp

 > * I find the dual nature of apply_visitor confusing.  With two 
arguments it
 >   is an imperative command: "apply this visitor."  With one argument
 >   it is a wrapper generator or something like that: "make this visitor
 >   into a function object that operates on variant<T0,...TN> instead
 >   of on T0, ... TN directly."  [Actually, I'm not sure the latter
 >   description is accurate and it wouldn't hurt to have it spelled out
 >   in the docs.]  I think it would be better to use two names rather
 >   than overloading one name in these two very different ways.

Would you prefer apply_visitor_delayed?

 > * I have a very clear mental model for what a non-recursive variant is
 >   doing, which gives me a clear of its costs and of where I'd use it:
 >   it's like a supercharged union.  From reading your docs it's very
 >   hard to form the same kind of mental model for a recursive variant.
 >   It would help to be explicit about what the library generates for
 >   you in the recursive case.
  >
 > * The make_recursive_variant example is vague.  It should describe
 >   what the result is equivalent to.  I presume it's something like:
 >
 >        variant<int, ...
 >
 >   ...uhh, interesting: I don't think you can express that example in
 >   terms of just variant and recursive_wrapper.  However, the text
 >   implies that make_recursive_variant is just a less-flexible way to
 >   achieve a subset of the same goals.

Suppose we could write a recursive variant like this

   RV = boost::variant< int, std::pair<RV,RV> >

Of course this is not syntactically possible in C++, but pretend for a
moment. The true problem, as the tutorial notes, is that this type would
require an infinite amount of storage to allocate statically.

Using boost::recursive_wrapper solves this problem by allocating its
contents dynamically. We could write

   RV = boost::variant<
      int,
      std::pair<
         boost::recursive_wrapper<RV>,
         boost::recursive_wrapper<RV>
         >
      >

Even if this syntax were possible, it would be inconvenient. The
short-hand alternative is to use make_recursive_variant:

   boost::make_recursive_variant<
      int,
      std::pair<
         boost::recursive_variant_,
         boost::recursive_variant_
         >
      >::type

Specifically, the pair's contents are dynamically-allocated.

 > * binary visitation:
 >
 >          "Notably this feature requires that binary visitors are
 >          incompatible with the visitor objects discussed in the
 >          tutorial above, as they must operate on two arguments."
 >
 >   I don't get it.  I can make an object whose function call operator
 >   overload set can operate on both 1 and 2 arguments.

Perhaps "incompatible" is not the best word choice. The intended point
is that a unary visitor is not usable as a binary visitor, but perhaps
it does not need to be stated.

 > * It seems like visitor_ptr is a very specialized name for something
 >   very general-purpose.  Doesn't bind(fp) (or some other boost
 >   construct) do the same thing?

Not that I know of. In particular, a visitor must be able to accept all
of the variant's bounded types. What boost::visitor_ptr does is to throw
boost::bad_visit in the event that it is visited with another type.

Eric