On Sun, Sep 21, 2008 at 7:58 PM, Hartmut Kaiser <hartmut.kaiser@gmail.com> wrote:
Hi all,
The review of Joel de Guzmans and Dan Marsdens Phoenix V2 library starts today, September 21st 2008, and will end on September 30th. I really hope to see your vote and your participation in the discussions on the Boost mailing lists!
---------------------------------------------------
Of course, it's hard to be subjective reviewing Phoenix, since I'm already a fan, but I'm going to try to approach this as I would any library and pretend like I don't know who the authors are. My review of Phoenix follows. I think Phoenix version 2 should be rejected as a Boost library. Instead, we should focus on version 3, i.e. Phoenix3/Lambda2. Obviously, there's nothing "wrong" with the Phoenix2 design, implementation, documentation, etc. However, I believe Boost users don't appreciably gain anything from a release of version 2 as a top-level Boost library, and I believe a premature release could actually come at a cost to Boost users as well as Boost developers. Accepting Phoenix2 as a Boost library is essentially just a rebranding of the current library included with Boost.Spirit. I don't see how users benefit from that rebranding. Potentially, the rebranding could deteriorate user experience, since Phoenix2 addresses the same problem domain as other Boost libraries and uses similar names (bind, function, _1) but is not compatible or interoperable with its cousins. Worse, it also overlaps/conflicts with the draft C++ standard library, which many users already have access to from their compiler vendors. This confusion is unnecessary and can be avoided by releasing Phoenix3 (which presumably will play nicely with the standard library) as an upgrade to Boost.Lambda. I'll briefly outline some specific technical concerns. I believe lack of support for the TR1 result_of protocol is a showstopper. Introducing yet another return type deduction protocol only makes users' lives more difficult. As has been suggested the old protocol could be removed from version 2 in favor of result_of, but in Phoenix3 this issue is already resolved. So, why not spend time finishing Phoenix3 rather than bringing version 2 into the post-TR1 world? In a similar vein, I think if a library uses placeholders, lazy evaluation, and other bind-like features, then it's important for the library to reconcile its relationship with the forthcoming std::is_placeholder, std::is_bind_expression, std::bind, etc. Why not expend effort getting it right in Phoenix3 rather than modernizing version 2? Giovanni noted version 2's lack of conformance to the standard algorithm concept requirements, namely Assignability. Conformance to the usual FunctionObject concepts also seems like something that should be required of newly accepted Boost FP libraries. Why not spend time insuring that Phoenix3 is conforming rather than retro-fitting version 2?
- What is your evaluation of the design?
If we were evaluating the library eight years ago, I would have no complaints. But we're evaluating it today, so I have to reiterate that lack of interoperability with the draft standard's FP features is an unacceptable design for a contemporary Boost library.
- What is your evaluation of the implementation?
I haven't really looked at the implementation. I'm kind of reluctant to invest too much time reviewing the current version when I know the Proto port is coming, and that seems to be the actual, valuable, long-term implementation. The code under consideration is known/intended to be transitory; what I'm actually looking forward to is the replacement! I mean we can already use Phoenix2 by including boost/spirit/phoenix.hpp. Moving the code to boost/phoenix isn't really a reason to get excited.
- What is your evaluation of the documentation?
The documentation is excellent. I mostly looked at the "starter kit," which is as good of a quick start guide as any I've seen. However, I seem to remember that the old documentation had an excellent introductory chapter that gave a brief overview of FP and touched on the history of Phoenix. I didn't notice that material in the current documentation. If it was removed, it should be brought back!
- What is your evaluation of the potential usefulness of the library?
Phoenix2 is certainly as useful as Boost.Lambda. However, there is some overhead (in man-months, not CPU cycles) in migrating existing Boost.Lambda code to Phoenix2. I suspect that many users will not find a compelling reason to incur that expense. Also, Phoenix2 makes no guarantees as to Boost.Lambda interoperability; i.e. to a new user, version 2 looks and feels like a completely different library with new documentation to read, new corner cases to discover, etc. So, I suspect that many users will not find a compelling reason to "learn" Phoenix2 for new projects when Boost.Lambda suffices. So, why not spend time making the "upgrade" from Boost.Lambda to Lambda2 (a.k.a. Phoenix3) intuitive and painless rather than making Phoenix2 more appealing/useful than Boost.Lambda?
- Did you try to use the library? With what compiler? Did you have any problems?
Yes, I've used Phoenix with several releases of gcc in the past. For the review, I used gcc 4.0. I've never had any problems.
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I've been following the review discussion. I've played around with Phoenix at various times, though I'm not sure that you would call my evaluation "in depth."
- Are you knowledgeable about the problem domain?
I have some experience using FP in C++. Finally, I believe that all the comments and discussion during this review period will strengthen Phoenix3 in the long run. And regardless of the final review decision, Phoenix2 will certainly be remembered in history as a widely admired library and a major milestone in the evolution of FP library techniques in C++. Daniel Walker