add ranged iterator support to mozilla::{Array,RangedArray,EnumerationArray}

RESOLVED FIXED in Firefox 44

Status

()

defect
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: heycam, Assigned: heycam)

Tracking

unspecified
mozilla44
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox44 fixed)

Details

Attachments

(1 attachment)

I want to use ranged for-loop syntax on an EnumeratedArray.  And I may as well add support to Array and RangedArray while I'm at it.
Assignee: nobody → cam
Status: NEW → ASSIGNED
Attachment #8675513 - Flags: review?(nfroyd)
Blocks: 1216043
Comment on attachment 8675513 [details] [diff] [review]
Add ranged iterator support to mozilla::{Array,RangedArray,EnumerationArray}.

Review of attachment 8675513 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with nitpicky consistency changes below.

::: mfbt/EnumeratedArray.h
@@ +77,5 @@
> +
> +  // Methods for range-based for loops.
> +  iterator begin() { return mArray.begin(); }
> +  const_iterator begin() const { return mArray.begin(); }
> +  const_iterator cbegin() const { return mArray.begin(); }

Nit: please make this cbegin() for consistency.  (Since you use the c* variants below.)

::: mfbt/EnumeratedRange.h
@@ +172,5 @@
>  
>  // Create a range to iterate from aBegin to aEnd, exclusive.
> +//
> +// (Once we can rely on std::underlying_type, we can remove the IntType
> +// template parameter.)

Feel free to fold these changes to this file into bug 1216038.

::: mfbt/RangedArray.h
@@ +46,5 @@
> +
> +  // Methods for range-based for loops.
> +  iterator begin() { return mArr.begin(); }
> +  const_iterator begin() const { return mArr.begin(); }
> +  const_iterator cbegin() const { return mArr.begin(); }

Here too.
Attachment #8675513 - Flags: review?(nfroyd) → review+
(In reply to Nathan Froyd [:froydnj] from comment #2)
> > +  const_iterator cbegin() const { return mArray.begin(); }
> 
> Nit: please make this cbegin() for consistency.  (Since you use the c*
> variants below.)

Sorry, yes I noticed these locally but forgot to update the patch.

> ::: mfbt/EnumeratedRange.h
> @@ +172,5 @@
> >  
> >  // Create a range to iterate from aBegin to aEnd, exclusive.
> > +//
> > +// (Once we can rely on std::underlying_type, we can remove the IntType
> > +// template parameter.)
> 
> Feel free to fold these changes to this file into bug 1216038.

Will do.
https://hg.mozilla.org/mozilla-central/rev/c3e202bf0f25
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Depends on: 1299489
You need to log in before you can comment on or make changes to this bug.