On 3 Jul 2015 at 16:50, Vladimir Prus wrote:
- Should the library be accepted?
Yes, conditional on the items below.
- How useful is it?
It is useful, though not as useful as it could be.
- What's your evaluation of - Design
The design is conservative and unsurprising, and is very similar to probably what 90% of us here would have chosen, myself included if one was tasked with an uncontroversial solution. I cannot fault it. My only qualm really, and this is entirely my fault, is there is a lack of type safety when loading symbols to a given type. This is my fault because I had said I would write a demangler which could check types matched, and I haven't had the time. If this feature existed, it could detect type mismatches and therefore ABI mismatch rather than just hoping a segfault should happen. A poor man's implementation could instead do demangle both symbols into a string, and do a nasty platform specific regex transformation into strings which are comparable. Slow and inaccurate though compared to a demangler based design.
- Implementation
I don't see any good reason why this library needs to be dependent on Boost. It uses little in Boost not also present in the C++ 11 STL, the only major blocker I noticed is some limited use of the MPL which is easily replaced with minimal constexpr as supported by VS2015. CONDITION: It should therefore support standalone usage decoupled from the rest of Boost on C++ 11 compilers. It can still use Boost on C++ 98 compilers. CONDITION: Namespace is not inline versioned on C++ 11 compilers. It should be. Otherwise implementation is very solid. I wouldn't expect any different from Antony.
- Documentations
CONDITION: BoostBook pages are missing badges for the CI test status. Readme has them though. CONDITION: No ability to launch example code in online web compiler. There is no acknowledgements section, and for good form there probably should be. Otherwise very good. I think I had sent Antony a list of docs problems last year, and he must have fixed them because I spotted nothing which particularly bothered me (that other reviews haven't already reported).
- Tests
CONDITION: No Appveyor integration. CONDITION: I saw a Coverity scan, but no clang-tidy + clang static analyser. A MSVC static analyser pass would do no harm either. CONDITION: The unit tests are not run under valgrind memcheck by default. Travis doesn't seem to have OS X testing enabled. The docs mention OS X isn't working yet, so that is understandable. Tests seem more functional than unit testing, but that is acceptable. I'm the same in AFIO.
- How much effort did you put into your evaluation?
About an hour, though I have been watching the development for a long time. I even promised some code I failed to deliver upon :(
- Did you attempt to use the library? On what systems and compilers?
Not on this occasion, but I did last year on Linux and Windows. It worked fine then. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/