GetIterator should check for next being a method

[bergus]

The GetIterator abstract operation sometimes does return a broken iterator, in particular one where the [[NextMethod]] is not a function object. It seems that this goes against the spirit of #976 (#988/#1021) and #2591: not only should the .next method be accessed only once, it should also need to be checked for being a method only once. If an Iterator Record is produced, it should be a working iterator, where [[NextMethod]] has a "function object" value.

There would only be a minimal observable change (but still making this normative and requiring consensus). As far as I can tell from the spec, and as previously suggested by @devsnek, the only thing that would have different observable behaviour is array destructuring. Every single other usage of GetIterator in the spec is immediately followed by an IteratorStep, which attempts to call the [[NextMethod]] and throws an exception then. In particular:

const [] = {
  [Symbol.iterator]: () => ({
    next: undefined, // or some other non-callable value, or no property at all
    return(){ console.log("done"); return {done: true}; },
  }),
};
// now: logs "done", without checking `.next`
// then: throws error

function getObj() { console.log("side effect"); return {}; }
[getObj().prop] = {
  [Symbol.iterator]: () => ({}),
};
// now: logs "side effect" before throwing a TypeError
// then: throws the TypeError immediately

This would make tc39/proposal-iterator-helpers#232 PR superfluous, and it would lead to more consistent behaviour (early failure) since that's also what the iterator helper methods from the proposal will do (e.g. const [] = Object.create(Iterator.prototype).take(0); will throw during the take call).

If this behaviour were not to be changed, the table that defines the shapes of the Iterator Records needs an editorial change to say "unknown" instead of "function object".

[bakkot]

Previously discussed in #1288.

[michaelficarra]

I've added a topic to the agenda of the upcoming plenary that proposes this change as a potential alternative to reverting the linked iterator helpers PR. I've also opened tc39/proposal-iterator-helpers#274 to do the revert, in case that's the committee preference. I'm neutral, but would prefer consistency.

[michaelficarra]

Today the committee decided that we would like to continue to delay failure for empty array destructuring of malformed iterators. We will track the Iterator Record slot type change in #3033.