Re: [boost] [local] Review (and resignation from review assistant)

Subject: Re: [boost] [local] Review (and resignation from review assistant)
From: Lorenzo Caminiti (lorcaminiti_at_[hidden])
Date: 2011-11-21 06:06:12

On Sun, Nov 20, 2011 at 7:53 PM, Gregory
Crosswhite<gcrosswhite_at_[hidden]> wrote:> Hey everyone,
Hello Greg and thank you very much for your review.
> The discussion on the mailing list has caused me to realize that I have> sufficient passion for the Local library that it is difficult for me to stay> neutral, so I will be recusing myself from my position as Review Assistant> for this library.  (I have learned that should I decide to help out again in> the future in this role for another review, I will make sure that it is for> a library that I don't feel strongly about either way!)> On the other hand, this means that there is no longer any reason not to post> a review of this library.> I have been a fan of this library for a long time because for me it solved> the problem of having a nice way to write (pseudo-)closures in C++03 without> having to write and maintain a lot of pointless boilerplate code, and> without having to use a completely foreign syntax that I could never seem to> get to work.  Here is my review of it.>> - What is your evaluation of the design?>> The library is very well designed.  The function declaration syntax looks a> little foreign at first, but it is very easy to get used to.  Furthermore,> error messages are usually able to point you to what you did wrong.  The> fact that you can write the body of the function in plain C++ code is a> major advantage of this library.  Some have referred to Boost.Bind,> Boost.Lambda and Boost.Phoenix as essentially being equivalent to this> library, but in none of those libraries can you express the body of your> function in plain C++;  one advantage of this is that errors in your code> result in normal C++ error messages rather than cryptic error messages.> It is also clear reading through the documentation that Lorenzo has put a> great deal of thought into possible use cases of his library;  there are a> great number of features which most users might never need to use but which> are available for those times when they are completely necessary --- such as> explicitly providing types of bound parameters, and marking functions inline> --- and they are provided in such a way that when they aren't needed they> introduce no additional overhead to the syntax.>> - What is your evaluation of the implementation?>> I am not familiar enough with preprocessor programming to be able to make a> fully qualified statement on this.  However, it looks reasonable based on> the Implementation appendix.  The virtual function call is a shame as, if I> recall correctly, it can make Boost.Local slower than Boost.Phoenix when the> compiler is not smart enough to optimize it into a direct call, but Lorenzo> has put so much time and expertise into the design of this library that I> doubt it is possible to improve on this while maintaining compatibility with
The final implementation does not use the virtual function, it
usesinstead a static_cast because it was measured to be faster.
Bothapproaches are shown
here:https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_local/Implementation.html#boost_local.Implementation.local_classes_as_template_parameters
> C++03.  For me this part of the overhead is not a big deal because I> primarily have been using Boost.Local in test suites where the difference in> performance does not matter.
Yes but on MSVC and C++11 there is no performance overhead
(withrespect to the alternative approaches including C++11 lambdas)
becausethe libraries uses optimizations from being able to directly
pass thelocal type as template parameter (that is not a standard C++03
featureso on other C++03 compilers there's a performance
overhead):https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_local/Alternatives.html#boost_local.Alternatives.local_function_benchmarking
> - What is your evaluation of the documentation?>> The documentation is some of the best well written I have seen for a Boost> library.  It does a good job both explaining how the library works,> providing a reference for remembering how the syntax works, and explaining> why the library was designed the way that it does.> As I have told Lorenzo in a separate e-mail, I do have a minor criticism of> the Tutorial section:  the ordering of the first four sections is probably> not what typical user reading it the first time would want because it puts> the most important and essential feature, variable binding, after two> features, default and empty argument features, which are nice to have but> are not likely to be needed in typical usage, especially considering that a> first-time user is most likely just looking for a convenient way to create a> simple (pseudo-)closure in C++03.  Thus, I suggest the following ordering:> "Local Functions"> "Variable Binding"> "Default Parameters"> "Empty Parameters"> This way a typical user, who will only care about writing a function that> binds a few local variables, will be able to get started immediately after> reading the second section rather than first having to wade through two> sections describing features that they are unlikely to care about right> away.  The rationale for putting "Default Parameters" before "Empty> Parameters" is that a typical user is very unlikely to need to know how to> call the macro with no arguments because there are few use cases where a> typical user won't want to bind *anything* from the local environment (since> most of the time this means that they won't be thinking in terms of local> functions in the first place), so they are less likely to care about how to> do this than they are about how to specify default parameters.
Yes, I will change the order of these subsections.
> - What is your evaluation of the potential usefulness of the library?>> This library is very useful for people who want to use much of the> functionality of C++11 lambdas on C++03 compilers, as I can say from> personal experience in using it.> As I have said before, the existence of Boost.Bind, Boost.Lambda and> Boost.Phoenix do not reduce the utility of this library.  They do have a> large advantage over Boost.Local of being succinct, which is why I often> prefer them over Boost.Local when it is easy to do so.  However, because> they require to you learn completely new syntax for *all* of the code in the> function body, and because they make use of heavy metatemplate programming> that results in absolutely horrendous error message that do not give you a> clue where they are wrong, for anything non-trivial Boost.Local is> hands-down *far* easier to use because you can use plain C++ statement> syntax which results in plain C++ error message when you get something> wrong.> Put another way, Boost.Bind, Boost.Lambda and Boost.Phoenix are> complementary to rather than competitors of Boost.Local.>> - Did you try to use the library?  With what compiler?  Did you have any> problems?>> I have used the library extensively in my own codes, though most often in> test suites.  I have used them on OSX with GCC 4.2, and on Linux with (if I> recall correctly) GCC 4.3 and 4.4.>> - How much effort did you put into your evaluation?  A glance?  A quick> reading?  In-depth study?>> I have become accustomed to using the library on a regular basis and hence> my evaluation is written based on my personal experience with it.>> Please state clearly whether you think this library should be accepted as> a Boost library.>> Definite yes.
:)
Thanks a lot for your comments.--Lorenzo