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

From: Zach Laine (whatwasthataddress_at_[hidden])
Date: 2019-12-26 18:35:07

On Thu, Dec 19, 2019 at 1:12 PM Krystian Stasiowski via Boost <
boost_at_[hidden]> wrote:

> My mini-review will focus on the container_interface.
>
> On Tue, Dec 10, 2019 at 7:21 PM Barrett Adair via Boost <
> boost_at_[hidden]> wrote:
>
> > - Your name
> >
>
> Krystian Stasiowski
>

Thanks for the review, Krystian.

> > - What is your evaluation of the library's
> >   * Design?
> >
>
> Overall, its pretty solid. One notable thing that comes to mind is that
> this should be made SFINAE friendly, ie disable certain overloads when the
> required functions are not defined in the derived class.

This is the case, within the limits of the sequence container concept.
That is, if there is some operation (say, insert()) that is required by all
sequence containers, it would be inappropriate to SNIFAE-away such an
operation if the derived class does not support it.  To do so would mean
that you end up with a container_interface-derived type that is a sequence
container.  However, there are optional sets of requirements for sequence
containers (the reversible container requirements are one example).  For
those optional requirements, I believe that they are all SFINAE-friendly.
If you find a counterexample, that's a bug; please report it.

> Additionally,
> providing a destructor is a big no-no in my opinion, and could lead to
> elements having their destructor called twice. Infact, the documentation
> says that the user must provide a destructor that destroys all the elements
> of the container, but the destructor provided by the container_interface
> clears the container anyways, which leads to UB.
>

Agreed.  As stated in another thread, I'll remove the destructor
implementation.

>
> >   * Implementation?
> >
>
> This implementation is pretty solid.
>
>
> >   * Documentation?
> >
>
> The documentation is _alright_. One thing I noticed is there is no cohesive
> list of which functions are provided by the interface when others are
> implemented by the user. It makes it quite a pain to navigate and even
> confusing.
>

Could you point specifically to the part of the docs that you find hard to
follow?  There is already this sort of cohesive list in the case of
container_interface.  For the iterators, there is not.  In both cases, the
list of operations you end up with is well documented on cppreference,com
and eel.is, because it comes from the standard.

Zach