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

[Francis Ricci via llvm-commits]

I tried to stay as close as possible to the check from std::distance,
which is the function that will fail to compile if you use the
templated override without an iterable type. std::distance doesn't
require a forward iterator, it only requires:
iterator_traits<_InputIter>::difference_type, which should be defined
in the same cases as iterator_traits<_InputIter>::pointer afaik.

On Wed, Jun 7, 2017 at 5:00 PM, David Blaikie <dblaikie at gmail.com> wrote:
> Looks a bit different from the is_forward_iterator type check that libc++
> uses. Any idea what the tradeoffs are there? Why this solution is
> equivalent/as good/correct?
>
> (should have test coverage for all 3 variants that are modified)
>
> On Wed, Jun 7, 2017 at 1:57 PM Francis Ricci via Phabricator
> <reviews at reviews.llvm.org> wrote:
>>
>> fjricci updated this revision to Diff 101813.
>> fjricci retitled this revision from "[ADT] Ensure that correct overrides
>> for SmallVector assign are selected" to "[ADT] Make iterable SmallVector
>> template overrides more specific".
>> fjricci edited the summary of this revision.
>> fjricci added a comment.
>>
>> Add unit test and migrate to iterator SFINAE checks
>>
>>
>> https://reviews.llvm.org/D33919
>>
>> Files:
>>   include/llvm/ADT/SmallVector.h
>>   unittests/ADT/SmallVectorTest.cpp
>>
>>
>> Index: unittests/ADT/SmallVectorTest.cpp
>> ===================================================================
>> --- unittests/ADT/SmallVectorTest.cpp
>> +++ unittests/ADT/SmallVectorTest.cpp
>> @@ -434,6 +434,15 @@
>>    this->assertValuesInOrder(this->theVector, 3u, 1, 2, 3);
>>  }
>>
>> +// Assign test
>> +TYPED_TEST(SmallVectorTest, AssignNonIterTest) {
>> +  SCOPED_TRACE("AssignTest");
>> +
>> +  this->theVector.push_back(Constructable(1));
>> +  this->theVector.assign(2, 2);
>> +  this->assertValuesInOrder(this->theVector, 2u, 2);
>> +}
>> +
>>  // Move-assign test
>>  TYPED_TEST(SmallVectorTest, MoveAssignTest) {
>>    SCOPED_TRACE("MoveAssignTest");
>> Index: include/llvm/ADT/SmallVector.h
>> ===================================================================
>> --- include/llvm/ADT/SmallVector.h
>> +++ include/llvm/ADT/SmallVector.h
>> @@ -388,7 +388,8 @@
>>    void swap(SmallVectorImpl &RHS);
>>
>>    /// Add the specified range to the end of the SmallVector.
>> -  template<typename in_iter>
>> +  template <typename in_iter,
>> +            typename = typename std::iterator_traits<in_iter>::pointer>
>>    void append(in_iter in_start, in_iter in_end) {
>>      size_type NumInputs = std::distance(in_start, in_end);
>>      // Grow allocated space if needed.
>> @@ -426,7 +427,9 @@
>>      std::uninitialized_fill(this->begin(), this->end(), Elt);
>>    }
>>
>> -  template <typename in_iter> void assign(in_iter in_start, in_iter
>> in_end) {
>> +  template <typename in_iter,
>> +            typename = typename std::iterator_traits<in_iter>::pointer>
>> +  void assign(in_iter in_start, in_iter in_end) {
>>      clear();
>>      append(in_start, in_end);
>>    }
>> @@ -579,7 +582,8 @@
>>      return I;
>>    }
>>
>> -  template<typename ItTy>
>> +  template <typename ItTy,
>> +            typename = typename std::iterator_traits<ItTy>::pointer>
>>    iterator insert(iterator I, ItTy From, ItTy To) {
>>      // Convert iterator to elt# to avoid invalidating iterator when we
>> reserve()
>>      size_t InsertElt = I - this->begin();
>> @@ -860,7 +864,8 @@
>>      this->assign(Size, Value);
>>    }
>>
>> -  template<typename ItTy>
>> +  template <typename ItTy,
>> +            typename = typename std::iterator_traits<ItTy>::pointer>
>>    SmallVector(ItTy S, ItTy E) : SmallVectorImpl<T>(N) {
>>      this->append(S, E);
>>    }
>>
>>
>