[PATCH] [libc++] Move enable_if to 2nd argument of std::vector(iterator, iterator) ctor

[Howard Hinnant]

Nice.  Committed revision 191145.

In the future please also test with:

$ export OPTIONS="-std=c++03 -stdlib=libc++"

I think there are many places like this that could use the same treatment.

Howard

On Sep 19, 2013, at 12:59 AM, Peter Collingbourne <peter at pcc.me.uk> wrote:

> If a pointer is passed as the third argument of the (iterator,
> iterator, allocator) constructor with the intention of it being
> implicitly converted to the allocator type, it is possible for overload
> resolution to favour the (iterator, iterator, enable_if) constructor.
> Eliminate this possibility by moving the enable_if to one of the
> existing arguments and removing the third argument.
> 
> http://llvm-reviews.chandlerc.com/D1723
> 
> Files:
>  include/vector
>  test/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp
> 
> Index: include/vector
> ===================================================================
> --- include/vector
> +++ include/vector
> @@ -519,25 +519,27 @@
>     vector(size_type __n, const_reference __x);
>     vector(size_type __n, const_reference __x, const allocator_type& __a);
>     template <class _InputIterator>
> -        vector(_InputIterator __first, _InputIterator __last,
> +        vector(_InputIterator __first,
>                typename enable_if<__is_input_iterator  <_InputIterator>::value &&
>                                  !__is_forward_iterator<_InputIterator>::value &&
>                                  is_constructible<
>                                     value_type,
> -                                    typename iterator_traits<_InputIterator>::reference>::value>::type* = 0);
> +                                    typename iterator_traits<_InputIterator>::reference>::value,
> +                                 _InputIterator>::type __last);
>     template <class _InputIterator>
>         vector(_InputIterator __first, _InputIterator __last, const allocator_type& __a,
>                typename enable_if<__is_input_iterator  <_InputIterator>::value &&
>                                  !__is_forward_iterator<_InputIterator>::value &&
>                                  is_constructible<
>                                     value_type,
>                                     typename iterator_traits<_InputIterator>::reference>::value>::type* = 0);
>     template <class _ForwardIterator>
> -        vector(_ForwardIterator __first, _ForwardIterator __last,
> +        vector(_ForwardIterator __first,
>                typename enable_if<__is_forward_iterator<_ForwardIterator>::value &&
>                                  is_constructible<
>                                     value_type,
> -                                    typename iterator_traits<_ForwardIterator>::reference>::value>::type* = 0);
> +                                    typename iterator_traits<_ForwardIterator>::reference>::value,
> +                                 _ForwardIterator>::type __last);
>     template <class _ForwardIterator>
>         vector(_ForwardIterator __first, _ForwardIterator __last, const allocator_type& __a,
>                typename enable_if<__is_forward_iterator<_ForwardIterator>::value &&
> @@ -1051,12 +1053,13 @@
> 
> template <class _Tp, class _Allocator>
> template <class _InputIterator>
> -vector<_Tp, _Allocator>::vector(_InputIterator __first, _InputIterator __last,
> +vector<_Tp, _Allocator>::vector(_InputIterator __first,
>        typename enable_if<__is_input_iterator  <_InputIterator>::value &&
>                          !__is_forward_iterator<_InputIterator>::value &&
>                          is_constructible<
>                             value_type,
> -                            typename iterator_traits<_InputIterator>::reference>::value>::type*)
> +                            typename iterator_traits<_InputIterator>::reference>::value,
> +                          _InputIterator>::type __last)
> {
> #if _LIBCPP_DEBUG_LEVEL >= 2
>     __get_db()->__insert_c(this);
> @@ -1084,11 +1087,12 @@
> 
> template <class _Tp, class _Allocator>
> template <class _ForwardIterator>
> -vector<_Tp, _Allocator>::vector(_ForwardIterator __first, _ForwardIterator __last,
> +vector<_Tp, _Allocator>::vector(_ForwardIterator __first,
>                                 typename enable_if<__is_forward_iterator<_ForwardIterator>::value &&
>                                 is_constructible<
>                                    value_type,
> -                                   typename iterator_traits<_ForwardIterator>::reference>::value>::type*)
> +                                   typename iterator_traits<_ForwardIterator>::reference>::value,
> +                                                   _ForwardIterator>::type __last)
> {
> #if _LIBCPP_DEBUG_LEVEL >= 2
>     __get_db()->__insert_c(this);
> Index: test/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp
> ===================================================================
> --- test/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp
> +++ test/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp
> @@ -19,17 +19,24 @@
> #include "../../../stack_allocator.h"
> #include "../../../min_allocator.h"
> 
> -template <class C, class Iterator>
> +template <class C, class Iterator, class A>
> void
> -test(Iterator first, Iterator last, const typename C::allocator_type& a)
> +test(Iterator first, Iterator last, const A& a)
> {
>     C c(first, last, a);
>     assert(c.__invariants());
>     assert(c.size() == std::distance(first, last));
>     for (typename C::const_iterator i = c.cbegin(), e = c.cend(); i != e; ++i, ++first)
>         assert(*i == *first);
> }
> 
> +template <class T>
> +struct implicit_conv_allocator : min_allocator<T>
> +{
> +    implicit_conv_allocator(void* p) {}
> +    implicit_conv_allocator(const implicit_conv_allocator&) = default;
> +};
> +
> int main()
> {
>     {
> @@ -52,6 +59,7 @@
>     test<std::vector<int, min_allocator<int>> >(bidirectional_iterator<const int*>(a), bidirectional_iterator<const int*>(an), alloc);
>     test<std::vector<int, min_allocator<int>> >(random_access_iterator<const int*>(a), random_access_iterator<const int*>(an), alloc);
>     test<std::vector<int, min_allocator<int>> >(a, an, alloc);
> +    test<std::vector<int, implicit_conv_allocator<int>> >(a, an, nullptr);
>     }
> #endif
> }
> <D1723.1.patch>_______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits