Skip to content

Commit

Permalink
[iterator-helpers] Check undefined instead of call
Browse files Browse the repository at this point in the history
This CL changes Symbol.iterator fallback from callable check to
undefined/null check. This is based on
tc39/proposal-iterator-helpers#272 and
discussion on May 2023 TC39 meeting.

Bug: v8:13558
Change-Id: If06bf77df383b06d1a8f6875f1760568089c08ae
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4545750
Commit-Queue: Rezvan Mahdavi Hezaveh <rezvan@chromium.org>
Reviewed-by: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#87758}
  • Loading branch information
rmahdav authored and V8 LUCI CQ committed May 19, 2023
1 parent c30a08d commit 79447a0
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 77 deletions.
95 changes: 21 additions & 74 deletions src/builtins/iterator-from.tq
Original file line number Diff line number Diff line change
Expand Up @@ -15,85 +15,32 @@ macro NewJSValidIteratorWrapper(implicit context: Context)(
}

// https://tc39.es/proposal-iterator-helpers/#sec-getiteratorflattenable
//
// Currently never used with the async hint, so only the sync path is
// implemented.
transitioning macro GetIteratorFlattenable(implicit context: Context)(
obj: JSReceiver): IteratorRecord {
try {
// 1. If obj is not an Object, throw a TypeError exception.
// (Done by caller.)

// 2. Let alreadyAsync be false.
//
// (Unimplemented because the async path is unused.)

// 3. Let method be undefined.
//
// (Done below.)

// 4. If hint is async, then
// a. Set method to ? Get(obj, @@asyncIterator).
// b. Set alreadyAsync to true.
//
// (Unimplemented because unused.)

// 5. If IsCallable(method) is false, then
// a. Set method to ? Get(obj, @@iterator).
const method = GetProperty(obj, IteratorSymbolConstant());

// b. Set alreadyAsync to false.
//
// (Unimplemented because unused.)

let iterator: JSAny;

// 6. If IsCallable(method) is false, then
if (!Is<Callable>(method)) {
// a. Let iterator be obj.
iterator = obj;

// b. Set alreadyAsync to true.
//
// (Unimplemented because unused.)
} else {
// 7. Else,
// a. Let iterator be ? Call(method, obj).
iterator = Call(context, UnsafeCast<Callable>(method), obj);
}
// 1. If obj is not an Object, throw a TypeError exception.
// (Done by caller.)

// 8. If iterator is not an Object, throw a TypeError exception.
const iteratorObj = Cast<JSReceiver>(iterator)
otherwise goto IteratorNotObject(obj, method);

// 9. Let nextMethod be ? Get(iterator, "next").
const nextMethod = GetProperty(iteratorObj, kNextString);
let iterator: JSAny;
try {
// 2. Let method be ? GetMethod(obj, @@iterator).
const method = GetMethod(obj, IteratorSymbolConstant())
otherwise IfNullOrUndefined;

// 4. Else (method is not undefined),
// a. Let iterator be ? Call(method, obj).
iterator = Call(context, method, obj);
} label IfNullOrUndefined {
// 3. If method is undefined, then
// a. Let iterator be obj.
iterator = obj;
}

// 10. If IsCallable(nextMethod) is false, throw a TypeError exception.
if (!Is<Callable>(nextMethod)) {
ThrowTypeError(
MessageTemplate::kPropertyNotFunction, nextMethod, kNextString, obj);
}
// 5. If iterator is not an Object, throw a TypeError exception.
const iteratorObj = Cast<JSReceiver>(iterator)
otherwise ThrowTypeError(MessageTemplate::kNotIterable, obj);

// 11. Let iteratorRecord be the Iterator Record { [[Iterator]]: iterator,
// [[NextMethod]]: nextMethod, [[Done]]: false }.
const iteratorRecord =
IteratorRecord{object: iteratorObj, next: nextMethod};

// 12. If hint is async and alreadyAsync is false, then
// a. Return CreateAsyncFromSyncIterator(iteratorRecord).
//
// (Unimplemented because unused.)

// 13. Return iteratorRecord.
return iteratorRecord;
} label IteratorNotObject(obj: JSAny, method: JSAny) deferred {
if (Is<Callable>(method)) {
ThrowTypeError(MessageTemplate::kSymbolIteratorInvalid);
} else {
ThrowTypeError(MessageTemplate::kNotIterable, obj);
}
}
// 6. Return ? GetIteratorDirect(iterator).
return GetIteratorDirect(iteratorObj);
}

// https://tc39.es/proposal-iterator-helpers/#sec-iterator.from
Expand Down
9 changes: 6 additions & 3 deletions test/mjsunit/harmony/iterator-from.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@ assertThrows(() => {
}, TypeError);

assertThrows(() => {
Iterator.from({[Symbol.iterator]() {
return { next: "not callable" };
}});
Iterator.from({
[Symbol.iterator]: 0,
next() {
return 42;
},
});
}, TypeError);

0 comments on commit 79447a0

Please sign in to comment.