Re: [review] [STLInterfaces] STLInterfaces review starts today

From: Daniela Engert (dani_at_[hidden])
Date: 2019-12-23 20:48:16

Hello,

this is my mini review of STLInterfaces.

> - Your name

Daniela Engert

> - Your knowledge of the problem domain

I've written some iterators and containers in the past in our companies
in-house production code base. Currently I am about to write another
container with very particular features where this library would be very
handy by saving me from writing much boilerplate code.

> - Whether you believe the library should be accepted into Boost (be clear
> about this)

I vote to *CONDITIONALLY* ACCEPT the library after fixing the problem
that I will point out below

> In addition, you are strongly encouraged to answer the following questions:
> - What is your evaluation of the library's
>   * Design?
It looks solid and pretty much matches my expectations in this regard
>   * Implementation?

 - I'm not too fond of the BOOST_STL_INTERFACES_DOXYGEN clutter, but I
can live with it

- there are two instances of '#if 0' in container_interface.hpp which
should be removed

- I like it pretty much. In particular I want to point out that this is
one of the few Boost libraries that caused no compiler warnings in msvc
14.1 at warning level 4 from the get-go.

>   * Documentation?

It's ok. If one is familiar with may other Boost library documentations
she will eventually find what one is looking for. The uninitated might
find it hard to get into this stuff and dismiss the library as too
mysterious to him to be useful.

>   * Tests?
The library is lacking the integration into Boost's test framework.
Therefore I didn't run any tests. Sorry, no CMake on this computer.
>   * Usefulness?
High. I really want to use it.
> - Did you attempt to use the library? If so:
>   * Which compiler(s)?
VS2019 with msvc 19.2.4,  msvc 19.1.9, msvc 19.0.3
>   * What was the experience? Any problems?

I have been playing with the static_vector example as a starter because
it engages most parts of the library.

It doesn't compile with VS2015 (msvc 19.0.3) because of problems with
the 'noexcept()' operator related to function overloading. Even without
this problem it wouldn't compile because of the unconditional use of
__has_include which is not part of C++ 14!

----
Condition 1 for acceptance: make use of __has_include conditional. There
are conforming compilers that don't implement it. Or consider the
removal of the optional dependency on 3rd-party non-Boost libraries.
----
I had no problems with VS2017 (msvc 19.1.9)Â  in both debug and release
mode. I didn't check older versions of msvc 19.1.x
With the current version of VS2019 (i.e. 19.4) I experience the
following problems:
 - the static_vector example doesn't compile in C++ 14 mode because of
the missing include of <tuple> (std::tie in line 222 is undefined).
Alas, when compiled in C++ 17 mode all is fine.
 - the same source also compiles in C++ 2a mode when the compiler is
instructed not to reveal the true value of __cplusplus. But when it does
(by means of /Zc:__cplusplus) hell breaks lose. In this case the
compiler defines __cplusplus to 201705L and __cpp_lib_concepts to
201806L. This enables the compilation of the section of lines 557-695 in
iterator_interface.hpp and the section of lines 698-996 in
container_interface.hpp, thereby exposing
 -- the syntax error in line  731 of container_interface.hpp
 -- the use of entities from the empty namespace 'v2_dtl' in various
places in container_interface.hpp (first occurence in line 750)
 -- the compilation failure in iterator_interface lines 618-621, most
likely due to the problem that Andrzej has already pointed out in
https://github.com/tzlaine/stl_interfaces/issues/13
It doesn't matter if the v1 or the v2 interface is used here (for
obvious reasons).
----
Condition 2 for acceptance: address the syntactical problems
Condition 3 for acceptance: address the missing entities from namespace
'v2_dtl'
Condition 4 for acceptance: fix the compilation failures when C++ 20
concepts are enabled (like with msvc19.2.4 in C++ 2a mode and full
conformance turned on) and the respective code paths are engaged.
----
> - How much effort did you put into your evaluation of the review?
>
About 5 hours studying the documentation and implementation, and playing
with various versions of msvc to figure out all of the problems
mentioned above.
Ciao
   Dani