Boost logo

Boost :

Subject: Re: [boost] [SPAM] Re: [review] The review of Boost.DoubleEnded starts today: September 21 - September 30
From: Benedek Thaler (thalerbenedek_at_[hidden])
Date: 2017-10-07 10:35:31

On Sat, Oct 7, 2017 at 1:54 AM, Louis Dionne via Boost <
boost_at_[hidden]> wrote:

> I vote to REJECT the library as currently proposed. However,
> I could see parts of the functionality provided by this library
> be added to Boost.Container.

Thanks for your review! (And the PR fixing typos)

> > - What is your evaluation of the design?
> Major points:
> - The first thing I thought when I saw the library was: why is this not
> an addition to Boost.Container?

I suppose this is now explained above.

- I don't see the use for the uninitialized constructors and
> `unsafe_push_back`/`unsafe_push_front`. I mean, I do understand the
> use _in theory_, but I'd really like to see a good motivation for them
> with a more concrete use case. This is pretty dangerous and tricky to
> work with, so I think this deserves a good justification. I can't see
> myself using this container or recommending anyone to use the container
> without this.

Again, benchmarks above - they show some things are faster this way. On the
other hand, I absolutely get your point.

> - The design rationale for `devector` at the beginning of the documentation
> is slightly misleading. `devector` can't be a drop-in replacement for
> `std::vector`, because of iterator invalidation guarantees of move
> operations (see

I suppose this happens only with a small buffer in effect.

- In section "devector usage":
> > Here, while push_back calls make use of the small buffer, push_front
> > results in an allocation: Push operations do not shift elements to
> > avoid breaking the promise of reserve.
> The "promise of reserve" needs to be explained; what does this mean?

d.reserve(d.size() + 10); // next 10 push_back will not allocate

> - How do those containers play with custom Allocators (I'm specifically
> thinking about those that provide fancy pointers)? The documentation
> should mention this.

Probably not supported.

> - I would really like to see actual benchmarks, even if this needs coming
> up with more realistic use cases. IMO, it is not acceptable for a library
> tailored for performance (and having unsafe operations for that purpose)
> to provide only a listing of assembly at `-O2`.

See above - tough I'm skeptic regarding micro-benchmarks. Too easy to get
them wrong, without knowing it, no way to prove one got them right.

> - Several minor things like typos and weird formulations. I submitted a PR
> for some of those, but the situation could still be improved. This is not
> a big deal, though, as it would be trivial to fix.

Thanks, merged.


Boost list run by bdawes at, gregod at, cpdaniel at, john at