Re: [boost] [type_erasure] Review started (July 18-27, 2012)

Subject: Re: [boost] [type_erasure] Review started (July 18-27, 2012)
From: Steven Watanabe (watanabesj_at_[hidden])
Date: 2012-07-27 16:19:18

AMDG

On 07/27/2012 11:14 AM, Julien Nitard wrote:
> Hi all,
> 
> Please find my review of Type Erasure below.
> 
> 
>> Please state clearly whether you think this library should be accepted
>> as a Boost library.
> 
> Yes.
> 
> 
> 
>> 1. What is your evaluation of the design?
> 
> The library is easy to use, at least for the use cases I am interested in.
> Still it provides what I consider to be very very advanced features.
> 
> Though, I'd question the need for some those very advanced functionalities.
> Will many people use something has cumbersome as :
> 
> tuple<requirements, _a, _b> t(&array[0], 2);
> any<requirements, _a> x(get<0>(t) + get<1>(t));
> 

The main benefit for this is for type
erased function arguments:

typedef mpl::vector<
  random_access_iterator<_iter>,
  same_type<random_access_iterator<_iter>::value_type, _a>,
  less_than_comparable<_a>,
  move_constructible<_a>,
  move_assignable<_a>,
  callable<bool(const _a&, const _a&), _f>,
  copy_constructible<_f>
> sort_requirements;

void sort_impl(iter_type, iter_type, func_type);

// capture the arguments and forward to the
// separately compiled implementation
template<class Iter, class F>
void sort(Iter, Iter, F);

Requirements like this are very common
in generic programming.

> I can't really evaluate if this has a huge cost in terms of complexity
> in the code but I think the usages (i.e. the benefits) are likely to
> be limited even among library developers. (By the way can this be
> considered multi dispatch in C++ ?)
> 

It isn't really multiple dispatch.  The
argument types can't vary independently.

> For instance, if removing this feature or simplifying the construction
> would allow removing the possibility to construct a "reference any"
> from a "reference-to-const any" I'd choose to remove the bug.

This shouldn't be possible.  Do you have
a test case?

> Now, I
> may be not knowledgeable enough to judge this point precisely. I think
> I may limit the use cases of Type Erasure to "super interface" and
> that there is more to it.
> 
> 
> 
>> 2. What is your evaluation of the implementation?
> 
> The implementation is nearly not documented. There are very few
> comments in the private part of the code and there is no coding
> guidelines. This makes a review of the implementation very hard
> especially because a library like this is composed mostly of
> boilerplate TMP code. I wanted to figure out how the vtable was
> created and used, but I got discouraged. Adding some help for would be
> hackers could be interesting.
> 

The vtable isn't terribly complex.  It's essentially
a statically initialized fusion::map.

template<class T1, class T2>
struct vtable2 {
    typename T1::type t1 = T1::value;
    typename T2::type t2 = T2::value;
    typename T1::type lookup(T1*) const { return t1; }
    typename T2::type lookup(T2*) const { return t2; }
};

A lot of the library internals are similar in
that they're essentially simple components
with a lot of preprocessor baggage around them.
The only real algorithmic complexity is in
normalize.hpp.

> Overall, the library has been submitted while still "young" (I am not
> disregarding the many month of efforts that were likely put into
> this). Steven mentions that he has not tried optimizing it yet. This
> is a sound choice for a young library but I'd recommend it to be done
> (and reviewed) before the library is released. We want to avoid a
> "boost::function effect" : a different guy slapping the library for
> being to slow and having a better design every 6 month. I think no one
> so far reviewed the implementation so it won't hurt to have a second
> round.
> 
> Another thing, would be to work on error messages, because this being
> C++ they are horrible. I don't know if static asserts may help a lot
> easily or not, but if anything can be done, it'll be welcome.
> 

I can make sure that the library doesn't
produce deep template instantiation
backtraces in most cases.  There are
a few cases where a static assertion
would help too.

> 
> 
>> 3. What is your evaluation of the documentation?
> 
> This is clearly the weak point of the library. The library offers a
> new style of programming. One that deserves a proper step by step
> tutorial. One that deserves a few guidelines.
> 
> Right now it's more or less backward. Very advanced concepts like
> placeholder types jointly capture variables are introduced before
> showing how to make your own concept and what's a concept by the way.
> 

The "Functions with multiple arguments" appears
early because it introduces placeholders, which
are used in many sections.

Any order is going to be somewhat problematic,
because of the way all the core components of the
library are connected.  Whatever I introduce
first is going to depend on things that aren't
explained until later.

I think the first examples in the docs need
to be based on
- only predefined concepts
- no placeholders used explicitly
- capture by value

> There are simple usages of Type Erasure that should be made more accessible.
> I have been writing a sample tutorial that reflects how I would have
> liked to learn what Type Erasure is about. Please find it attached. It
> doesn't pretend to be complete or correct (it could use a serious
> amount of re reading) but it shows what I believe to be a much more
> natural progression.
> Please excuse me for the ugly shell in a .txt but work happened. I had
> no time for better.
> 
> Some other pieces of advice:
>  - I'd rename "concept map" to "concept adapter". There's no map for
> the user, it's your internal logic that is using one to find the
> correct adapter for a given concept and type.

The term "Concept map" is taken from the C++ concept proposals.

>  - I'd rewrite the "Concept Definitions" page. You define 3 kinds of
> high level concepts first, then explain what a primitive concept is.
> I'd keep the placeholder stuff for a different section.
> 

In this section, I'm trying to specify exactly
what makes a valid concept.  It's a bit hard
to do that without discussing placeholders.
Also, I'm more focussed on precision than
on readability here.  Informal descriptions
can go in earlier sections.

> The good point is that the reference documentation looks complete and helpful.
> 

In Christ,
Steven Watanabe