1478879 - Introduce Iterator and ModIterator in HashTable.h

Status: RESOLVED FIXED

(Core :: MFBT, enhancement)

Description

[Nicholas Nethercote [inactive]]

Range and Enum are basically just iterators with non-standard names. This change will bring HashTable.h in line with PLDHashTable/nsTHashtable.

Comment 1

[Nicholas Nethercote [inactive]]

I was originally planning to remove Range and Enum, but I underestimated how widely they are used within SpiderMonkey. Furthermore, there are plenty of other FooRange classes for iterating over other data structures.

Changing all of those would take a bunch of time without providing that much value. So I plan to convert the few existing uses of Enum/Range outside of SpiderMonkey to Iterator/ModIterator -- for consistency with PLDHashTable/nsTHashtable -- but leave the ones within SpiderMonkey as is.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: Bug 1478879 - Remove zero-arg constructor for Range.

Review commit: https://reviewboard.mozilla.org/r/260672/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/260672/

Comment 3

[Andrew McCreight [:mccr8]]

Comment on attachment 8996567 [details]
Bug 1478879 - Remove zero-arg constructor for Range.

https://reviewboard.mozilla.org/r/260672/#review267674

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: Bug 1478879 - Introduce Iterator and ModIterator in HashTable.h.

These basically duplicate the existing Range and Enum classes, but use more
familiar terminology, similar to the iterators we have for
PLDHashTable/nsTHashtable:

- Hash{Set,Map}::all()  Hash{Set,Map}::iter()
- Enum constructor      Hash{Set,Map}::modIter()
- Range::front()        Iterator::get()
- Range::popFront()     Iterator::next()
- Range::empty()        Iterator::done()
- Enum::mutableFront()  ModIterator::getMutable()
- Enum::removeFront()   ModIterator::remove()
- Enum::rekeyFront()    ModIterator::rekey()

The next patch will reduce the amount of code duplication.

Review commit: https://reviewboard.mozilla.org/r/260674/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/260674/

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: Bug 1478879 - Define Range/Enum in terms of Iterator/ModIterator.

To reduce the code duplication.

Review commit: https://reviewboard.mozilla.org/r/260676/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/260676/

Comment 6

[Nicholas Nethercote [inactive]]

Attachment: Bug 1478879 - Remove Range/Enum use in testHashTable.cpp.

Review commit: https://reviewboard.mozilla.org/r/260678/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/260678/

Comment 7

[Nicholas Nethercote [inactive]]

Attachment: Bug 1478879 - Remove Range/Enum use in DMD.

Review commit: https://reviewboard.mozilla.org/r/260680/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/260680/

Comment 8

[Nicholas Nethercote [inactive]]

Attachment: Bug 1478879 - Remove Range/Enum use in Bench.cpp.

Review commit: https://reviewboard.mozilla.org/r/260682/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/260682/

Comment 9

[Nicholas Nethercote [inactive]]

Attachment: Bug 1478879 - Remove Range/Enum use in dom/.

Review commit: https://reviewboard.mozilla.org/r/260684/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/260684/

Comment 10

[Nicholas Nethercote [inactive]]

Attachment: Bug 1478879 - Remove Range/Enum use in devtools/.

Review commit: https://reviewboard.mozilla.org/r/260686/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/260686/

Comment 11

[Nicholas Nethercote [inactive]]

Attachment: Bug 1478879 - Remove Range/Enum use in XPConnect.

Review commit: https://reviewboard.mozilla.org/r/260688/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/260688/

Comment 12

[Nicholas Nethercote [inactive]]

Comment on attachment 8996567 [details]
Bug 1478879 - Remove zero-arg constructor for Range.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/260672/diff/1-2/

Comment 13

[Nicholas Nethercote [inactive]]

mccr8: apologies, by accident I originally pushed just a single patch of the stack, which you then r+'d, but now with the full stack pushed MozReview wants you to r+ again.

Comment 14

