1386838 - Give nsTObserverArray standard iterator types

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement, P3)

Description

[(no longer active)]

Basically things adhering to this type definition for nsTArray: https://searchfox.org/mozilla-central/rev/bbc1c59e460a27b20929b56489e2e55438de81fa/xpcom/ds/nsTArray.h#864

This will allow these iterators to be used in C++11 for loops, Reverse(), etc.

Comment 1

[Eric Rahm [:erahm]]

This is a bit tricky since we support mutating the array during iteration. We have a couple types of iterators in there, ForwardIterator and EndLimitedIterator, that behave differently depending on mutation behavior. They also require that `HasMore` is checked before retrieving the next item.

I'm not even sure how we could define `end()` in this case.

Comment 2

[Simon Giesecke [:sg] [he/him]]

(In reply to Eric Rahm [:erahm] from comment #1)

This is a bit tricky since we support mutating the array during iteration.
We have a couple types of iterators in there, ForwardIterator and
EndLimitedIterator, that behave differently depending on mutation behavior.

That's not a fundamental issue for providing standard iterators. They could be returned by non-standard member functions, e.g. forward_begin, forward_end, end_limited_begin, end_limited_end.

Only if we want to be a Range (and support range-based for), we would need to decide which of these iterator flavours should be used for that. If we think the user needs the choice, we could provide them via a simple wrapper type templates such that one could write:

nsTObserverArray<Foo> array;
for (const Foo& foo : ForwardIterated(array)) { ... }
for (const Foo& foo : EndLimitedIterated(array)) { ... }

with (just a sketch)

template<typename ObserverArrayT> struct ForwardIterated {
// maybe define value_type etc.

ForwardIterated(ObserverArrayT &aArray);

auto begin() { return mArray.forward_begin(); }
auto end() { return mArray.forward_end(); }

private:
ObserverArrayT &aArray;
};

They also require that HasMore is checked before retrieving the next item.

I'm not even sure how we could define end() in this case.

A Range can now have different types for begin and end iterators, maybe this could be exploited here.

Range-based for also allows for that with C++17.

Comment 3

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1386838 - Add STL-style ranges for nsTObserverArray based on ForwardIterator/EndLimitedIterator/BackwardIterator. r=#xpcom-reviewers

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f75d1d3e2f64
Add STL-style ranges for nsTObserverArray based on ForwardIterator/EndLimitedIterator/BackwardIterator. r=froydnj

Comment 5

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/f75d1d3e2f64