[llvm][ADT] Fix invalid reference type of depth-first, breadth-first and post order iterators

C++s iterator concept requires operator* to return the same type as is specified by the iterators reference type. This functionality is especially important for older generic code that did not yet make use of auto.
An example from within LLVM is iterator_adaptor_base which uses the reference type of the iterator it is wrapping as its return type for operator* (this class is used as base for a lot of other functionality like filter iterators and so on).
Using any of the graph traversal iterators listed above with it would previously fail to compile due to reference being non-const while operator* returned a const reference.

This patch fixes that by correctly specifying reference and using it as the return type of operator* explicitly to prevent further issues in the future.

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

Author: zero9178

llvm/include/llvm/ADT/BreadthFirstIterator.h

@@ -50,7 +50,7 @@ class bf_iterator : public bf_iterator_storage<SetType> {
   using value_type = typename GT::NodeRef;
   using difference_type = std::ptrdiff_t;
   using pointer = value_type *;
-  using reference = value_type &;
+  using reference = const value_type &;
 
 private:
   using NodeRef = typename GT::NodeRef;
@@ -123,7 +123,7 @@ class bf_iterator : public bf_iterator_storage<SetType> {
 
   bool operator!=(const bf_iterator &RHS) const { return !(*this == RHS); }
 
-  const NodeRef &operator*() const { return VisitQueue.front()->first; }
+  reference operator*() const { return VisitQueue.front()->first; }
 
   // This is a nonstandard operator-> that dereferences the pointer an extra
   // time so that you can actually call methods on the node, because the

llvm/include/llvm/ADT/DepthFirstIterator.h

@@ -88,7 +88,7 @@ class df_iterator : public df_iterator_storage<SetType, ExtStorage> {
   using value_type = typename GT::NodeRef;
   using difference_type = std::ptrdiff_t;
   using pointer = value_type *;
-  using reference = value_type &;
+  using reference = const value_type &;
 
 private:
   using NodeRef = typename GT::NodeRef;
@@ -165,7 +165,7 @@ class df_iterator : public df_iterator_storage<SetType, ExtStorage> {
   }
   bool operator!=(const df_iterator &x) const { return !(*this == x); }
 
-  const NodeRef &operator*() const { return VisitStack.back().first; }
+  reference operator*() const { return VisitStack.back().first; }
 
   // This is a nonstandard operator-> that dereferences the pointer an extra
   // time... so that you can actually call methods ON the Node, because

llvm/include/llvm/ADT/PostOrderIterator.h

@@ -100,7 +100,7 @@ class po_iterator : public po_iterator_storage<SetType, ExtStorage> {
   using value_type = typename GT::NodeRef;
   using difference_type = std::ptrdiff_t;
   using pointer = value_type *;
-  using reference = value_type &;
+  using reference = const value_type &;
 
 private:
   using NodeRef = typename GT::NodeRef;
@@ -161,7 +161,7 @@ class po_iterator : public po_iterator_storage<SetType, ExtStorage> {
   }
   bool operator!=(const po_iterator &x) const { return !(*this == x); }
 
-  const NodeRef &operator*() const { return std::get<0>(VisitStack.back()); }
+  reference operator*() const { return std::get<0>(VisitStack.back()); }
 
   // This is a nonstandard operator-> that dereferences the pointer an extra
   // time... so that you can actually call methods ON the BasicBlock, because

llvm/unittests/ADT/BreadthFirstIteratorTest.cpp

@@ -70,4 +70,8 @@ TEST(BreadthFristIteratorTest, Cycle) {
   EXPECT_EQ(It, End);
 }
 
+static_assert(
+    std::is_convertible_v<decltype(*std::declval<bf_iterator<Graph<3>>>()),
+                          typename bf_iterator<Graph<3>>::reference>);
+
 } // end namespace llvm

llvm/unittests/ADT/DepthFirstIteratorTest.cpp

@@ -50,4 +50,9 @@ TEST(DepthFirstIteratorTest, ActuallyUpdateIterator) {
 
   EXPECT_EQ(3, S.InsertVisited);
 }
+
+static_assert(
+    std::is_convertible_v<decltype(*std::declval<df_iterator<Graph<3>>>()),
+                          typename df_iterator<Graph<3>>::reference>);
+
 }

llvm/unittests/ADT/PostOrderIteratorTest.cpp

@@ -36,6 +36,10 @@ TEST(PostOrderIteratorTest, Compiles) {
   PIExt.insertEdge(std::optional<BasicBlock *>(), NullBB);
 }
 
+static_assert(
+    std::is_convertible_v<decltype(*std::declval<po_iterator<Graph<3>>>()),
+                          typename po_iterator<Graph<3>>::reference>);
+
 // Test post-order and reverse post-order traversals for simple graph type.
 TEST(PostOrderIteratorTest, PostOrderAndReversePostOrderTraverrsal) {
   Graph<6> G;