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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D70735#1765776 <https://reviews.llvm.org/D70735#1765776>, @fhahn wrote:

> In D70735#1765288 <https://reviews.llvm.org/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 <https://reviews.llvm.org/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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70735/new/

https://reviews.llvm.org/D70735