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

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.

Reviewed By: rriddle

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

llvm/include/llvm/ADT/STLExtras.h

@@ -272,13 +272,15 @@ template <typename T> auto drop_begin(T &&RangeOrContainer, size_t N = 1) {
 // be applied whenever operator* is invoked on the iterator.
 
 template <typename ItTy, typename FuncTy,
-          typename FuncReturnTy =
-            decltype(std::declval<FuncTy>()(*std::declval<ItTy>()))>
+          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> {
+          mapped_iterator<ItTy, FuncTy>, ItTy,
+          typename std::iterator_traits<ItTy>::iterator_category,
+          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)) {}
@@ -287,7 +289,7 @@ public:
 
   const FuncTy &getFunction() const { return F; }
 
-  FuncReturnTy operator*() const { return F(*this->I); }
+  ReferenceTy operator*() const { return F(*this->I); }
 
 private:
   FuncTy F;

llvm/include/llvm/ADT/iterator.h

@@ -95,6 +95,22 @@ protected:
     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,
@@ -172,19 +188,21 @@ public:
     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);
   }
 };
 

mlir/include/mlir/IR/BlockSupport.h

@@ -52,7 +52,6 @@ class PredecessorIterator final
   static Block *unwrap(BlockOperand &value);
 
 public:
-  using reference = Block *;
 
   /// Initializes the operand type iterator to the specified operand iterator.
   PredecessorIterator(ValueUseIterator<BlockOperand> it)
@@ -151,7 +150,7 @@ public:
                                                                 &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
@@ -163,7 +162,6 @@ class op_iterator
   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)
@@ -171,7 +169,7 @@ public:
                               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

mlir/include/mlir/IR/BuiltinAttributes.h

@@ -299,9 +299,6 @@ public:
 
     /// 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.
@@ -314,9 +311,6 @@ public:
     /// Initializes the float element iterator to the specified iterator.
     ComplexFloatElementIterator(const llvm::fltSemantics &smt,
                                 ComplexIntElementIterator it);
-
-  public:
-    using reference = std::complex<APFloat>;
   };
 
   //===--------------------------------------------------------------------===//

mlir/include/mlir/IR/TypeRange.h

@@ -128,8 +128,6 @@ class ValueTypeIterator final
   static Type unwrap(Value value) { return value.getType(); }
 
 public:
-  using reference = Type;
-
   /// Provide a const dereference method.
   Type operator*() const { return unwrap(*this->I); }
 

mlir/include/mlir/IR/TypeUtilities.h

@@ -75,8 +75,6 @@ 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);
@@ -92,8 +90,6 @@ 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);

mlir/include/mlir/IR/UseDefLists.h

@@ -286,9 +286,6 @@ class ValueUserIterator final
   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 &)>(

mlir/include/mlir/TableGen/Operator.h

@@ -84,8 +84,6 @@ public:
   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 *,