On Thu, Nov 10, 2011 at 12:52 AM, Jeffrey Lee Hellrung, Jr.
The review of Lorenzo Caminiti's proposed Boost.Local library begins tomorrow, ***November 10, 2011***, and ends on ***November 19, 2011***.
Boost.Local implements local functions, local blocks, and local exits for the C++ programming language. It allows one to define a function at any declarative scope, including function scope; bind variables from the enclosing scope; and pass this function to templated STL-style algorithms.
First comment: when I saw the name Boost.Local, I immediately assumed it was related somehow to Boost.Locale. That is, the name suggests that it somehow helps you localize an internationalized application. The description above came as a surprise to me. Though I can't remember exactly where, I think there was a suggestion to name this library something more like "Boost.Scope," which would be more intuitive. I would like to request that (or similar) name change. Why does the name matter? Because there are so many Boost libraries that one's first problem is to determine at a glance whether Boost even has a library to address one's current problem -- and which library that might be. Once you've focused on a particular library, of course you read its documentation to discover the details. But for most of us, reading through all documentation for all Boost libraries is simply not practical. If I were looking for a tactic to pass a nested function to an STL algorithm, I'm afraid I'd have skimmed right past the name "Boost.Local" without further investigation. If the name of a proposed Boost library might possibly be ambiguous, it seems important to me to try to make it less so.
Please state clearly whether you think this library should be accepted as a Boost library.
I vote conditional YES. Conditions synopsized at end.
Other questions you may want to consider:
- What is your evaluation of the design? - What is your evaluation of the implementation? - What is your evaluation of the documentation?
The documentation is laudably clear.
- What is your evaluation of the potential usefulness of the library?
I definitely like the ability to construct nested functions to pass to STL algorithms. Thumbs up for BOOST_LOCAL_FUNCTION_*. I must admit I'm not fully convinced of the utility of BOOST_LOCAL_BLOCK, given the existing ability to do this: int i = 17; { const int& locali(i); ++locali; // ERROR } Using BOOST_LOCAL_BLOCK seems like a lot of typing, and the risk of making the code harder for your colleagues to read, for what it gains. Having worked with both try/finally (in some languages, that's all you get) and with RAII, courtesy of stack objects with constructors and destructors, I much prefer RAII. So for me, BOOST_LOCAL_EXIT is of dubious utility as well. I recognize the value of being able to express cleanup immediately following the corresponding acquisition; but if I'm going to acquire this resource in one place, chances are I'll end up needing to acquire it in other places too, so I may as well write the class that logically encapsulates it. That encapsulation is usually harder to express with try/finally, and I don't see how BOOST_LOCAL_EXIT would do better. That said, I recognize that others may differ on this point.
- Did you try to use the library? With what compiler? Did you have any problems?
did not try
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I read the online documentation.
Please also consider the following issues and proposals specific to Boost.Local. Your opinion is welcome on any or all of these.
- 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?
BOOST_LOCAL_EXIT does indeed seem wholly redundant with Boost.ScopeExit. I don't think both should be equivalently supported in Boost: how should a coder pick? If there really are semantic differences between them, then the documentation for each must be very clear about those distinctions. If there are not, one should be eliminated. It would make some sense to me to fold Boost.ScopeExit into this library, perhaps with forwarding headers for backwards compatibility.
- Lorenzo is proposing to add boost::local::function::overload to Boost.Function as boost::function::overload. See https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost/loca...
YES! overload has no compelling relationship to Boost.Local. It really does belong in Boost.Function.
- Lorenzo is proposing to add BOOST_IDENTITY_TYPE to boost/utility. See https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/BOOST_IDEN...
This seems generally useful, yes. In fact it should probably show up in a Boost FAQ for trying to pass arbitrary types to macros.
- Likewise, Lorenzo is proposing to add BOOST_IDENTITY_VALUE to boost/utility. See https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/BOOST_IDEN...
This is one point on which the documentation is not clear. For an
expression that evaluates to a runtime value, what's the benefit of
(e.g. from the cited page) BOOST_IDENTITY_VALUE((key_sizeof
--------
Lastly, please note that Boost.Local has included a copy of the Variadic Macro Data (VMD) library in boost/detail/preprocessor/variadic_macro_data. Since then, VMD has been modified and integrated into Boost.Preprocessor. After the review has concluded, Lorenzo will remove this local copy of the VMD library and replace its usage within Boost.Local with the Boost.Preprocessor variadic extensions.
--------
Conditions: 1. Rename library to something less ambiguous (avoid resonance with internationalization). 2. Cage match with Boost.ScopeExit. 3. Move 'overload' to Boost.Function. It doesn't belong in Boost.Local. 4. Move BOOST_IDENTITY_TYPE to boost/utility. 5. Use official Boost.Preprocessor VMD support rather than a library-specific copy. Comments: * For me the value of this library is BOOST_LOCAL_FUNCTION_PARAMS/NAME. It doesn't matter much to me whether BOOST_LOCAL_EXIT or BOOST_LOCAL_BLOCK come along for the ride. * I don't really see the point of BOOST_IDENTITY_VALUE either.