- 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
Giovanni Piero Deretta wrote:
- 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.
Thank you.
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.
Wow, I haven't quite realized the effect of my works on people. I too stand in the shoulder of giants, those before me, like Jaakko, Gary Powell, Brian McNamara, Yannis, etc. Most welcome!
- 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.
I think I am convinced. Even with phx functions, the expressions expressing the arguments are already "optionally lazy" anyway due to C++: foo(123 + 456) // foo is a phoenix function The expression (resulting to the argument) is eagerly evaluated. Whereas: foo(_1 + 456) it is not (eagerly evaluated). So it's really a case by case basis depending on what C++ can evaluate eagerly. So, it follows that: foo(123 + 456) can be evaluated eagerly, not only the arguments, but foo itself. While: foo(_1 + 456) can remain lazy, as usual. Sooo... It's just a matter of determining the impact of this change on the overall scheme of things. I suggest we have it on a branch and play with it some. I also urge people to chime in and give us their opinions on this (another) breaking change. The implication of this, as you opined, is immense. For one, it allows us to have only one set of STL algorithms (for example) that can be applied lazily or immediately. We can make everything a lazy function and reap the benefits of currying/partial application, etc.
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.
This I am not a fan of. I enjoy the terse expressions. There are problems, I agree, like the assignability that Peter mentions. But I don't think they are showstoppers, enough to require a strict syntax.
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 :).
Agreed.
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.
Right. Proto fixes this.
- 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]
Thank you for your review and thanks for starting the discussions. Your comments and suggestions are invaluable. Regards, -- Joel de Guzman http://www.boostpro.com http://spirit.sf.net
on Wed Oct 01 2008, Joel de Guzman <joel-AT-boost-consulting.com> wrote:
I think I am convinced. Even with phx functions, the expressions expressing the arguments are already "optionally lazy" anyway due to C++:
foo(123 + 456) // foo is a phoenix function
The expression (resulting to the argument) is eagerly evaluated. Whereas:
foo(_1 + 456)
it is not (eagerly evaluated).
So it's really a case by case basis depending on what C++ can evaluate eagerly.
So, it follows that:
foo(123 + 456)
can be evaluated eagerly, not only the arguments, but foo itself. While:
foo(_1 + 456)
can remain lazy, as usual.
It may be the right choice anyway, but keep in mind that any such nonuniformity limits genericity. I think we already have that with Boost.Bind ("nested binds are special") but this effect is worth thinking about. -- Dave Abrahams BoostPro Computing http://www.boostpro.com
David Abrahams wrote:
on Wed Oct 01 2008, Joel de Guzman <joel-AT-boost-consulting.com> wrote:
I think I am convinced. Even with phx functions, the expressions expressing the arguments are already "optionally lazy" anyway due to C++:
foo(123 + 456) // foo is a phoenix function
The expression (resulting to the argument) is eagerly evaluated. Whereas:
foo(_1 + 456)
it is not (eagerly evaluated).
So it's really a case by case basis depending on what C++ can evaluate eagerly.
So, it follows that:
foo(123 + 456)
can be evaluated eagerly, not only the arguments, but foo itself. While:
foo(_1 + 456)
can remain lazy, as usual.
It may be the right choice anyway, but keep in mind that any such nonuniformity limits genericity. I think we already have that with Boost.Bind ("nested binds are special") but this effect is worth thinking about.
Yeah, I was thinking about that too. That is why I suggested to work on this on a branch for further investigation. Regards, -- Joel de Guzman http://www.boostpro.com http://spirit.sf.net
Joel de Guzman wrote:
David Abrahams wrote:
on Wed Oct 01 2008, Joel de Guzman <joel-AT-boost-consulting.com> wrote:
I think I am convinced. Even with phx functions, the expressions expressing the arguments are already "optionally lazy" anyway due to C++:
foo(123 + 456) // foo is a phoenix function
The expression (resulting to the argument) is eagerly evaluated. Whereas:
foo(_1 + 456)
it is not (eagerly evaluated).
So it's really a case by case basis depending on what C++ can evaluate eagerly.
So, it follows that:
foo(123 + 456)
can be evaluated eagerly, not only the arguments, but foo itself. While:
foo(_1 + 456)
can remain lazy, as usual.
It may be the right choice anyway, but keep in mind that any such nonuniformity limits genericity. I think we already have that with Boost.Bind ("nested binds are special") but this effect is worth thinking about.
Yeah, I was thinking about that too. That is why I suggested to work on this on a branch for further investigation.
One possibly nasty effect of this is that this makes phoenix unusable as imperative callback function like in this example: enum { begining, continue_ } set_menu_action("play", do_play(begining)); or in Spirit where in some cases, all args are known yet you want to have a semantic action that's not executed immediately: "what:" >> eps[foo()] I don't want foo() to be immediate! Oh my... this behavior, while cool, might not be practical after all. Regards, -- Joel de Guzman http://www.boostpro.com http://spirit.sf.net
On Thu, Oct 2, 2008 at 6:17 PM, Joel de Guzman <joel@boost-consulting.com> wrote:
Joel de Guzman wrote:
David Abrahams wrote:
on Wed Oct 01 2008, Joel de Guzman <joel-AT-boost-consulting.com> wrote:
I think I am convinced. Even with phx functions, the expressions expressing the arguments are already "optionally lazy" anyway due to C++:
foo(123 + 456) // foo is a phoenix function
The expression (resulting to the argument) is eagerly evaluated. Whereas:
foo(_1 + 456)
it is not (eagerly evaluated).
So it's really a case by case basis depending on what C++ can evaluate eagerly.
So, it follows that:
foo(123 + 456)
can be evaluated eagerly, not only the arguments, but foo itself. While:
foo(_1 + 456)
can remain lazy, as usual.
It may be the right choice anyway, but keep in mind that any such nonuniformity limits genericity. I think we already have that with Boost.Bind ("nested binds are special") but this effect is worth thinking about.
Yeah, I was thinking about that too. That is why I suggested to work on this on a branch for further investigation.
One possibly nasty effect of this is that this makes phoenix unusable as imperative callback function like in this example:
enum { begining, continue_ }
set_menu_action("play", do_play(begining));
An explicit use of bind might to the trick: bind(do_play, beginning)
or in Spirit where in some cases, all args are known yet you want to have a semantic action that's not executed immediately:
"what:" >> eps[foo()]
I don't want foo() to be immediate!
wouldn't "what:" >> eps[ foo ] work?
Oh my... this behavior, while cool, might not be practical after all.
I guess it depends a lot on what the most common use case is. -- gpd
Giovanni Piero Deretta wrote:
On Thu, Oct 2, 2008 at 6:17 PM, Joel de Guzman <joel@boost-consulting.com> wrote:
Joel de Guzman wrote:
David Abrahams wrote:
on Wed Oct 01 2008, Joel de Guzman <joel-AT-boost-consulting.com> wrote:
I think I am convinced. Even with phx functions, the expressions expressing the arguments are already "optionally lazy" anyway due to C++:
foo(123 + 456) // foo is a phoenix function
The expression (resulting to the argument) is eagerly evaluated. Whereas:
foo(_1 + 456)
it is not (eagerly evaluated).
So it's really a case by case basis depending on what C++ can evaluate eagerly.
So, it follows that:
foo(123 + 456)
can be evaluated eagerly, not only the arguments, but foo itself. While:
foo(_1 + 456)
can remain lazy, as usual. It may be the right choice anyway, but keep in mind that any such nonuniformity limits genericity. I think we already have that with Boost.Bind ("nested binds are special") but this effect is worth thinking about. Yeah, I was thinking about that too. That is why I suggested to work on this on a branch for further investigation. One possibly nasty effect of this is that this makes phoenix unusable as imperative callback function like in this example:
enum { begining, continue_ }
set_menu_action("play", do_play(begining));
An explicit use of bind might to the trick: bind(do_play, beginning)
ermmm...
or in Spirit where in some cases, all args are known yet you want to have a semantic action that's not executed immediately:
"what:" >> eps[foo()]
I don't want foo() to be immediate!
wouldn't "what:" >> eps[ foo ]
work?
Perhaps. How about this: "what:" >> eps[ foo(123, "busted") ] ah, yeah, bind again... Tedious.
Oh my... this behavior, while cool, might not be practical after all.
I guess it depends a lot on what the most common use case is.
Probably :-* Regards, -- Joel de Guzman http://www.boostpro.com http://spirit.sf.net
participants (3)
-
David Abrahams -
Giovanni Piero Deretta -
Joel de Guzman