GetKeysIterator should defer IsCallable check on next

[syg]

After getting consensus for tc39/proposal-iterator-helpers#274 for iterator helpers, GetKeysIterator here should also defer the IsCallable check.

[bakkot]

Yeah, makes sense. I'll bring this next meeting.

[michaelficarra]

same for GetSetRecord

[bakkot]

I'm a bit less convinced for GetSetRecord - there are paths not involving empty sets where one of these methods doesn't get invoked at all.

[michaelficarra]

Yeah, so to be consistent with our choices at the last meeting, we should not validate the interface.

[bakkot]

I am not convinced that consistency with the choice at the last meeting requires that, especially because these are string-named methods and there's multiple of them. An iterator whose Symbol.iterator method returns something with a non-callable next is much more obviously broken (i.e., a much harder mistake to make) than accidentally passing an object which happens to have a .size property but not keys and/or has methods, so it seems like it's worth more explicitly guarding against the latter.

Also we did explicitly discuss exactly this question and decided on exactly these semantics. I don't want to revisit that. You can add something to the agenda if you really want to, but I'm not going to.

And to be concrete, the thing that I plan to do is make all of the Set methods in this proposal eagerly access the .size getter, the .has property the .keys property, and then check that the .has property is callable and the .keys property is callable and call ToNumber on the size property, and then having done that to implement the remainder of the algorithm using those particular things. And in the case of union, it will have text that checks .has is callable even though it will never in fact call it, but at least this ensures that Union and intersection require the same interface even if not necessarily the same behavior for that interface.

[michaelficarra]

That's fair, but I think you're still obligated to bring it up and reconfirm that plan, given the recent iterator helpers decisions.

[zloirock]

What's the result discussion of this issue?

[michaelficarra]

@zloirock It was approved. Notes will be out in 2 weeks.

[bakkot]

Specifically, dropping the IsCallable check in GetKeysIterator was approved, but we're keeping the others mentioned above.