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

[Markus Böck via Phabricator via llvm-commits]

zero9178 added inline comments.


================
Comment at: llvm/include/llvm/ADT/BreadthFirstIterator.h:53
   using pointer = value_type *;
-  using reference = value_type &;
+  using reference = const value_type &;
 
----------------
kuhar wrote:
> Could we add const conditionally, either with some type traits or define `reference` in terms of `decltype(*declval<pointer>())`?
> 
> Do you know if it there's any existing use of non-const references with these iterators?
The result of `operator*` has always been a const-reference so there couldn't have been (or at the very least can't currently be) any real properly compiling code that relies on `reference` being non-const while `operator*` isn't. 

 Defining it to `decltype(*declval<pointer>())` would just be equal to `value_type&` no? That would at the very least not be possible in a non-const `operator*` and I think is intentionally not the case, since a `NodeRef&` would allow breaking the invariants of the iterators (would modify the `NodeRef` within `VisitQueue`)

Hope I understood your comment correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151198