Re: [boost] Boost.Local Review - comments by Andrzej

Subject: Re: [boost] Boost.Local Review - comments by Andrzej
From: Lorenzo Caminiti (lorcaminiti_at_[hidden])
Date: 2011-11-15 08:46:38

On Tue, Nov 15, 2011 at 6:05 AM, Andrzej Krzemienski <akrzemi1_at_[hidden]> wrote:
> Hi Lorenzo,
> I would like to provide some comments on Boost.Local.

Hi Andrzej, thank you very much for your review (even if there was no
yes/no conclusion :) ).

>> Please state clearly whether you think this library should be accepted
>> as a Boost library.
>
> I am unable to give a yes/no answer. This is mostly because I am not clear
> who is the intended audience of the library. I explain this in detail below.
>
> I am impressed with the library documentation. It is very informative with
> lots of examples, benchmarks, comparisons, rationale, and layered design
> (overview, and then details). This is how a Boost library's documentation
> (and every documentation, IMO) should look like. Yet, one thing that I am
> missing is what practical problems (that are not already solved) this
> library is trying to solve. I am not saying that it doesn't solve any. I am
> just saying I cannot figure it out. A typical story for a Boost library is
> that there is a common problem that many developers face fairly often, and
> at some point someone decides to fix this problem comprehensively and
> ultimately. This is the impression that I had, for example, with
> Boost.Optional. When I read about it I thought "Yes! This is what I was
> missing for a long time". I cannot see this in Boost.Local. Perhaps I am
> not the intended audience of the library, but it would help me if it was
> clear who the audience is. (I provide examples of what I mean below.)

Yes, Boost.Local vs. C++11 lambdas vs. Boost.Lambda vs. Boost.Phenix
vs. etc was subject of long discussions when I asked for interest in
Boost.Local:
http://lists.boost.org/Archives/boost/2010/08/170138.php
This culminated with the Alternatives section:
https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_local/Alternatives.html#boost_local.Alternatives.local_functions

I think in a nutshell the conclusion (which I will detail in the rest
of my comments below) was that Boost.Local is handy but on compilers
with C++11 lambdas you'd just use lambdas. At the same time,
Boost.Local allows you to write local functions that work also on
C++03 compilers and that have the same compile-time and run-time
performances of lambdas on C++11 compilers. So the /main/ audience
would be someone (like myself) that wants to do local function stuff
and expects lambda-like performances on C++11 but also has to deal
with C++03 compilers. There are other benefits like const& binding
which is not supported by C++11 lambdas (so how can I bind by const a
variable that has a very expensive copy constructor using C++11
lambdas?) and improve scope exit (i.e., local exit) but these other
benefits are probably minor in the general domain (they can be very
important or even essential is a specific and narrow domain like
Contract Programming but not critical in the general case).

> Documentation clearly separates three parts: local functions, local scopes
> and local exits. Also my impression is different on the three parts. This
> suggests to me that they could be three separate libraries. What do they
> have in common? The syntax, I guess. And the fact that you declare them
> inside functions. But is that enough to bundle them together? Consequently,
> I divide my comments into three parts.

The reasons why these are together are:
1) They are all about declaring stuff at local scope.
2) Once you have local functions then local exits and local blocks are
trivial corollaries to implement (define a local function and call it
right away, define a local function can call if from a destructor).
I can add this rational in the docs (currently it's not there).

> LOCAL FUNCTIONS
>
> Why would I want to use it? Personally, I have one compiler that does
> support local classes as template parameters. All my other compilers

You might still want to use Boost.Local on this compiler so to:
1) Avoid the boiler-plate code for binding variables in scope
(especially if you don't want to repeat their types for example to
facilitate maintenance).
2) And, to write code that will work on other C++03 compilers that do
not accept local types as template parameters.

> support lambdas. So I guess local functions are not for me. So who will use

