[boost] [Phoenix] review

Subject: [boost] [Phoenix] review
From: Giovanni Piero Deretta (gpderetta_at_[hidden])
Date: 2008-09-30 11:10:00

- Please always state in your review, whether you think the library should be
  accepted as a Boost library!

Having to review a library that is already expected to change is hard,
OTOH functional programming in c++ need to go beyond boost.lambda
(which has been basically in maintenance mode for a long time), and I
think that Phoenix is the way to the future. Thus I think that Phoenix
should be accepted.

I support having an extra mini review after the implementation has
been updated, but my vote is not under this condition.

I think that the review plan proposed by Eric is great.

- What is your evaluation of the documentation?

I think that documentation of Phoenix is probably the most important
part of the library and I consider it of high quality: I've been an
happy lambda user for a couple of years. I went beyond its documented
interface and started to play with its internal. I've been able to add
lazy functions to lambda and other nice additions. But I would never
even have attempted that if I hadn't been exposed to the Phoenix
documentation.

I've never really used the phoenix library, but I've read its
documentation many times (starting maybe 4 years ago, I think it was
the old V1 documentation). I remember that the first time I read it I
understood basically nothing. But it got me interested in functional
programming. Every time I read it I learned a bit more until something
clicked. After that everything seemed so obvious. Thanks Joel, you
exposed me to a new world.

- What is your evaluation of the design?

Very good. I'm a big fan of lazy functions (I try to make all my
polymorphic functions object lazy, and leave bind only for good old
function pointers), and I hope that Joel will make them 'optionally
lazy'. But even if he didn't, thanks to phoenix extensibility, it
wouldn't be hard to add it on top of the current design.

Again, I vote for always requiring lambda[] around every lambda
expression, as this could really help to solve many problems, like
assignability and operator& problems. It will also open up the road
for further improvements, like overriding capture defaults or
selecting perfect forwarding.

About perfect forwarding: it would be great if phoenix allowed it by
default at least on compilers that support rvalue references.

IMHO result_of support is necessary for a library released today :).

I cannot really comment on the details of extension interface as it
seems that it is bound to change, but I think it is, together with
lazy functions, the bigger improvement to boost.lambda.

- What is your evaluation of the implementation?

I did only take a quick glance at the implementation, but the code
looks surprisingly clean (even if comments are a bit spotty), with few
compiler workarounds. This is most likely due to the clean separations
of the various parts of the library. Even the use of the preprocessor
seems limited to the strict neces.

I did notice that stl function object instances are declared, as
const, in an header file. This could cause ODR violations.

- What is your evaluation of the potential usefulness of the library?

Great, at least if one is interested in a more functional approach to C++.

- Did you try to use the library?  With what compiler?  Did you have any
problems?

I tried some examples with gcc 4.2. Didn't have any problem after I
figured out that the header placement inside spirit changed from boost
1.35 to 1.36.

- How much effort did you put into your evaluation? A glance? A quick
reading? In-depth study?

An in depth study of the documentation and a quick reading of the source.

- Are you knowledgeable about the problem domain?

A bit. I've never implemented a full lambda library, but played a lot
with boost.lambda.

[ I would have liked to make a more in-depth review, but I'm in the
middle of a deadline, sorry]
-- 
gpd