https://codereview.chromium.org/938443002/diff/20001/src/flag-definitions.h
File src/flag-definitions.h (right):
https://codereview.chromium.org/938443002/diff/20001/src/flag-definitions.h#newcode199
src/flag-definitions.h:199: V(harmony_spreadcalls, "harmony
spread-calls")
On 2015/02/18 15:32:08, caitp wrote:
On 2015/02/18 15:07:05, arv wrote:
> harmony_spread_calls maybe?
>
> It is not clear why this is two flags?
I thought it might work well to put spread calls and spread elements
behind
different flags, and `harmony_spread` would implicate both of them.
Good idea. I like.
https://codereview.chromium.org/938443002/diff/20001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/938443002/diff/20001/src/parser.cc#newcode5501
src/parser.cc:5501: list->Add(factory()->NewSmiLiteral(i,
RelocInfo::kNoPosition), zone());
On 2015/02/18 15:32:08, caitp wrote:
On 2015/02/18 15:07:06, arv wrote:
> It is not clear to me what the list contains? I think we are storing
the
indexes
> of the position of the spread expressions?
>
> Maybe add a comment explaining what this looks like?
I tried to draw a graph of it in a comment, but it didn't add much
clarity. For
reference though, it's sort of like this:
0. The function
1. The receiver
2. The index, in the list, of the index of the first spread expression
(this is
where it gets confusing!)
3...N Each parameter expression passed to the call
N...M Index of each spread expression in the arguments list.
It's organized this way so that there isn't a bunch of extra array
allocations
(arrays make it a bit more complicated for the preparser)
I think this is a really good solution but it is not obvious what is
going on here.
(I might borrow this approach for traceur)
https://codereview.chromium.org/938443002/diff/20001/src/preparser.cc
File src/preparser.cc (right):
https://codereview.chromium.org/938443002/diff/20001/src/preparser.cc#newcode1021
src/preparser.cc:1021: ParseArguments(&spread_pos, ok);
On 2015/02/18 15:32:08, caitp wrote:
On 2015/02/18 15:07:06, arv wrote:
> Does this work?
No --- In the other patch I was throwing an error if it finds a spread
argument
calling an intrinsic. But it might be good to make it work somehow
instead
Maybe a DCHECK for now then?
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.
