https://codereview.chromium.org/477263002/diff/80001/src/parser.h
File src/parser.h (right):
https://codereview.chromium.org/477263002/diff/80001/src/parser.h#newcode577
src/parser.h:577: FunctionLiteral::IsGeneratorFlag is_generator,
On 2014/08/22 08:22:46, marja wrote:
Are these flag mutually exclusive; Afaik an arrow function cannot be a
generator, so, it would make sense to have a three-value flag (normal,
arrow,
generator) instead of 2 bools.
How does IsConcise relate to them? Looks like it cannot be an arrow
function and
it cannot be a generator either?
So would it make sense to have a 4-value flag (normal, arrow,
generator,
concise) here?
Generator methods can be concise, too. But that's the only possible
overlap, AFAICT. It's probably better to just extend the existing
function kind enum with both ConciseMethod and ConciseGeneratorMethod,
because that's clearer and excludes all invalid combinations by
construction. (Although, in fact, there doesn't even seem to be much of
a need for distinguishing generators from concise generators at all, so
perhaps we can just completely avoid making that a separate case.)
Likewise, it would make sense to change SharedFunctionInfo,
FastNewClosureStub and other relevant places to also use this enum
instead of separate flags (in particular, the stub will eventually need
to handle arrow functions as well).
(Alternatively, we should at least have assertions in various places
that exclude all invalid combinations of the separated flags.)
https://codereview.chromium.org/477263002/diff/80001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/477263002/diff/80001/src/preparser.h#newcode1864
src/preparser.h:1864: switch (next) {
Perhaps add a TODO that if both harmony_object_literals and
harmony_generators is active, you need to allow for a '*' before this.
https://codereview.chromium.org/477263002/diff/80001/src/v8natives.js
File src/v8natives.js (right):
https://codereview.chromium.org/477263002/diff/80001/src/v8natives.js#newcode1772
src/v8natives.js:1772: ? ''
Hm, does this need another case for IsConcise && IsGenerator?
https://codereview.chromium.org/477263002/diff/80001/test/cctest/test-parsing.cc
File test/cctest/test-parsing.cc (right):
https://codereview.chromium.org/477263002/diff/80001/test/cctest/test-parsing.cc#newcode3394
test/cctest/test-parsing.cc:3394: const char* context_data[][2] =
{{"({", "});"},
On 2014/08/22 08:22:46, marja wrote:
Is there a reason why you do
({ stuff });
and not just
{ stuff }
?
Yes, because then it would be a block, not an object. ;) Because
JavaScript.
https://codereview.chromium.org/477263002/diff/80001/test/cctest/test-parsing.cc#newcode3397
test/cctest/test-parsing.cc:3397: const char* statement_data[] = {
Nit: statement_data seems like the wrong name in this case; method_data?
https://codereview.chromium.org/477263002/diff/80001/test/cctest/test-parsing.cc#newcode3416
test/cctest/test-parsing.cc:3416: const char* statement_data[] = {
Nit: name_data?
https://codereview.chromium.org/477263002/diff/80001/test/cctest/test-parsing.cc#newcode3488
test/cctest/test-parsing.cc:3488: const char* statement_data[] = {
Nit: parameter_data?
https://codereview.chromium.org/477263002/
--
--
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.
