Hello Boost community, 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 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 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. 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! Best regards, Pierre Morcello -- View this message in context: http://boost.2283326.n4.nabble.com/Boost-Local-Review-tp4078276p4078276.html Sent from the Boost - Dev mailing list archive at Nabble.com.
On Wed, Nov 16, 2011 at 6:16 PM, Pierre Morcello <pmorcell-cppfrance@yahoo.fr> 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...
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
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.
On Thu, Nov 17, 2011 at 4:33 AM, Vicente Botet <vicente.botet@wanadoo.fr> wrote:
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 ); }();
I can't check with a C++11 compiler right now, but my understand is "yes". Using decltype, you don't even have to repeat the type (to make maintenance easier). I think the most general way to write that is: xtype x; decltype(x) const& const_ref_x = x; [xcr]() { assert( const_ref_x == 0 ); }(); The down sides are: 1) The lambda cannot refer to the bound variable using its original name x (this will actually look very strange in Boost.Contract if I had to indicate the user to use some magic names for variables in constant assertions). 2) The extra type manipulation. I understand these are not big disadvantages in general but IMO the following is more clear an concise (I'm probably biased ;) ): xtype x; BOOST_LOCAL_BLOCK(const bind& x) { assert( x == 0 ); } BOOST_LOCAL_BLOCK_END I would instead prefer the following if C++11 allowed it: xtype x; [const&x]() { assert( x == 0 ); }(); But C++11 only allows to qualify the bind with the reference &x and not with the additional const const&x. Thanks. --Lorenzo
Le 17/11/11 13:15, Lorenzo Caminiti a écrit :
On Thu, Nov 17, 2011 at 4:33 AM, Vicente Botet<vicente.botet@wanadoo.fr> wrote:
lcaminiti wrote: 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 ); }(); I can't check with a C++11 compiler right now, but my understand is "yes". Using decltype, you don't even have to repeat the type (to make maintenance easier). I think the most general way to write that is:
xtype x; decltype(x) const& const_ref_x = x; [xcr]() { assert( const_ref_x == 0 ); }();
The down sides are: 1) The lambda cannot refer to the bound variable using its original name x (this will actually look very strange in Boost.Contract if I had to indicate the user to use some magic names for variables in constant assertions). 2) The extra type manipulation.
Could you add the preceding lambda alternative to the Alternative sections once you could verify it?
I would instead prefer the following if C++11 allowed it:
xtype x; [const&x]() { assert( x == 0 ); }();
But C++11 only allows to qualify the bind with the reference&x and not with the additional const const&x. Maybe it is worth to post the idea to the comp.lang.c++ ML.
Best, Vicente
On Thu, Nov 17, 2011 at 12:10 PM, Vicente J. Botet Escriba <vicente.botet@wanadoo.fr> wrote:
Le 17/11/11 13:15, Lorenzo Caminiti a écrit :
On Thu, Nov 17, 2011 at 4:33 AM, Vicente Botet<vicente.botet@wanadoo.fr> wrote:
lcaminiti wrote: 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 ); }();
I can't check with a C++11 compiler right now, but my understand is "yes". Using decltype, you don't even have to repeat the type (to make maintenance easier). I think the most general way to write that is:
xtype x; decltype(x) const& const_ref_x = x; [xcr]() { assert( const_ref_x == 0 ); }();
The down sides are: 1) The lambda cannot refer to the bound variable using its original name x (this will actually look very strange in Boost.Contract if I had to indicate the user to use some magic names for variables in constant assertions). 2) The extra type manipulation.
Could you add the preceding lambda alternative to the Alternative sections once you could verify it?
Yes.
I would instead prefer the following if C++11 allowed it:
xtype x; [const&x]() { assert( x == 0 ); }();
But C++11 only allows to qualify the bind with the reference&x and not with the additional const const&x.
Maybe it is worth to post the idea to the comp.lang.c++ ML.
Other people have raised the same question (I can't find another reference I read a while back): http://stackoverflow.com/questions/3772867/lambda-capture-as-const-reference I guess I could ask comp.lang.c++ why C++11 does not allow const& bindings (I'm not really sure myself). Thanks. --Lorenzo
participants (4)
-
Lorenzo Caminiti -
Pierre Morcello -
Vicente Botet -
Vicente J. Botet Escriba