[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