Re: [boost] Boost.DLL formal review

Subject: Re: [boost] Boost.DLL formal review
From: Bjorn Reese (breese_at_[hidden])
Date: 2015-07-11 07:51:41

On 06/29/2015 03:11 PM, Vladimir Prus wrote:

> - Should the library be accepted?

Yes.

> - How useful is it?

It hides most of the complexity if you want to build a plugin
framework.

> - What's your evaluation of
>    - Design

I like how reference counting can be used to manage life-times.

It is unclear to me whether there are any limitations about aliasing.
For instance, can I alias constructors, operators, or functions in an
anonymous namespace?

>    - Implementation

Looks clean from my brief examination.

My only complaint is that some headers includes system headers that
typically brings in their own macros and non-namespaced types which
may or may not colide with the user code. For example,
<boost/dll/runtime_symbol_info.hpp> includes <dlfnc.h> on Posix
platforms. This is not a major complaint, and I realize that this
can be difficult to avoid in a header-only library.

>    - Documentations

Generally good and well-structured.

Some suggestions and minor comments:

Some sentences are difficult to read due to their grammar. For example,
the "Linking plugin into the executable" tutorial, or the the second
and third questions in the FAQ.

Tutorial "Searching for a symbol in multiple plugins" should mention
that previously loaded libraries are unloaded on subsequent loads (this
is mentioned in the reference documentation, but it would be nice to
have here as well.)

Tutorial "Symbol shadowing problem (Linux)" should start by describing
the problem it solves.

Reference BOOST_DLL_SECTION says: ''Permissions Can be "read" or
"write" (without quotes!).'' I suggest that you list the names as
code-blocks instead (e.g. use `read` in the .qbk)

Reference runtime_symbol_info: When I first saw the *_location names
I thought that they referred to the addresses (especially
this_line_location.) I suggest that they be renamed to *_path.

Reference dll::import and dll::import_alias uses string_ref for
func_name, yet states that func_name must be a "Null-terminated C or
C++ mangled name of the function to import. Can handle std::string,
char*, const char*."

FAQ ends with "[endsect]"

The design rationale and the FAQ seems to be in disagreement about ABI
portability.

>    - Tests

Did not look.

> - How much effort did you put into your evaluation?

A couple of hours reading the documentation and browsing the code.

> - Did you attempt to use the library? On what systems and compilers?

No.