Support value-typed references in iterator facade's operator->()

[csigg]

Summary

Add a PointerProxy similar to the existing iterator_facade_base::ReferenceProxy and return it from the arrow operator. This prevents iterator facades with a reference type that is not a true reference to take the address of a temporary.

Forward the reference type of the mapped_iterator to the iterator adaptor which in turn forwards it to the iterator facade. This fixes mlir::op_iterator::operator->() to take the address of a temporary.

Make some polishing changes to op_iterator and op_filter_iterator.

[csigg]

Rebase.

[rriddle]

Thanks!

mlir/include/mlir/IR/BlockSupport.h

166

Is this the only situation this happens? Can you fix any others as well.

[csigg]

Fix other mapped_iterators as well.

llvm/include/llvm/ADT/STLExtras.h

template <typename T> auto drop_begin(T &&RangeOrContainer, size_t N = 1) {
   return make_range(std::next(adl_begin(RangeOrContainer), N),
                     adl_end(RangeOrContainer));
 }
 
 // mapped_iterator - This is a simple iterator adapter that causes a function to
 // be applied whenever operator* is invoked on the iterator.
 
 template <typename ItTy, typename FuncTy,
-          typename FuncReturnTy =
+          typename ReferenceTy =
               decltype(std::declval<FuncTy>()(*std::declval<ItTy>()))>
 class mapped_iterator
     : public iterator_adaptor_base<
