| 19:37 | <TabAtkins> | I'm translating the Map/Set prototype methods over to WebIDL-ese (so we can define maplikes and setlikes in terms of Infra maps and sets, rather than doing weird and fraught indirections to actual ES Maps and Sets), and I noticed that while the @@iterator for them records the initial length and only iterates to that length at max (can stop earlier if things are deleted so it hits the end before that point), the forEach methods just visit every entry "live" and can potentially run forever if the callback keeps adding entries. |
| 19:37 | <TabAtkins> | Is this behavior difference intentional? |
| 19:43 | <bakkot> | uh |
| 19:43 | <bakkot> | that's probably a bug from when we refactored those |
| 19:44 | <bakkot> | oh, wait, no it's not |
| 19:44 | <bakkot> | you missed a step: |
| 19:44 | <bakkot> | CreateMapIterator step 2.d.iii.6: Set numEntries to the number of elements of entries. |
| 19:44 | <TabAtkins> | Right, I mentioned that. |
| 19:44 | <bakkot> | no, that's in the loop |
| 19:45 | <bakkot> | it updates numEntries within the loop |
| 19:45 | <bakkot> | or I am not understanding your question |
| 19:45 | <TabAtkins> | Oh dang you're right, I did miss/misunderstand that. |
| 19:48 | <TabAtkins> | Hm then, the two methods can both run infinitely, ok. Is there a particular reason why the two have their iteration written significantly differently, or is that intended to just be an editorial detail? (Iterator goes over the entries by index and makes no mention of "empty" values; forEach iterates the entries list directly and explicitly jumps over "empty" values. (I note that "empty" is supposed to be a spec convenience for deleted entries and not an actual author-exposed thing.)) |
| 19:50 | <bakkot> | Iterator does handle empty values, it seems to me?
|
| 19:51 | <bakkot> | They actually look pretty similar to me, all told |
| 19:51 | <TabAtkins> | Sigh, I'm blind. |
| 19:51 | <TabAtkins> | Yeah, so it's just the explicit index-based vs just looping over the List directly, I guess |
| 19:52 | <bakkot> | yeah, and that one... I think it's a path-dependence thing |
| 19:53 | <bakkot> | in earlier editions iterators were specified in terms of explicitly keeping all of the state on internal slots, but in https://github.com/tc39/ecma262/pull/2045 we made it possible to specify a "spec generator" which is more similar to how you'd write it in JS |
| 19:53 | <bakkot> | prior to that, the iterator needed to be specified in terms of an index so it could store the index on an internal slot, as in https://tc39.es/ecma262/2016/#sec-%mapiteratorprototype%.next |
| 19:54 | <bakkot> | with the refactoring that's no longer strictly necessary but the refactoring was written as a delta from what was originally there |
| 19:54 | <TabAtkins> | Ahhhhh, ok, thanks for the history, that makes sense. |
| 20:16 | <TabAtkins> | Okay, more questions in this regard: Array's forEachdoes compute the length once at the beginning and then doesn't go past it https://tc39.es/ecma262/#sec-array.prototype.foreach, so you can't infinite-loop yourself by pushing to the array in the callback. Is the behavior difference between arr.forEach and map.forEach intentional or accidental? (And which should we copy for general web-platform purposes when we define iteration better for Infra types in https://github.com/whatwg/infra/issues/396?) |
| 20:18 | <bakkot> | That one is before my time, so someone else will have to give the actual answer |
| 20:18 | <bakkot> | but I do note that looking up the length of an array is observable, whereas looking up the length of a Map's internal list is not, so it's possible that's the reason for the difference? |
| 20:18 | <TabAtkins> | Hm, but Array's @@iterator does allow infinite-loops, so I guess that might just be a legacy detail we got stuck with |
| 20:18 | <bakkot> | ah, yeah, in that case probably |
| 20:22 | <bakkot> | that does seem to be the consensus in other languages also |
| 20:22 | <TabAtkins> | Excellent, thanks. Looks like we just need to add some details to Infra to make it clear how mid-iteration changes to the collection should work; we can match JS and define it in terms of an internal index that advances until it's past the (current) end. |
| 20:22 | <TabAtkins> | And that'll mean I get to simplify some of my text in WebIDL that's doing this by hand, which is nice. |