-
Notifications
You must be signed in to change notification settings - Fork 758
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change ArenaVector<T>::Iterator to satisfy standard (Legacy)RandomAccessIterator concept #1962
Change ArenaVector<T>::Iterator to satisfy standard (Legacy)RandomAccessIterator concept #1962
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Ryooooooga! Some comments below.
src/mixed_arena.h
Outdated
} | ||
|
||
bool operator<(const Iterator& other) const { | ||
return (parent->data + index) < (other.parent->data + other.index); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's enough to compare the indexes here - as the parent must be identical? (perhaps we should assert on that?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since a < b
must be equal to b - a > 0
[*1], I agree that it is better to order only among iterators with the same parent.
509dd76
*1: https://en.cppreference.com/w/cpp/named_req/RandomAccessIterator
} | ||
|
||
T* operator->() const { | ||
return &(*parent)[index]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it common to define ()
to be identical to *
like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not operator ()
but operator ->
to allow it->member_of_T
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, right, I misread that.
…pairs that have the same parent.
Great, thanks again @Ryooooooga! |
to pass iterators to the functions defined in <algorithm>.