Re: [boost] Boost.DLL formal review is ongoing

Subject: Re: [boost] Boost.DLL formal review is ongoing
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2015-07-04 12:31:23

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/