[review] Fusion review begins
The review of Joel de Guzman's Fusion library begins today, May 1, 2006, and continues through May 10, 2006. :Download: http://spirit.sourceforge.net/dl_more/fusion_v2.zip :Description: Fusion is a library of heterogenous containers and views and algorithms. A set of heterogenous containers (vector, list, set and map) is provided out of the box along with view classes that present various composable views over the data. The containers and views follow a common sequence concept with an underlying iterator concept that binds it all together, suitably making the algorithms fully generic over all sequence types. The architecture is somewhat modeled after MPL which in turn is modeled after STL. It is code-named "fusion" because the library is the "fusion" of compile time metaprogramming with runtime programming. Review questions ================ Please always explicitly state in your review, whether you think the library should be accepted into Boost. You might want to comment on the following questions: - What is your evaluation of the design? - What is your evaluation of the implementation? - What is your evaluation of the documentation? - What is your evaluation of the potential usefulness of the library? - Did you try to use the library? With what compiler? Did you have any problems? - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? - Are you knowledgeable about the problem domain? Cheers, Ron
On Mon, May 01, 2006 at 10:30:52AM -0400, Neal Becker <ndbecker2@gmail.com> wrote:
http://spirit.sourceforge.net/dl_more/fusion_v2/libs/fusion/doc/html/index.h... I guess these are current: "Last revised: April 28, 2006 at 08:48:02 GMT" Regards, Andreas Pokorny
Andreas Pokorny wrote:
Assuming these are the up-to-date docs. Is the example code in the Docs: Extension/Enabling Tag Dispatching correct? Shouldn't: namespace boost { namespace fusion { namespace traits { template<> struct tag_of<example_struct> { typedef example_sequence_tag type; }; template<> struct tag_of<example_sequence_struct const> //*** { typedef example_tag type; //*** }; }}} be replaced with: namespace boost { namespace fusion { namespace traits { template<> struct tag_of<example_struct> { typedef example_sequence_tag type; }; template<> struct tag_of<example_struct const> //*** { typedef example_sequence_tag type; //*** }; }}} Jeff
----- Original Message ---- From: Jeff Flinn <TriumphSprint2000@hotmail.com> To: boost@lists.boost.org Sent: Monday, 1 May, 2006 6:52:33 PM Subject: Re: [boost] [review] Fusion review begins Andreas Pokorny wrote:
The docs are up to date, this is a typo in the documentation, the example code itself shows the correct definition. Thanks for spotting this, I've corrected it for later versions. Cheers Dan _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Mon, May 01, 2006 at 09:49:52AM -0400, Ronald Garcia <garcia@cs.indiana.edu> 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.
- 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.
- 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;
- Are you knowledgeable about the problem domain?
Yes. In my opinion fusion-2 should be part of boost. Regards Andreas Pokorny
Andreas Pokorny wrote:
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.
Noted.
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.
Thank you very much and thank you for your review. Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
On 05/01/2006 08:49 AM, Ronald Garcia wrote:
The review of Joel de Guzman's Fusion library begins today, May 1, 2006, and continues through May 10, 2006.
In boost/fusion/tuple/tuple.hpp there's this: get(Tuple& tup) { typedef result_of::at_c<Tuple, N> at_meta; return extension::at_impl <typename traits::tag_of<Tuple>::type >::template apply<Tuple, mpl::int_<N> >::call(tup); } I don't see any use of the at_meta typedef. What's the purpose of this typedef? TIA.
This appears to be the result of a bit of cut and paste from the at and at_c implementations, which also have the redundant typedef. The get implementations should be calling at_c directly, rather than implementing more tag_dispatching to the at_impl type themselves. I've tidied things up, removing the redundant typedefs in both get and at, and modifying get to call at_c directly. Thanks for pointing this one out. Cheers Dan
On 05/03/2006 01:54 PM, dan marsden wrote:
Could you post the location of the revised code?
Thanks for pointing this one out.
Sure. There may be a similar cut and paste problem in /sequence/intrinsic/begin.hpp which has: template <typename Sequence> inline typename result_of::begin<Sequence>::type begin(Sequence& seq) { typedef result_of::begin<Sequence> begin_meta; return extension::begin_impl <typename traits::tag_of<Sequence>::type>:: template apply<Sequence>::call(seq); } Also, the docs in notes.html under "Tag Dispatching" at the 2nd item 3 has: 3. result_of::begin is the template which is specialized to provide an implementation for each tag type yet, in the above begin.hpp, it's not specialized, and a grep of all the .hpp file turned up no likely candidates; however, a grep for 'struct begin_impl' did show several specializations. Is the aforementioned correct?
Its under the spirit parser library cvs: http://tinyurl.com/s7qdw On the branch FUSION_V2
You seem to have hit a bit of a rich seam here! I suspect this may recur in some of the other intrinsics, I'll do a pass through and tidy this up. We may need to do a bit of work on these anyway based on the MPL style comments made by Dave A about calling the _impl functions.
Yes, that is another typo, the correct structure is metafunction XXX tag dispatching to implementation XXX_impl. Now fixed. Thanks again! Dan
On 05/04/2006 12:27 PM, dan marsden wrote:
Well I tried looking here: http://cvs.sourceforge.net/viewcvs.py/spirit/spirit/boost/fusion/tuple/?only... but that showed the last change to tuple.hpp was made by djowel 6 weeks ago. Is there some other place I should look? TIA. -regards Larry [snip]
Larry Evans wrote:
I think this is due to the delay between sourceforge developer CVS and the read only CVS. Hopefully this will sync up, but if not we may look at posting some revised code later in the week when we've incorporated the bulk of the feedback. Sorry if this is frustrating in the meantime. Thanks Dan
On 05/01/2006 08:49 AM, Ronald Garcia wrote:
The review of Joel de Guzman's Fusion library begins today, May 1, 2006, and continues through May 10, 2006.
In: doc/html/fusion/iterators/operators/operator_unary_star.htm I was confused by Return type: and Semantics: seeming to say the same thing. Shouldn't Return type: be: Equivalent to deref<I>::type. or simply: deref<I>::type ? NOTE: the use of I instead of i.
Larry Evans wrote: Hi Larry!
I agree. The equivalence can simply be a note somewhere. But I'll wait for Dan to concur, he wrote that part of the docs. Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
A very interesting library. I look forward to trying it out and will hopefully find enough time to review it in the coming days. For now just some quick feedback on a doc error that I haven't seen reported yet. The "tag_of" page's example seems to use is_sequence<> when it intends tag_of<>. Further, the first line in this example is missing a '>'. Also, I do have to agree with Dave A. in that the transition into the reference section seemed rather abrupt. Cheers, -Ryan
Ryan Gallagher <ryan.gallagher@gmail.com> writes:
Also, I do have to agree with Dave A. in that the transition into the reference section seemed rather abrupt.
Actually, my problem is that the doc seems to go in and out of "reference mode" freely and without warning. -- Dave Abrahams Boost Consulting www.boost-consulting.com
Ronald Garcia <garcia@cs.indiana.edu> writes:
- What is your evaluation of the design?
It's very good overall. I find the dispatching and function selection mechanisms cumbersome and inelegant, but I'm not sure if there's a better solution. Maybe since usually iterator intrinsics have to be implemented all together, it would be best to allow them to be defined in one big class template, rather than being forced to piecemeal create a bunch of little nested templates. I'm a bit surprised to see static call() functions used instead of operator() for the implementation of a function call, and also to see the use of nested apply<> metafunctions for computing the result types of functions. I thought Joel had decided to use something compatible with boost::result_of. I'm surprised and a little concerned about what I perceive to be redundancy in the value_of metafunction and the deref_impl metafunction class. In the extension example, I see repeatedly the same patter transferring the cv-ref qualification from one type to another. Can't that be made simpler for extenders? typedef typename mpl::if_< is_const<Sequence>, std::string const&, std::string&>::type type;
- What is your evaluation of the implementation?
What I've seen is excellent.
- What is your evaluation of the documentation?
A decent start, but needs more attention. Forgive me for being too pedantic (I know that I am): * The top-level heading "Support" is misleading at best. * "As with MPL and STL iterators describe positions, and provide..." ^ move this comma-----^ ^--- to here * "A Forward Iterator traverses a Sequence allowing movement" ^^ You seem to have double spaces------------^^ where commas should be. As with most English writing, the missing comma is much less common than the extraneous one in this doc. I'm just not going to point out all the extra ones. * How does an "expression requirement" differ from a "requirement?" * IMO there's no need for novel approaches to concept documentation here [unless you are going to start using ConceptGCC notation ;-)]. The expression semantics should be an additional column in the regular requirements table. * IMO the doc formatting needs to better reflect the structure. For example, at libs/fusion/doc/html/fusion/iterators/functions.html there's no indication that the "Functions" section I'm in is a subsection of "Iterators" rather than "Sequences" Maybe we should show Fusion > Iterators > Functions above the horizontal rule on that page, for example. * On that note, the section title "Adapted" under Sequences is unacceptably non-descriptive without context. * why do sequences have "intrinsics" while iterators only have "functions?" * IMO too many of the pages At libs/fusion/doc/html/fusion/extension.html: * Is all the information in this section available in * is "ftag" a typo? Was "tag" intended? If not, what is the "f" for? * "As it is straightforward to do, we are going to provide a random access iterator in our example." Doesn't providing random access imply providing a whole bunch of iterator comparison and offsetting functionality that you wouldn't otherwise have to? Why complicate the example? Oh, you have the iterator claim it's random access, but then you refer the reader to the example. Hm. * "Notice we need both const and non const partial specialization." There's no such thing; this is confusing. Reword * it's "metafunction," not "meta function" * "A few details need explaining here: 1. The iterator is parameterized by the type it is iterating over, and the index of the current element." 2. The typedef struct_type provides the type being iterated over, and the typedef index provides the index of the current element. These will become useful later in our implementation." Aren't these points redundant with one another? What details are being explained? Just the meaning of the template parameters? The intro set me up for something a lot less self-evident. ... 5. All that remains is a constructor that initializes a reference to the example_struct being iterated over. "All that remains" doesn't seem right here, and there's plenty in the example that bears explanation but that you've left out. Maybe you should just add comments numbering the interesting lines template<typename Struct, int Pos> // 1 struct example_struct_iterator : iterator_base<example_struct_iterator<Struct, Pos> > // 2 { BOOST_STATIC_ASSERT(Pos >=0 && Pos < 3); // 3 typedef Struct struct_type; // 4 typedef mpl::int_<Pos> index; // 5 typedef example_struct_iterator_tag ftag; // 6 typedef random_access_traversal_tag category; // 7 example_struct_iterator(Struct& str) // 8 : struct_(str) {} Struct& struct_; }; and then your numbered list can contain simple phrases like 8. The constructor stores a reference to the struct so that when the iterator is dereferenced it can return the appropriate element. * "So how does value_of_impl get used? Well value_of is implemented as follows:" IMO this answering-your-own-question style doesn't work. We tried it in the MPL book, and took it out. Just my 2c. "template <typename Iterator> struct value_of { typedef typename extension::value_of_impl<typename Iterator::ftag>:: template apply<Iterator>::type type; };" I think you should say "get used by the library." Why don't you show the idiomatic version for metafunction classes? template <typename Iterator> struct value_of : mpl::apply_wrap< extension::value_of_impl<typename Iterator::ftag> , Iterator > {}; or maybe better, template <typename Iterator> struct value_of : extension::value_of_impl<typename Iterator::ftag> ::template apply<Iterator> {}; * "The runtime functionality used by deref is provided by the call static function of the selected MPL Metafunction Class. The actual implementation of deref_impl is slightly more complex than that of value_of_impl. We wish to return references to the element pointed to by the iterator, but we need a little bit of metaprogramming to return const references if the underlying sequence is const. We also need to implement the call function, which returns a reference to the appropriate member of the underlying sequence." This bit is hard to read. It feels like the presentation is probably in the wrong order, there's redundancy, and too much extra verbiage. But I could be wrong :) * You start using fields::name and fields::age without saying what they are or where they come from. I object to this policy: Unless otherwise noted, components are in namespace boost::fusion. For the sake of simplicity, code in this quick start implies using directives for the fusion components we will be using. because it is utterly ambiguous whether some of these functions are supposed to be called without qualification and found via ADL or not. Without warning that policy is carried forward from the QuickStart to the reference sections (which, incidentally, should be grouped as subsections of a section called Reference so the transition is clear -- this doc seems to wander in and out of formal documentation mode) so that in libs/fusion/doc/html/fusion/sequences/concepts/forward_sequence.html it is obvious that result_of is found within namespace boost::fusion... but where is begin found in begin(s)? Also I think the cover page credits me for Doug Gregor's work :)
- What is your evaluation of the potential usefulness of the library?
Very useful.
- Did you try to use the library? With what compiler?
Yes, with several; I don't remember which exactly.
Did you have any problems?
No.
Very. I think this library should be accepted. However 1. the documentation should undergo an editorial review, to remove ambiguity, clarify the line between formal and informal, and with Strunk & White's mantra to "omit needless words" (and commas ;->) in mind. I only scratched the surface here. This is not necessarily a criticism of this particular library. All of our submissions need that. :) 2. I would like the authors to have a transition plan in place for replacement of the existing boost tuples. As long as the boost tuple library is there in its current form it will be the official tuple library of boost and I won't be able to use any fusion algorithms in my libraries because people will want to pass me boost::tuple. Making boost::tuple a valid fusion sequence would probably be sufficient. -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams <dave@boost-consulting.com> writes:
- What is your evaluation of the documentation?
A decent start, but needs more attention.
It would be really nice for reviewers if there were a website where the doc was posted (with your edits, if you like -- the one in the vault is stable) so that we can refer to it in our postings. -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams <dave@boost-consulting.com> writes:
I also note in fusion's tests, constructs like this: void test() { using namespace boost::fusion; using namespace boost; { // Testing deref, next, prior, begin, end char const* s = "Hello"; typedef FUSION_SEQUENCE<int, char, double, char const*> seq_type; seq_type v(1, 'x', 3.3, s); result_of::begin<seq_type>::type i(v); ^^^^^^^^^ which might (depending on whether ADL is intended) fail to test some important things in the library, and are certain to fail compilation once boost/utility/result_of.hpp gets included by a few more boost headers. -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams wrote:
Understood.
Right. Ok, noted. Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
Please always explicitly state in your review, whether you think the library should be accepted into Boost.
Abstain: this library is too big for the time I have, so I'll just post some documentation comments.
- What is your evaluation of the documentation?
The Quick Start is going nicely, but then stops suddenly with a "Tip of the Iceberg" excuse. The remainder of the documentation is reference documentation (i.e. the examples in all subsequent sections seem to be demonstrations of syntax rather than realistic usage examples). In particular a real-world example for lazy evaluation and some algorithms other than for_each, showing the traditional C++ code and the fusion way, to make it clear what advantage it brings. I'd like to see a page describing the inefficiencies: increased run-time (or none?) and increased compile-time. Are they basically the same as boost::tuple? http://www.boost.org/libs/tuple/doc/tuple_users_guide.html#performance (Pedantic documentation comments moved to bottom of this mail)
- What is your evaluation of the potential usefulness of the library?
I think the programmer-time-saving for writing load/save/output functions of classes that are a bunch of variables is useful, and also helps accuracy. BTW, can it also do operator== as a one-liner? On the downside the learning curve (not just this library but it assumes familiarity with Tuples and MPL) would probably stop me using it in any project where other programmers may have to maintain my code. At least until I see "Boost Fusion Made Easy" in the bookshops :-). Does this library supersede Boost Tuples, allowing the latter to be deprecated if Fusion is accepted into boost? If not, what are the remaining advantages of boost::tuples? Darren Pedantic Documentation Comments: * The first example in QuickStart should show the using namespace line. Or footnote 2 should be promoted from being a footnote to become the 2nd paragraph of this page. * The first paragraph of the introduction has basically the same first and last sentence. I think the last sentence should be dropped, and the first modified slightly to mention tuples. * In same paragraph, "A list<X> can only hold X instances" sounds ambiguous: does X mean a number of a type. So, here is my suggestion for that first paragraph (with a few minor syntax changes as well): An advantage of languages such as Python and Lisp/ Scheme, ML and Haskell, etc., over C++ is the ability to have heterogeneous containers (also known as tuple types) that can hold arbitrary element types. In C++ all the standard library containers can only hold a specific type. A vector<int> can only hold ints, a list<X> can only hold instances of X, and so on.
There has been quite a bit of feedback that better quickstart / motivation material is required. We expect to improve this for the final version of the documentation.
Yes, seq1 == seq2, seq1 < seq2 etc. all work as you would expect.
I believe that it is possible Fusion may supercede Boost Tuple. If that is the case, we will probably make Boost Tuple a conforming Fusion sequence in order to ease the transition to using the new library.
Thanks for the detailed documentation feedback. I've incorporated most of your suggested changes into the latest version of the documentation. Cheers Dan
Hi, Not a full review of fusion, but just some (slightly confused...apologies) thoughts. I started to look into fusion to try to understand what it is exactly. I have an idea what it might be about so I decided to look in the examples directory. Is there only one example? I am surprised. is that due to lack of time? There also doesnt seem to be much in way of examples in the documentation. Would it be possible to have a simple definition of what a view is, as it doesnt seem to be defined anywhere, or is it? How expensive is it to construct a view at runtime. Is it likely that the view is easy for the compiler to optimise (IOW so that the actual code references the original sequence). I am kind of interested in this library ( though unsure if it can help exactly with my problems), I have a matrix which is in fact a tuple of tuples and I'm wondering if fusion could help me in iterating over it. For example currently I am writing out each co_factor function explicitly ( of which there are 16) though I presume it would be possible to find the generic algorithm, so writing one function: // cofactor function of 3D homogeneous matrix // constrained to cofactor(0,0) using enable_if template<int R, int C> typename boost::enable_if_c< R == 0 && C ==0, value_type >::type cofactor()const { value_type t = ( this->at<1,1>() * ( this->at<2,2>() * this->at<3,3>() - this->at<2,3>() * this->at<3,2>()) - this->at<1,2>() * ( this->at<2,1>() * this->at<3,3>() - this->at<2,3>() * this->at<3,1>()) + this->at<1,3>() * ( this->at<2,1>() * this->at<3,2>() - this->at<2,2>() * this->at<3,1>()) ); return t; } There probably isnt enough detail here to provide a solution to this particular problem, but does it look like the sort of problem that fusion could help with?. regards Andy Little
I think there are a couple of points here: 1) Examples: In the docs I think there are quite a few examples, roughly one per algorithm in the algorithms section, similarly one per iterator function. I've not checked the whole of the sequences stuff, but that also seems to have a few examples. Was there a section in particular here where you felt we were lacking in examples? Or were you looking for larger / different types of example? As to code examples, yes one example covering the extension stuff is pretty minimal. The tests should provide a great deal of additional examples, although we agree more code specifically intended for examples would be better. We did not add more pre-review as we lacked inspiration for short but interesting examples that would prove instructive. We've had a few ideas since, and will aim to add at least a couple more examples to the final release. 2) Motiviation There has been quite a bit of feedback that we need to express the motivation for using the library in more detail. Again we've got some ideas in this area, helped by feedback from other reviewers, so we will produce improvements in this area for the final documentation.
There's a bit of detail in the views section, but I'll have a look at addressing the points you mention in more detail, as the views are an important concept for the lib.
IMO this is a good example of the type of code that would benefit from the features of fusion. You seem to have data of known type and size at compile time, which is the main prerequisite. If the code above operated on a sequence of fusion sequences, application of fusion::fold and fusion::transform looks like the right place to start replacing the iteration boilerplate with library code. Thanks for the feedback. Cheers Dan
* Documentation 1. A quick reference / one-page index is needed real badly! It takes far too many clicks to get from one component to the other. 2. Some examples are needed - illustration of common practice is a must for this library. 3. MPL (especially MPL-Lambda) interoperability must be outlined in more detail (or is the user expected to wire the result functions in terms of "typename ... >::type")? Sometimes the nested metafunction is called "apply" in the examples (an artefact?) and the function object with its "result" metafunction is said to refine an MPL metafunction class that uses "apply" instead. What?! The structure seems a bit over-categorized to me and sometimes even misleading (e.g. when looking for fusion::pair I have to navigate to "Support" where my first guess would be "Containers", "Sequence"->"Views" is another example). The concept of views, the impact of cv-qualification and by-reference/by-value passing should be outlined in more detail (or is it somewhere and I was unable to find it?). * Design 1. I'm missing an extension point in the generators to turn them into a type-preserving variadic input facility. Elaboration: Currently e.g. "make_vector" is an inline function. My suggestion is to turn it into a global constant of the type of a class template instantiation e.g. "vector_generator<F>". vector_generator<identity_function> const make_vector = ... This way it's easy to create user-defined function objects, that take a variable number of arguments, turn them into a tuple and finally use this information to create a user-defined object (that is, entirely without using Boost.PP). Of course that beast would be able to take a stateful functor in its ctor. 2. I'm also missing a fusion-equivalent to mpl::unpack_args (that would be about the inverse of 1). The header structure seems a bit over-categorized and counter intuitive -- maybe the categorization chosen makes sense from an implementor's perspective but it might be too much for the user (maybe improved docs can sort this out). * Implementation Fine. Maybe could use already-preprocessed files to speed up compilation times... * Potential usefulness A must-have for those who reprogram their compilers and indirectly useful for those who use libraries that do so. * Did you try to use the library? With what compiler? Did you have any problems? Yes. I used it with several compilers, such as GCC and different versions of VisualC++. Initially I had severe problems finding out which parts in the result computation require const qualification (had to read the source for reference), I still don't know the preferred way of using MPL-lambda for specifying the result (the source doesn't do it this way, I guess for compile time efficiency) and I had some problems following the "namespace result_of scheme" due to ambiguous template names (ADL, VisualC++ only). I'm still playing with it and hope I can add some details here, soon. * How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I'd say nearly an in-depth study, so far. * Are you knowledgeable about the problem domain? Yes. * Should the library be accepted as a Boost library? Yes, please! I hope the numbered paragraphs can be addressed beforehand, though.
Hi again, It would be very cool if fusion::pair would be compatible with mpl::pair, so that MPL's "first" and "second" metafunctions work... Maybe the names of the type members in "mpl::pair" should be changed from "first/second" to "first_type/second_type", because then 1. "std::pair" could be used with MPL and Fusion (*), 2. "fusion::pair" could be used with MPL, and 3. "mpl::pair" could be used as a Fusion pair, if no runtime data is required (*) in place of a fusion::pair and not as an "adapted sequence". Regards, Tobias
Tobias Schwinger wrote:
Here's my current thinking on this: 1) mpl::pair can be a conforming mpl sequence 2) fusion::pair can be a conforming fusion sequence 3) Hence, mpl::pair and fusion::pair can both be conforming mpl sequences and at the same time, also be conforming fusion sequences. Maybe: 1) fusion::map elements can be generalized to be any fusion sequence with at least 2 elements. 2) mpl::map elements can be generalized to be any mpl sequence with at least 2 elements. Conclusion: The type/type-value sequence is such a powerful concept. Almost any data structure can be defined in terms of sequence. If all our data structures are also conforming sequences, The C++ world would be such an exciting world! Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
// fusion::vector::at_impl requires specialization of mpl::int_ // (and doesn't work with other Integral Constants). #include <iostream> #include <boost/mpl/integral_c.hpp> #include <boost/mpl/int.hpp> #include <boost/fusion/sequence/container/vector.hpp> #include <boost/fusion/sequence/generation/make_vector.hpp> #include <boost/fusion/sequence/intrinsic/at.hpp> int main() { using namespace boost::fusion; namespace mpl = boost::mpl; std::cout << at< mpl::int_<0> >( make_vector("hello","bug")) << std::endl; // OK std::cout << at< mpl::integral_c<int,0> >( make_vector("hello","bug")) << std::endl; // ERROR return 0; } -- Regards, Tobias
Tobias Schwinger wrote:
2. I'm also missing a fusion-equivalent to mpl::unpack_args (that would be about the inverse of 1).
Actually there already is a fusion::unpack_args, alas it is undocumented. I was the one who implemented this feature and should have written the docs for it. My apologies to Joel and Dan and to the reviewers and users of the library. Anyway, you can find this in boost/fusion/sequence/utility/unpack_args.hpp and use it thusly, boost::fusion::unpack_args(f, seq); where f is a functor or function pointer and seq is a fusion Sequence. Either argument may be const qualified. The effect is that of invoking f with each element of seq as an argument. HTH, João
On 05/07/2006 02:19 PM, João Abecasis wrote:
The last statement might be interpreted to mean: typedef boost::fusion::vector<object, int> vector2; vector2 v2(o, 17); unpack_args(f,v2) == vector2(f(o),f(17)) instead of: unpack_args(f,v2) == f(o,17) where the above code is from that in: libs/fusion/test/sequence/unpack_args.cpp BTW, the purpose of unpack_args would be easier to see if the actual tests were more like: BOOST_TEST(f(o,17) == unpack_args( f , v2 )); instead of: BOOST_TEST(f.get() - 17 == unpack_args( f , v2 )); Then the reader would just have to look at the v2 declaration: vector2 v2(o, 17); to see the purpose, IMHO.
Larry Evans wrote:
How about: seq - a model of Forward Sequence f - a N-ary functor or function pointer, with N = result_of::size<Seq>::value Semantics: returns the result of invoking n-ary f, using the elements of seq as arguments. ?
I guess I didn't write that test thinking that it would get used as documentation... :-( It seems I should. I'll give it another try soonish. Thanks for your comments, João
Another one (hope my piecemeal reports are OK): There is no "result_of::make_pair" metafunction. It is needed because of the 'detail::as_fusion_element' type transformation in "fusion::make_pair". With this // missing result metafunction for fusion::make_pair template<typename First, typename Second> struct result_of_make_pair { typedef boost::fusion::pair<First, typename boost::fusion::detail::as_fusion_element<Second>::type> type; }; workaround in place fusion::pair gives a very useful context structure for folding... Regards, Tobias
Tobias Schwinger wrote:
Thanks! Duly noted. Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
Tobias Schwinger wrote:
Hmmm -- I had hoped for a comment on this one... Did my review read too negative? Was something unclear?
* Did you try to use the library? With what compiler? Did you have any problems?
Yes.
<snip>
I'm still playing with it and hope I can add some details here, soon.
I wanted to use Fusion to write something (well, at least half-way) real for review, so I chose to solve this problem: Call one of a heterogenous set of functors (their call operators might be templates) based on a runtime index with a complexity of O(log N) (basically what an n-ary tree structure of nested switch/case blocks does). The solution o handles unary functors only and doesn't bother with the result (simplicity), o contains numerous workarounds and is far away from being perfect, o requires an MPL-patch [ http://tinyurl.com/ljfdm] to compile, but o nevertheless documents my fight with the library, hopefully including lots of mistakes a new user is tempted to make, so it might work good to inspire upcomnig examples. http://tinyurl.com/l5bxl (source file in the vault) Fusion rocks! Regards, Tobias
Tobias Schwinger wrote:
I think we just missed it. Sorry. Yes, I think your proposal makes sense. That in fact is the very essence of Phoenix. Let me think about it some more to see how it would fit in the overall picture. At the very least, I like it. Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
Joel de Guzman wrote:
I recently noticed that (at least) the gmane web interface seems to have problems catching all messages -- maybe /some clients/ missed it...
Here's a naive "bind" in pseudo Fusion, just for the taste of it: gen_tuple<make_bound_func> bind; make_bound_func(tuple binding) { return gen_tuple< bound_func<tuple> >(bound_func(binding)); } bound_func<binding_tuple>[state: binding_tuple binding](tuple in) { return unpack_args( front(binding), transform(pop_front(binding), if_(is_ph(_), in[ph_index(_)], _ )) ); }
At the very least, I like it.
Thanks, Tobias
Tobias Schwinger wrote:
Tobias Schwinger wrote:
Very cool! That might be a good example for a tutorial if you are willing to document and share it. In fact, I need something like that in Spirit-2 (but I need the result and more args).
Fusion rocks!
Thanks! Kind words like that make all those long coding nights all worth while! :) Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
Joel de Guzman wrote:
Sure (that is, for the willingness-part). I'm not sure whether it qualifies for a good tutorial. There might be better ways to do things (I wanted to get my hands on the library quickly, so I didn't put too much time into planning, plus I still have some questions, below). Given that my implementation can not be radically simplified, it might also be too much code for a tutorial. Maybe a linear version for a tutorial and the logarithmic version for an inline-documented source code example would work better... BTW. implementation improvements, anyone? I'm all ear! So if we want to turn it into a source code example or tutorial, we should try to establish common practice. Therefore I'd like to ask you to comment on a few problems for which there might be a preferred/recommended solution: 1. How to set up the client namespaces most conveniently? Too many qualified names clutter my code. I actually like the scheme the library uses but I encountered following portability problems on the client side: namespace a_scope { using namespace boost::fusion; namespace result_of { using namespace boost::fusion::result_of; } namespace mpl = boost::mpl; struct a_copy_function; // omitted for transparency namespace result_of { template<typename Seq> struct noop_transform : transform<typename boost::add_const<Seq>::type, a_copy_function> // ^^^^^^^^^ vc 7.1 and gcc 3.4 choke here { }; } template<typename Seq> inline typename result_of::noop_transform<Seq>::type // ^^^^^^^^^ found ambiguous by some compilers // (IIRC it was gcc, couldn't reproduce the case, though) noop_transform(Seq const & seq) { return transform(seq, an_identity_function()); // ^^^^^^^^^ found ambiguous by vc 8 (visible + ADL) } } 2. Fusion and MPL placeholder expressions MPL placeholder expressions are very convenient for specifying the result because of their lazy nature (see lines 267, 312). Of course, this approach trades compile time for syntactic sugar. The syntactic sugar might not be entirely irrelevant, because it allows us to emphasize even complex problems with little code. So, I still wonder, whether it is a good idea to use MPL lambda for the result computation or not. Is it? If so (even occasionanlly), it would be necessary to have at least accessors for pair members (they are needed to keep the evaluation lazy, see line 211) or, ideally, if there would be a normalization between the STL and the MPL pair concepts (then MPL's accessors, 'first' and 'second', could be borrowed). (It would also be cool if one could just typedef the lambda expression to nested result or inherit from a wrapper. I don't know whether it's possible and a good idea - this part is just loud thinking)... 4. Choice of datastructures I used cons lists for the tree because I figured they might be limitless and because the structure is built one element at a time. There will be a lot of traversal later -- does it make a difference / would have 'vector' been a better choice? 5. BTW... I found it surprising that "iter->second" didn't work in place of "(*iter).second" (see lines 456, 485)... The other issues that require workarounds in the code have already been reported and acknowledged elsewhere in this branch of the thread.
In fact, I need something like that in Spirit-2 (but I need the result and more args).
In fact, it is Spirit-inspired [ spirit.devel, "self-adjusting alternative parser", bottom of the post ]. I used an unary functor with a void return for the example, but calling a function, adding another parameter and returning a bool value is nearly as trivial as its initial removal from the concept.
It must've been plenty, guessing from another 17218 lines of clean-looking & heavily templated code for the 'boost' directory... **** Another thundering applause **** Regards, Tobias
Tobias Schwinger wrote:
Right.
I was contemplating on something like the one you are doing. The inputs are: 1) A runtime value (a primitive integer; e.g. char,int) 2) A tuple of functions 3) A tuple of args (usually tiers) I was thinking of using the boost preprocessor instead to generate actual switch code. Something like: template <int Size> struct switch_dispatch; template <> struct switch_dispatch<1> { template <typename RT, typename Int , typename Functions, typename Args> static RT call(int n, Functions const& dispatch, Args const& args) { return at_c<0>(dispatch)(args); } }; template <> struct switch_dispatch<2> { template <typename RT, typename Int , typename Functions, typename Args> static RT call(int n, Functions const& dispatch, Args const& args) { if (n == 0) return at_c<0>(dispatch)(args); else return at_c<1>(dispatch)(args); } }; template <> struct switch_dispatch<3> { template <typename RT, typename Int , typename Functions, typename Args> static RT call(int n, Functions const& dispatch, Args const& args) { switch (n) { case 0: return at_c<0>(dispatch)(args); case 1: return at_c<1>(dispatch)(args); } } }; ++ A little change, we can use an mpl::vector_c to hold the cases. Something like: case mpl::at_c<cases, 0>::value : return at_c<0>(dispatch)(args); case mpl::at_c<cases, 1>::value : return at_c<1>(dispatch)(args); where "cases" is an mpl::vector_c. Such a utility will provide the backbone for 1) Phoenix2's switch_ statement 2) Spirit's switch_p 3) Spirit-2's predictive parsing schemes, and all those stuff that intends to optimize spirit's alternative. ++ Some more code to handle defaults; etc... see Phoenix2 switch and Spirit switch_p for reference. ... More comments on your post later. Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
Joel de Guzman wrote:
Tobias Schwinger wrote:
Joel de Guzman wrote:
<snip>
<snip code> Yeah, sure - that probably works better in practice, but I would've been a pretty bad one for learning/using Fusion ;-). In contrast to switch/case, however, the toy code I posted uses a decision tree. It depends on the optimizer in use whether switch/case ends up like that. Example: n=3 (2 bits are used for each level in the dispatch id) root seq. / | \ / | \ o o f /|\ /|\ f f f f f f Legend: o: node (no data, it's an external tree) f: function object Adding some more MPL code could make it a compile time huffman coder ;-). Not that I'd have any use for it...
It might be interesting to compare the full optimized disassembly and performance of: - switch/case - the Fusion-based tree dispatcher - linear Fusion iteration with if/else
... More comments on your post later.
Wow! These were a lot of posts... Thanks, Tobias
Tobias Schwinger wrote:
Joel de Guzman wrote:
Tobias Schwinger wrote:
This is tough. operator-> is required to be a member function. Unfortunately, that would also mean that the type dereferenced should also be known at compile time. This will ultimately break the MPL requirement that an iterator pointing at the *end* should not be dereferenced-- that's what will happen by the very act of computing the operator->'s return type. Nullary (non-templatable) member functions such as this and operator() with no args are such PITAs! If I have a wish for C++0x, one would be to allow them to be templated non members. In Phoenix, I had to go through hoops just to disable_if the nullary operator(). Eager evaluation wreaks havoc! Look: typename mpl::eval_if< typename Eval::no_nullary // avoid calling apply_actor when this is true , mpl::identity<detail::error_expecting_arguments> , apply_actor<eval_type, basic_environment<> > >::type operator()() const; Eval::no_nullary has to search the expression tree! A compile-time expensive operation! Unfortunately for Fusion iterators, I can't find a suitable workaround to enable operator-> return type deduction only when appropriate. In general, an iterator does not know if it is at the end. Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
Joel de Guzman <joel@boost-consulting.com> writes:
I don't buy that argument. The past-the-end-iterator has a different type from all the others, and since it's not dereferenceable anyway, it doesn't need to have an operator->. Furthermore,
That part seems strange to me. It certainly could know whether it's at the end, I think. -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams wrote:
Sure, all iterators have different types, but there's nothing special with past-the-end-iterator. For example, an iterator adapter treats all its held iterators equally.
Thinking about it some more, you may be right. Maybe I can use the disable_if technique. At the very least, I'll need a is_dereferencable<I> metafunction so I can also disable -> on adapted iterators. I'll see if I can come up with something. Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
Joel de Guzman wrote:
*hits hand against forehead* ...and cannot be a template because there are no arguments.
Those arbitrary and irregular restrictions on operator overloading are in general!
If I have a wish for C++0x, one would be to allow them to be templated non members.
Absolutely. I guess, this part of the current standard annoys both C++ programmers and implementors...
Interesting. Just curious: how would you mask "nullaries" whith a non-member template function?
There might be none ;-(...
In general, an iterator does not know if it is at the end.
Something different (that sentence just brought it back to my mind): I noticd that fusion::pair<end>(begin) works quite well as a "compound iterator" but has a pretty uncool interface. A wrapper around it could give a useful utility... 'in' is of type fusion::pair<end>(pos), '==>' denotes interface transformation provided by an imaginary wrapper *in.second ==> *in equal_to<typename In::first_type, typename In::second_type> ==> eoi<In> Since this thing knows when it's at the end, it could (BTW) even have an operator-> that disappears in this case... Regards, Tobias
Tobias Schwinger wrote:
Joel de Guzman wrote:
Tobias Schwinger wrote:
You don't. It just works because templates are delayed. If operator()() can be a non-member, I would write that as: template <typename Actor> actor_result<typename Actor::eval_type, basic_environment<> > operator()(Actor const& actor);
I'd refrain from commenting right now because I realized I might have been wrong. I think now that Dave is correct that we can detect an iterator pointing nowhere and simply return void_ or something like I did above for Phoenix. Thanks! -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
Joel de Guzman wrote:
Of course! Confused me.
OK. Then I wouldn't need an "end iterator" for a full iteration and could ask the *one* iterator whether it's at the end, right?
Yeah. But for my vote: I'd rather live without 'operator->' if it costs me more template instantiations per iteration frame. Further it's probably even more consistent to enforce explicit dereferentiation, so runtime code is kept in sync with result computation code (we'd have to use result_of::deref there, anyway).
Thanks!
My pleasure. Regards, Tobias
Tobias Schwinger wrote:
Joel de Guzman wrote:
Tobias Schwinger wrote:
To be precise, the issue is not really knowing we are at the end. In all cases, you really can't. For example, an iterator range's end iterator can not know it is the end. You have to compare it with the begin iterator to really know. The real issue is knowing if an iterator is dereferencable. And, for that, it is certainly possible to have an is_dereferencable<I> trait.
You have a point there. I'm guessing though that the impact is not that significant. After all, you'll, more often than not, dereference the iterator anyway. So, in effect, bringing op-> will mean eager evaluation at compile time of result_of::deref plus an eval_if plus is_dereferencable<I>. result_of::deref will be memoized when you actually do the deref. is_dereferencable<I> is cheap compared to phoenix's no_nullaries. Then again, all these wouldn't be needed if C++ allowed -> to be a non-member template!!! Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
Tobias Schwinger wrote:
It's really hard to tell. Only testing and benchmarking will reveal which to choose. I'm sure you noticed that from the recent Spirit-dev tests conducted. I too get mild surprises every now and then with the test results. Anyway, in general, vectors are way faster to traverse and have the smallest memory (struct) footprints. Yet, YMMV; and good for us, switching data types is easy :) I remember the old days when there is only one type: cons, then the ensuing debate as to which data structure is best suitable for a tuple, then I wrote a vectorized tuple (which became Fusion1), etc... Now, I do not want to go back!!! Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
David Abrahams wrote:
In terms of memory footprint? That would be true if we take advantage of empty base class optimization for, say, cons-lists. We are not doing that yet. I think we should. At the other extreme, if all elements are empty, we can just use mpl::vector or mpl::list. Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
On 05/15/2006 07:49 AM, David Abrahams wrote:
Isn't the list<T0,T1,T2,...> memory the same as that of: struct { T0 car; struct { T1 car; struct { T2 car; struct { ... } cdr; } cdr; } cdr; }; and the vector<T0,T1,T2,...> memory the same as that of: struct { T0 m0; T1 m1; T2 m2; ... } ? If so, what's the reason the memory footprints would be different? If not, could you explain what is the footprint, preferably using some struct counterpart like those above? TIA.
Larry Evans wrote:
Here's a sketch: template <class Head, class Tail> struct cons : compressed_pair<Head, Tail> { /*...*/ }; So, given empty elements such as: struct T0 {}; struct T1 {}; struct T2 {}; We'd get: struct cons { struct cons { struct cons { }; }; }; Also empty! See attached: size:1 Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net #include <boost/compressed_pair.hpp> #include <iostream> template <class Head, class Tail = nil> struct cons : boost::compressed_pair<Head, Tail> { }; struct T0 {}; struct T1 {}; struct T2 {}; struct nil {}; int main() { std::cout << "size:" << sizeof(cons<T0, cons<T1, cons<T0, nil> > >) << std::endl; return 0; }
Tobias Schwinger wrote:
Again, I'm not sure about the best use practice. On the client side, I would think that you will want to use Fusion, MPL, interspersed with STL. Those libraries (esecially MPL and Fusion) have lots of names in common. I am not sure if using directives/declarations will be practical at all. The least I can suggest is: namespace mpl = boost::mpl; namespace fusion = boost::fusion; Those are short prefixes already. Or maybe even: namespace fpl = boost::fusion; // fusion programming library. hah! :) namespace bfl = boost::fusion; // boost fusion library. hahah! :) Choose your pick. I kinda like the "fpl" synonym. It jives well with mpl and stl. Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
Joel de Guzman wrote:
<non working namespace setup>
You might have slightly misinterpreted my intention. I just tried to get rid of: - qualifiaction for 'result_of' in a namespace where result computation happens - qualification for 'fusion' in the "runtime code" namespace - qualification for 'mpl' in the "pure metaprogramming" namespace so we're talking about three different client namespaces -- not about stuffing three libraries into one. I was really surprised that it's indeed hard to find a convenient setup that works with more than one compiler...
namespace f = boost::fusion; // ;P
I kinda like the "fpl" synonym. It jives well with mpl and stl.
Yeah, for saying fpl::nil in the "result computation" namespace ;-). Seriously, too much qualified names are tedious to write and even more tedious to read... Regards, Tobias
Tobias Schwinger wrote:
Sure.
So, I still wonder, whether it is a good idea to use MPL lambda for the result computation or not. Is it?
I think so, yes.
If my suggestion to make mpl/fusion pair fully conforming mpl/fusion sequences is accepted, then you can use code like mpl::begin<_> or mpl::at<_, N>.
Hmmm... can you elaborate. I lost you here. Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
Joel de Guzman wrote:
It's a very cool suggestion, IMO (just for the protocol: seems 'mpl::begin' was supposed to read 'mpl::front'). But it seems we're still talking about different pairs of shoes (shoes of pairs?): Given the (actually) unspecified type members of 'mpl::pair' get "_type"-suffixes, any pair would just work with MPL *as a pair*. That is, the 'first' and 'second' metafunctions are trivial and (unlike all sequence-stuff) not tag-dispatched, so their instantiation is very lightweight and especially nice for within iteration...
OK. I believe it works best with some imaginary code: // rtc stands for runtime code // rmf stands for result metafunctions struct a_func : result< munch< _1, tweak<_2> > > { template<typename A, typename B> typename result<A,B>::type operator()(A const & a, B const & b) const { return rtc::munch(a,rtc::tweak(b)); } }; ------------- namespace rmf { typedef result< munch<_1,tweak<_2> > > a_func; } namespace rtc { struct a_func : rmf:: a_func { template<typename A, typename B> typename result<A,B>::type operator()(A const & a, B const & b) const { return munch(a,tweak(b)); } }; } Regards, Tobias
Tobias Schwinger wrote:
Understood.
[snip cool code] Interesting! at the very least :) More ideas to meditate on! Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
On 05/01/2006 08:49 AM, Ronald Garcia wrote:
The review of Joel de Guzman's Fusion library begins today, May 1,
In extension.htm, there's a section: Designing a suitable iterator which has code: template<typename Struct, int Pos> struct example_struct_iterator : iterator_base<example_struct_iterator<Struct, Pos> > { BOOST_STATIC_ASSERT(Pos >=0 && Pos < 3); typedef Struct struct_type; typedef mpl::int_<Pos> index; typedef example_struct_iterator_tag ftag; typedef random_access_traversal_tag category; example_struct_iterator(Struct& str) : struct_(str) {} Struct& struct_; }; However, there's no declaration of example_struct_iterator_tag. I assume it can just be declared (no need to define it) like the example_sequence_tag occuring just below the section titled: Enabling Tag Dispatching Actually, I think that's another typo, IOW instead of example_sequence_tag, it should be example_tag. But that makes me wonder if you didn't meand that example_struct_iterator_tag should also be example_tag. Also, since the above code is all for a specific structure, i.e. example_struct, why is there any need for including a Struct template parameter. Why not just: template<int Pos> struct example_struct_iterator : iterator_base<example_struct_iterator<example_struct, Pos> > { BOOST_STATIC_ASSERT(Pos >=0 && Pos < 3); typedef example_struct struct_type; ... }; ?
Larry Evans wrote:
There's going to be a couple of changes in this example. You are correct though that a forward declaration of example_struct_iterator_tag is needed, if only to make the docs clearer.
The correct structure should be that the tag_of the example_struct should be example_sequence_tag, and the tag_of the iterator should be example_struct_iterator_tag. (There should be 2 distinct tags). This should be tidied up and consistent in the next version of the docs.
The Struct parameter allows us to capture whether the iterator points into an example_struct or a const example_struct. Some of the rest of the example code needs to make this distinction. Thanks for the feedback Cheers Dan
On 05/06/2006 05:11 PM, dan marsden wrote:
Sure. And thanks for the fusion_v2 and the quick response to feedback. However, I've got another question. I removed the value_of_impl from my code since it looks like deref_impl defines what's needed and my code ran OK. Is value_of_impl really needed to define a fusion iterator for a new struct?
Larry Evans wrote:
No problem
value_of_impl is only used by the value_of metafunction, and is a requirement of the Fusion forward iterator concept. It is not needed in order for deref to work correctly. There is a distinction between deref and value_of: fusion::result_of::deref<It>::type - Returns the result of dereferencing It fusion::result_of::value_of<It>::type - Returns the element stored at It Now if an element of type T is stored at It then: fusion::result_of::deref<It>::type is T& fusion::result_of::value_of<It>::type is T If on the other hand an element of type T& is stored at It then: fusion::result_of::deref<It>::type is T& (as before) fusion::result_of::value_of<It>::type is T& So value_of allows us to distinguish members that are references. Hope that was what you were after. Cheers Dan
Ronald Garcia:
The review of Joel de Guzman's Fusion library begins today, May 1, 2006, and continues through May 10, 2006.
Hello! One note about fusion::tuple I/O manipulators: "The library defines three manipulators for changing the default behavior: Manipulators tuple_open(arg) Defines the character that is output before the first element. tuple_close(arg) Defines the character that is output after the last element. tuple_delimiter(arg) Defines the delimiter character between elements." "Example: std::cout << tuple_open('[') << tuple_close(']') << tuple_delimiter(", ") << a;" I see a usability issue here. I believe that if one needs to specify tuple_open then he would need to specify tuple_close too. I propose to change tuple manipulators to be: 1) tuple_bounds("(", ")") // if need to specify one bound, I need to specify both. 2) tuple_delimiter(",") // I'm happy with bounds, but delimiter should be specified. 3) tuple_punc("(", ",", ")") // I should specify all punctuation. furthermore, I believe that these manipulators are more general. They could be used with any sequence, not only with tuples. May be it should be factored out to separate library and named appropriately? 1) seq_bounds("(", ")") // if need to specify one bound, I need to specify both. 2) seq_delimiter(",") // I'm happy with bounds, but delimiter should be specified. 3) seq_punc("(", ",", ")") // I should specify all punctuation. seq stands for sequence. Best, Oleg Abrosimov.
My review of the proposed Boost.Fusion library follows. In general, I think the library will be useful, at least for library developers. My time for the review was very limited, and so I have not actually spent any significant amount of time using the library. Hopefully my feedback will be useful nonetheless.
- What is your evaluation of the design?
I appreciate the concern about efficiency and the consequent use of laziness in constructing views. But since the underlying sequence is held by reference, a view cannot outlive the underlying sequence. What is the solution if this is in fact desired? Is the solution to use as_vector or one of its kin on the resulting lazy view? There is no discussion or acknowledgment of this in the documentation as far as I saw. The extensibility feature of the library is potentially very useful. That several foreign libraries -- MPL, std::pair, etc. -- are supported as first-class Fusion sequences without modification to those libraries is compelling. From my brief glance at the implementation, the extensibility feature also seems to simplify the support of the several Fusion sequence types. However, it seems that there is significant overhead in taking advantage of Fusion's extensibility. For major existing components such as std::pair, this overhead can be taken care of once and included with Fusion. But for an end-user of the library the story is far more complicated. In particular, the amount of effort required to support even the trivial example_struct indicates to me that very few user types will ever have Fusion support in this manner. Dave Abrahams made a similar comment, but I think this more thought is needed here. If I had more time for this review, I would have done some of the thinking myself :) With regard to naming, I think fusion::pair is a bad name. Perhaps it should be instead named fusion::keyed? I think the name pair should be reserved for two values (std::pair) or two types (mpl::pair). The mixing of the two was unexpected. I find the metafunctions in the library not as straightforward as they should be. On the one hand, it seems nearly all of the metafunctions are the same in functionality and use, simply providing the result type of the various functions in the library. Then there is something like fusion::result_of::value_of that does not correspond to any function. Why? Of what function exactly is this the result? I think metafunctions like this should be in the plain fusion namespace. Aren't the arguments to the function object called by the fold algorithm "backwards?" That is, to follow the MPL convention, it would be (state, elem) not (elem, state). Maybe other functional languages do it differently? If so, I am not sure where it is better to have consistency, though I would lean towards consistency with MPL. I was going to suggest an "apply" algorithm, which would invoke a function with the elements of a sequence as arguments. From reading the newsgroup, I see this comes in the form of unpack_args. As noted elsewhere, this should be documented.
- What is your evaluation of the implementation?
I did not look at it extensively.
- What is your evaluation of the documentation?
There needs to be more discussion of where Fusion is appropriate or not appropriate. I see there has been discussion about this on the newsgroup recently. This sort of discussion should appear in the documentation as well. Thinking about how to explain this may also lead to a good simple example to demonstrate a case where the library would be useful. For instance, I initially wondered why there is not a filter algorithm that takes a runtime predicate. The answer, of course, is that this would not be possible, since the length of the resultant sequence would not be known at compile-time. Though this is obvious upon even the most basic reflection, it was not immediately obvious to me. Documentation could have helped here. There should be discussion of the approach Fusion takes with respect to laziness and holding the underlying views by reference. The performance claim made here is probably, though maybe not, obvious. More important, though, and also not discussed is how to make a deep copy of a view, in case it needs to outlive its underlying sequence. Not to unnecessarily belabor the point, but the documentation badly needs examples other than how to extend the library. I understand the problem of coming up with a simple (but also non-trivial) example is not easy. Even if the example is somewhat involved or complex, ddiscussing it in the documentation would be incredibly useful to people such as myself approaching the library for the first time. As for the audience of such a discussion, I think you can assume your readers are familiar with metaprogramming and other advanced C++ techniques. What I want is a discussion that shows me how Fusion can be so useful -- not merely leaves me with the sense that it probably is, kinda, maybe :) I think the documentation as a whole needs some reorganization. If nothing else, I think the documentation for the metafunctions should appear in a separate section from the regular functions. As it is, the metafunctions appear quite predictable, mostly for determining return types, and so the metafunction reference disrupts the flow of reading the documentation. On a related note, if there is in fact anything unusual or special about some of the metafunctions (apart from my comment above about value_of, etc.), I missed it in the monotony of their documentation. I also have a few minor complaints about typos, etc. in the documentation, which I note below.
- What is your evaluation of the potential usefulness of the library?
Since my time with the library was limited, I am left only with an impression. My impression is that it is potentially very useful in the development of libraries.
- Are you knowledgeable about the problem domain?
Somewhat. I used MPL quite a bit in the development of Boost.Variant, and I have used metaprogramming techniques in other projects, for serialization/deserialization of data to pre-established wire formats. Particularly in the latter project, I think Fusion's facilities would have been helpful. -- Minor documentation issues: - example for zip algorithm has syntax "make_vector((1,2),('a','b'))" -- not real, is it? - example for pop_back spells it "___pop_back___" for some reason
Eric Friedman wrote:
You're right. Deep copying is a job for as_vector and its friends. You are also right that this should be documented.
Well, to be fair, it is not more difficult than extending MPL. The trivial example is not so trivial after all. The example presents a full random-access and associative sequence. In the common case, a forward-sequence is all that's needed, and with it, you only need to supply a few intrinsics. But I get your point. We'll find a way to ease the burden of extension. I am confident that we can find something nice.
Agreed! I like the name "keyed".
I retrospect, all metafunctions originally resided in the namespace fusion::meta. Right now, there's a multitude of namespaces for different types of metafunctions in Fusion. There's result_of, there's traits, there's extension. Thinking back now, the original "meta" namespace might have been the right choice after all. It does not introduce a confusion as to what to use for what ("hey, what namespace is XXX in?").
Ok. Noted.
Noted.
Again, noted. I think Dan's reply is a good start towards that.
Indeed.
Understood.
I'm currently working on that, but I got sidetracked by some personal matters. It would also help if the community posed some possible *easy* uses. As far as I am concerned, my uses of Fusion have been for rather complex libraries like Spirit and Phoenix-- not your typical day to day code. I definitely agree that we need more "cook-book" style examples.
I'll let Dan comment on that.
- example for pop_back spells it "___pop_back___" for some reason
Noted. Thanks a lot for your review! Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
Joel de Guzman <joel@boost-consulting.com> writes:
I don't think that's the problem. I think Eric was saying the classification of that particular metafunction into namespace result_of didn't seem to be consistent with your other choices. -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams wrote:
Right. Yes, I know that. What I am saying is that the situation is a bit awkward. I am not sure if putting value_of in the main fusion namespace is good. It'll be like an orphan there (IMO). Having all metafunctions in "meta" as originally was, does not have that problem. In addition, it also does not have the problem of having to remember which metafunction resides in which namespace. I know how you dislike the namespace "meta", so I'll leave it at that. Maybe others have other names in mind or other suggestions worth considering. Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
Sorry if following occurs twice. I sent it 2 hrs. ago and haven't seen it yet. On 05/01/2006 08:49 AM, Ronald Garcia wrote:
The review of Joel de Guzman's Fusion library begins today, May 1, 2006, and continues through May 10, 2006.
The boost/fusion/iterator/equal_to.hpp has: operator==(Iter1 const&, Iter2 const&) { return result_of::equal_to<Iter1, Iter2>::value; } Obviously only the types are compared and not the values. Shouldn't the values be compared as well? I've got my own tuple implementation where I need to compare the types as well as values. To do this it looks like I have to specialize the above operator==. Was this intended, or is there some better way to do what I need?
Larry Evans
I believe that the position of a fusion iterator must be encoded in the information available at compile. This is needed for fusion sequences to serve as conforming MPL sequences (and probably various other things). I've not needed any runtime comparisons in any of the sequences I've implemented, although I must admit I found the definition of operator== surprising to start off with. Can you post some code or a summary of the design you are using? With regard to the ability of fusion sequences to act as MPL sequences, it looks like there has been a bit of an omission in the documentation on that front... #include <boost/fusion/sequence/intrinsic/mpl.hpp> may provide a starting point to get the general idea of the MPL stuff if you are interested, until we get chance to update the docs (assuming I'm not just failing to find the relevant section at the moment). Cheers Dan
On 05/10/2006 03:04 PM, dan marsden wrote:
OK, maybe I misunderstood. I thought that since operator* returns a value instead of a type (obviously), that two iterators would be equal only when they were the same length and each value returned by operator* was equal. Is that not the case?
Although it's not the latest code, the code here: http://cvs.sourceforge.net/viewcvs.py/boost-sandbox/boost-sandbox/boost/inde... contains the essentials. Briefly, the factor classes calculate the size of a buffer to hold all the elements in the tuple as well as their offsets. Now, the way I've tried to use fusion's extension mechanism is that the iterator_product<TypeIt>(void*rest) takes mpl::begin<TypeList>::type as TypeIt where TypeList is the TypeList used to form the tuple. In the above code, this TypeList is a compile-time function of TypeMap. Then each next_impl simply increments the rest arg to CTOR by sizeof(deref<TypeIt>::type) to locate the next element in the tuple. Obviously, two iterators formed from two different tuples formed from the same TypeList would always compare equal with the current definition of fusion::result_of::equal_to. WARNING: I haven't tested that assumption, it just seems that way from looking at the code. I believe that would be unexpected by most users.
On 05/10/2006 04:10 PM, David Abrahams wrote:
Nope, you're right. However, the reason I mentioned operator* was to emphasize that the operators, whether * or == applied to values, not types. The operator== applied to std::vector<I>::iterator and std::vector<I>::iterator would probably compare an I* with an I* and not use: is_same< std::vector<I>::iterator, std::vector<I>::iterator>::value I'll admit I shouldn't have mentioned anything about length; however, if two stl iterator's do compare equal then the operator* would return the same value for the same number of operator++'s, wouldn't they?
Larry Evans <cppljevans@cox-internet.com> writes:
Yes, in the STL they apply to values, but not in Fusion.
Yes, STL has iterators whose position can only be measured at runtime. Fusion has iterators whose position can be measured at compile-time. But -- not to put too fine a point on it -- so what?
Assuming the comparison was a well-defined operation in the first place, yes. If you're comparing iterators from different sequences, all bets are off. -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams wrote:
Larry Evans <cppljevans@cox-internet.com> writes:
Larry, in Fusion, the type of the iterator actually encodes its position. Unlike STL, where the *type* of c.begin() is the same as c.begin()++ or even c.end(), In Fusion, like in MPL, the iterators, from begin to end, all have different types. IOTW, Fusion iterators are polymorphic. It changes as you traverse the sequence. The type of begin(s) is not the same as next(begin(s)), for example. Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
On 05/10/2006 11:18 PM, David Abrahams wrote:
Thanks! That last statement convinces me. I remember vaguely reading about that somewhere in the standard. Dan's remark about "Comparing iterators from different sequences" being undefined was what first prompted my memory about that.
I believe 2 fusion iterators should compare equal iff they refer to the same position within the same sequence. Comparing iterators from different sequences should be documented to result in undefined behaviour. I'll tighten up the documentation in this area. Joel, am I correct in the above? Cheers Dan
Joel de Guzman wrote:
The issue here is (if it's not clear yet) that the terminal condition of all fusion algorithms should be known at compile time. IOTW, all fusion sequences should be traversable at compile time. If this is not the case, then (to list a few): 1) the sequences will not be traversable by MPL 2) we can't create views (e.g. iterator_range): views, like all sequences, have a fixed length that should be known at compile time. This constraint is also the very reason why Fusion algorithms are so tight and fast. The compilers can optimize the recursions into tight unrolled inline functions. Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
Larry Evans <cppljevans@cox-internet.com> writes:
There's no reason to compare the values. The only data you ever need in such an iterator is a reference to the tuple being traversed, and since it's illegal to compare iterators into different sequences, that test would always be true. -- Dave Abrahams Boost Consulting www.boost-consulting.com
Hi! This is my review of the Fusion library. Sorry for the late review... I should probably state my bias, I've known Fusion since it was bundled with Spirit and have wanted to see it in Boost ever since I started to grasp its usefulness (I'll admit it wasn't right away... docs were scarce back then ;-) ). I've commented on Fusion occasionally and have contributed patches and code.
Please always explicitly state in your review, whether you think the library should be accepted into Boost.
Yes, I think Fusion should be an official part of Boost. Fusion has been in development for a couple of years and is a mature library. It is unofficially part of Boost, it is available under the Spirit tree. Fusion is being used by a growing number of official and proposed Boost libraries. It is instrumental in the redesign of Spirit and Phoenix (in queue for review?) and also in the fork and generalization of Proto (out of Xpressive), as a general-purpose Expression Templates library. There's really no excuse for a library of this caliber to be relegated to an implementation detail. There are a few configuration macros whose default value is hardcoded to 10. They are, FUSION_MAX_UNPACK_ARG_SIZE FUSION_MAX_VECTOR_SIZE FUSION_MAX_SET_SIZE FUSION_MAX_LIST_SIZE FUSION_MAX_MAP_SIZE . I'd like to see a single macro to globally set the default value for all the others. Which also reminds me, all macros still need to be BOOST_ified.
- What is your evaluation of the design?
Fusion builds on top of the design of MPL and also of the STL so I don't have much to say in this regard. It is also a mature library that has been in development for some time. In the past I've noticed minor inconsistencies between MPL and Fusion, which I reported back (the lack of unpack_args was one of them). I think these two libraries, as much as possible, should be kept in sync. Apart from interface exposed to library users, another important aspect is the interface for extension. In Fusion-1, implementing a new iterator type required one to specialize not only Fusion intrinsics, but also mpl intrinsics. Things have improved since then, but I didn't look into this specifically for the review. I'll try to share any comments at another time. The algorithms also look straightforward to implement and the algorithms already in the library are a testament to that. On the directory structure used for the headers, I like the modular approach but sometimes get the feeling that it's too fragmented. Having more top-level reflections of individual components would ease my concerns. Another aspect is that this structure diverges from what is used in MPL. Given the parallel between the two libraries I think an effort should be made to keep the structure similar. In fact, this is also part of the user interface. Still, in the end, I recognize that they're two independent libraries and must each follow its path.
- What is your evaluation of the implementation?
The implementation is very readable and even simple to grasp, once one understands a few fundamental concepts. Although this aspect is not required of a Boost library, I think it is great when it can be achieved. Fusion is coded with the expectation that the library code will be optimized into very tight code, which seems to be a reasonable assumption with (any?) modern compilers. I would like to see more compile-time concept checks and "friendlier" error messages added to the implementation. Ideally, the user should never see errors beyond what s/he directly uses. Unfortunately, however, at places this will be at odds with the simplicity and readability of the code.
- What is your evaluation of the documentation?
I think I shouldn't be allowed to comment on this department... Anyway, I think the docs still need some work, but I generally like the direction. I'll try to offer Joel and Dan specific comments later. For the record, and while it is no excuse, it should be noted that Joel de Guzman is one of the lead developers of QuickBook (the other one being Eric Niebler), which makes it harder to write the docs... I mean, he can't simply complain about the tools. I'm sure Joel is taking notes and, in the end, QuickBook and the rest of us will benefit as well :-)
- What is your evaluation of the potential usefulness of the library?
I believe Fusion has great potential as a library building block but also as a more application-oriented library. On the library building block side, I believe it has the potential to become a fundamental/core piece of Boost. For application developers, Fusion takes tuples to a new level, allowing one to make the most of information that is available at compile-time.
- Did you try to use the library? With what compiler?
Yes. Mostly with gcc (3.4 and over), also with intel-9.0 (IIRC), both under linux.
Did you have any problems?
No. Joel and Dan have been very helpful and supportive for any doubts and issues I have had and given the readability of the code I have been known to provide fixes to typos and other minor issues.
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I suppose it qualifies as an in-depth study for the code and a quick reading for the docs. I've followed Fusion evolution since version 1 and have perused the code thoroughly. I implemented unpack_args for fusion-1 for my own purposes and later ported and contributed it to fusion-2. As a user of the library, I've privately used Fusion to re-implement and extend Spirit's closures, to allow members to selected and aggregated for return. Something like, closure<int, return_<float>, return_<double> >; , in the context of Spirit, would make a parser return a Fusion.Sequence of <float, double>. If also used Fusion in other pet projects.
- Are you knowledgeable about the problem domain?
I'm not sure I can categorize Fusion into a single and specific problem domain. I think the potential use cases are vast and the domain too broad. I can't claim to be knowledgeable in all the possibilities it allows. I'd like to congratulate the developers of the library on the great library and thank them for exploring the concepts and bringing it to us! I really expect to see Fusion as an official Boost library, soon ;-) Best regards, João
João Abecasis wrote:
Thanks, Joao!
Right. Ok.
Understood.
Yes, all fusion sequences automatically get MPL support out of the box without any additional coding. That's one of the advantages of Fusion2. I think this should be given emphasis in the docs.
Understood. I'll give this a lot of thinking. At the very least, we'll surely have a flat set of headers that forward to the main headers. Where to place them is a matter to deliberate on. Maybe <boost/fusion/hdr/xxx.hpp> ? or simply <boost/fusion/xxx.hpp> and push the main tree downwards in another directory, say, "main"?
I hope g++ will catch up! See Dan's post regarding some Fusion speed tests. g++ produces the fattest and slowest code :( I wish I was using incorrect flags or I haven't found the best compiler switches to use for g++. MS VC8.0 produces the tightest and fastest code, so far. (No, I'm not a fan of MS. Those are just the facts.)
Good points. I'll see how this can be addressed.
Sure. FWIW, a lot of QuickBook improvements recently can be attributed to Fusion documentation, in as much as QuickBook came into being when Eric needed to document Xpressive. And further down history, QuickBook had a former life as QuickDoc, which I wrote as a Spirit example and later, as a doc tool for Phoenix. Often times, one has to write tools to write tools to write tools to write tools...
Thank you very much! Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
Joel de Guzman wrote:
Why do you need to have them in separate directories? Are there conflicting/overlaping headers when you mix the approaches (or are you just trying to keep everything nice and tidy ;-) ) ?
It will catch up :-) On a side note, it seems the tests were run with gcc 3.4. IIRC, gcc 4 and beyond introduced a couple of interesting optimizations so I wonder how far do those go towards narrowing the gap.
MS VC8.0 produces the tightest and fastest code, so far. (No, I'm not a fan of MS. Those are just the facts.)
Of course ;-) I like facts! From my limited experience, I've also seen MSVC produce the tightest code. Perhaps there should be a way to extract some benchmarks (e.g., code size, performance) from the Boost regression tests, or separate benchmark-tests for the different compilers could be integrated into the system. This could be interesting and useful information for library developers.
I wanted to make an additional suggestion that might make Fusion's usefulness more visible, but it slipped my mind at the last minute. While I'm not that familiar with Boost.Serialization it seems to be a fairly popular library with developers of varying technical level. My suggestion is that Fusion Sequences be made serializable, using Boost.Serialization. This would go in a separate header (e.g., in fusion/sequence/io/serialization.hpp) and have the requirement that serialization be implemented for individual element types. If possible, the serialization would be generic, i.e., tied to the type sequence and not to the particular Fusion Sequence. With serialization in place, Fusion sequences could be used to manage a class's data and its serialization or they could be used as a proxy during serialization: archive & tie(a, b, c); Hmm... I see that the Quick Start mentions serialization as an example. Is there a reason for not offering the interface to Boost.Serialization in the library? Best regards, João
João Abecasis wrote:
Haha! Right. I think I am somewhat autistic in that regard-- I like them all lined up and structured perfectly. I like the thought that someone out there will really read and try to understand the code. For me, structure is part of the poetry ;)
I would love to see better numbers for g++.
Definitely!
Why not! That would be a good direction. Are you volunteering? Ahem ahem... duck... duck!!! :-) I also thought about an easy way to make an arbitrary struct a valid fusion associative sequence (perhaps through macros). I can't start to imagine what that would bring. Fusionized in-memory relational database support perhaps? Regards, -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
Joel de Guzman wrote:
Don't know if it helps, but GCC has some optimization hints: // no side effects, may work on memory passed in through pointers // and may read globals int __attribute__((__pure__)) func(int); // no side effects, may neither access memory passed in through // pointers nor globals int __attribute__((__const__)) func(int); // always inline int __attribute__((__always_inline__)) func(int); Regards, Tobias
participants (15)
-
Andreas Pokorny -
Andy Little -
dan marsden -
Darren Cook -
David Abrahams -
Eric Friedman -
Jeff Flinn -
Joel de Guzman -
João Abecasis -
Larry Evans -
Neal Becker -
Oleg Abrosimov -
Ronald Garcia -
Ryan Gallagher -
Tobias Schwinger