[ADT] Add non-const operator* to iterator_adaptor_base and df_iterator.

[fhahn]

Summary

The missing non-const operator* implementation are causing issues when
using filter_iterator with df_iterator: without the non-const
implementation, we can only return const references to the underlying
values.

This patch also adjusts the return type of operator* cont in
interator_adaptor_base.

[merge_guards_bot]

Build result: FAILURE -
Log files: console-log.txt, CMakeCache.txt

[fhahn]

In D70735#1760590, @merge_guards_bot wrote:

Build result: FAILURE -
Log files: console-log.txt, CMakeCache.txt

failed because connection timeout.

[dblaikie]

Could you add some test coverage (showing what would break, specifically - an iterator with only a const op* being adapted that would previously (well, previous to this patch, so currenctly in tree - might just take updating an existing iterator in the test case to fall into this situation) fail to compile) to llvm/unittests/ADT/IteratorTest.cpp

[fhahn]

In D70735#1765288, @dblaikie wrote:

Could you add some test coverage (showing what would break, specifically - an iterator with only a const op* being adapted that would previously (well, previous to this patch, so currenctly in tree - might just take updating an existing iterator in the test case to fall into this situation) fail to compile) to llvm/unittests/ADT/IteratorTest.cpp

I just gave it a try with the iterators available in the unit tests, but so far I did not manage to replicate the compilation failure. My motivation is the vpbb_iterator_adaptor in D70734, which cannot be used to iterate over const VPlan objects. @dblaikie do you think this patch actually makes sense or just papers over a different issue?

[dblaikie]

In D70735#1765776, @fhahn wrote:

In D70735#1765288, @dblaikie wrote:

Could you add some test coverage (showing what would break, specifically - an iterator with only a const op* being adapted that would previously (well, previous to this patch, so currenctly in tree - might just take updating an existing iterator in the test case to fall into this situation) fail to compile) to llvm/unittests/ADT/IteratorTest.cpp

I just gave it a try with the iterators available in the unit tests, but so far I did not manage to replicate the compilation failure. My motivation is the vpbb_iterator_adaptor in D70734, which cannot be used to iterate over const VPlan objects. @dblaikie do you think this patch actually makes sense or just papers over a different issue?

Ah, OK - well it sounds like there are two independent changes here, then. (which I guess was sort of obvious from the changes) - I'm guessing, currently, the depth first iterator would not work as the underlying iterator to the iterator_adaptor_base? So this patch is currently making two changes to "meet in the middle"? (ie: they're still incompatible until both these changes are made)

Does the issue not reproduce if you make an underlying iterator with only a const op* (that returns a const ref) & wrap that with iterator_adaptor_base?

[fhahn]

I resolved my original motivation for the patch in a different way, so I won't be pushing this any further for now

llvm/include/llvm/ADT/DepthFirstIterator.h

public:
   static df_iterator end(const GraphT &G, SetType &S) { return df_iterator(S); }
 
   bool operator==(const df_iterator &x) const {
     return VisitStack == x.VisitStack;
   }
   bool operator!=(const df_iterator &x) const { return !(*this == x); }
 
   const NodeRef &operator*() const { return VisitStack.back().first; }
+  NodeRef &operator*() { 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
   // the contained type is a pointer.  This allows BBIt->getTerminator() f.e.
   //
   NodeRef operator->() const { return **this; }
 
   df_iterator &operator++() { // Preincrement

llvm/include/llvm/ADT/iterator.h

public:
   bool operator==(const DerivedT &RHS) const { return I == RHS.I; }
   bool operator<(const DerivedT &RHS) const {
     static_assert(
         BaseT::IsRandomAccess,
         "Relational operators are only defined for random access iterators.");
     return I < RHS.I;
   }
 
-  ReferenceT operator*() const { return *I; }
+  const ReferenceT operator*() const { return *I; }
+  ReferenceT operator*() { return *I; }
 };
 
 /// An iterator type that allows iterating over the pointees via some
 /// other iterator.
 ///
 /// The typical usage of this is to expose a type that iterates over Ts, but
 /// which is implemented with some iterator over T*s:
 ///