Hi Lasse,
thanks for reviewing the code for me. Please take another look.
Thank you!
Martin
http://codereview.chromium.org/6335010/diff/1/src/parser.cc
File src/parser.cc (right):
http://codereview.chromium.org/6335010/diff/1/src/parser.cc#newcode3037
src/parser.cc:3037: GetAccessor = 0x01,
On 2011/01/21 12:01:13, Lasse Reichstein wrote:
Add a "k" in front of constants, i.e., kGetAccessor, kSetAccessor,
kAccessor,
kData.
Done.
http://codereview.chromium.org/6335010/diff/1/src/parser.cc#newcode3043
src/parser.cc:3043: HashMap props;
On 2011/01/21 12:01:13, Lasse Reichstein wrote:
Properties should be at the end of the class, after methods (including
static
methods).
Done.
http://codereview.chromium.org/6335010/diff/1/src/parser.cc#newcode3050
src/parser.cc:3050: default:
On 2011/01/21 12:01:13, Lasse Reichstein wrote:
Move the default to the end instead.
I just think it reads better :)
Done.
http://codereview.chromium.org/6335010/diff/1/src/parser.cc#newcode3077
src/parser.cc:3077: ASSERT(!name->AsArrayIndex(&hash));
On 2011/01/21 12:01:13, Lasse Reichstein wrote:
I'm a little worried that we might have a symbol that is also a valid
ArrayIndex.
We might not, but I haven't been able to convince myself that it can't
happen
yet.
I am less concerned since the same checks are performed in ast.cc
ObjectLiteral::CalculateEmitStore which has been around for a while.
http://codereview.chromium.org/6335010/diff/1/src/parser.cc#newcode3108
src/parser.cc:3108: // Data property conflicting with an accessor.
On 2011/01/21 17:37:55, MarkM wrote:
On WebKit nightly Version 5.0.1 (5533.17.8, r75891) non-strict
({x: 33, get x(){}});
SyntaxError: Parse error
On Minefield/FF 4.0b10pre (2011-01-21) non-strict
({x: 33, get x(){}});
SyntaxError: property name x appears more than once in object
literal
Thank you, Mark, for doing the research. The shipping versions of
Firefox, Safari do not report error, however. Since their future
versions are implementing this I would recommend we keep the full checks
in place as implemented, although there may be good reason (like fast
Chrome release cycle) that may lead us to wait with full check until
some future date and leave them all under strict mode for now.
http://codereview.chromium.org/6335010/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
