search

tc39 / proposal-joint-iteration

a TC39 proposal to synchronise the advancement of multiple iterators
https://tc39.es/proposal-joint-iteration
51 stars 2 forks source link

Iterator.zip reads properties of the "bag of iterables" overload twice #7

Closed bakkot closed 7 months ago

bakkot commented 7 months ago

Iterator.zip step 14.c.i does iterables.[[GetOwnProperty]](key), and then 14.d.i does Get(iterables, key). That's silly: we already have the property descriptor in the earlier step and can just use the [[Value]] of it directly, instead of doing another observable lookup. The loops in c and d should be collapsed.

michaelficarra commented 7 months ago

Collapsing the loops means we will open iterators before enumerating all the keys. I guess that's fine?

bakkot commented 7 months ago

Yeah I don't see a problem with that. Enumerating keys shouldn't be side-effecting anyway, and if it is I think there's no reasonable expectation about ordering of those effects vs opening iterators.

michaelficarra commented 7 months ago

I will also have to handle getters. I don't suppose there's any AO I can re-use for that?

bakkot commented 7 months ago

Not that I'm seeing.

But now that I'm actually going through things, it looks like the pattern in Object.assign is to not re-use the property descriptor, and in fact just invoke Get after checking the descriptor for enumerability. That seems silly but I guess we should follow precedent?

I still think the loops should be collapsed though.

michaelficarra commented 7 months ago

I don't think we should care all that much about following precedent here.