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

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

dexonsmith added a comment.

In D113158#3120023 <https://reviews.llvm.org/D113158#3120023>, @dblaikie wrote:

>> 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?
>
> I think that covers all cases I can think of, and seems like the right thing - but if it means lots of cleanup, and just keeping/making two overloads (const/non-const) that both return non-const ref (well, return ref, whatever that is - const ref for const_iterators (that themselves may or may not be const... you know what I mean)) is significantly cheaper right now - I'd be OK with that. Seems like it'd make things incrementally better at least - leave a comment aobut it being a stop-gap, not a complete solution, maybe some comments about the functionality or patterns that are deprecated/urge people towards just writing the one const op*?

I think I'd rather commit as-is with just one `operator*() const` if you agree it's the right answer. (I found the counter-examples by doing a grep for `operator*() {`, but they aren't being used in iterator adaptors right now... so I don't think we need a stop-gap.)

Think there's value in adding a test that a second operator non-const-qualified operator isn't added as a place to add a comment, something like:

  // There should only be one `operator*()` and it should be const-qualified.
  // Rather than adding overloads, fields in adapted iterators should be made
  // mutable or ReferenceT should be `const`-qualified.
  auto x = &decltype(make_filter_range(...).begin())::operator*;
  auto x = &decltype(make_map_range(...).begin())::operator*;
  auto x = &decltype(reverse(...).begin())::operator*;
  ...

?


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

https://reviews.llvm.org/D113158