[boost] Phoenix v3 review

Subject: [boost] Phoenix v3 review
From: Mathias Gaunard (mathias.gaunard_at_[hidden])
Date: 2011-02-24 11:52:29

Here is my review of Phoenix v3.

First, let me say that I am a big user of Phoenix v2, Spirit and Proto, 
and that I therefore envision myself a big user of Phoenix v3 as well.

It is a great improvement on top of Phoenix v2 (in particular, no 
strange bugs due to broken type deduction and correct usage of 
result_of), and for that reason alone it warrants my approval.
So here it is: I vote yes for inclusion.

I however have concerns about its compatibility with Phoenix v2.
I tried to see how Spirit fared if I symlinked its underlying phoenix 
directory to that of Phoenix v3, and it just doesn't work.
Mixing Phoenix v3 with Spirit doesn't work either.

I believe this is a very important issue. I'm not sure it needs to be 
fixed before the first release of Phoenix as a first-class Boost 
citizen, but it certainly needs to be fixed ASAP by porting Spirit to 
use the new version.

Apart from that, the library is pretty satisfying:
- As I said, usage of result_of has been a great improvement, and bind 
for example is now polymorphic and fully compatible with boost/std::bind.
- it supports perfect forwarding up to a limit
- It uses Proto in a minimalistic and modular way
- It is reasonably fast to compile, thanks to the instrumentation of 
wave to preprocess headers. Compiling the STL module could be made 
faster by using free functions instead of global objects (which should 
be the recommended way to define lazy functions!).
- The docs haven't changed much, a few rectifications here and there 
plus the addition of the new internals. It still contains typos and some 
things that need to be updated.

Let's start on with more detailed remarks:

Misc
----
Phoenix v3 is missing a phoenix.hpp header file that includes all 
modules, which Phoenix v2 had (but in the parent directory).
Lazy functions
--------------
Using phoenix::function to turn a PFO into a lazy function is the most 
basic way to extend Phoenix with new functionality, and clearly the 
recommended way to do so according to the docs. [1] [2]
It is directly inherited from Phoenix v2, and only the result type 
deduction mechanism changed.
This approach has several problems, at least in the way it is presented 
in the documentation.
- global objects potentially increase binary size
- those objects are not PODs and therefore requires runtime 
initialization, adding some runtime overhead at application startup
- instantiation happens regardless of whether the function is used or 
not, which affects compilation time negatively.
This method is massively used to define a whole lot of lazy functions 
that forward to standard algorithms and container functions [3], which 
suggests that this is indeed the recommended way to proceed.
It should however be avoided; prefer defining a template function that 
constructs and applies the adapted function object, if the function 
object can stay a POD that's better too.
I think it would also be valuable to add a "lazy" function (or some 
other name) that takes a PFO and returns its lazy version, that could be 
used inline in lambda expressions in a way similar to bind.
[1] 
<http://svn.boost.org/svn/boost/sandbox/SOC/2010/phoenix3/libs/phoenix/doc/html/phoenix/starter_kit/lazy_functions.html>
[2] 
<http://svn.boost.org/svn/boost/sandbox/SOC/2010/phoenix3/libs/phoenix/doc/html/phoenix/modules/bind.html>
[3] 
<http://svn.boost.org/svn/boost/sandbox/SOC/2010/phoenix3/libs/phoenix/doc/html/phoenix/modules/stl/container.html>
Phoenix as a Proto-based library
--------------------------------
Extension of Phoenix is very nice: you can plug custom subgrammars and 
subnodes non-intrusively, making it very modular.
Phoenix expressions are also refinements of Proto expressions, so I can 
use Proto transforms to alter the tree.
It is also possible to specify custom evaluation strategies on a 
per-node basis, which paves the way for many exotic applications.
Terminals can also be customized for evaluation within the default 
strategy. I'm not entirely sure this feature is not redundant, but I'm 
not familiar enough with the code to tell.
Phoenix as Proto with statements
--------------------------------
One thing I was hoping Phoenix v3 would be is Proto extended to support 
statements.
While it comes pretty for some uses, it still fails one one point: the 
ability to define custom languages that embed Phoenix statements.
The missing piece is handling of domains (extends are missing too, but I 
don't think they make sense at the statement level).
I think that area needs some research though, so it's not relevant to 
acceptance at all.
Documentation
-------------
<http://svn.boost.org/svn/boost/sandbox/SOC/2010/phoenix3/libs/phoenix/doc/html/phoenix/modules/bind.html> 
says that bind is monomorphic.
I thought it was polymorphic now?
<http://svn.boost.org/svn/boost/sandbox/SOC/2010/phoenix3/libs/phoenix/doc/html/phoenix/actor.html> 
doesn't talk about perfect forwarding.
<http://svn.boost.org/svn/boost/sandbox/SOC/2010/phoenix3/libs/phoenix/doc/html/phoenix/inside/actor.html> 
Actor cannot be both a concept and a model, it's a concept and a 
refinement. It would also be nice to highlight the difference with the 
PFO concept.
"The problem is that given an arbitrary function F, using current C++ 
language rules, one cannot create a forwarding function FF that 
transparently assumes the arguments of F. "
That's wrong. It's not impossible, it just requires an exponential 
number of overloads. C++0x rvalue references allow to reduce this to a 
linear amount.
<http://svn.boost.org/svn/boost/sandbox/SOC/2010/phoenix3/libs/phoenix/doc/html/phoenix/inside/expression.html> 
spurious ] at the end.
The layout of this section is weird, by the way. The macros appear on 
top but are not explained until a later page.
I remember I saw a couple other typos and similar but I can't find them 
anymore. Anyway it needs some proofreading.
Features I would like to see in a future version
-------------------------------------------------
Ok, this has little to do in a review, but here it is anyway.
I would like to have an adapter to turn a PFO into a monomorphic 
function object (i.e. a function object with a result_type) that can be 
passed to legacy algorithms.
This can be done two ways: either by giving the return type or the type 
of the arguments explicitly.
This could also be used to define variant visitors with lambdas.
I would like it if Phoenix could detect function objects that are 
monomorphic and automatically propagate that monomorphism (but that's 
probably hard to do).
I would like it if the polymorphic function objects generated by Phoenix 
were masked out using SFINAE if their body-expression would result in a 
hard error.
Essentially, to do this, one needs to be able to test whether the 
default Proto transform of an expression leads to an error. While this 
should be possible with compilers supporting extended SFINAE, I haven't 
been able to grok the Proto internals well enough to do it yet myself.