I ended up having to mostly rewrite ParsePropertyDefinition again to better
support numeric names.
I also changed the FunctionLiteral::KindFlag to FunctionKind which is a bit
mask that is used throughout the code base.
I'm on vacation next week so take your time.
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 09:34:31, rossberg wrote:
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.)
I initially added kConciseMethod to FunctionLiteral::KindFlag but after
realizing that we will eventually need concise generator methods I
changed to individual flags.
Adding a kConciseGeneratorMethod when we need it seems like a cleaner
solution. I was not happy adding all these bool like parameters.
Looking forward, it seems like ES7 will support async functions, async
arrows as well as async concise functions and maybe even async
generators. At that point we would need to go back to individual flags
but we can deal with that then.
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) {
On 2014/08/22 09:34:31, rossberg wrote:
Perhaps add a TODO that if both harmony_object_literals and
harmony_generators
is active, you need to allow for a '*' before this.
Done.
I already have changes coming that changes this function further to
handle class elements correctly. I also plan to add support for
`*method()`s.
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: ? ''
On 2014/08/22 09:34:31, rossberg wrote:
Hm, does this need another case for IsConcise && IsGenerator?
Eventually. Adding a todo for now.
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#newcode3397
test/cctest/test-parsing.cc:3397: const char* statement_data[] = {
On 2014/08/22 09:34:31, rossberg wrote:
Nit: statement_data seems like the wrong name in this case;
method_data?
Done.
https://codereview.chromium.org/477263002/diff/80001/test/cctest/test-parsing.cc#newcode3416
test/cctest/test-parsing.cc:3416: const char* statement_data[] = {
On 2014/08/22 09:34:31, rossberg wrote:
Nit: name_data?
Done.
https://codereview.chromium.org/477263002/diff/80001/test/cctest/test-parsing.cc#newcode3488
test/cctest/test-parsing.cc:3488: const char* statement_data[] = {
On 2014/08/22 09:34:31, rossberg wrote:
Nit: parameter_data?
Done.
https://codereview.chromium.org/477263002/diff/100001/src/ast.h
File src/ast.h (right):
https://codereview.chromium.org/477263002/diff/100001/src/ast.h#newcode2422
src/ast.h:2422: if (is_arrow()) return FunctionKind::kArrowFunction;
These are currently stored as a bitfield_. I could change this to store
the value as a FunctionKind (enum) with the trade-off that it would need
more memory.
I'm not sure what to optimize for here?
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.
