Re: [boost] [review] Fusion review begins

From: Eric Niebler (eric_at_[hidden])
Date: 2006-05-10 13:34:46

Joel de Guzman wrote:
> Eric Niebler wrote:
> 
>>>- What is your evaluation of the design?
>>
>>
>>Very nice. Joel and Dan have put a lot of thought into the design of 
>>Fusion. Lazy heterogeneous sequences are very powerful. As Joel and I 
>>have discussed extensively in recent weeks, support for segmented data 
>>structures would make traversal of some sorts of sequences much more 
>>efficient. Joel seems interested and committed to making these changes, 
>>and I'm happy to help.
> 
> 
> I think the segmentation idea is very good. I love the direction that
> this is leading to. Thank you very much for spear-heading the effort.

My pleasure. FYI - I now have a general implementation of 
fusion::segmented_iterator<>, so we can proceed with our simplified 
segmentation interface.

>>Like Dave, I'd like to see some more thought put into the extensibility 
>>mechanism. Defining new Fusion iterators is a bit painful, and something 
>>like a Fusion iterator adaptor library might be very useful. That might 
>>be possible if the metafunctions were all part of the same struct. I'd 
>>be satisfied with an analysis that breaks down the pros and cons of such 
>>an approach.
> 
> 
> As I mentioned in my reply to Eric Friedman, the extensibility mechanism
> is no more difficult than extending MPL. It just so happens that the
> extension section gave a not so trivial example (that of a random access
> and associative sequence). Anyway, I do agree that a better extension
> mechanism, if one can be found, would be a definite boon to using
> Fusion.

I am imagining an interface similar to Boost.Iterator's 
iterator_facade<>. That would make it possible to easily adapt the 
implementation from another fusion iterator. The Fusion filter_view and 
transform_view can be build on top just as Boost.Iterator's 
filter_iterator and transform_iterator are built on top of 
iterator_facade<>.

>>Some miscelaneous questions:
>>
>>- Why is there a traits::tag_of<> for extracting the sequence tag, but 
>>no corresponding metafunction for extracting the iterator's tag?
> 
> 
> traits::tag_of<> should be usable for iterators too. The docs should
> make that clear.

Nope. There's code like this, from fusion/iterator/next.hpp:

     namespace result_of
     {
         template <typename Iterator>
         struct next
         {
             typedef typename
                 extension::next_impl<typename Iterator::ftag>::
                     template apply<Iterator>::type
             type;
         };
     }

You're accessing the nested Iterator::ftag directly. You should be using 
the tag_of<> metafunction here, otherwise specializing it will have no 
effect.

>>- Why do I have to declare both my sequence's category *and* my 
>>iterator's category? Won't there always be an obvious relation between them?
> 
> Iterators do not have the exact same categories as sequences. For
> example, iterators do not have the associative category.

But you have an is_associative<> metafunction. Why do you need an 
associative sequence category? IMO, sequences shouldn't have catogories. 
Iterators should have categories, and sequences have iterators. Extra 
information, like whether the sequence is associative, should be 
provided by specializing external traits, like is_associative. 
Otherwise, you have duplication. (Eg., of course a forward sequence has 
forward iterators!)

>>- how is is_view<> used? The Extensibility section says I have to 
>>specialize it, but I don't think it says anywhere why.
> 
> 
> I'll discuss this in another post.
> 
> 
>>- Where is as_tuple? I need it!
> 
> 
> It resides in the future :)
> 
> 
>>>- What is your evaluation of the implementation?
>>
>>
>>Top notch. Clean and very granular headers. The directory structure 
>>could be a bit simpler, but that's a nit.
> 
> 
> Thanks! What directory structure do you have in mind?

Less nesting. IMO there is no reason for there to be iteration/, query/, 
and transformation/ sub-directories under algorithm. Just put all the 
algorithms in algorithm/. IMO, all the subdirectories under sequence/ 
are unnecessary, too. I've had to grep through the tree structure to 
find the header I'm looking for. That's crazy.

If you don't want to flatten the tree structure (seems we spend a lot of 
time talking about flattening trees ;-), I like the approach taken by 
Boost.PP. There is a tree structure, but also top-level headers that 
simply #include the correct header from the sub-directory. That way, if 
I want to use boost::fusion::begin() I can just #include 
<boost/fusion/begin.hpp> without needing to know that it actually lives 
at <boost/fusion/sequence/intrinsic/begin.hpp>.

-- 
Eric Niebler
Boost Consulting
www.boost-consulting.com