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

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 12 11:29:03 PST 2021


dexonsmith added a comment.

Thanks for the review! Landed in 1b651be0465de70cfa22ce4f715d3501a4dcffc1 <https://reviews.llvm.org/rG1b651be0465de70cfa22ce4f715d3501a4dcffc1>.

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

>> 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*;
>>   ...
>>
>> ?
>
> Can we put that in the iterator adapter base somehow, so we don't have to make it a statement about each use of iterator adapters?

I tried adding something like `static_assert(&DerivedT::operator*, "")` to `iterator_adaptor_base` but at the point of evaluation (DerivedT's parent class), `DerivedT` doesn't have an `operator*` yet. For now I've left this alone.

(BTW, I have some follow-ups to clean up `operator->` and `operator[]`.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113158



More information about the llvm-commits mailing list