Updated patch, PTAL. I can give another go at the code generation stuff.
Marja thank you very much for pointing out my embarrassing not-working
hack. I
have fixed it and tested again in GDB to ensure that we hit the pre-parsing
case.
https://codereview.chromium.org/332443002/diff/20001/src/ast.h
File src/ast.h (right):
https://codereview.chromium.org/332443002/diff/20001/src/ast.h#newcode1455
src/ast.h:1455: bool is_computed_name);
On 2014/06/13 15:19:24, rossberg wrote:
Is this flag needed? Isn't it implied by the type of key Expression
node?
The flag is needed. ['foo']:42 has a Literal as the expression node.
We can't just treat it as foo:42 because of computed names don't
participate in the early-error-on-duplicate-key stuff.
https://codereview.chromium.org/332443002/diff/20001/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):
https://codereview.chromium.org/332443002/diff/20001/src/ia32/full-codegen-ia32.cc#newcode1704
src/ia32/full-codegen-ia32.cc:1704: // Object literals have two parts.
The "static" part on the left contains no
On 2014/06/13 15:19:24, rossberg wrote:
I'm not convinced that it's best to duplicate this whole loop, given
that it
shares a lot of (sub)cases. I found the previous version with the
phase flag
somewhat more pleasant.
I tried to do this at first, but ran into problems with accessors.
Notably, in the original code accessors are not set until the end, to
allow the AccessorsTable to accumulate both the getter and the setter
for a value. These values are then set together in a post-pass.
However in the computed properties case we need to set the "get" and
"set" accessors separately, when we see them.
I also had problems getting the switch fallthrough logic to do the right
thing. I can give it another go if you think it's the right thing.
https://codereview.chromium.org/332443002/diff/20001/src/ia32/full-codegen-ia32.cc#newcode1761
src/ia32/full-codegen-ia32.cc:1761:
accessor_table.lookup(key->AsLiteral())->second;
On 2014/06/13 15:19:24, rossberg wrote:
Is this correct? Can't accessors have computed property names as well?
According to spec, yes, but not in the current patch. I can add a TODO.
https://codereview.chromium.org/332443002/diff/20001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/332443002/diff/20001/src/preparser.h#newcode1607
src/preparser.h:1607: is_computed_name = true;
On 2014/06/13 15:19:24, rossberg wrote:
On 2014/06/13 08:14:18, Michael Starzinger wrote:
> Shouldn't the detection of this syntax live behind a flag for now?
I agree, we should have --harmony-object-literals (which later might
also
include short method forms).
Done.
https://codereview.chromium.org/332443002/diff/20001/src/runtime.cc
File src/runtime.cc (right):
https://codereview.chromium.org/332443002/diff/20001/src/runtime.cc#newcode5498
src/runtime.cc:5498: static MaybeHandle<Object> PutOwnElement(Isolate*
isolate,
On 2014/06/13 15:19:24, rossberg wrote:
Nit: SetOwnElement
Done.
https://codereview.chromium.org/332443002/diff/20001/src/runtime.cc#newcode5521
src/runtime.cc:5521: static MaybeHandle<Object> PutOwnProperty(Isolate*
isolate,
On 2014/06/13 15:19:24, rossberg wrote:
Nit: SetOwnProperty
Done.
https://codereview.chromium.org/332443002/diff/20001/src/runtime.h
File src/runtime.h (right):
https://codereview.chromium.org/332443002/diff/20001/src/runtime.h#newcode228
src/runtime.h:228: F(PutOwnProperty, 3, 1) \
On 2014/06/13 15:19:25, rossberg wrote:
Let's call this SetOwnProperty for consistency.
Done.
https://codereview.chromium.org/332443002/diff/20001/test/mjsunit/object-literal-computed-names.js
File test/mjsunit/object-literal-computed-names.js (right):
https://codereview.chromium.org/332443002/diff/20001/test/mjsunit/object-literal-computed-names.js#newcode5
test/mjsunit/object-literal-computed-names.js:5: // Test computed
properties syntax.
On 2014/06/13 08:14:19, Michael Starzinger wrote:
This file should definitely go into the "harmony" or one of the "esN"
sub-directories.
Done.
https://codereview.chromium.org/332443002/
--
--
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.
