Re: [boost] Boost.Local Review

Subject: Re: [boost] Boost.Local Review
From: Lorenzo Caminiti (lorcaminiti_at_[hidden])
Date: 2011-11-17 03:56:26

On Wed, Nov 16, 2011 at 6:16 PM, Pierre Morcello
<pmorcell-cppfrance_at_[hidden]> wrote:
> Hello Boost community,

Hi Pierre. Thank you very much for your review!

> Here is my review of the local function library.
>
> - What is your evaluation of the design?
> The design is clear. Several iterations were done already to come to what it
> is today. Of course no one is particularly fond of macros, but a huge work
> was done to reduce the potential problems and on the other hand the library
> can really help a lot.
>
> - What is your evaluation of the implementation?
> I did not get too much into the current implementation. I dug it almost one
> year ago, but there were changes since then. The implementation is correct
> given the features of the library (use of 'this', functor,, name,...). If
> there were less features, then some optimisations would have been possible
> (example : not use a virtual function, not use a functor). But the library

One change in the implementation was to use static_cast instead of a
virtual function to allow to pass the local class as a template
parameter. I did some benchmarking and the static_cast approach had
better (somewhat faster) run-time than the virtual function approach
with essentially same compilation times.
https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_local/Implementation.html#boost_local.Implementation.local_classes_as_template_parameters

> would in that case have fewer cases of use. I know how the first verion of
> Lorenzo worked but I did not check the latest.
> Given the current features, I don't see how to do even better than what
> Lorenzo did. So I am happy with the implementation.
>
> - What is your evaluation of the documentation?
> I really appreciate the detailled documentation. On a 'merchandising note',
> the first page is too big. I think this could 'afraid' some new readers.
> Introduction + an 8-line motivating example should be enough to appeal (or
> not) the new reader. I am happy to review a submitted library with a

Yes, I will revamp the Introduction section making it shorter and
adding a motivation note as suggested by Andrzej in his review I will
probably only show local functions in the Introduction example and
mention the existence of local blocks and exits for which I will refer
the Tutorial section. I will probably start the Tutorial section with
the current Introduction section and its example.

> complete documentation (that does not always happen).
>
> - What is your evaluation of the potential usefulness of the library?
> This library is filling a few strong needs. Several users including myself
> needed local function and const block. I am lazy when it comes to functors,
> I also find this library is pretty nice to make them quick. C++11 will on
> that last part change it , but const block are a must have in my opinion
> which are not available in C++11.

That is correct. I want to clarify your point for everyone with an
example. The only way you can make const blocks using C++11 lambdas is
to bind by value:

xtype x;
[x]() {
    assert( x == 0 );
}();

However, this lambda solution for const block will:
1) Obviously, only work on C++11 (and not on C++03).
2) Not compile at all if xtype is non-copyable.
3) Significantly increase run-time if xtype has an expensive copy constructor.

Boost.Local allows instead to bind by const& so no copy is necessary
(plus it also works on C++03):

xtype x;
BOOST_LOCAL_BLOCK(const bind& x) {
    assert( x == 0 );
} BOOST_LOCAL_BLOCK_END

Again, local blocks might not be needed everywhere but if you need
them for your application domain, C++11 lambdas do /not/ provide a
good solution (while I think Boost.Local does).
https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_local/Alternatives.html

> The local function can greatly reduce the presence of internal functions in
> ".h", and thus make the interfaces much better in some cases.
>
> - Did you try to use the library?  With what compiler?  Did you have any
> problems?
> I use regularly this library on VC9, but mostly for local function. As I
> don't try to push the limits, I did not come into problems.
>
> - How much effort did you put into your evaluation?  A glance?  A quick
> reading?  In-depth study?
>
> I have followed Lorenzo work since his proposition. At the beginning I
> followed it much more closely, but right now I dont' have the time to redo
> an in depth study. I use a previous version of Lorenzo library regularly. So
> for the review, I simply had a glance. But I already read the documentation
> in detail several month ago.
>
> - Are you knowledgeable about the problem domain?
>
> I implemented a my own version of local functions for boost almost 2 years
> ago I think.
>
> - Boost.Local's local exits provide the same functionality (and then some)
> as Boost.ScopeExit.  Does this duplication of functionality need to be
> dealt with, and if so, how?
>
> I think it's important to check if the author are ok with merging or not.
> Obviously Lorenzo is very motivated and gives great feedback on his works
> with boost. Just make sure this won't hurt his motivation ? Because then
> there are the question of maintening the libraries which is not always the
> easiest.
> I am ok if one replace the other.
>
> - Do you vote for the library?
>
> Yes, I vote for its admission.
>
> I also want to add that I am amazed by the overall results in term of
> quality that Lorenzo was able to get with this library. Lorenzo is
> constently gathering the users needs and updating his library in that way.
> Like a marathonian. Very inspiring!

:))

Thanks a lot.
--Lorenzo