[Bug libstdc++/63500] [4.9/5 Regression] bug in debug version of std::make_move_iterator?

[Jonathan Wakely]

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.