Re: Improve insert/emplace robustness to self insertion

2016-07-12 Thread Jonathan Wakely
On 08/07/16 17:38 +0100, Jonathan Wakely wrote: On 06/07/16 21:46 +0200, François Dumont wrote: Don't you plan to add it to the testsuite ? Done with the attached aptch. Just for completeness, I'm also adding the example from LWG 2164, which is related. Tested x86_64, committed to trunk.

Re: Improve insert/emplace robustness to self insertion

2016-07-10 Thread Jonathan Wakely
On 09/07/16 20:29 -0400, David Edelsohn wrote: On Sat, Jul 9, 2016 at 7:59 PM, Jonathan Wakely wrote: On 09/07/16 13:47 -0400, David Edelsohn wrote: This patch has caused some new libstdc++ testsuite failures on AIX. Which patch? My last patch only added a new test,

Re: Improve insert/emplace robustness to self insertion

2016-07-09 Thread David Edelsohn
On Sat, Jul 9, 2016 at 7:59 PM, Jonathan Wakely wrote: > On 09/07/16 13:47 -0400, David Edelsohn wrote: >> >> This patch has caused some new libstdc++ testsuite failures on AIX. > > > Which patch? > > My last patch only added a new test, that can't have caused failures > in

Re: Improve insert/emplace robustness to self insertion

2016-07-09 Thread Jonathan Wakely
On 09/07/16 13:47 -0400, David Edelsohn wrote: This patch has caused some new libstdc++ testsuite failures on AIX. Which patch? My last patch only added a new test, that can't have caused failures in unrelated tests. https://gcc.gnu.org/ml/libstdc++-cvs/2016-q3/msg00021.html FAIL:

Re: Improve insert/emplace robustness to self insertion

2016-07-09 Thread David Edelsohn
This patch has caused some new libstdc++ testsuite failures on AIX. FAIL: 23_containers/list/debug/insert4_neg.cc (test for excess errors) Excess errors: /tmp/20160708/powerpc-ibm-aix7.1.0.0/libstdc++-v3/include/debug/formatter.h:387:7: error: __gnu_debug::_Error_formatter&

Re: Improve insert/emplace robustness to self insertion

2016-07-08 Thread Jonathan Wakely
On 06/07/16 21:46 +0200, François Dumont wrote: Don't you plan to add it to the testsuite ? Done with the attached aptch. On my side I rebase part of my patch to reorganize a little bit code. I reintroduced _M_realloc_insert which isolates the code of _M_insert_aux used when we need to

Re: Improve insert/emplace robustness to self insertion

2016-07-06 Thread Jonathan Wakely
On 06/07/16 21:46 +0200, François Dumont wrote: On 05/07/2016 12:47, Jonathan Wakely wrote: On 04/07/16 15:55 +0100, Jonathan Wakely wrote: I'm getting nervous about the smart insertion trick to avoid making a copy, I have a devious testcase in mind which will break with that change. I'll

Re: Improve insert/emplace robustness to self insertion

2016-07-06 Thread François Dumont
On 05/07/2016 12:47, Jonathan Wakely wrote: On 04/07/16 15:55 +0100, Jonathan Wakely wrote: I'm getting nervous about the smart insertion trick to avoid making a copy, I have a devious testcase in mind which will break with that change. I'll share the testcase later today. Here's a testcase

Re: Improve insert/emplace robustness to self insertion

2016-07-04 Thread Jonathan Wakely
On 02/07/16 08:37 +0200, François Dumont wrote: I haven't consider in this patch your remark about using allocator to build instance so don't hesitate to commit what you want and I will rebase. Here's what I've committed to trunk. I'm getting nervous about the smart insertion trick to avoid

Re: Improve insert/emplace robustness to self insertion

2016-07-02 Thread François Dumont
On 01/07/2016 11:54, Jonathan Wakely wrote: On 30/06/16 21:51 +0200, François Dumont wrote: On 29/06/2016 23:30, Jonathan Wakely wrote: iterator insert(const_iterator __position, value_type&& __x) { return emplace(__position, std::move(__x)); } That's suboptimal, since in the

Re: Improve insert/emplace robustness to self insertion

2016-07-01 Thread Jonathan Wakely
On 01/07/16 10:54 +0100, Jonathan Wakely wrote: On 30/06/16 21:51 +0200, François Dumont wrote: On 29/06/2016 23:30, Jonathan Wakely wrote: iterator insert(const_iterator __position, value_type&& __x) { return emplace(__position, std::move(__x)); } That's suboptimal, since in the

Re: Improve insert/emplace robustness to self insertion

