[message] Improve IteratorSymbolNonCallable error message [v8/v8 : main]
王澳 (Gerrit)
Attention is currently required from: Igor Sheludko, 王澳.
Patch set 3:Commit-Queue +1
To view, visit change 3813069. To unsubscribe, or for help writing mail filters, visit settings.
王澳 (Gerrit)
Attention is currently required from: Igor Sheludko.
Patch set 3:-Code-Review
王澳 (Gerrit)
Attention is currently required from: Igor Sheludko.
1 comment:
Patchset:
The error messages for other cases including Array.from and new Uint8Array() etc are broken after the CL https://chromium-review.googlesource.com/c/v8/v8/+/3798522. E.g.
```
const a = {
[Symbol.iterator]: 1
};
Array.from(a)
```
will throw "Spread syntax requires ...iterable[Symbol.iterator] to be a function".
I changed the error message back and add the receiver to the IteratorSymbolNonCallable to make it clearer. Igor, WDYT? Thanks!
To view, visit change 3813069. To unsubscribe, or for help writing mail filters, visit settings.
Feng Yu (Gerrit)
Attention is currently required from: Igor Sheludko, 王澳.
1 comment:
Patchset:
The error messages for other cases including Array. […]
Thanks for catching this!
How about sperate the message usage into two parts:
1. IteratorSymbolNonCallableWithSpread: Spread syntax requires ...iterable[Symbol.iterator] to be a function
2. IteratorSymbolNonCallable: Found non-callable @@iterator in %This first error code is used in `builtins-call-gen.cc`'s `CallOrConstructBuiltinsAssembler::CallOrConstructWithSpread`, the second error code used in other cases: typed-array-createtypedarray.tq, array-from.tq, typed-array-from.tq.
IMO, "Spread syntax requires ...iterable[Symbol.iterator] to be a function" will be more appropriate in spread(...) operation because it convey more context and message.
To view, visit change 3813069. To unsubscribe, or for help writing mail filters, visit settings.
Igor Sheludko (Gerrit)
Attention is currently required from: Feng Yu, 王澳.
1 comment:
Patchset:
Thanks for catching this! […]
Thanks!
I think as long as we can't point error message to the bad argument of the builtin adding "in #<Object>" to the message does't make it more clear.
How about mimicking the Safari's message again for the [Typed]Array.from() case:%s requires that the property of the first argument, items[Symbol.iterator], when exists, be a function
The message IDs can be something like:
1. SpreadIteratorSymbolNonCallable
2. FirstArgumentIteratorSymbolNonCallable
To view, visit change 3813069. To unsubscribe, or for help writing mail filters, visit settings.
王澳 (Gerrit)
Attention is currently required from: Feng Yu, Igor Sheludko.
Patch set 4:Commit-Queue +1
1 comment:
Patchset:
Thanks! […]
Done. Thanks all!
To view, visit change 3813069. To unsubscribe, or for help writing mail filters, visit settings.
Igor Sheludko (Gerrit)
Attention is currently required from: Feng Yu, 王澳.
Patch set 7:Code-Review +1
3 comments:
Patchset:
lgtm with nits
File src/builtins/typed-array-createtypedarray.tq:
Patch Set #7, Line 348:
labels IfConstructByArrayLike(JSReceiver, uintptr) {Could you please move the IfIteratorNotCallable block implementation to the caller side:
`, label IfIteratorNotCallable(JSAny)`
File src/builtins/typed-array-from.tq:
Patch Set #7, Line 156:
'TypedArray.from'`kBuiltinNameFrom` to be uniform.
To view, visit change 3813069. To unsubscribe, or for help writing mail filters, visit settings.
王澳 (Gerrit)
Attention is currently required from: Feng Yu.
Patch set 8:Commit-Queue +2
3 comments:
Patchset:
Thanks!
File src/builtins/typed-array-createtypedarray.tq:
Patch Set #7, Line 348:
labels IfConstructByArrayLike(JSReceiver, uintptr) {
Could you please move the IfIteratorNotCallable block implementation to the caller side: […]
Done. Thanks!
File src/builtins/typed-array-from.tq:
Patch Set #7, Line 156:
'TypedArray.from'`kBuiltinNameFrom` to be uniform.
Done
To view, visit change 3813069. To unsubscribe, or for help writing mail filters, visit settings.
王澳 (Gerrit)
Attention is currently required from: Feng Yu, 王澳.
Patch set 9:Commit-Queue +2
王澳 (Gerrit)
Attention is currently required from: Feng Yu, 王澳.
Patch set 10:Commit-Queue +2
V8 LUCI CQ (Gerrit)
V8 LUCI CQ submitted this change.
7 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: src/builtins/array-from.tq
Insertions: 2, Deletions: 1.
@@ -180,7 +180,8 @@
return a;
} label IteratorNotCallable(_value: JSAny) deferred {
ThrowTypeError(
- MessageTemplate::kFirstArgumentIteratorSymbolNonCallable, 'Array.from');
+ MessageTemplate::kFirstArgumentIteratorSymbolNonCallable,
+ '%Array%.from');
}
}
}
```
```
The name of the file: src/builtins/typed-array-createtypedarray.tq
Insertions: 8, Deletions: 6.
@@ -345,7 +345,8 @@
transitioning macro ConstructByJSReceiver(implicit context: Context)(
obj: JSReceiver): never
- labels IfConstructByArrayLike(JSReceiver, uintptr) {
+ labels IfConstructByArrayLike(JSReceiver, uintptr),
+ IfIteratorNotCallable(JSAny) {
try {
// TODO(v8:8906): Use iterator::GetIteratorMethod() once it supports
// labels.
@@ -364,10 +365,6 @@
goto IfConstructByArrayLike(obj, length);
} label IfInvalidLength(length: Number) {
ThrowRangeError(MessageTemplate::kInvalidTypedArrayLength, length);
- } label IfIteratorNotCallable(_value: JSAny) deferred {
- ThrowTypeError(
- MessageTemplate::kFirstArgumentIteratorSymbolNonCallable,
- 'TypedArray\'s constructor');
}
}
@@ -391,7 +388,8 @@
ConstructByTypedArray(typedArray) otherwise IfConstructByArrayLike;
}
case (obj: JSReceiver): {
- ConstructByJSReceiver(obj) otherwise IfConstructByArrayLike;
+ ConstructByJSReceiver(obj) otherwise IfConstructByArrayLike,
+ IfIteratorNotCallable;
}
// The first argument was a number or fell through and is treated as
// a number. https://tc39.github.io/ecma262/#sec-typedarray-length
@@ -412,6 +410,10 @@
// 56 for constructorName.
const elementsInfo = GetTypedArrayElementsInfo(map);
return ConstructByArrayLike(map, arrayLike, length, elementsInfo);
+ } label IfIteratorNotCallable(_value: JSAny) deferred {
+ ThrowTypeError(
+ MessageTemplate::kFirstArgumentIteratorSymbolNonCallable,
+ 'TypedArray\'s constructor');
}
}
```
```
The name of the file: tools/v8heapconst.py
Insertions: 24, Deletions: 24.
@@ -544,30 +544,30 @@
("old_space", 0x04a2d): "RegExpMultipleCache",
("old_space", 0x04e35): "SingleCharacterStringTable",
("old_space", 0x0523d): "BuiltinsConstantsTable",
- ("old_space", 0x0568d): "AsyncFunctionAwaitRejectSharedFun",
- ("old_space", 0x056b1): "AsyncFunctionAwaitResolveSharedFun",
- ("old_space", 0x056d5): "AsyncGeneratorAwaitRejectSharedFun",
- ("old_space", 0x056f9): "AsyncGeneratorAwaitResolveSharedFun",
- ("old_space", 0x0571d): "AsyncGeneratorYieldResolveSharedFun",
- ("old_space", 0x05741): "AsyncGeneratorReturnResolveSharedFun",
- ("old_space", 0x05765): "AsyncGeneratorReturnClosedRejectSharedFun",
- ("old_space", 0x05789): "AsyncGeneratorReturnClosedResolveSharedFun",
- ("old_space", 0x057ad): "AsyncIteratorValueUnwrapSharedFun",
- ("old_space", 0x057d1): "PromiseAllResolveElementSharedFun",
- ("old_space", 0x057f5): "PromiseAllSettledResolveElementSharedFun",
- ("old_space", 0x05819): "PromiseAllSettledRejectElementSharedFun",
- ("old_space", 0x0583d): "PromiseAnyRejectElementSharedFun",
- ("old_space", 0x05861): "PromiseCapabilityDefaultRejectSharedFun",
- ("old_space", 0x05885): "PromiseCapabilityDefaultResolveSharedFun",
- ("old_space", 0x058a9): "PromiseCatchFinallySharedFun",
- ("old_space", 0x058cd): "PromiseGetCapabilitiesExecutorSharedFun",
- ("old_space", 0x058f1): "PromiseThenFinallySharedFun",
- ("old_space", 0x05915): "PromiseThrowerFinallySharedFun",
- ("old_space", 0x05939): "PromiseValueThunkFinallySharedFun",
- ("old_space", 0x0595d): "ProxyRevokeSharedFun",
- ("old_space", 0x05981): "ShadowRealmImportValueFulfilledSFI",
- ("old_space", 0x059a5): "SourceTextModuleExecuteAsyncModuleFulfilledSFI",
- ("old_space", 0x059c9): "SourceTextModuleExecuteAsyncModuleRejectedSFI",
+ ("old_space", 0x05689): "AsyncFunctionAwaitRejectSharedFun",
+ ("old_space", 0x056ad): "AsyncFunctionAwaitResolveSharedFun",
+ ("old_space", 0x056d1): "AsyncGeneratorAwaitRejectSharedFun",
+ ("old_space", 0x056f5): "AsyncGeneratorAwaitResolveSharedFun",
+ ("old_space", 0x05719): "AsyncGeneratorYieldResolveSharedFun",
+ ("old_space", 0x0573d): "AsyncGeneratorReturnResolveSharedFun",
+ ("old_space", 0x05761): "AsyncGeneratorReturnClosedRejectSharedFun",
+ ("old_space", 0x05785): "AsyncGeneratorReturnClosedResolveSharedFun",
+ ("old_space", 0x057a9): "AsyncIteratorValueUnwrapSharedFun",
+ ("old_space", 0x057cd): "PromiseAllResolveElementSharedFun",
+ ("old_space", 0x057f1): "PromiseAllSettledResolveElementSharedFun",
+ ("old_space", 0x05815): "PromiseAllSettledRejectElementSharedFun",
+ ("old_space", 0x05839): "PromiseAnyRejectElementSharedFun",
+ ("old_space", 0x0585d): "PromiseCapabilityDefaultRejectSharedFun",
+ ("old_space", 0x05881): "PromiseCapabilityDefaultResolveSharedFun",
+ ("old_space", 0x058a5): "PromiseCatchFinallySharedFun",
+ ("old_space", 0x058c9): "PromiseGetCapabilitiesExecutorSharedFun",
+ ("old_space", 0x058ed): "PromiseThenFinallySharedFun",
+ ("old_space", 0x05911): "PromiseThrowerFinallySharedFun",
+ ("old_space", 0x05935): "PromiseValueThunkFinallySharedFun",
+ ("old_space", 0x05959): "ProxyRevokeSharedFun",
+ ("old_space", 0x0597d): "ShadowRealmImportValueFulfilledSFI",
+ ("old_space", 0x059a1): "SourceTextModuleExecuteAsyncModuleFulfilledSFI",
+ ("old_space", 0x059c5): "SourceTextModuleExecuteAsyncModuleRejectedSFI",
}
# Lower 32 bits of first page addresses for various heap spaces.
```
```
The name of the file: test/message/fail/iterator-non-callable-2.out
Insertions: 2, Deletions: 2.
@@ -1,6 +1,6 @@
-*%(basename)s:7: TypeError: TypedArray.from requires that the property of the first argument, items[Symbol.iterator], when exists, be a function
+*%(basename)s:7: TypeError: %%TypedArray%%.from requires that the property of the first argument, items[Symbol.iterator], when exists, be a function
Uint8Array.from(x);
^
-TypeError: TypedArray.from requires that the property of the first argument, items[Symbol.iterator], when exists, be a function
+TypeError: %%TypedArray%%.from requires that the property of the first argument, items[Symbol.iterator], when exists, be a function
at Function.from (<anonymous>)
at *%(basename)s:7:12
```
```
The name of the file: test/message/fail/iterator-non-callable.out
Insertions: 2, Deletions: 2.
@@ -1,6 +1,6 @@
-*%(basename)s:7: TypeError: Array.from requires that the property of the first argument, items[Symbol.iterator], when exists, be a function
+*%(basename)s:7: TypeError: %%Array%%.from requires that the property of the first argument, items[Symbol.iterator], when exists, be a function
Array.from(x);
^
-TypeError: Array.from requires that the property of the first argument, items[Symbol.iterator], when exists, be a function
+TypeError: %%Array%%.from requires that the property of the first argument, items[Symbol.iterator], when exists, be a function
at Function.from (<anonymous>)
at *%(basename)s:7:7
```
```
The name of the file: src/builtins/typed-array-from.tq
Insertions: 1, Deletions: 1.
@@ -153,7 +153,7 @@
} label IteratorNotCallable(_value: JSAny) deferred {
ThrowTypeError(
MessageTemplate::kFirstArgumentIteratorSymbolNonCallable,
- 'TypedArray.from');
+ kBuiltinNameFrom);
}
const finalLengthNum = Convert<Number>(finalLength);
```
[message] Improve IteratorSymbolNonCallable error message
Add the receiver to the IteratorSymbolNonCallable error
message.
Bug: v8:12918
Change-Id: Ib863a357474282ec3723cc4e7e012052979ca2d6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3813069
Reviewed-by: Igor Sheludko <ish...@chromium.org>
Commit-Queue: 王澳 <wangao...@bytedance.com>
Cr-Commit-Position: refs/heads/main@{#82308}
---
M src/builtins/array-from.tq
M src/builtins/base.tq
M src/builtins/builtins-call-gen.cc
M src/builtins/typed-array-createtypedarray.tq
M src/builtins/typed-array-from.tq
M src/common/message-template.h
A test/message/fail/iterator-non-callable-2.js
A test/message/fail/iterator-non-callable-2.out
A test/message/fail/iterator-non-callable-3.js
A test/message/fail/iterator-non-callable-3.out
A test/message/fail/iterator-non-callable.js
A test/message/fail/iterator-non-callable.out
M test/unittests/interpreter/bytecode_expectations/PrivateAccessorAccess.golden
M test/unittests/interpreter/bytecode_expectations/PrivateMethodAccess.golden
M test/unittests/interpreter/bytecode_expectations/StaticPrivateMethodAccess.golden
M tools/v8heapconst.py
16 files changed, 121 insertions(+), 54 deletions(-)