Re: [boost] proposal a movable_initializer_list

Subject: Re: [boost] proposal a movable_initializer_list
From: Sebastian Redl (sebastian.redl_at_[hidden])
Date: 2014-09-15 08:08:56

On 15 Sep 2014, at 9:39, 何子杰Hzj_jie <hzj_jie_at_[hidden]> wrote:

> hi, all,
> 
> So I proposal a movable_initializer_list, it accepts two ways to construct, with an initializer_list<T*> or initializer_list<T>, the prototype is,
> 
> The implementation of T* overloads is something like
> 
> movable_initializer_list<std::initializer_list<T*> x)
> 
> {
> 
>    for(auto it : x)
> 
>    {
> 
>        assert((*it) != nullptr);
> 
>        v.push_back(std::move(**it));
> 
>        delete *it;
> 
>    }
> }

Should be

for (auto p : x) {
  v.push_back(std::move(*p));
  delete p;
}

because of how for-range works. Seems an incomplete manual change from your published source.

However, this is highly problematic because:
- There is absolutely no way to make this exception-safe. The intended usage is obviously:
  SomeType st = { new Foo(123), new Foo(456), new Foo(789) };
  But if any constructor here throws, a lot of memory is leaked, and destructors are not called.
- If efficiency is your goal, I somehow doubt that an additional dynamic allocation per element for the initial list, then building a vector element by element, is the best way to achieve it.
- The interface to this is just horrible. Looking at your test case, there is an absurd amount of boilerplate. Here’s what we really want:
    SomeType st{ Foo(123), Foo(456), Foo(789) };
  Here’s what your thing forces me to do:
    auto pointers = { new Foo(123), new Foo(456), new Foo(789) };
    SomeType st(pointers); // Absolute best case: no ambiguity in overload resolution - actually rather doubtful.
  If you do it that way, you might as well do it this way:
    Foo[] foos = [ Foo(123), Foo(456), Foo(789) }; // No dynamic allocation
    SomeType st(make_move_iterator(begin(foos)), make_move_iterator(end(foos)));

More efficient, and you could make helpers that combine the make_move_iterator and begin/end calls to reduce boilerplate.

> 
> While the implementation of T overloads is something like
> 
> moveable_initializer_list<std::initializer_list<T> x)
> {
> 
>    for(auto it : x) v.push_back(move(*it));
> }

Again, that’s not how for-range works. What you want is

for (auto&& e : x) v.push_back(std::move(e));

But that doesn’t actually work, because a std::initializer_list never gives you non-const access to its elements.

I notice that your test case doesn’t test this constructor.

Sebastian