[llvm] r207069 - [ADT] Add a generic iterator utility for adapting iterators much like

[Duncan P. N. Exon Smith]

On 2014-Apr-24, at 13:37, David Blaikie <dblaikie at gmail.com> wrote:

> On Thu, Apr 24, 2014 at 1:24 PM, Chandler Carruth <chandlerc at gmail.com> wrote:
>> 
>> On Thu, Apr 24, 2014 at 1:16 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>> 
>>>> +/// \brief An iterator type that allows iterating over the pointees via
>>>> some
>>>> +/// other iterator.
>>>> +///
>>>> +/// The typical usage of this is to expose a type that iterates over
>>>> Ts, but
>>>> +/// which is implemented with some iterator over T*s:
>>>> +///
>>>> +/// \code
>>>> +///   typedef pointee_iterator<SmallVectorImpl<T *>::iterator>
>>>> iterator;
>>>> +/// \endcode
>>>> +template <
>>>> +    typename WrappedIteratorT,
>>>> +    typename T = typename std::remove_pointer<
>>>> +        typename
>>>> std::iterator_traits<WrappedIteratorT>::value_type>::type>
>>> 
>>> Just as a strawman, how do you feel about:
>>> 
>>> template <typename WrappedIteratorT, typename T =
>>> decltype(**std::declval<WrappedIteratorT>())>
>>> 
>>> I /think/ your version doesn't work for smart pointers, does it?
>>> (judging by http://en.cppreference.com/w/cpp/types/remove_pointer )
>>> and this version should.
>> 
>> 
>> Entertainingly, I almost used the version you posted prior to getting
>> std::remove_pointer to work.
>> 
>> I feel strongly that pointee_iterator should work with smart pointers, and
>> I'm somewhat bummed that std::remove_pointer doesn't work.
>> 
>> How about a slight variation: T =
>> decltype(*std::declval<WrappedIteratorT::value_type>())
> 
> Are there cases you have in mind where this would be more
> appropriate/effective/correct than what I've suggested?
> 
> In the case where the underlying iterator lies about its value type
> (such that the value type differs from the result of operator*) we're
> not going to get the desired behavior - at best the types will
> mismatch and the pointee_iterator's operator* will fail to compile,
> right? (since it's applying * to the result of the underlying
> iterator's op*, not to the value type (in the case where they're
> different)).

I agree that David's version is safer.

> Seems more terse and more correct to have the type basically match the
> expression in the op*.
> 
> But I'm not entirely confident here - the sorts of shenanigans people
> might have going on with iterator traits and mismatched
> value/reference/pointer types are something I try not to dwell on, as
> a rule. They make me sad.

I sure wish Boost's iterator concepts [1] were in C++11.  I think using `value_type`
as `reference` is a legitimate use case (as is `vector<bool>::iterator`), and it's
frustrating not to have STL support for it.

[1]: http://www.boost.org/doc/libs/1_55_0/libs/iterator/doc/new-iter-concepts.html