Thanks
https://codereview.chromium.org/938443002/diff/80001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/938443002/diff/80001/src/parser.cc#newcode5497
src/parser.cc:5497: int i = 0;
On 2015/02/25 14:28:18, rossberg wrote:
On 2015/02/23 17:55:56, arv wrote:
> This could use a comment explaining what it desugars to.
+1
Acknowledged.
https://codereview.chromium.org/938443002/diff/80001/src/parser.cc#newcode5508
src/parser.cc:5508: args->Add(factory()->NewArrayLiteral(unspread,
literal_index,
On 2015/02/23 17:55:56, arv wrote:
Will this be observable if someone overrides
Array.prototoype[Symbol.iterator]?
The unspread array literals aren't iterated via iterator protocol, so
no. Could be observable if the length property were defined though.
https://codereview.chromium.org/938443002/diff/160001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/938443002/diff/160001/src/preparser.h#newcode2358
src/preparser.h:2358: int unspread_sequences_count = 0;
On 2015/02/25 14:28:18, rossberg wrote:
A brief comment explaining what this actually counts would be useful.
I gather
it's the number of non-empty sequences of non-spread arguments?
That's right --- will add a comment
https://codereview.chromium.org/938443002/diff/160001/src/runtime/runtime-function.cc
File src/runtime/runtime-function.cc (right):
https://codereview.chromium.org/938443002/diff/160001/src/runtime/runtime-function.cc#newcode641
src/runtime/runtime-function.cc:641: // TODO(caitp): Support larger
element indexes (up to 2^53-1).
On 2015/02/25 14:28:19, rossberg wrote:
Ah, no. We likely won't, ever, at least not for calls -- you will
never have a
stack that large anyway.
sorry, that's copy/pasted from similar code I wrote in ArrayConcat,
where the "length" property is spec'd to support up to 2^53-1. I agree
the comment doesn't really apply here.
https://codereview.chromium.org/938443002/diff/160001/src/runtime/runtime-function.cc#newcode643
src/runtime/runtime-function.cc:643:
ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate, val,
On 2015/02/25 14:28:19, rossberg wrote:
I think it's more adequate to raise an exception here. RangeError or
OOM.
The only reason ToLength() would throw is if the value is an object with
a custom toString() or valueOf() which throws, iirc. So, it wouldn't
really be representative of OOM or RangeError
There isn't really a good reason to use ToLength() here anyways, though.
The arguments object should always be a JSArray, so I guess I'll just
remove the else-case here.
https://codereview.chromium.org/938443002/diff/160001/src/runtime/runtime-function.cc#newcode682
src/runtime/runtime-function.cc:682: if (!fun->IsJSFunction()) {
On 2015/02/25 14:28:19, rossberg wrote:
In a similar vein, this logic belongs into Execution::New.
The problem with letting Execution::New() and Execution::Call() do it,
is you end up with an illegal instruction if they don't find something
callable.
https://codereview.chromium.org/938443002/diff/160001/src/runtime/runtime.h
File src/runtime/runtime.h (right):
https://codereview.chromium.org/938443002/diff/160001/src/runtime/runtime.h#newcode62
src/runtime/runtime.h:62: F(ApplyCall, 3, 1) \
On 2015/02/25 14:28:19, rossberg wrote:
How about slightly more descriptive names? E.g. ApplyAll and
ApplyConstructAll.
Acknowledged.
https://codereview.chromium.org/938443002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
