sorry, my fault of the mistake of for-range usage, you are correct, I am using iterator in my implementation. The efficiency I am taking care is the difference between copy-construction and move-construction. It will be significant if we are using lambda expression. Surely we still have something else to improve, say, provide the size of the vector at construction. The way using movable_initializer_list in the test case is not typical, indeed I am trying another not related issue. <the order of the move-constructions are not expected, i.e. line 46 will throw errors by g++, but no problem by msvc> Usually if a class A accept movable_initializer_list<T> as the parameter of a constructor, we only need A({T(), T()}), i.e. same as initializer_list. You properly would like to have a look at the event class, which is the typical scenario of movable_initializer_list, at https://github.com/Hzj-jie/osi/blob/master/delegates/event.hpp, with a test at https://github.com/Hzj-jie/osi/blob/master/utt_cases/delegates/event_test.hp.... briefly it uses movable_initializer_list to avoid a copy of std::function instances, but can provide a way to call constructor with { } syntax. .Hzj_jie From: Sebastian Redl Sent: Monday, September 15, 2014 8:09 PM To: boost@lists.boost.org On 15 Sep 2014, at 9:39, 何子杰Hzj_jie <hzj_jie@hotmail.com> 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 _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost