[PATCH] D33919: [ADT] Make iterable SmallVector template overrides more specific

[David Blaikie via llvm-commits]

What sort of failure did you see? My simple test seemed to compile
successfully with &arr[0], &arr[2], basically. Maybe showing the specific
code and compiler messages would be helpful.

In any case, I did figure out why this wasn't being as overly restrictive
as I thought it would be - the "::type" was missing at the end of
std::enable_if. This meant the enable_if check wasn't doing anything -
other than housing the iterator_traits check which did mostly enough SFINAE
to help most cases.

The test case that's missing to demonstrate that it's not doing enough
checking, would be with a non-forward iterator (a basic iterator that only
supports dereferencing, but not incrementing) - such an iterator would
still be accepted by this overload, but shouldn't be. This could be tested
by having such a basic iterator that was convertible to an integer type -
and demonstrating that the overload set favors the integer conversion of
both arguments rather than trying to use it in the range based flavor.

Once the "::type" is added to the "std::enable_if<...>" then it starts
failing a lot & then the change to is_convertible fixes those failures.

On Thu, Jun 8, 2017 at 12:55 PM Francis Ricci via Phabricator <
reviews at reviews.llvm.org> wrote:

> fjricci added a comment.
>
> `assign(&arr[0], &arr[2])` fails both with `is_same` and `is_convertible`.
> Is that an acceptable usage of `assign` (and what you meant by random
> access iterator)? There's already a test case for `assign(std::begin(arr),
> std::end(arr))`, and that works fine with `is_same`.
>
> It looks like `random_access_iterator_tag` inherits from
> `input_iterator_tag` (
> http://en.cppreference.com/w/cpp/iterator/iterator_tags), so maybe that's
> why `is_same` works?
>
>
> https://reviews.llvm.org/D33919
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170608/2bae7155/attachment.html>