-             mapped_iterator<ItTy, FuncTy>, ItTy,
-             typename std::iterator_traits<ItTy>::iterator_category,
-             typename std::remove_reference<FuncReturnTy>::type> {
+          std::remove_reference_t<ReferenceTy>,
+          typename std::iterator_traits<ItTy>::difference_type,
+          std::remove_reference_t<ReferenceTy> *, ReferenceTy> {
 public:
   mapped_iterator(ItTy U, FuncTy F)
     : mapped_iterator::iterator_adaptor_base(std::move(U)), F(std::move(F)) {}
 
   ItTy getCurrent() { return this->I; }
 
   const FuncTy &getFunction() const { return F; }
 
-  FuncReturnTy operator*() const { return F(*this->I); }
+  ReferenceTy operator*() const { return F(*this->I); }
 
 private:
   FuncTy F;
 };
 
 // map_iterator - Provide a convenient way to create mapped_iterators, just like
 // make_pair is useful for creating pairs...
 template <class ItTy, class FuncTy>

llvm/include/llvm/ADT/iterator.h

class ReferenceProxy {
     DerivedT I;
 
     ReferenceProxy(DerivedT I) : I(std::move(I)) {}
 
   public:
     operator ReferenceT() const { return *I; }
   };
 
+  /// A proxy object for computing a pointer via indirecting a copy of a
+  /// reference. This is used in APIs which need to produce a pointer but for
+  /// which the reference might be a temporary. The proxy preserves the
+  /// reference internally and exposes the pointer via a arrow operator.
+  class PointerProxy {
+    friend iterator_facade_base;
+
+    ReferenceT R;
+
+    template <typename RefT>
+    PointerProxy(RefT &&R) : R(std::forward<RefT>(R)) {}
+
+  public:
+    PointerT operator->() const { return &R; }
+  };
+
 public:
   DerivedT operator+(DifferenceTypeT n) const {
     static_assert(std::is_base_of<iterator_facade_base, DerivedT>::value,
                   "Must pass the derived type to this template!");
     static_assert(
         IsRandomAccess,
         "The '+' operator is only defined for random access iterators.");
     DerivedT tmp = *static_cast<const DerivedT *>(this);
#endif
   }
   bool operator>=(const DerivedT &RHS) const {
     static_assert(
         IsRandomAccess,
         "Relational operators are only defined for random access iterators.");
     return !(static_cast<const DerivedT &>(*this) < RHS);
   }
 
-  PointerT operator->() { return &static_cast<DerivedT *>(this)->operator*(); }
-  PointerT operator->() const {
-    return &static_cast<const DerivedT *>(this)->operator*();
+  PointerProxy operator->() {
+    return static_cast<DerivedT *>(this)->operator*();
+  }
+  PointerProxy operator->() const {
+    return static_cast<const DerivedT *>(this)->operator*();
   }
   ReferenceProxy operator[](DifferenceTypeT n) {
     static_assert(IsRandomAccess,
                   "Subscripting is only defined for random access iterators.");
-    return ReferenceProxy(static_cast<DerivedT *>(this)->operator+(n));
+    return static_cast<DerivedT *>(this)->operator+(n);
   }
   ReferenceProxy operator[](DifferenceTypeT n) const {
     static_assert(IsRandomAccess,
                   "Subscripting is only defined for random access iterators.");
-    return ReferenceProxy(static_cast<const DerivedT *>(this)->operator+(n));
+    return static_cast<const DerivedT *>(this)->operator+(n);
   }
 };
 
 /// CRTP base class for adapting an iterator to a different type.
 ///
 /// This class can be used through CRTP to adapt one iterator into another.
 /// Typically this is done through providing in the derived class a custom \c
 /// operator* implementation. Other methods can be overridden as well.

mlir/include/mlir/IR/BlockSupport.h

 /// are embedded into terminator operations. From the operand, we can get the
 /// terminator that contains it, and its parent block is the predecessor.
 class PredecessorIterator final
     : public llvm::mapped_iterator<ValueUseIterator<BlockOperand>,
                                    Block *(*)(BlockOperand &)> {
   static Block *unwrap(BlockOperand &value);
 
 public:
-  using reference = Block *;
 
   /// Initializes the operand type iterator to the specified operand iterator.
   PredecessorIterator(ValueUseIterator<BlockOperand> it)
       : llvm::mapped_iterator<ValueUseIterator<BlockOperand>,
                               Block *(*)(BlockOperand &)>(it, &unwrap) {}
   explicit PredecessorIterator(BlockOperand *operand)
       : PredecessorIterator(ValueUseIterator<BlockOperand>(operand)) {}
 
class op_filter_iterator
   static bool filter(Operation &op) { return llvm::isa<OpT>(op); }
 
 public:
   op_filter_iterator(IteratorT it, IteratorT end)
       : llvm::filter_iterator<IteratorT, bool (*)(Operation &)>(it, end,
                                                                 &filter) {}
 
   /// Allow implicit conversion to the underlying iterator.
-  operator IteratorT() const { return this->wrapped(); }
+  operator const IteratorT &() const { return this->wrapped(); }
 };
 
 /// This class provides iteration over the held operations of a block for a
 /// specific operation type.
 template <typename OpT, typename IteratorT>
 class op_iterator
     : public llvm::mapped_iterator<op_filter_iterator<OpT, IteratorT>,
                                    OpT (*)(Operation &)> {
   static OpT unwrap(Operation &op) { return cast<OpT>(op); }
 
 public:
-  using reference = OpT;
 
   /// Initializes the iterator to the specified filter iterator.
   op_iterator(op_filter_iterator<OpT, IteratorT> it)
       : llvm::mapped_iterator<op_filter_iterator<OpT, IteratorT>,
                               OpT (*)(Operation &)>(it, &unwrap) {}
 
   /// Allow implicit conversion to the underlying block iterator.
-  operator IteratorT() const { return this->wrapped(); }
+  operator const IteratorT &() const { return this->wrapped(); }
 };
 } // end namespace detail
 } // end namespace mlir
 
 namespace llvm {
 
 /// Provide support for hashing successor ranges.
 template <>

mlir/include/mlir/IR/BuiltinAttributes.h

public:
   /// Iterator for walking over APFloat values.
   class FloatElementIterator final
       : public llvm::mapped_iterator<IntElementIterator,
                                      std::function<APFloat(const APInt &)>> {
     friend DenseElementsAttr;
 
     /// Initializes the float element iterator to the specified iterator.
     FloatElementIterator(const llvm::fltSemantics &smt, IntElementIterator it);
-
-  public:
-    using reference = APFloat;
   };
 
   /// Iterator for walking over complex APFloat values.
   class ComplexFloatElementIterator final
       : public llvm::mapped_iterator<
             ComplexIntElementIterator,
             std::function<std::complex<APFloat>(const std::complex<APInt> &)>> {
     friend DenseElementsAttr;
 
     /// Initializes the float element iterator to the specified iterator.
     ComplexFloatElementIterator(const llvm::fltSemantics &smt,
                                 ComplexIntElementIterator it);
-
-  public:
-    using reference = std::complex<APFloat>;
   };
 
   //===--------------------------------------------------------------------===//
   // Value Querying
   //===--------------------------------------------------------------------===//
 
   /// Returns true if this attribute corresponds to a splat, i.e. if all element
   /// values are the same.

mlir/include/mlir/IR/TypeRange.h

 
 /// This class implements iteration on the types of a given range of values.
 template <typename ValueIteratorT>
 class ValueTypeIterator final
     : public llvm::mapped_iterator<ValueIteratorT, Type (*)(Value)> {
   static Type unwrap(Value value) { return value.getType(); }
 
 public:
-  using reference = Type;
-
   /// Provide a const dereference method.
   Type operator*() const { return unwrap(*this->I); }
 
   /// Initializes the type iterator to the specified value iterator.
   ValueTypeIterator(ValueIteratorT it)
       : llvm::mapped_iterator<ValueIteratorT, Type (*)(Value)>(it, &unwrap) {}
 };
 

mlir/include/mlir/IR/TypeUtilities.h

 // Utility Iterators
 //===----------------------------------------------------------------------===//
 
 // An iterator for the element types of an op's operands of shaped types.
 class OperandElementTypeIterator final
     : public llvm::mapped_iterator<Operation::operand_iterator,
                                    Type (*)(Value)> {
 public:
-  using reference = Type;
-
   /// Initializes the result element type iterator to the specified operand
   /// iterator.
   explicit OperandElementTypeIterator(Operation::operand_iterator it);
 
 private:
   static Type unwrap(Value value);
 };
 
 using OperandElementTypeRange = iterator_range<OperandElementTypeIterator>;
 
 // An iterator for the tensor element types of an op's results of shaped types.
 class ResultElementTypeIterator final
     : public llvm::mapped_iterator<Operation::result_iterator,
                                    Type (*)(Value)> {
 public:
-  using reference = Type;
-
   /// Initializes the result element type iterator to the specified result
   /// iterator.
   explicit ResultElementTypeIterator(Operation::result_iterator it);
 
 private:
   static Type unwrap(Value value);
 };
 
 using ResultElementTypeRange = iterator_range<ResultElementTypeIterator>;
 
 } // end namespace mlir
 
 #endif // MLIR_SUPPORT_TYPEUTILITIES_H

