Re: [boost] [Review] Phoenix review starts today, September 21st

Subject: Re: [boost] [Review] Phoenix review starts today, September 21st
From: Joel de Guzman (joel_at_[hidden])
Date: 2008-09-26 23:44:41

Daniel Walker wrote:

 > 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.

Thank you for your review.

 > I think Phoenix version 2 should be rejected as a Boost library.
 > Instead, we should focus on version 3, i.e. Phoenix3/Lambda2.

I understand your concerns.

Will you change your vote if I say that Phoenix2 will not be
repackaged into Boost? The real intent is to have Phoenix3/Lambda2
into Boost, not Phoenix2. I asked for a review to flesh out
the few remaining issues and to achieve the goal of having a
top-notch library. All the major issues raised thus far are
interface related. These can be implemented in a short period
of time in Phoenix. They are not major issues in terms of
implementation. Phoenix3 is the proof. Fusion is also a proof.
Fusion started out without Typeof and Result_of support. Soon
thereafter, these were added.

Perhaps it was premature to ask for a review, but if you look
closely into the Proto port, Phoenix3 *is* Phoenix2 targetting a
different back-end (proto, instead of hand-coded). It's not a
total rewrite, like say, Spirit2 from Spirit1 or Phoenix2 from
Phoenix1.

I was confident that, since Phoenix3 is essentially Phoenix2+proto,
that it's good enough to ask for a review pending the finalization
of Phoenix3. I also thought that it was a good idea to have the
review for Phoenix2 for those who have already invested in the
library in order to realize a good transition path from the old
to the new.

And, not to be forgotten, there's also the Lambda Phoenix
relationship. I personally would not want to have Phoenix2
as-is into Boost without providing a strategy for a painless
transition for old Lambda into the new. The review is a good
opportunity for all these matters to be discussed.

 > 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.

There won't be a premature release. Promise. I thought that was
clear from the roadmap I sketched. In as much as many (most?)
libraries in Boost get conditional aceptance (they get accepted
if certain conditions are met), I was hoping it would be the
same for Phoenix. I haven't seen a library getting accepted as-is
into Boost. Typically, a post review version of a library --
one that addresses the concerns raisied in the review, is
crafted before merging into the trunk.

 > Accepting Phoenix2 as a Boost library is essentially just a rebranding
 > of the current library included with Boost.Spirit. I don't see how

I agree!

 > 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.

IMO, Phoenix3(alpha) is not enough, even. So far, some issues like
the upgrade to Boost.Lambda you mentioned hasn't been addressed yet.
Allow me conditional acceptance and all the issues raised will
be addressed in Phoenix3.

 > 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?

Exactly. No more effort will be put into Phoenix2. Phoenix3 is the
future. Let Phoenix2 stand on its merits in terms of design/implementation/
documentation. Phoenix3 is Phoenix2 plus all of the above and
will be the post-review implementation.

 > 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?

Yes. That's the idea. To put matters into perspective, the features
you want is just a fraction of the totality of the whole Phoenix
library. It's a promise that all those features you want will be
put in place. But, please don't overlook the things that are already
in place.

 > 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?

Agreed.

 >> - 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.

Again, agreed.

 >> - 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.

Phoenix3 is essentially Phoenix2 plus proto and some of the
features that brings it into the post TR1 world. If it is the
implementation you want to evaluate, you can look into either
renditions.

 >> - 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!

Hmm. Some people say I'm overly verbose and chatty. I recently
tend to snip portions of the documentation because of that.
I'll see what I can do. Thanks for the compliment.

 >> - 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?

Yes. That is actually the plan. Obviously, this review will
make things a lot clearer. I would definitely want to hear
from you what you want to see/expect. All those wishes/wants
will be assimilated into Phoenix3/Lambda2.

 >> - 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++.

Thank you. I'm still hoping that my explanations will make you
change your vote to conditional acceptance. I believe that we
actually have an edge over other Boost libraries that were
conditional accepted because we actually have a proof of the
future that addresses the majority of the issues raised.
Nevertheless, your review will definitely help in improving
the library.

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