tc39 / proposal-async-iteration

Asynchronous iteration for JavaScript
https://tc39.github.io/proposal-async-iteration/
MIT License
858 stars 44 forks source link

Add a note that the type checks in async-from-sync-iterator can be omitted in some engines #105

Closed domenic closed 5 years ago

domenic commented 7 years ago

Via @caitp in #jslang.

In particular they are only necessary if you can get access to the functions via .caller, or similar nonstandard extensions. And for engines that implement built-ins as strict, they are already prohibited from .caller returning the functions.

Now that I state it this I'm not sure whether we should keep the type checks any more, since .caller is nonstandard, and we don't generally add type checks in places just in case someone implements a nonstandard feature that hits an unexpected spec code path. Hmm.

Opinions, @bterlson @allenwb?

allenwb commented 7 years ago

Are you talking about these checks: If Type(O) is not Object, or if O does not have a [[SyncIterator]] internal slot, then ?

If so, I think they are necessary. Otherwise, there would be unguarded accesses to O.[[SyncIterator]].

Implementations can always skip checks that they can prove will never fail (in their implementation).

domenic commented 7 years ago

Yep, those are the ones.

Otherwise, there would be unguarded accesses to O.[[SyncIterator]].

Well, the issue is that this class and its methods are only ever created/called by for-await-of syntax. Thus this whole class is somewhat similar to the Iterator object created by EnumerateObjectProperties.

However, unlike the EnumerateObjectProperties iterator, it has the added wrinkle that this inaccessible-class wraps a user-provided object, which means you could use .caller to get access to the class's methods. Thus the OP's confusion about what to do.

allenwb commented 7 years ago

However, unlike the EnumerateObjectProperties iterator, it has the added wrinkle that this inaccessible-class wraps a user-provided object, which means you could use .caller to get access to the class's methods.

Which is why I'd say the guard is necessary. Anywhere it is possible for an extension to invalidate type dataflow derived slot access soundness then there should be a guard.

domenic commented 7 years ago

Right, but the issue is that you can only access it when using nonstandard extensions. What if someone added a nonstandard extension (call it for.in) which gave you access to the Iterator created by EnumerateObjectProperties? Should we go edit the spec for that Iterator to add guards?

(To be clear, I pretty much agree the safe thing to do is leave the guards in, and probably will do that. I am just curious about the reasoning and trying to figure out a consistent principle we use here.)

caitp commented 7 years ago

I'm not sure why it's not feasible to just Assert: O is an Object with a [SyncIterator] internal slot, which is likely what implementations will do to ensure it's not exposed