You'd probably always use lambdas unless you want to write local
functions that also work on C++03 compilers (or the pathological case
when you must bind my const and you can't copy the variable value).

> them? Someone that does not have too new compiler (because they have
> superior - in terms of performance - alternatives: lambdas, local classes).
> Someone that does not have a too old compiler, because we read in the
> documentation that "The implementation of this library uses preprocessor
> and template metaprogramming (as supported by
> Boost.Preprocessor<http://www.boost.org/doc/libs/release/libs/preprocessor/doc/index.html>and
> Boost.MPL <http://www.boost.org/doc/libs/release/libs/mpl/doc/index.html>),
> templates with partial specializations and function pointers (similarly to
> Boost.Function<http://www.boost.org/doc/libs/release/doc/html/function.html>).
> As a consequence, this library is fairly demanding on compilers' compliance
> with the C++03 <http://www.open-std.org/JTC1/SC22/WG21/docs/standards>standard."

Yes, your compiler needs to be C++03 compliant and maybe old
supposedly C++03 compilers were not. Plus support for typeof is needed
if you don't want to specify the bound types manually. Your point is
correct but I must say that the compilers I have to use at work follow
under this category where Boost.Local will be useful (and probably
that won't change for another 5 years or so :( ).

> Further someone who does not mind compile-time and run-time
> performance overhead. And someone,who prefers the new syntax to creating a

The overhead goes away on C++11 compilers but yes it is there for
C++03 (unless the compiler has the non standard feature that allows
local types ad template parameters).

> functor class non-locally. I am not sure how many users that  would be?

Yes, it is a very basic trait of the library users that they want to
program the function locally without using global functors. IMO, this
is a common case and there is no real answer for this without C++11
lambdas.

> The documentation mentions proposal
> N2511<http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2511.html>,
> which says that even though with lambdas and local classes we already have
> a good tool, we still want to have a normal function syntax for local
> functions. Boost.LocalFunction does not enable a normal function syntax
> either, so it is no better an alternative than lambdas or local classes.

Yes, I don't buy the N2511 syntax argument neither.

> Except for "const binding" which I find hard to appreciate. And except for

I agree (const binding is important/essential only in narrow and
specific domains).

> compilers that don't support C++11 features, but than again, working only
> with the new compilers I may not be the right person to review the library.
>
> Is this supposed to provide a seamless migration of the code from compilers
> w/o lambdas to those that do support lambdas?

Yes: Boost.Local allows to program local functions that compile on
both C++03 and C++11, and have the same compile-time and run-time
performance as lambdas on C++11 compilers. Is this useful?

> I am not comfortable with the syntax: we first specify parameters, then
> function body and last, we specify the function name. I am not comfortable
> that the other Lorenzo's library (that I await impatiently) -

:)

> Boost.Contract - will provide a different function syntax than Local
> Functions. This will render the situation where every library will require
> of me to learn different function declaration syntax.

Boost.Local and Boost.Contract have different domains so their DSELs
are different.

> On the other hand, the documentation clearly says why this syntax is
> necessary, so I am not saying "change the syntax".

Yep.

> I am suspicious about the local function overloading. While I can imagine
> writing a local function; writing a bunch of them inside another function
> would be likely be some error in my design.

BTW, the overload<> template allows to overload any functor, not just
local functions.

> LOCAL BLOCKS
>
> Again, why would I need them? The example with assertion does not convince
> me. That is, I face the practical tradeoff: additional enforcement on
> assertions at the cost of slower build, slower execution time and less
> readable syntax. I will choose not to use the blocks.

I understand but in some domains (e.g., Contract Programming) this
trade off might be different.

> The only application that I can think of is a "temporary usage" while
> refactoring. If I work with the code where functions are 1000-line-long, I
> would like to divide them into blocks and check which parts reference what
> variables. In this case, as one of the footnotes in documentation suggests,
> allowing "return;" inside blocks would be a dangerous thing. But again, if
> I had a luxury of lambdas, I would choose lambdas for that purpose, because
> they allow specifying the default binding ( [&], [=] ).

Sure.

> It looks like local blocks are only a syntax sugar over local functions. Am
> I right?

Yes, you define a local function with no result type and no parameters
and call it right away. However, the fact that local blocks are a
corollary of local functions hat does not mean that local blocks are
not useful.

> LOCAL EXITS
>
> While exception specifications for locals blocks seam strange, I find
> allowing exception specifications for local exits an error. Local exits
> should be subject to the same restrictions as destructors (they are
> implemented as destructor calls anyways): you must not throw from them. And
> I expect the library documentation to say it explicitly. I find this
> "warning" missing from Boost.ScopeExit. So I suggest to disallow (if
> possible) specifying exceptions in local exit, and adding a section in
> documentation that warns against potential throws from an exit.

Oops, I missed that! I will add a warning to the docs that to comply
with STL exception safety, local exits should never throw.

> I do not see anything against having two libraries for the same purpose.
> This is pretty much the same what we already have in Boost in case of state
> machines. But since we have two, it is useful to ask why I would choose
> local exit over scope exit. My answer is:
>
> 1. Local Exit allows me to specify an empty parameter list (I cannot do it
> with ScopeExit). - this is a practical problem I faced
> 2. I can bind to "this".

Without the rest of Boost.Local, I'd just improve the current
Boost.ScopeExit to (1) support empty parameter list and (2) handle
this (these both require just some pp tricks).

> I do not find const binding a practical advantage. Typically for short
> functions it is hardly a real problem that I confuse = with ==. And exits
> would typically be one-liners in my code.
> On the other hand there are reasons why I would prefer ScopeExit to
> LocalExit: it spells shorter. In LocalExit I have to type word 'bind' for
> every single parameter even though it is obvious I need to bind them all
> (because scope guards do not accept normal parameters). The same argument
> goes for blocks, obviously. I also like ScopeExit's potential to be
> implemented with lambdas, which might result in performance benefits, and
> allow the 'default binding'. On the other hand LocalExit cannot be
> implemented with a lambda, if not for any other reason, then due to the
> const binding.
>
> It is really nice that i can bind to variable of the name 'bind'; although
> I cannot use type 'bind' in local functions.

Yes, a local function parameter cannot have a type named bind. I think
this is fine or is there a name different than bind that you'd prefer?

> GENERAL THOUGHTS
>
> I am really impressed with the usage of macros. It looks like thanks to
> Lorenzo we are reaching a new level of preprocessor programming. Who knows

It all starts with Boost.Preprocessor!

> how far we can go. I do not think they are overused, given the difficult
> task. On the other hand, error messages that I encountered give me no clue
> on what I have typed wrong.

Unfortunately, when using macros there are intrinsic limitations of
the errors you can detect and therefore raise intelligibly. However,
after a little while that you use Boost.Local, you'll get the function
declarations right or be able to find the errors just inspecting the
macro calls by eye. More complex errors are usually within the
function definition which is programmed outside the macros so compiler
errors will help you there as usual.

> Decsriptions in the documentation, and the rationale clearly indicates the
> very detailed knowledge of many C++ aspects.
>
> One suggestion for the future, although I am not sure how useful it would
> turn out in practice is to have something like local expression, or even
> not necessarily a local one. This would be a function whose body consists
> of a single expression. The benefit you get is that the programmer does not
> need to specify a return type for it (you can use BOOST_TYPEOF for that),
> or doesn't even have to type keyword 'return', but this may be too much off
> topic.

Can you clarify with an example?

> Term 'C99 <http://www.open-std.org/jtc1/sc22/wg14/www/projects#9899> and
> later compilers' may be a bit of an overuse for C++ compilers. I suggest
> 'compilers supporting variadic macros'.

OK.

> Also, in reply to particular review questions:
>
>
>> What is your evaluation of the design?
> It is clear. Some macros are difficult to read, e.g. Function arguments
> first, function name second, but I guess this is a necessity given the
> domain.
>
>> What is your evaluation of the implementation?
> The impressive macro magic is beyond my cognitive capabilities
>
>> What is your evaluation of the documentation?
> It is very good. It meets Boost's and my personal criteria for a good
> documentation.
>
>> What is your evaluation of the potential usefulness of the library?
>> Are you knowledgeable about the problem domain?
> This is the part where I have problems. I am not sure about the scope and
> intended audience of the library.

IMO: The main use is if I need to write local functions which have to
perform (compiler and run times) as well as lambda on C++11 compilers
and they also have to work on C++03 compilers.

>> Did you try to use the library?  With what compiler?  Did you have any
> problems?
> I tried to use the library (basic examples) with VC++8 and VC++10 and Boost
> 1.42. I found that it does not compile if I use compiler option /ZI (Debug
> Information Format: Program Database for Edit & Continue) on either
> compiler.

I will look at why it doesn't compile with /ZI.

>> How much effort did you put into your evaluation?  A glance?  A quick
> reading?  In-depth study?
> I spent one evening on reading the documentation and playing with the basic
> examples.
>
>
> For the last remark, I believe that the criterion for accepting or
> rejecting a library is that if it addresses a real-life problems and if it
> is well designed. Boost.Local is well designed, but the intended audience
> and primary motivation is not clearly stated, so I find it hard to assess
> if the first criterion is met.
>
> All in all, Boost.Local is an impressive piece of work.

Thanks a lot for your review!
--Lorenzo