lcaminiti wrote:
On Wed, Nov 16, 2011 at 6:16 PM, Pierre Morcello <pmorcell-cppfrance@> 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_loca...
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_loca...
I have not used yet C++ lambdas, couldn't the following be used to support const binding? xtype x; xtype const& xcr; // const binder [xcr]() { assert( xcr == 0 ); }(); Best, Vicente -- View this message in context: http://boost.2283326.n4.nabble.com/Boost-Local-Review-tp4078276p4079413.html Sent from the Boost - Dev mailing list archive at Nabble.com.