Hi Lorenzo, I would like to provide some comments on Boost.Local.
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.) 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. 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 support lambdas. So I guess local functions are not for me. So who will use 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." Further someone who does not mind compile-time and run-time performance overhead. And someone,who prefers the new syntax to creating a functor class non-locally. I am not sure how many users that would be? 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. Except for "const binding" which I find hard to appreciate. And except for 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? 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. On the other hand, the documentation clearly says why this syntax is necessary, so I am not saying "change the syntax". 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. 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. 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 ( [&], [=] ). It looks like local blocks are only a syntax sugar over local functions. Am I right? 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. 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". 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. 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 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. 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. 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'. 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.
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.
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. Regards, &rzej