Message ID | e21ee73e-fa91-2c95-2c0e-8648b299216b@gmail.com |
---|---|
State | New |
Headers | show |
Series | Protect from comma operator overload | expand |
On 16 April 2018 at 20:52, François Dumont wrote: > Hi > > While working on something else on libstdc++ I started having a test > failing because of the missing comma overload protection in deque.tcc. So I > looked for other similar places in the code and here is a patch to fix the > places I found. > > Let me know if it is still time to commit. The changes look right, but please add new tests to demonstrate the code that used to fail. You can use <testsuite_iterators.h> because the iterator types defined in there have deleted comma operators that should cause errors in these places.
On 16 April 2018 at 21:08, Jonathan Wakely wrote: > On 16 April 2018 at 20:52, François Dumont wrote: >> Hi >> >> While working on something else on libstdc++ I started having a test >> failing because of the missing comma overload protection in deque.tcc. So I >> looked for other similar places in the code and here is a patch to fix the >> places I found. >> >> Let me know if it is still time to commit. > > The changes look right, but please add new tests to demonstrate the > code that used to fail. > > You can use <testsuite_iterators.h> because the iterator types defined > in there have deleted comma operators that should cause errors in > these places. Something like this (but in four separate tests): #include <deque> #include <list> #include <vector> #include <testsuite_iterators.h> int main() { using namespace __gnu_test; int a[1] = {}; test_container<int, input_iterator_wrapper> t(a, a+1); std::deque<int> d; d.assign(t.begin(), t.end()); std::list<int> l; l.assign(t.begin(), t.end()); std::vector<int> v; v.assign(t.begin(), t.end()); std::vector<bool> b; b.assign(t.begin(), t.end()); } Given how rare it is for real code to overload the comma operator, and that nobody has reported these bugs, I think this can wait for after GCC 8.
Hi Here is the patch I eventually would like to commit. Tested under Linux x86_64, ok for trunk ? François On 17/04/2018 22:34, François Dumont wrote: > Yes, I also think there is no rush to fix this issue. > > I had already written a test for a different purpose using the > input_iterator_wrapper. This is why I detected this std::deque issue. > > On 16/04/2018 22:16, Jonathan Wakely wrote: >> On 16 April 2018 at 21:08, Jonathan Wakely wrote: >>> On 16 April 2018 at 20:52, François Dumont wrote: >>>> Hi >>>> >>>> While working on something else on libstdc++ I started having >>>> a test >>>> failing because of the missing comma overload protection in >>>> deque.tcc. So I >>>> looked for other similar places in the code and here is a patch to >>>> fix the >>>> places I found. >>>> >>>> Let me know if it is still time to commit. >>> The changes look right, but please add new tests to demonstrate the >>> code that used to fail. >>> >>> You can use <testsuite_iterators.h> because the iterator types defined >>> in there have deleted comma operators that should cause errors in >>> these places. >> Something like this (but in four separate tests): >> >> #include <deque> >> #include <list> >> #include <vector> >> #include <testsuite_iterators.h> >> >> int main() >> { >> using namespace __gnu_test; >> int a[1] = {}; >> test_container<int, input_iterator_wrapper> t(a, a+1); >> >> std::deque<int> d; >> d.assign(t.begin(), t.end()); >> >> std::list<int> l; >> l.assign(t.begin(), t.end()); >> >> std::vector<int> v; >> v.assign(t.begin(), t.end()); >> >> std::vector<bool> b; >> b.assign(t.begin(), t.end()); >> } >> >> >> Given how rare it is for real code to overload the comma operator, and >> that nobody has reported these bugs, I think this can wait for after >> GCC 8. >> > diff --git a/libstdc++-v3/include/bits/deque.tcc b/libstdc++-v3/include/bits/deque.tcc index fe96330..1337394 100644 --- a/libstdc++-v3/include/bits/deque.tcc +++ b/libstdc++-v3/include/bits/deque.tcc @@ -297,7 +297,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER std::input_iterator_tag) { iterator __cur = begin(); - for (; __first != __last && __cur != end(); ++__cur, ++__first) + for (; __first != __last && __cur != end(); ++__cur, (void)++__first) *__cur = *__first; if (__first == __last) _M_erase_at_end(__cur); diff --git a/libstdc++-v3/include/bits/list.tcc b/libstdc++-v3/include/bits/list.tcc index 22538c9..e90d957 100644 --- a/libstdc++-v3/include/bits/list.tcc +++ b/libstdc++-v3/include/bits/list.tcc @@ -312,7 +312,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER iterator __first1 = begin(); iterator __last1 = end(); for (; __first1 != __last1 && __first2 != __last2; - ++__first1, ++__first2) + ++__first1, (void)++__first2) *__first1 = *__first2; if (__first2 == __last2) erase(__first1, __last1); diff --git a/libstdc++-v3/include/bits/stl_bvector.h b/libstdc++-v3/include/bits/stl_bvector.h index 05e9ff2..4b52c17 100644 --- a/libstdc++-v3/include/bits/stl_bvector.h +++ b/libstdc++-v3/include/bits/stl_bvector.h @@ -1229,7 +1229,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER std::input_iterator_tag) { iterator __cur = begin(); - for (; __first != __last && __cur != end(); ++__cur, ++__first) + for (; __first != __last && __cur != end(); ++__cur, (void)++__first) *__cur = *__first; if (__first == __last) _M_erase_at_end(__cur); diff --git a/libstdc++-v3/include/bits/vector.tcc b/libstdc++-v3/include/bits/vector.tcc index 2516e35..f33da03 100644 --- a/libstdc++-v3/include/bits/vector.tcc +++ b/libstdc++-v3/include/bits/vector.tcc @@ -273,7 +273,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER { pointer __cur(this->_M_impl._M_start); for (; __first != __last && __cur != this->_M_impl._M_finish; - ++__cur, ++__first) + ++__cur, (void)++__first) *__cur = *__first; if (__first == __last) _M_erase_at_end(__cur); diff --git a/libstdc++-v3/testsuite/23_containers/deque/modifiers/assign/1.cc b/libstdc++-v3/testsuite/23_containers/deque/modifiers/assign/1.cc new file mode 100644 index 0000000..fbab09b --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/deque/modifiers/assign/1.cc @@ -0,0 +1,36 @@ +// Copyright (C) 2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +#include <deque> + +#include <testsuite_iterators.h> +#include <testsuite_hooks.h> + +typedef __gnu_test::test_container<int, __gnu_test::input_iterator_wrapper> + input_iterator_seq; + +int main() +{ + std::deque<int> d; + + int array[] { 0, 1, 2 }; + input_iterator_seq seq(array, array + 3); + + d.assign(seq.begin(), seq.end()); + VERIFY( d.size() == 3 ); + return 0; +} diff --git a/libstdc++-v3/testsuite/23_containers/list/modifiers/assign/1.cc b/libstdc++-v3/testsuite/23_containers/list/modifiers/assign/1.cc new file mode 100644 index 0000000..c5fde47 --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/list/modifiers/assign/1.cc @@ -0,0 +1,36 @@ +// Copyright (C) 2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +#include <list> + +#include <testsuite_iterators.h> +#include <testsuite_hooks.h> + +typedef __gnu_test::test_container<int, __gnu_test::input_iterator_wrapper> + input_iterator_seq; + +int main() +{ + std::list<int> l; + + int array[] { 0, 1, 2 }; + input_iterator_seq seq(array, array + 3); + + l.assign(seq.begin(), seq.end()); + VERIFY( !l.empty() ); + return 0; +} diff --git a/libstdc++-v3/testsuite/23_containers/vector/bool/modifiers/assign/1.cc b/libstdc++-v3/testsuite/23_containers/vector/bool/modifiers/assign/1.cc new file mode 100644 index 0000000..833201b --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/vector/bool/modifiers/assign/1.cc @@ -0,0 +1,41 @@ +// Copyright (C) 2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +#include <vector> + +#include <testsuite_iterators.h> +#include <testsuite_hooks.h> + +void test01() +{ + typedef __gnu_test::test_container<bool, __gnu_test::input_iterator_wrapper> + input_iterator_seq; + + std::vector<bool> bv; + + bool array[] { false, true, true }; + input_iterator_seq seq(array, array + 3); + + bv.assign(seq.begin(), seq.end()); + VERIFY( bv.size() == 3 ); +} + +int main() +{ + test01(); + return 0; +} diff --git a/libstdc++-v3/testsuite/23_containers/vector/modifiers/assign/1.cc b/libstdc++-v3/testsuite/23_containers/vector/modifiers/assign/1.cc new file mode 100644 index 0000000..ca7b125 --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/vector/modifiers/assign/1.cc @@ -0,0 +1,41 @@ +// Copyright (C) 2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +#include <vector> + +#include <testsuite_iterators.h> +#include <testsuite_hooks.h> + +void test01() +{ + typedef __gnu_test::test_container<int, __gnu_test::input_iterator_wrapper> + input_iterator_seq; + + std::vector<int> v; + + int array[] { 0, 1, 2 }; + input_iterator_seq seq(array, array + 3); + + v.assign(seq.begin(), seq.end()); + VERIFY( v.size() == 3 ); +} + +int main() +{ + test01(); + return 0; +}
On 02/05/18 07:11 +0200, François Dumont wrote: >Hi > > Here is the patch I eventually would like to commit. > > Tested under Linux x86_64, ok for trunk ? Ah yes, this is the one I remember thinking would be OK for stage 1. OK for trunk, thanks.
diff --git a/libstdc++-v3/include/bits/deque.tcc b/libstdc++-v3/include/bits/deque.tcc index fe96330..1337394 100644 --- a/libstdc++-v3/include/bits/deque.tcc +++ b/libstdc++-v3/include/bits/deque.tcc @@ -297,7 +297,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER std::input_iterator_tag) { iterator __cur = begin(); - for (; __first != __last && __cur != end(); ++__cur, ++__first) + for (; __first != __last && __cur != end(); ++__cur, (void)++__first) *__cur = *__first; if (__first == __last) _M_erase_at_end(__cur); diff --git a/libstdc++-v3/include/bits/list.tcc b/libstdc++-v3/include/bits/list.tcc index 22538c9..e90d957 100644 --- a/libstdc++-v3/include/bits/list.tcc +++ b/libstdc++-v3/include/bits/list.tcc @@ -312,7 +312,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER iterator __first1 = begin(); iterator __last1 = end(); for (; __first1 != __last1 && __first2 != __last2; - ++__first1, ++__first2) + ++__first1, (void)++__first2) *__first1 = *__first2; if (__first2 == __last2) erase(__first1, __last1); diff --git a/libstdc++-v3/include/bits/stl_bvector.h b/libstdc++-v3/include/bits/stl_bvector.h index 05e9ff2..4b52c17 100644 --- a/libstdc++-v3/include/bits/stl_bvector.h +++ b/libstdc++-v3/include/bits/stl_bvector.h @@ -1229,7 +1229,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER std::input_iterator_tag) { iterator __cur = begin(); - for (; __first != __last && __cur != end(); ++__cur, ++__first) + for (; __first != __last && __cur != end(); ++__cur, (void)++__first) *__cur = *__first; if (__first == __last) _M_erase_at_end(__cur); diff --git a/libstdc++-v3/include/bits/vector.tcc b/libstdc++-v3/include/bits/vector.tcc index 2516e35..f33da03 100644 --- a/libstdc++-v3/include/bits/vector.tcc +++ b/libstdc++-v3/include/bits/vector.tcc @@ -273,7 +273,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER { pointer __cur(this->_M_impl._M_start); for (; __first != __last && __cur != this->_M_impl._M_finish; - ++__cur, ++__first) + ++__cur, (void)++__first) *__cur = *__first; if (__first == __last) _M_erase_at_end(__cur);