On Sun, Jul 26, 2015 at 5:54 PM, Glen Fernandes wrote:

Some feedback:

Thanks for the feedback!

1. For a given type T whose constructor can throw, and you have a loop constructing n objects of type T via std::allocator_traits<T>::construct - should you not be handling that potential construction failure? (i.e. Destroying those elements that were constructed before the constructor that throws, before re-throwing)

[snip]

(My apologies if you already do this, and I missed it).

There are two such constructors: explicit devector(size_type n, const Allocator& allocator = Allocator()) devector(size_type n, const T& value, const Allocator& allocator = Allocator()) The first one has a `construction_guard` which takes care cleanup on failure. The second one calls `opt_copy` at the end, which also has the same kind of guard if needed. The failure scenario for both constructor (and for other metods) is tested: https://github.com/erenon/container/blob/devector/test/devector_test.cpp#L22... https://github.com/erenon/container/blob/devector/test/devector_test.cpp#L26... Did I missed something?

2. size_type being unsigned int instead of std::size_t - I understand the motivation, but could this not be customized - e.g. via another policy? Or should it not instead be based on std::allocator_traits<Allocator>::size_type instead?

We could provide a way. However, using allocator_traits is not the best option, I guess, since std::allocator::size_type is size_t, and for most users unsigned would be enough. GrowthPolicy could be used, however.

3. I understand deriving from Allocator for EBO, but should that type be std::allocator_traits<Allocator>::rebind_alloc<T> instead? (And have the 'allocator_type' member typedef also be this rebound type) .

What would this buy us? The user is expected to provide an allocator for T. As far as I know, the usecase of rebind is to allocate other types with the provided allocator, e.g: nodes, in node based containers. I'm not against the idea, I just don't know how is it better. Thanks, Benedek

On Sun, Jul 26, 2015 at 11:13 PM, Glen Fernandes wrote:

It's up to you. It may be more intuitive having a separate policy, instead of turning GrowthPolicy into GrowthAndSizePolicy.

I'd like to avoid having too many policies. One could argue, that the chosen size type limits the maximum growth, thus it fits into the concept of the GrowthPolicy. I'll think on it.

It only enables someone to supply an allocator type without having to supply the rebound allocator type themselves.

e.g. The following code: template<class A> struct T { container1 c1; container2 c2; container3 c3; };

In this case, one should write: template <class A> struct T { template <class Y> using AY = typename std::allocator_traits<A>::rebind_alloc<Y>; container1 c1; // etc. }

.... looks cleaner than: template<class A> struct T { container1 c1; container2 c2; container3 c3; };

I know that some standard library vendors do this already: e.g. Using Dinkumware with Intel C++, the following will compile { std::vector v; v.push_back(1); } Others do not: e.g. Using libc++ with Clang there is a static_assert "Allocator::value_type must be the same as value_type".

It's up to you. Personally I think the former behavior is better, and if it isn't tolerable by the standard, it should be.

I think this version obscures intent and behaviour, and encourages sloppy use. It looks like the container uses allocator<X>, but really, it uses allocator<Y>, which is not nice. Or maybe it's just me being too strict. Thanks, Benedek

On Sun, Jul 26, 2015 at 11:53 AM, Benedek Thaler wrote:

On Sun, Jul 26, 2015 at 5:54 PM, Glen Fernandes wrote:

...

Some feedback:

Thanks for the feedback!

...

1. For a given type T whose constructor can throw, and you have a loop constructing n objects of type T via std::allocator_traits<T>::construct - should you not be handling that potential construction failure? (i.e. Destroying those elements that were constructed before the constructor that throws, before re-throwing)

[snip]

(My apologies if you already do this, and I missed it).

There are two such constructors:

explicit devector(size_type n, const Allocator& allocator = Allocator()) devector(size_type n, const T& value, const Allocator& allocator = Allocator())

The first one has a `construction_guard` which takes care cleanup on failure. The second one calls `opt_copy` at the end, which also has the same kind of guard if needed.

The failure scenario for both constructor (and for other metods) is tested:

https://github.com/erenon/container/blob/devector/test/devector_test.cpp#L22...

https://github.com/erenon/container/blob/devector/test/devector_test.cpp#L26...

Did I missed something?

An alternate approach, if you can require C++11, is to use "delegating constructors". See libc++'s dynarray implementation for an example of this. ( https://android.googlesource.com/platform/ndk/+/5de42e6621b3d0131472c3f8838b..., default constructor at line 138, other constructor at line 231) Basic idea: * Call the default constructor first. That can be noexcept. * Insert the elements, one by one, maintaining the class invariants. * If one of the insertions throws, then the destructor will be called, and it can do the cleanup. -- Marshall

On Sun, Jul 26, 2015 at 9:35 PM, Matthias Vallentin < vallentin@icsi.berkeley.edu> wrote:

From the documentation, it sounds like a devector is strictly superior to std::vector, as it offers a superset of functionality. It would be great if you could discuss a bit more the trade-offs, for example, in which cases a devector is inferior to a std::vector performance-wise.

Thanks for the feedback! The only compromise I know about is using `unsigned` vs. `std::size_t` as `size_type`, described in the docs: " The devector class template intends to be a drop in replacement of std:: vector. However, to keep its size slim (16 bytes on 32 bit, 24 bytes on a typical 64 bit architecture), its size_type is defined as unsigned int which reduces max_size() to UINT_MAX. " Assuming no small buffer optimization is used, devector should have the same performance as std::vector. Unfortunately, I have no benchmarks to prove it, yet. However, even if the current implementation falls behind std::vector, that only means I missed some optimization, it shouldn't be something inherent, to my understanding. Benedek