https://codereview.chromium.org/938443002/diff/80001/src/bootstrapper.cc
File src/bootstrapper.cc (right):
https://codereview.chromium.org/938443002/diff/80001/src/bootstrapper.cc#newcode1619
src/bootstrapper.cc:1619:
EMPTY_NATIVE_FUNCTIONS_FOR_FEATURE(harmony_spreadcalls)
On 2015/02/25 00:00:24, caitp wrote:
On 2015/02/24 16:22:46, rossberg wrote:
> Why do we need a separate flag?
See the answer I gave to Erik --- If spread-calls and spread-elements
are behind
different flags, it seemed nice to have a third flag which would
implicate both
of them (also, it would be convenient to put any self-hosted runtime
support for
both in one script).
If that's not cool, I'll just get rid of it
Well, since they currently imply each other, the separation doesn't
really exist. :) But yeah, I don't think we need feature subset flags.
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/23 17:55:56, arv wrote:
This could use a comment explaining what it desugars to.
+1
https://codereview.chromium.org/938443002/diff/160001/src/flag-definitions.h
File src/flag-definitions.h (right):
https://codereview.chromium.org/938443002/diff/160001/src/flag-definitions.h#newcode199
src/flag-definitions.h:199: V(harmony_spreadcalls, "harmony
spread-calls")
Nit: spread_calls
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;
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?
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).
Ah, no. We likely won't, ever, at least not for calls -- you will never
have a stack that large anyway.
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,
I think it's more adequate to raise an exception here. RangeError or
OOM.
https://codereview.chromium.org/938443002/diff/160001/src/runtime/runtime-function.cc#newcode659
src/runtime/runtime-function.cc:659: if (!fun->IsJSFunction()) {
Why is this needed? Isn't that already taken care of by Execution::Call?
https://codereview.chromium.org/938443002/diff/160001/src/runtime/runtime-function.cc#newcode682
src/runtime/runtime-function.cc:682: if (!fun->IsJSFunction()) {
In a similar vein, this logic belongs into Execution::New.
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) \
How about slightly more descriptive names? E.g. ApplyAll and
ApplyConstructAll.
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.