[Nathan Froyd [:froydnj]]

Comment on attachment 8996578 [details]
Bug 1478879 - Remove Range/Enum use in Bench.cpp.

https://reviewboard.mozilla.org/r/260682/#review267738

Comment 15

[Andrew McCreight [:mccr8]]

Comment on attachment 8996577 [details]
Bug 1478879 - Remove Range/Enum use in DMD.

https://reviewboard.mozilla.org/r/260680/#review267756

Comment 16

[Andrew McCreight [:mccr8]]

Comment on attachment 8996579 [details]
Bug 1478879 - Remove Range/Enum use in dom/.

https://reviewboard.mozilla.org/r/260684/#review267758

Comment 17

[Andrew McCreight [:mccr8]]

Comment on attachment 8996581 [details]
Bug 1478879 - Remove Range/Enum use in XPConnect.

https://reviewboard.mozilla.org/r/260688/#review267760

Comment 18

[Luke Wagner [:luke]]

Comment on attachment 8996567 [details]
Bug 1478879 - Remove zero-arg constructor for Range.

https://reviewboard.mozilla.org/r/260672/#review267780

Nice!

Comment 19

[Luke Wagner [:luke]]

Comment on attachment 8996574 [details]
Bug 1478879 - Introduce Iterator and ModIterator in HashTable.h.

https://reviewboard.mozilla.org/r/260674/#review267782

Great!  I like the new names better.  It's also lovely, now that move semantics are a thing, to have modIter() return ModIterator by value.

Comment 20

[Luke Wagner [:luke]]

Comment on attachment 8996575 [details]
Bug 1478879 - Define Range/Enum in terms of Iterator/ModIterator.

https://reviewboard.mozilla.org/r/260676/#review267784

Comment 21

[Luke Wagner [:luke]]

Comment on attachment 8996576 [details]
Bug 1478879 - Remove Range/Enum use in testHashTable.cpp.

https://reviewboard.mozilla.org/r/260678/#review267786

Comment 22

[Luke Wagner [:luke]]

Comment on attachment 8996580 [details]
Bug 1478879 - Remove Range/Enum use in devtools/.

https://reviewboard.mozilla.org/r/260686/#review267788

(stealing)

Comment 23

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8996580 [details]
Bug 1478879 - Remove Range/Enum use in devtools/.

https://reviewboard.mozilla.org/r/260686/#review267790

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/591d08c5c22b
Remove zero-arg constructor for Range. r=luke
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e45ce266b66
Introduce Iterator and ModIterator in HashTable.h. r=luke
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c3b5f4eda62
Define Range/Enum in terms of Iterator/ModIterator. r=luke
https://hg.mozilla.org/integration/mozilla-inbound/rev/3bd2221b034d
Remove Range/Enum use in testHashTable.cpp. r=luke
https://hg.mozilla.org/integration/mozilla-inbound/rev/41aeecb74b97
Remove Range/Enum use in DMD. r=mccr8
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f7dbe4598b9
Remove Range/Enum use in Bench.cpp. r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/b0b47dcc5194
Remove Range/Enum use in dom/. r=mccr8
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd4f3d39873c
Remove Range/Enum use in devtools/. r=sfink
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c7c754ce8fd
Remove Range/Enum use in XPConnect. r=mccr8

Comment 25

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/591d08c5c22b
https://hg.mozilla.org/mozilla-central/rev/1e45ce266b66
https://hg.mozilla.org/mozilla-central/rev/2c3b5f4eda62
https://hg.mozilla.org/mozilla-central/rev/3bd2221b034d
https://hg.mozilla.org/mozilla-central/rev/41aeecb74b97
https://hg.mozilla.org/mozilla-central/rev/2f7dbe4598b9
https://hg.mozilla.org/mozilla-central/rev/b0b47dcc5194
https://hg.mozilla.org/mozilla-central/rev/fd4f3d39873c
https://hg.mozilla.org/mozilla-central/rev/3c7c754ce8fd