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

Subject: Re: [boost] [type_erasure] Review started (July 18-27, 2012)
From: Fabio Fracassi (f.fracassi_at_[hidden])
Date: 2012-07-29 15:28:35

On 7/18/12 7:13 AM, Lorenzo Caminiti wrote:
> *** The review of Steven Watanabe's proposed Boost.TypeErasure library

> Please state clearly whether you think this library should be accepted
> as a Boost library.

A clear Yes, here.

> Other questions you may want to consider:
> 1. What is your evaluation of the design?

Let me start by saying that this library is a very impressive piece of 
software. It opens up new possibilities in solving several problems in 
developing large scale software systems in a way I haven't seen before.

I have two concerns about the design:

a. downcasting.
It has been mentioned in the discussion, that this would be a useful 
feature. Since this feature is coupled with how the any type stores the 
concepts and cannot be implemented without breaking encapsulation, it 
needs to be part of the library, or the library has to provide stable 
hooks for it.
Steven has already bumped up the priority of this, so I am confident 
that this will be solved in some way. Since this feature needs quite a 
bit of user facing machinery a mini review for this might be needed.
I do not make the inclusion of this feature a condition on acceptance, 
because I trust Steven to include at some time.

b. creating custom concepts/concept interfaces.

Consider this paragraph as dreaming out loud. I like the way Steven 
solved the problem of defining concepts but I still like it to be 
easier, because I believe that how easy it is to define concepts is the 
key to widespread adoption.
Ideally it should not be more complex to define a concept than to define 
a (virtual) interface. My ideal solution would be able to do this:

struct int_container_concept {
//...
   void push_back(int) = 0;
   size_t size() = 0;                        // b.2
//...
};

void legacy_func(int_container_concept& c); // b.1

BOOST_CREATE_CONCEPT(int_container_concept);

void test()
   std::vector<int> v = /*filles somehow*/;
   any<int_container_concept> con = v;

   legacy_func(con);
}

I know that it is impossible to implement the BOOST_CREATE_CONCEPT macro 
in the above example without language support. So until we get compile 
time reflection in c++ we need to live with the additional boilerplate 
in some way.
In the "Design" section Steven explains that to scrap the boilerplate he 
departed from defining the user facing interface using abstract classes.
My concern with this departure is that it is quite different, and not 
directly compatible.
There are two things type erasures concepts cannot do:

b.1. I cannot use a concept without using any<>. (i.e. the legacy_func 
marked above would need to take an any<int_container_concept>& instead)
This might be a deal-breaker for some users of the library.

b.2. I cannot define a concept that has more than one function directly, 
i.e. I have to define concepts for push_back and size and then compose 
them (in this case 35 lines of code). This is of course mainly an issue 
of syntactic sugar, but in Stevens concept definition boiler plate and 
interface information (member name, arguments, composition lists) are 
intermixed.

I seem to recall that there was some discussion about having some macros 
to simplify the boilerplate. I think something along the lines of

BOOST_CREATE_CONCEPT(container_concept, typename T,
        (void (push_back, T),
          void (size))
)

wouldn't be too bad.

> 2. What is your evaluation of the implementation?

I only skimmed through the implementation, and found it surprisingly 
easy to understand for such a meta programming heavy library. Even so it 
could use a few comments.
The Implementation is clean, yet still unoptimized. I think this is 
reasonable at this point in time. I trust Steven to apply optimizations 
as time goes by. It would be nice to see the Small Object Optimization 
relatively soon.
The error messages are not to good, especially a "concept missmatch" 
spews out several rather unhelpful implementation details. (This is also 
already aknowledged by Steven.)

> 3. What is your evaluation of the documentation?

The documentation that is there is solid. As others have noted It needs 
more Motivation, Tutorials and Examples. I especially would like to 
second one point Eric Niebler made:

>>> In short, I'd put a handful of examples at the front of the users' guide
> that are relate-able and illustrative. Otherwise, you just have a
> listing of features and sample usages, with no motivation, no rationale,
> and no thread tying the features together.
>
> It might even help to describe what "type erasure" is, how it's been
> traditionally used, and what one of the simpler examples would look like
> without Boost.TypeErasure. That way, folks can get a feel for the
> drudge-work that your library saves them.

I think this (especially the second paragraph) is the key to making the 
docu more accessible.

> 4. What is your evaluation of the potential usefulness of the library?

Very useful, a potential game changer!
It opens the door to a compelling alternative to building systems with 
it rather than inheritance based polymorphism.

It also provides a compelling motivation for bringing "static 
reflection" to C++ because this feature would allow trivial definition 
of custom concepts and knowing which concepts a concrete class supports.

> 5. Did you try to use the library?  With what compiler?  Did you have
> any problems?

Build the examples with clang-3.1(Apple) and a build a small toy example 
myself to gain some insights. No problems there.

> 6. How much effort did you put into your evaluation?  A glance?  A
> quick reading?  In-depth study?

Deep reading of the documentation, superficial code reading, some 
experiments, lots of thinking. 2-3 days.

> 7. Are you knowledgeable about the problem domain?

Quite.

Best regards, and thanks to Steven for this great library.

Fabio