Correctly instantiate `iterator_adaptor_base` when defining `pointer_iterator`
authorDavid Blaikie <dblaikie@gmail.com>
Wed, 14 Nov 2018 07:19:21 +0000 (07:19 +0000)
committerDavid Blaikie <dblaikie@gmail.com>
Wed, 14 Nov 2018 07:19:21 +0000 (07:19 +0000)
The definition of `pointer_iterator` omits what should be a `iterator_traits::<>::iterator_category` parameter from `iterator_adaptor_base`. As a result, iterators based on `pointer_iterator` always have defaulted value types and the wrong iterator category.

The definition of `pointee_iterator` just a few lines above does this correctly.

This resolves [[ https://bugs.llvm.org/show_bug.cgi?id=39617 | bug 39617 ]].

Patch by Dylan MacKenzie!

Reviewers: dblaikie

Differential Revision: https://reviews.llvm.org/D54377

llvm-svn: 346833

llvm/include/llvm/ADT/iterator.h
llvm/unittests/ADT/IteratorTest.cpp

index 7f7ed69..40e490c 100644 (file)
@@ -309,8 +309,10 @@ make_pointee_range(RangeT &&Range) {
 template <typename WrappedIteratorT,
           typename T = decltype(&*std::declval<WrappedIteratorT>())>
 class pointer_iterator
-    : public iterator_adaptor_base<pointer_iterator<WrappedIteratorT, T>,
-                                   WrappedIteratorT, T> {
+    : public iterator_adaptor_base<
+          pointer_iterator<WrappedIteratorT, T>, WrappedIteratorT,
+          typename std::iterator_traits<WrappedIteratorT>::iterator_category,
+          T> {
   mutable T Ptr;
 
 public:
index 50c3b01..de0a671 100644 (file)
@@ -7,6 +7,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm/ADT/ilist.h"
 #include "llvm/ADT/iterator.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
@@ -35,6 +36,34 @@ static_assert(std::is_same<typename AdaptedIter::pointer, Shadow<2>>::value,
 static_assert(std::is_same<typename AdaptedIter::reference, Shadow<3>>::value,
               "");
 
+// Ensure that pointe{e,r}_iterator adaptors correctly forward the category of
+// the underlying iterator.
+
+using RandomAccessIter = SmallVectorImpl<int*>::iterator;
+using BidiIter = ilist<int*>::iterator;
+
+template<class T>
+using pointee_iterator_defaulted = pointee_iterator<T>;
+template<class T>
+using pointer_iterator_defaulted = pointer_iterator<T>;
+
+// Ensures that an iterator and its adaptation have the same iterator_category.
+template<template<typename> class A, typename It>
+using IsAdaptedIterCategorySame =
+  std::is_same<typename std::iterator_traits<It>::iterator_category,
+               typename std::iterator_traits<A<It>>::iterator_category>;
+
+// pointeE_iterator
+static_assert(IsAdaptedIterCategorySame<pointee_iterator_defaulted,
+                                        RandomAccessIter>::value, "");
+static_assert(IsAdaptedIterCategorySame<pointee_iterator_defaulted,
+                                        BidiIter>::value, "");
+// pointeR_iterator
+static_assert(IsAdaptedIterCategorySame<pointer_iterator_defaulted,
+                                        RandomAccessIter>::value, "");
+static_assert(IsAdaptedIterCategorySame<pointer_iterator_defaulted,
+                                        BidiIter>::value, "");
+
 TEST(PointeeIteratorTest, Basic) {
   int arr[4] = {1, 2, 3, 4};
   SmallVector<int *, 4> V;