2016-07-01 Thread Jonathan Wakely
On 30/06/16 21:51 +0200, François Dumont wrote: On 29/06/2016 23:30, Jonathan Wakely wrote: iterator insert(const_iterator __position, value_type&& __x) { return emplace(__position, std::move(__x)); } That's suboptimal, since in the general case we need an extra construction for

Re: Improve insert/emplace robustness to self insertion

2016-06-30 Thread François Dumont
On 29/06/2016 23:30, Jonathan Wakely wrote: On 29/06/16 21:36 +0100, Jonathan Wakely wrote: On 29/06/16 21:43 +0200, François Dumont wrote: As asked here is now a patch to only fix the robustness issue. The consequence is that it is reverting the latest optimization as, without smart algo, we

Re: Improve insert/emplace robustness to self insertion

2016-06-29 Thread Jonathan Wakely
On 29/06/16 21:43 +0200, François Dumont wrote: I tried those changes too but started having failing tests in vector/ext_pointer so prefer to not touch that for the moment. I think the compilation error was coming from the change of begin() + (__position - cbegin()) into begin() + __n because

Re: Improve insert/emplace robustness to self insertion

2016-06-29 Thread Jonathan Wakely
On 29/06/16 21:36 +0100, Jonathan Wakely wrote: On 29/06/16 21:43 +0200, François Dumont wrote: As asked here is now a patch to only fix the robustness issue. The consequence is that it is reverting the latest optimization as, without smart algo, we always need to do a copy to protect against

Re: Improve insert/emplace robustness to self insertion

2016-06-29 Thread Jonathan Wakely
On 29/06/16 21:43 +0200, François Dumont wrote: As asked here is now a patch to only fix the robustness issue. The consequence is that it is reverting the latest optimization as, without smart algo, we always need to do a copy to protect against insertion of values contained in the vector as

Re: Improve insert/emplace robustness to self insertion

2016-06-29 Thread François Dumont
On 29/06/2016 11:10, Jonathan Wakely wrote: On 28/06/16 21:59 +0200, François Dumont wrote: @@ -303,16 +301,20 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER emplace(const_iterator __position, _Args&&... __args) { const size_type __n = __position - begin(); It looks like this should

Re: Improve insert/emplace robustness to self insertion

2016-06-29 Thread Paolo Carlini
Hi, On 29/06/2016 12:07, Jonathan Wakely wrote: On 29/06/16 11:35 +0200, Paolo Carlini wrote: Hi, On 29/06/2016 10:57, Jonathan Wakely wrote: On 28/06/16 21:59 +0200, François Dumont wrote: + if (_M_data_ptr(__position.base()) <= __ptr + && __ptr <

Re: Improve insert/emplace robustness to self insertion

2016-06-29 Thread Jonathan Wakely
On 29/06/16 11:35 +0200, Paolo Carlini wrote: Hi, On 29/06/2016 10:57, Jonathan Wakely wrote: On 28/06/16 21:59 +0200, François Dumont wrote: + if (_M_data_ptr(__position.base()) <= __ptr + && __ptr < _M_data_ptr(this->_M_impl._M_finish - 1)) This is undefined behaviour. If the

Re: Improve insert/emplace robustness to self insertion

2016-06-29 Thread Paolo Carlini
Hi, On 29/06/2016 10:57, Jonathan Wakely wrote: On 28/06/16 21:59 +0200, François Dumont wrote: + if (_M_data_ptr(__position.base()) <= __ptr + && __ptr < _M_data_ptr(this->_M_impl._M_finish - 1)) This is undefined behaviour. If the object is not contained in the vector then you

Re: Improve insert/emplace robustness to self insertion

2016-06-29 Thread Jonathan Wakely
On 28/06/16 21:59 +0200, François Dumont wrote: @@ -303,16 +301,20 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER emplace(const_iterator __position, _Args&&... __args) { const size_type __n = __position - begin(); It looks like this should use __position - cbegin(), to avoid an

Re: Improve insert/emplace robustness to self insertion

2016-06-29 Thread Jonathan Wakely
On 28/06/16 21:59 +0200, François Dumont wrote: template void vector<_Tp, _Alloc>:: -_M_insert_aux(iterator __position, const _Tp& __x) +_M_insert_value_aux(iterator __position, const _Tp& __x) #endif { - if (this->_M_impl._M_finish != this->_M_impl._M_end_of_storage)

Improve insert/emplace robustness to self insertion

2016-06-28 Thread François Dumont
Hi Following below discussion here is a patch to make sure we correctly deal with insertion of instances stored into the vector itself. When we don't need reallocation I have implemented the libc++ trick to avoid an intermediate copy. I did my best to detect when a value_type