<div dir="ltr">__is_input_iterator would be a private implementation detail of libc++ - it won't be portable to use that.<br><br>What do you mean by 'InputIterator ~= iterator_traits-input_iterator_tag' ?</div><br><div class="gmail_quote"><div dir="ltr">On Thu, Jun 8, 2017 at 10:29 AM Francis Ricci <<a href="mailto:francisjricci@gmail.com">francisjricci@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Unfortunately, it appears that InputIterator !=<br>
iterator_traits->input_iterator_tag, as that was one of my original<br>
attempts. However, it looks like std::vector's implementation uses<br>
std::__is_input_iterator, which appears to work so far. I'll finish<br>
testing and update if it works.<br>
<br>
On Thu, Jun 8, 2017 at 12:42 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
><br>
><br>
> On Wed, Jun 7, 2017 at 2:03 PM Francis Ricci <<a href="mailto:francisjricci@gmail.com" target="_blank">francisjricci@gmail.com</a>><br>
> wrote:<br>
>><br>
>> I tried to stay as close as possible to the check from std::distance,<br>
>> which is the function that will fail to compile if you use the<br>
>> templated override without an iterable type. std::distance doesn't<br>
>> require a forward iterator, it only requires:<br>
>> iterator_traits<_InputIter>::difference_type, which should be defined<br>
>> in the same cases as iterator_traits<_InputIter>::pointer afaik.<br>
><br>
><br>
> It still seems best to not diverge from what the standard requires here..<br>
><br>
> "This overload only participates in overload resolution if InputIt satisfies<br>
> InputIterator." (to quote the more accessible (than the stancdard)<br>
> <a href="http://en.cppreference.com/w/cpp/container/vector/assign" rel="noreferrer" target="_blank">http://en.cppreference.com/w/cpp/container/vector/assign</a> as an example)<br>
><br>
> Though maybe it's impractical/not worth the hassle, I'm not sure?<br>
><br>
> I think it'd be a matter of using enable_if and checking the<br>
> iterator_category from the iterator traits, though.<br>
><br>
>><br>
>><br>
>> On Wed, Jun 7, 2017 at 5:00 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
>> > Looks a bit different from the is_forward_iterator type check that<br>
>> > libc++<br>
>> > uses. Any idea what the tradeoffs are there? Why this solution is<br>
>> > equivalent/as good/correct?<br>
>> ><br>
>> > (should have test coverage for all 3 variants that are modified)<br>
>> ><br>
>> > On Wed, Jun 7, 2017 at 1:57 PM Francis Ricci via Phabricator<br>
>> > <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br>
>> >><br>
>> >> fjricci updated this revision to Diff 101813.<br>
>> >> fjricci retitled this revision from "[ADT] Ensure that correct<br>
>> >> overrides<br>
>> >> for SmallVector assign are selected" to "[ADT] Make iterable<br>
>> >> SmallVector<br>
>> >> template overrides more specific".<br>
>> >> fjricci edited the summary of this revision.<br>
>> >> fjricci added a comment.<br>
>> >><br>
>> >> Add unit test and migrate to iterator SFINAE checks<br>
>> >><br>
>> >><br>
>> >> <a href="https://reviews.llvm.org/D33919" rel="noreferrer" target="_blank">https://reviews.llvm.org/D33919</a><br>
>> >><br>
>> >> Files:<br>
>> >> include/llvm/ADT/SmallVector.h<br>
>> >> unittests/ADT/SmallVectorTest.cpp<br>
>> >><br>
>> >><br>
>> >> Index: unittests/ADT/SmallVectorTest.cpp<br>
>> >> ===================================================================<br>
>> >> --- unittests/ADT/SmallVectorTest.cpp<br>
>> >> +++ unittests/ADT/SmallVectorTest.cpp<br>
>> >> @@ -434,6 +434,15 @@<br>
>> >> this->assertValuesInOrder(this->theVector, 3u, 1, 2, 3);<br>
>> >> }<br>
>> >><br>
>> >> +// Assign test<br>
>> >> +TYPED_TEST(SmallVectorTest, AssignNonIterTest) {<br>
>> >> + SCOPED_TRACE("AssignTest");<br>
>> >> +<br>
>> >> + this->theVector.push_back(Constructable(1));<br>
>> >> + this->theVector.assign(2, 2);<br>
>> >> + this->assertValuesInOrder(this->theVector, 2u, 2);<br>
>> >> +}<br>
>> >> +<br>
>> >> // Move-assign test<br>
>> >> TYPED_TEST(SmallVectorTest, MoveAssignTest) {<br>
>> >> SCOPED_TRACE("MoveAssignTest");<br>
>> >> Index: include/llvm/ADT/SmallVector.h<br>
>> >> ===================================================================<br>
>> >> --- include/llvm/ADT/SmallVector.h<br>
>> >> +++ include/llvm/ADT/SmallVector.h<br>
>> >> @@ -388,7 +388,8 @@<br>
>> >> void swap(SmallVectorImpl &RHS);<br>
>> >><br>
>> >> /// Add the specified range to the end of the SmallVector.<br>
>> >> - template<typename in_iter><br>
>> >> + template <typename in_iter,<br>
>> >> + typename = typename<br>
>> >> std::iterator_traits<in_iter>::pointer><br>
>> >> void append(in_iter in_start, in_iter in_end) {<br>
>> >> size_type NumInputs = std::distance(in_start, in_end);<br>
>> >> // Grow allocated space if needed.<br>
>> >> @@ -426,7 +427,9 @@<br>
>> >> std::uninitialized_fill(this->begin(), this->end(), Elt);<br>
>> >> }<br>
>> >><br>
>> >> - template <typename in_iter> void assign(in_iter in_start, in_iter<br>
>> >> in_end) {<br>
>> >> + template <typename in_iter,<br>
>> >> + typename = typename<br>
>> >> std::iterator_traits<in_iter>::pointer><br>
>> >> + void assign(in_iter in_start, in_iter in_end) {<br>
>> >> clear();<br>
>> >> append(in_start, in_end);<br>
>> >> }<br>
>> >> @@ -579,7 +582,8 @@<br>
>> >> return I;<br>
>> >> }<br>
>> >><br>
>> >> - template<typename ItTy><br>
>> >> + template <typename ItTy,<br>
>> >> + typename = typename std::iterator_traits<ItTy>::pointer><br>
>> >> iterator insert(iterator I, ItTy From, ItTy To) {<br>
>> >> // Convert iterator to elt# to avoid invalidating iterator when we<br>
>> >> reserve()<br>
>> >> size_t InsertElt = I - this->begin();<br>
>> >> @@ -860,7 +864,8 @@<br>
>> >> this->assign(Size, Value);<br>
>> >> }<br>
>> >><br>
>> >> - template<typename ItTy><br>
>> >> + template <typename ItTy,<br>
>> >> + typename = typename std::iterator_traits<ItTy>::pointer><br>
>> >> SmallVector(ItTy S, ItTy E) : SmallVectorImpl<T>(N) {<br>
>> >> this->append(S, E);<br>
>> >> }<br>
>> >><br>
>> >><br>
>> ><br>
</blockquote></div>