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

[Francis Ricci via llvm-commits]

My issue was that I forgot that the end pointer was not inclusive, so
I was off by 1, so that's working now.

You're right, when I add the `::type`, `is_convertible` is required (I
apologize, I don't use SFINAE much, so I'm still getting up to speed).

On Thu, Jun 8, 2017 at 4:13 PM, David Blaikie <dblaikie at gmail.com> wrote:
> 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
>>
>>
>>
>