mlir/include/mlir/IR/UseDefLists.h

 /// a specific use iterator.
 template <typename UseIteratorT, typename OperandType>
 class ValueUserIterator final
     : public llvm::mapped_iterator<UseIteratorT,
                                    Operation *(*)(OperandType &)> {
   static Operation *unwrap(OperandType &value) { return value.getOwner(); }
 
 public:
-  using pointer = Operation *;
-  using reference = Operation *;
-
   /// Initializes the user iterator to the specified use iterator.
   ValueUserIterator(UseIteratorT it)
       : llvm::mapped_iterator<UseIteratorT, Operation *(*)(OperandType &)>(
             it, &unwrap) {}
   Operation *operator->() { return **this; }
 };
 
 } // namespace mlir
 
 #endif

mlir/include/mlir/TableGen/Operator.h

protected:
     // The TableGen definition of this decorator.
     const llvm::Record *def;
   };
 
   // A utility iterator over a list of variable decorators.
   struct VariableDecoratorIterator
       : public llvm::mapped_iterator<llvm::Init *const *,
                                      VariableDecorator (*)(llvm::Init *)> {
-    using reference = VariableDecorator;
-
     /// Initializes the iterator to the specified iterator.
     VariableDecoratorIterator(llvm::Init *const *it)
         : llvm::mapped_iterator<llvm::Init *const *,
                                 VariableDecorator (*)(llvm::Init *)>(it,
                                                                      &unwrap) {}
     static VariableDecorator unwrap(llvm::Init *init);
   };
   using var_decorator_iterator = VariableDecoratorIterator;