[PATCH] D113158: ADT: Fix const-correctness of iterator adaptors

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith marked an inline comment as done.
dexonsmith added a comment.

@dblaikie / others, I have some doubts after doing a deeper audit of custom iterators in LLVM.

There are lots of custom iterators that *only* define a non-const qualified `operator*()`. In some cases, it'd make sense to make a member `mutable`, or change the return type to always be `const`-qualified. But perhaps not in every case?

Another option, which would be consistent with iterator_facade_base's implementations of `[]` and `->`, would be to define both here but not adjust the return type:

  ReferenceT operator*() { return *I; } // Note: return has no `const`
  ReferenceT operator*() const { return *I; }

Other adaptors could also overload `operator*()`, but also would avoid adding `const` to the return.

This would:

- Conform to the `ReferenceT` defined in the template parameters (avoids my main concern from https://reviews.llvm.org/D112981).
- Support adapting iterators where `operator*()` is non-`const`.
- Error on `const`-qualified dereferences if the adapted iterator doesn't return `ReferenceT` for that.

WDYT?


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

https://reviews.llvm.org/D113158