Re: [boost] [review] Fusion review begins

From: Joel de Guzman (joel_at_[hidden])
Date: 2006-05-02 02:38:59

Andreas Pokorny wrote:
> On Mon, May 01, 2006 at 09:49:52AM -0400, Ronald Garcia <garcia_at_[hidden]> wrote:
>> - What is your evaluation of the design?
> 
> Well done, it is both clean and extensible. 
> 
> I do like the separation of metafunctions yielding the result of an 
> operation in the dedicated result_of namespace and the runtime functions 
> in the plain fusion namespace. 
> 
> But I would like to emphasize the laziness of the algorithms. It allowed 
> me to store fusions pair types, and a self written compile time only ct_pair,
> which does not have an instance of the second(value) type, in the 
> associative map tuple container. Operating on the different pair types 
> does not cause any problems. Only trying to dereference an iterator 
> pointing on a ct_pair to access the value will cause compile errors.
> By applying a simple filter it is still possible to access all (runtime) 
> values of the tuple. 
> 
> In the end I was able to store information existing during run and compile time
> and compile time only information inside a fusion tuple. 

That's in my TODO list: formalize the "pair" concept. Right now,
the docs mention using fusion::pair, yet as you've mentioned,
fusion::map is not really hardwired to require it. It can use any
"pair" as long as it conforms to the concept (even std::pair[**]
or mpl::pair might do ok (AFAICT, but untested)).

[**] This might be useful because I am now aware of some cases where
you need to attach some runtime data to the keys.

>> - What is your evaluation of the implementation?
> 
> The code is clean - readable. I learned using fusion-2 by reading the formerly 
> sparse or non existing docs and the source code. It took me some time understanding 
> the dispatching system, but right after that everything turned out to be
> clear and obvious. 
> 
> I am using fusion from spirits cvs for several months now. In all that
> time I detected only one bug, which got fixed within hours. 
> 
> 
>> - What is your evaluation of the documentation?
> 
> The entry pages are fine to read, the reference is complete.
> But I think someone new to type sequences, could need a more detailed
> documentation. 

Noted.

>> - What is your evaluation of the potential usefulness of the library?
> 
> Very useful. In my opinion it is a must-have for boost. 
> 
> 
>> - Did you try to use the library? With what compiler?
>>   Did you have any problems?
> 
> I currently write a dsl framework using fusion, with a domain specific 
> rule system. Fusion maps are used to store node attributes in the expression
> tree. I used different versions of gcc to compile. 
> Apart from a gcc linker bug that shows up in a certain version of
> binutils, and with certain locale settings, I had no problems. 
> I wonder if there are compiler limitations in symbol name lengths.
> 
> 
>> - How much effort did you put into your evaluation? A glance? A
>>   quick reading? In-depth study?
> 
> Nearly an in depth study, since I learned fusion-2 by reading the source 
> code. When reading the documentation today I discovered several nice features 
> like the vector_tie, allowing something like 
>  int i; char c; double d; 
>  vector_tie(i, c, d) = make_vector(1,'a', 5.5);
> 
> which is nearly as cool as:
>  int i; char c; double d; 
>  i,c,d = 1, 'a', 5.5;

That part is taken from the Boost.Tuple docs, for which I should
give proper credit. Right now, there's a simple acknowledgment to
Jaakko, its author. I should mention too that he, and Jeremiah
Willcock were also involved in Fusion coding in its early days
when it started as a TR1 implementation of tuples.

>> - Are you knowledgeable about the problem domain?
> 
> Yes. 
> 
> In my opinion fusion-2 should be part of boost. 

Thank you very much and thank you for your review.

Regards,
-- 
Joel de Guzman
http://www.boost-consulting.com
http://spirit.sf.net