Re: PR 58148 patch
Hi, On 08/29/2013 09:37 PM, François Dumont wrote: Indeed, I check the Standard about const_pointer, so here is another attempt. Tested under Linux x86_64. 2013-08-29 François Dumont fdum...@gcc.gnu.org PR libstdc++/58148 * include/debug/functions.h (__foreign_iterator_aux4): Use sequence const_pointer as common type to compare pointers. Add a fallback overload in case pointers cannot be cast to sequence const_pointer. * testsuite/23_containers/vector/modifiers/insert/58148.cc: New. Ok to commit ? Seems pretty good to me. I have been thinking: when the non-trivial __foreign_iterator_aux4 is selected it actually has available as the last two arguments std::addressof(*(__it._M_get_sequence()-_M_base().begin())) std::addressof(*__other) we could as well give the parameters names and avoid passing __other. Also, I think we can do everything with std::less. I'm attaching below something I quickly hacked, untested, see if you like it in case commit something similar. Thanks! Paolo. // Index: functions.h === --- functions.h (revision 202068) +++ functions.h (working copy) @@ -36,7 +36,7 @@ #include bits/move.h// for __addressof and addressof #if __cplusplus = 201103L # include bits/stl_function.h // for less and greater_equal -# include type_traits // for common_type +# include type_traits // for is_lvalue_reference and __and_ #endif #include debug/formatter.h @@ -172,27 +172,28 @@ } #if __cplusplus = 201103L - templatetypename _Iterator, typename _Sequence, - typename _InputIterator, - typename _PointerType1, - typename _PointerType2 + // Default implementation. + templatetypename _Iterator, typename _Sequence inline bool __foreign_iterator_aux4(const _Safe_iterator_Iterator, _Sequence __it, - _InputIterator __other, - _PointerType1, _PointerType2) + typename _Sequence::const_pointer __begin, + typename _Sequence::const_pointer __other) { - typedef typename std::common_type_PointerType1, - _PointerType2::type _PointerType; + typedef typename _Sequence::const_pointer _PointerType; constexpr std::less_PointerType __l{}; - constexpr std::greater_equal_PointerType __ge{}; - return (__l(std::addressof(*__other), - std::addressof(*(__it._M_get_sequence()-_M_base().begin( - || __ge(std::addressof(*__other), - std::addressof(*(__it._M_get_sequence()-_M_base().end() - - 1)) + 1)); + return (__l(__other, __begin) + || __l(std::addressof(*(__it._M_get_sequence()-_M_base().end() + - 1)), __other)); } - + + // Fallback when address type cannot be implicitely casted to sequence + // const_pointer. + templatetypename _Iterator, typename _Sequence +inline bool +__foreign_iterator_aux4(const _Safe_iterator_Iterator, _Sequence, ...) +{ return true; } + templatetypename _Iterator, typename _Sequence, typename _InputIterator inline bool __foreign_iterator_aux3(const _Safe_iterator_Iterator, _Sequence __it, @@ -209,12 +210,12 @@ - std::addressof(*(__it._M_get_sequence()-_M_base().begin())) == __it._M_get_sequence()-size() - 1) return (__foreign_iterator_aux4 - (__it, __other, + (__it, std::addressof(*(__it._M_get_sequence()-_M_base().begin())), std::addressof(*__other))); return true; } - + /* Fallback overload for which we can't say, assume it is valid. */ templatetypename _Iterator, typename _Sequence, typename _InputIterator inline bool @@ -223,7 +224,7 @@ std::false_type) { return true; } #endif - + /** Checks that iterators do not belong to the same sequence. */ templatetypename _Iterator, typename _Sequence, typename _OtherIterator inline bool
Re: PR 58148 patch
Yes, this was cleaner, tested and committed this way. 2013-08-30 François Dumont fdum...@gcc.gnu.org PR libstdc++/58148 * include/debug/functions.h (__foreign_iterator_aux4): Use sequence const_pointer as common type to compare pointers. Add a fallback overload in case pointers cannot be cast to sequence const_pointer. * testsuite/23_containers/vector/modifiers/insert/58148.cc: New. François On 08/30/2013 11:44 AM, Paolo Carlini wrote: Hi, On 08/29/2013 09:37 PM, François Dumont wrote: Indeed, I check the Standard about const_pointer, so here is another attempt. Tested under Linux x86_64. 2013-08-29 François Dumont fdum...@gcc.gnu.org PR libstdc++/58148 * include/debug/functions.h (__foreign_iterator_aux4): Use sequence const_pointer as common type to compare pointers. Add a fallback overload in case pointers cannot be cast to sequence const_pointer. * testsuite/23_containers/vector/modifiers/insert/58148.cc: New. Ok to commit ? Seems pretty good to me. I have been thinking: when the non-trivial __foreign_iterator_aux4 is selected it actually has available as the last two arguments std::addressof(*(__it._M_get_sequence()-_M_base().begin())) std::addressof(*__other) we could as well give the parameters names and avoid passing __other. Also, I think we can do everything with std::less. I'm attaching below something I quickly hacked, untested, see if you like it in case commit something similar. Thanks! Paolo. //
Re: PR 58148 patch
Indeed, I check the Standard about const_pointer, so here is another attempt. Tested under Linux x86_64. 2013-08-29 François Dumont fdum...@gcc.gnu.org PR libstdc++/58148 * include/debug/functions.h (__foreign_iterator_aux4): Use sequence const_pointer as common type to compare pointers. Add a fallback overload in case pointers cannot be cast to sequence const_pointer. * testsuite/23_containers/vector/modifiers/insert/58148.cc: New. Ok to commit ? François On 08/28/2013 10:50 PM, Paolo Carlini wrote: Hi, On 08/28/2013 09:30 PM, François Dumont wrote: - std::addressof(*(__it._M_get_sequence()-_M_base().begin( + (*(__it._M_get_sequence()-_M_base().begin( I'm not convinced that you can avoid these std::addressof: it seems to me that the value_type can still have an overloaded operator Paolo. Index: include/debug/functions.h === --- include/debug/functions.h (revision 201966) +++ include/debug/functions.h (working copy) @@ -36,7 +36,7 @@ #include bits/move.h// for __addressof and addressof #if __cplusplus = 201103L # include bits/stl_function.h // for less and greater_equal -# include type_traits // for common_type +# include type_traits // for is_lvalue_reference and __and_ #endif #include debug/formatter.h @@ -172,17 +172,16 @@ } #if __cplusplus = 201103L + // Default implementation. templatetypename _Iterator, typename _Sequence, - typename _InputIterator, - typename _PointerType1, - typename _PointerType2 + typename _InputIterator inline bool __foreign_iterator_aux4(const _Safe_iterator_Iterator, _Sequence __it, _InputIterator __other, - _PointerType1, _PointerType2) + typename _Sequence::const_pointer, + typename _Sequence::const_pointer) { - typedef typename std::common_type_PointerType1, - _PointerType2::type _PointerType; + typedef typename _Sequence::const_pointer _PointerType; constexpr std::less_PointerType __l{}; constexpr std::greater_equal_PointerType __ge{}; @@ -192,7 +191,16 @@ std::addressof(*(__it._M_get_sequence()-_M_base().end() - 1)) + 1)); } - + + // Fallback when address type cannot be implicitely casted to sequence + // const_pointer. + templatetypename _Iterator, typename _Sequence, + typename _InputIterator +inline bool +__foreign_iterator_aux4(const _Safe_iterator_Iterator, _Sequence, + _InputIterator, ...) +{ return true; } + templatetypename _Iterator, typename _Sequence, typename _InputIterator inline bool __foreign_iterator_aux3(const _Safe_iterator_Iterator, _Sequence __it, @@ -223,7 +231,7 @@ std::false_type) { return true; } #endif - + /** Checks that iterators do not belong to the same sequence. */ templatetypename _Iterator, typename _Sequence, typename _OtherIterator inline bool Index: testsuite/23_containers/vector/modifiers/insert/58148.cc === --- testsuite/23_containers/vector/modifiers/insert/58148.cc (revision 0) +++ testsuite/23_containers/vector/modifiers/insert/58148.cc (revision 0) @@ -0,0 +1,35 @@ +// Copyright (C) 2013 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/. + +// { dg-options -std=gnu++11 } +// { dg-do compile } + +#include vector + +void +test01() +{ + std::vectorwchar_t v; + char c = 'a'; + v.insert(v.begin(), c, c); +} + +int main() +{ + test01(); + return 0; +}