* [Bug libstdc++/63500] [4.9/5 Regression] bug in debug version of std::make_move_iterator?
[not found] ` <bug-63500-19885-6PZlFsivrJ@http.gcc.gnu.org/bugzilla/>
@ 2014-10-14 21:52 ` François Dumont
2014-10-15 11:10 ` Jonathan Wakely
0 siblings, 1 reply; 5+ messages in thread
From: François Dumont @ 2014-10-14 21:52 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 647 bytes --]
Hi
Here is a proposal to fix the issue with iterators which do not
expose lvalue references when dereferenced. I simply chose to detect
such an issue in c++11 mode thanks to the is_lvalue_reference meta function.
2014-10-15 François Dumont <fdumont@gcc.gnu.org>
PR libstdc++/63500
* include/bits/cpp_type_traits.h (__true_type): Add __value constant.
(__false_type): Likewise.
* include/debug/functions.h (__foreign_iterator_aux2): Do not check for
foreign iterators if input iterators returns rvalue reference.
* testsuite/23_containers/vector/63500.cc: New.
Tested under Linux x86_64.
François
[-- Attachment #2: debug.patch --]
[-- Type: text/x-patch, Size: 3237 bytes --]
Index: include/bits/cpp_type_traits.h
===================================================================
--- include/bits/cpp_type_traits.h (revision 216158)
+++ include/bits/cpp_type_traits.h (working copy)
@@ -79,9 +79,12 @@
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION
- struct __true_type { };
- struct __false_type { };
+ struct __true_type
+ { enum { __value = 1 }; };
+ struct __false_type
+ { enum { __value = 0 }; };
+
template<bool>
struct __truth_type
{ typedef __false_type __type; };
Index: include/debug/functions.h
===================================================================
--- include/debug/functions.h (revision 216158)
+++ include/debug/functions.h (working copy)
@@ -34,7 +34,7 @@
// _Iter_base
#include <bits/cpp_type_traits.h> // for __is_integer
#include <bits/move.h> // for __addressof and addressof
-# include <bits/stl_function.h> // for less
+#include <bits/stl_function.h> // for less
#if __cplusplus >= 201103L
# include <type_traits> // for is_lvalue_reference and __and_
#endif
@@ -252,8 +252,21 @@
const _InputIterator& __other,
const _InputIterator& __other_end)
{
+#if __cplusplus >= 201103L
+ typedef std::iterator_traits<_InputIterator> _InputIteTraits;
+ typedef typename _InputIteTraits::reference _InputIteRefType;
+#endif
return __foreign_iterator_aux3(__it, __other, __other_end,
+#if __cplusplus < 201103L
_Is_contiguous_sequence<_Sequence>());
+#else
+ typename std::conditional<
+ std::__and_<std::integral_constant<
+ bool, _Is_contiguous_sequence<_Sequence>::__value>,
+ std::is_lvalue_reference<_InputIteRefType> >::value,
+ std::__true_type,
+ std::__false_type>::type());
+#endif
}
/* Handle the case where we aren't really inserting a range after all */
Index: testsuite/23_containers/vector/63500.cc
===================================================================
--- testsuite/23_containers/vector/63500.cc (revision 0)
+++ testsuite/23_containers/vector/63500.cc (working copy)
@@ -0,0 +1,39 @@
+// -*- C++ -*-
+
+// Copyright (C) 2014 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 <memory>
+#include <iterator>
+#include <debug/vector>
+
+class Foo
+{};
+
+void
+test01()
+{
+ __gnu_debug::vector<std::unique_ptr<Foo>> v;
+ __gnu_debug::vector<std::unique_ptr<Foo>> w;
+
+ v.insert(end(v),
+ make_move_iterator(begin(w)),
+ make_move_iterator(end(w)));
+}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Bug libstdc++/63500] [4.9/5 Regression] bug in debug version of std::make_move_iterator?
2014-10-14 21:52 ` [Bug libstdc++/63500] [4.9/5 Regression] bug in debug version of std::make_move_iterator? François Dumont
@ 2014-10-15 11:10 ` Jonathan Wakely
2014-10-15 20:14 ` François Dumont
0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Wakely @ 2014-10-15 11:10 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 14/10/14 23:51 +0200, François Dumont wrote:
>Hi
>
> Here is a proposal to fix the issue with iterators which do not
>expose lvalue references when dereferenced. I simply chose to detect
>such an issue in c++11 mode thanks to the is_lvalue_reference meta
>function.
>
>2014-10-15 François Dumont <fdumont@gcc.gnu.org>
>
> PR libstdc++/63500
> * include/bits/cpp_type_traits.h (__true_type): Add __value constant.
> (__false_type): Likewise.
> * include/debug/functions.h (__foreign_iterator_aux2): Do not check for
> foreign iterators if input iterators returns rvalue reference.
> * testsuite/23_containers/vector/63500.cc: New.
>
>Tested under Linux x86_64.
>
>François
>
>Index: include/bits/cpp_type_traits.h
>===================================================================
>--- include/bits/cpp_type_traits.h (revision 216158)
>+++ include/bits/cpp_type_traits.h (working copy)
>@@ -79,9 +79,12 @@
> {
> _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
>- struct __true_type { };
>- struct __false_type { };
>+ struct __true_type
>+ { enum { __value = 1 }; };
>
>+ struct __false_type
>+ { enum { __value = 0 }; };
>+
> template<bool>
> struct __truth_type
> { typedef __false_type __type; };
This change isn't necessary with my suggestion below.
>Index: include/debug/functions.h
>===================================================================
>--- include/debug/functions.h (revision 216158)
>+++ include/debug/functions.h (working copy)
>@@ -34,7 +34,7 @@
> // _Iter_base
> #include <bits/cpp_type_traits.h> // for __is_integer
> #include <bits/move.h> // for __addressof and addressof
>-# include <bits/stl_function.h> // for less
>+#include <bits/stl_function.h> // for less
> #if __cplusplus >= 201103L
> # include <type_traits> // for is_lvalue_reference and __and_
> #endif
>@@ -252,8 +252,21 @@
> const _InputIterator& __other,
> const _InputIterator& __other_end)
> {
>+#if __cplusplus >= 201103L
>+ typedef std::iterator_traits<_InputIterator> _InputIteTraits;
>+ typedef typename _InputIteTraits::reference _InputIteRefType;
>+#endif
> return __foreign_iterator_aux3(__it, __other, __other_end,
>+#if __cplusplus < 201103L
> _Is_contiguous_sequence<_Sequence>());
>+#else
>+ typename std::conditional<
>+ std::__and_<std::integral_constant<
>+ bool, _Is_contiguous_sequence<_Sequence>::__value>,
>+ std::is_lvalue_reference<_InputIteRefType> >::value,
>+ std::__true_type,
>+ std::__false_type>::type());
>+#endif
> }
I find this much easier to read:
#if __cplusplus < 201103L
typedef _Is_contiguous_sequence<_Sequence> __tag;
#else
using __lvalref = std::is_lvalue_reference<
typename std::iterator_traits<_InputIterator>::reference>;
using __contiguous = _Is_contiguous_sequence<_Sequence>;
using __tag = typename std::conditional<__lvalref::value, __contiguous,
std::__false_type>::type;
#endif
return __foreign_iterator_aux3(__it, __other, __other_end, __tag());
It only has one preprocessor condition and it avoids mismatched
parentheses caused by opening the function parameter list once but
closing it twice in two different branches.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Bug libstdc++/63500] [4.9/5 Regression] bug in debug version of std::make_move_iterator?
2014-10-15 11:10 ` Jonathan Wakely
@ 2014-10-15 20:14 ` François Dumont
2014-10-15 20:41 ` Jonathan Wakely
2014-10-16 9:57 ` Jonathan Wakely
0 siblings, 2 replies; 5+ messages in thread
From: François Dumont @ 2014-10-15 20:14 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1274 bytes --]
On 15/10/2014 13:10, Jonathan Wakely wrote:
>
> I find this much easier to read:
>
> #if __cplusplus < 201103L
> typedef _Is_contiguous_sequence<_Sequence> __tag;
> #else
> using __lvalref = std::is_lvalue_reference<
> typename std::iterator_traits<_InputIterator>::reference>;
> using __contiguous = _Is_contiguous_sequence<_Sequence>;
> using __tag = typename std::conditional<__lvalref::value,
> __contiguous,
> std::__false_type>::type;
> #endif
> return __foreign_iterator_aux3(__it, __other, __other_end, __tag());
>
> It only has one preprocessor condition and it avoids mismatched
> parentheses caused by opening the function parameter list once but
> closing it twice in two different branches.
>
>
That's much better indeed.
Shall we go with this ? Of course we are simply considering that we
can't check for foreign iterators when some iterator adapters comes
in-between. I hope one day to detect invalid usages even in this context.
2014-10-16 François Dumont <fdumont@gcc.gnu.org>
PR libstdc++/63500
* include/debug/functions.h (__foreign_iterator_aux2): Do not check for
foreign iterators if input iterators returns rvalue reference.
* testsuite/23_containers/vector/63500.cc: New.
François
[-- Attachment #2: debug.patch --]
[-- Type: text/x-patch, Size: 2737 bytes --]
Index: include/debug/functions.h
===================================================================
--- include/debug/functions.h (revision 216279)
+++ include/debug/functions.h (working copy)
@@ -34,7 +34,7 @@
// _Iter_base
#include <bits/cpp_type_traits.h> // for __is_integer
#include <bits/move.h> // for __addressof and addressof
-# include <bits/stl_function.h> // for less
+#include <bits/stl_function.h> // for less
#if __cplusplus >= 201103L
# include <type_traits> // for is_lvalue_reference and __and_
#endif
@@ -252,8 +252,16 @@
const _InputIterator& __other,
const _InputIterator& __other_end)
{
- return __foreign_iterator_aux3(__it, __other, __other_end,
- _Is_contiguous_sequence<_Sequence>());
+#if __cplusplus < 201103L
+ typedef _Is_contiguous_sequence<_Sequence> __tag;
+#else
+ using __lvalref = std::is_lvalue_reference<
+ typename std::iterator_traits<_InputIterator>::reference>;
+ using __contiguous = _Is_contiguous_sequence<_Sequence>;
+ using __tag = typename std::conditional<__lvalref::value, __contiguous,
+ std::__false_type>::type;
+#endif
+ return __foreign_iterator_aux3(__it, __other, __other_end, __tag());
}
/* Handle the case where we aren't really inserting a range after all */
Index: testsuite/23_containers/vector/63500.cc
===================================================================
--- testsuite/23_containers/vector/63500.cc (revision 0)
+++ testsuite/23_containers/vector/63500.cc (working copy)
@@ -0,0 +1,39 @@
+// -*- C++ -*-
+
+// Copyright (C) 2014 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 <memory>
+#include <iterator>
+#include <debug/vector>
+
+class Foo
+{};
+
+void
+test01()
+{
+ __gnu_debug::vector<std::unique_ptr<Foo>> v;
+ __gnu_debug::vector<std::unique_ptr<Foo>> w;
+
+ v.insert(end(v),
+ make_move_iterator(begin(w)),
+ make_move_iterator(end(w)));
+}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Bug libstdc++/63500] [4.9/5 Regression] bug in debug version of std::make_move_iterator?
2014-10-15 20:14 ` François Dumont
@ 2014-10-15 20:41 ` Jonathan Wakely
2014-10-16 9:57 ` Jonathan Wakely
1 sibling, 0 replies; 5+ messages in thread
From: Jonathan Wakely @ 2014-10-15 20:41 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 15/10/14 22:06 +0200, François Dumont wrote:
>On 15/10/2014 13:10, Jonathan Wakely wrote:
>>
>>I find this much easier to read:
>>
>>#if __cplusplus < 201103L
>> typedef _Is_contiguous_sequence<_Sequence> __tag;
>>#else
>> using __lvalref = std::is_lvalue_reference<
>> typename std::iterator_traits<_InputIterator>::reference>;
>> using __contiguous = _Is_contiguous_sequence<_Sequence>;
>> using __tag = typename std::conditional<__lvalref::value,
>>__contiguous,
>>std::__false_type>::type;
>>#endif
>> return __foreign_iterator_aux3(__it, __other, __other_end, __tag());
>>
>>It only has one preprocessor condition and it avoids mismatched
>>parentheses caused by opening the function parameter list once but
>>closing it twice in two different branches.
>>
>>
>That's much better indeed.
>
> Shall we go with this ?
Yes, it looks good to me, thanks.
>Of course we are simply considering that
>we can't check for foreign iterators when some iterator adapters comes
>in-between. I hope one day to detect invalid usages even in this
>context.
I agree that's OK.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Bug libstdc++/63500] [4.9/5 Regression] bug in debug version of std::make_move_iterator?
2014-10-15 20:14 ` François Dumont
2014-10-15 20:41 ` Jonathan Wakely
@ 2014-10-16 9:57 ` Jonathan Wakely
1 sibling, 0 replies; 5+ messages in thread
From: Jonathan Wakely @ 2014-10-16 9:57 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 15/10/14 22:06 +0200, François Dumont wrote:
>On 15/10/2014 13:10, Jonathan Wakely wrote:
>>
>>I find this much easier to read:
>>
>>#if __cplusplus < 201103L
>> typedef _Is_contiguous_sequence<_Sequence> __tag;
>>#else
>> using __lvalref = std::is_lvalue_reference<
>> typename std::iterator_traits<_InputIterator>::reference>;
>> using __contiguous = _Is_contiguous_sequence<_Sequence>;
>> using __tag = typename std::conditional<__lvalref::value,
>>__contiguous,
>>std::__false_type>::type;
>>#endif
>> return __foreign_iterator_aux3(__it, __other, __other_end, __tag());
>>
>>It only has one preprocessor condition and it avoids mismatched
>>parentheses caused by opening the function parameter list once but
>>closing it twice in two different branches.
>>
>>
>That's much better indeed.
>
> Shall we go with this ? Of course we are simply considering that
>we can't check for foreign iterators when some iterator adapters comes
>in-between. I hope one day to detect invalid usages even in this
>context.
>
>2014-10-16 François Dumont <fdumont@gcc.gnu.org>
>
> PR libstdc++/63500
> * include/debug/functions.h (__foreign_iterator_aux2): Do not check for
> foreign iterators if input iterators returns rvalue reference.
> * testsuite/23_containers/vector/63500.cc: New.
>
>François
>
As this is a regression it should go on the 4.9 branch too.
>Index: include/debug/functions.h
>===================================================================
>--- include/debug/functions.h (revision 216279)
>+++ include/debug/functions.h (working copy)
>@@ -34,7 +34,7 @@
> // _Iter_base
> #include <bits/cpp_type_traits.h> // for __is_integer
> #include <bits/move.h> // for __addressof and addressof
>-# include <bits/stl_function.h> // for less
>+#include <bits/stl_function.h> // for less
> #if __cplusplus >= 201103L
> # include <type_traits> // for is_lvalue_reference and __and_
> #endif
>@@ -252,8 +252,16 @@
> const _InputIterator& __other,
> const _InputIterator& __other_end)
> {
>- return __foreign_iterator_aux3(__it, __other, __other_end,
>- _Is_contiguous_sequence<_Sequence>());
>+#if __cplusplus < 201103L
>+ typedef _Is_contiguous_sequence<_Sequence> __tag;
>+#else
>+ using __lvalref = std::is_lvalue_reference<
>+ typename std::iterator_traits<_InputIterator>::reference>;
>+ using __contiguous = _Is_contiguous_sequence<_Sequence>;
>+ using __tag = typename std::conditional<__lvalref::value, __contiguous,
>+ std::__false_type>::type;
>+#endif
>+ return __foreign_iterator_aux3(__it, __other, __other_end, __tag());
> }
>
> /* Handle the case where we aren't really inserting a range after all */
>Index: testsuite/23_containers/vector/63500.cc
>===================================================================
>--- testsuite/23_containers/vector/63500.cc (revision 0)
>+++ testsuite/23_containers/vector/63500.cc (working copy)
>@@ -0,0 +1,39 @@
>+// -*- C++ -*-
>+
>+// Copyright (C) 2014 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 <memory>
>+#include <iterator>
>+#include <debug/vector>
>+
>+class Foo
>+{};
>+
>+void
>+test01()
>+{
>+ __gnu_debug::vector<std::unique_ptr<Foo>> v;
>+ __gnu_debug::vector<std::unique_ptr<Foo>> w;
>+
>+ v.insert(end(v),
>+ make_move_iterator(begin(w)),
>+ make_move_iterator(end(w)));
>+}
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-10-16 9:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <bug-63500-19885@http.gcc.gnu.org/bugzilla/>
[not found] ` <bug-63500-19885-6PZlFsivrJ@http.gcc.gnu.org/bugzilla/>
2014-10-14 21:52 ` [Bug libstdc++/63500] [4.9/5 Regression] bug in debug version of std::make_move_iterator? François Dumont
2014-10-15 11:10 ` Jonathan Wakely
2014-10-15 20:14 ` François Dumont
2014-10-15 20:41 ` Jonathan Wakely
2014-10-16 9:57 ` Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).