Re: [whatwg/streams] assert invariants in InvokeOrNoop and friends (#584)

domenic requested changes on this pull request.

This doesn't seem quite right. I assume this is in reaction to https://github.com/whatwg/streams/pull/573#issuecomment-257418589 ? Can you help me understand that a bit more? For example if `O` is undefined then the current code will fail, right? So it's not every argument.

Maybe what's just missing is the assert on `P`?

> @@ -62,6 +62,10 @@ exports.IsFiniteNonNegativeNumber = v => {
 };
 
 exports.InvokeOrNoop = (O, P, args) => {
+  assert(exports.typeIsObject(O) === true);

This is not accurate. We use GetV, not Get. So in theory `O` could be anything.

> @@ -62,6 +62,10 @@ exports.IsFiniteNonNegativeNumber = v => {
 };
 
 exports.InvokeOrNoop = (O, P, args) => {
+  assert(exports.typeIsObject(O) === true);
+  assert(typeof P === 'string');

This is already in the spec, sort of, as "Assert: ! IsPropertyKey(P) is true." Probably there should be a (non-exported?) helper for that in this file, which checks if typeof is string or symbol.

> @@ -62,6 +62,10 @@ exports.IsFiniteNonNegativeNumber = v => {
 };
 
 exports.InvokeOrNoop = (O, P, args) => {
+  assert(exports.typeIsObject(O) === true);
+  assert(typeof P === 'string');
+  assert(Array.isArray(args));

This is not accurate; the spec says "If args was not passed, let args be a new empty List."

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/streams/pull/584#pullrequestreview-6526197

Received on Monday, 31 October 2016 21:12:55 UTC