[cfe-commits] [patch] TransformIterator (motivating use case in Clang)

[Douglas Gregor]

On May 6, 2012, at 7:02 PM, David Blaikie wrote:

> This patch adds an iterator much like boost's transform_iterator
> (without some extra bells & whistles) for use in some changes I'd like
> to make to Clang to correct some misimplemented iterators there.
> 
> A few gotchas that could be improved/changed depending on opinions:
> * I may be playing a little loose with the return type of the functor
> (see the example/motivating functor, to_pointer) - the return type
> actually must be a reference, though the result_type provides the
> underlying value type, not the reference type. If this is violated
> Clang will emit a warning, but I could make it more robust with a
> compile time assertion in the TransformIterator that the result_type
> is actually a reference type, and strip that off to provide the
> value_type of the TransformIterator.

It's much more correct for the value and reference types of the iterator type to be, e.g.,

  typedef typename Functor::result_type reference;
  typedef typename remove_reference<reference>::type value_type;

> * I realize adding pointer-yness back onto an iterator over references
> when the underlying data structure (in the Clang patch you can see
> this situation) is a sequence of pointers may be a bit overkill -
> though the alternatives are writing different algorithms in the two
> cases where I've used TransformIterator, or having the decl_iterator
> iterate over pointers (in which case I should probably change the
> other iterators I've modified to iterate over pointers for symmetry).

diff --git include/clang/AST/DeclBase.h include/clang/AST/DeclBase.h
index 6aef681..0a16ea5 100644
--- include/clang/AST/DeclBase.h
+++ include/clang/AST/DeclBase.h
@@ -1175,17 +1175,18 @@ public:
     Decl *Current;
 
   public:
-    typedef Decl*                     value_type;
-    typedef Decl*                     reference;
-    typedef Decl*                     pointer;
+    typedef Decl                     value_type;
+    typedef value_type&              reference;
+    typedef value_type*              pointer;

Since we tend to traffic in declaration pointers, not references, it's really beneficial to have value_type be Decl*. Of course, it's fine for the reference type to be Decl* as well.


> At some point I'll also be looking to add a FilterIterator (& possibly
> a specific Filtering+Transforming iterator, as I'm not sure the
> composition of the two independently will be great for the use case I
> have in mind) as well since that's one of the motivation for tidying
> up these iterators - to pull out the common functionality in the
> specific/filtered_iterators in DeclContext.


+//===- llvm/Support/type_traits.h - Simplfied type traits -------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===/

Header needs updating.

+  pointer operator->() const {
+    return m_func(*m_iter);
+  }

This isn't going to work; m_func returns a value_type or reference. Dealing with -> is a real pain, because in the general case you need a proxy class.

+// a convenient utility for a common adaptation
+template<typename T>
+struct to_pointer {
+  typedef T * result_type;
+  typedef T argument_type;
+  result_type &operator()(T &t) const {
+    ptr = &t;
+    return ptr;
+  }
+private:
+  mutable T *ptr;
+};

'to_pointer' is a very non-LLVM-y name. 

More generally, this particular function object seems like a bad idea. There's nothing wrong with having an iterator type whose reference type is not a true reference, which means that there is no reason to do this little dance with the mutable 'ptr'.

	- Doug