Thanks, fixed these issues, added couple more tests and committed.
Thank you! Martin http://codereview.chromium.org/6144005/diff/32001/src/messages.js File src/messages.js (right): http://codereview.chromium.org/6144005/diff/32001/src/messages.js#newcode205 src/messages.js:205: strict_function_name: "Function name may not be eval or arguments in strict mode", Agreed. Opened a bug on this: http://code.google.com/p/v8/issues/detail?id=1052 On 2011/01/18 13:40:59, Lasse Reichstein wrote:
Eventually we must check (if you haven't already) what error messages
JSC gives
in strict mode. So far they haven't released a browser with strict
mode, so
there is no rush.
http://codereview.chromium.org/6144005/diff/32001/src/parser.cc File src/parser.cc (right): http://codereview.chromium.org/6144005/diff/32001/src/parser.cc#newcode326 src/parser.cc:326: strict_mode_ = (parent_ != NULL) ? parent_->strict_mode_ : false; On 2011/01/18 13:40:59, Lasse Reichstein wrote:
Can be written (parent_ != NULL) && parent_->strict_mode_ Probably doesn't make any difference, though.
Done. http://codereview.chromium.org/6144005/diff/32001/src/parser.cc#newcode1101 src/parser.cc:1101: temp_scope_->EnableStrictMode(); On 2011/01/18 13:40:59, Lasse Reichstein wrote:
You can set directive_prologue to false here, since there is no more
reason to
look at the directive prologue.
On the other hand, any realistic code won't have any directive
prologue except
"use strict", if any.
Done. http://codereview.chromium.org/6144005/diff/32001/src/scopes.cc File src/scopes.cc (right): http://codereview.chromium.org/6144005/diff/32001/src/scopes.cc#newcode417 src/scopes.cc:417: for (int i = 0, length = num_parameters(); i < length; i ++) { On 2011/01/18 13:40:59, Lasse Reichstein wrote:
Perhaps ASSERT that both name and parameter(i)->name() are symbols.
They must
be, but better safe than sorry.
Done. http://codereview.chromium.org/6144005/diff/32001/test/mjsunit/strict-mode.js File test/mjsunit/strict-mode.js (right): http://codereview.chromium.org/6144005/diff/32001/test/mjsunit/strict-mode.js#newcode58 test/mjsunit/strict-mode.js:58: } On 2011/01/18 13:40:59, Lasse Reichstein wrote:
How about adding a case where there are more directive prologues than
"use
strict", and it's neither first nor last?
Done. http://codereview.chromium.org/6144005/diff/32001/test/mjsunit/strict-mode.js#newcode88 test/mjsunit/strict-mode.js:88: CheckStrictMode("var arguments;", SyntaxError) On 2011/01/18 13:40:59, Lasse Reichstein wrote:
Test that we catches: var o = {set foo(eval) {}}; and likewise for "arguments". (It uses ParseFunctionLiteral, so it should just work).
Done. http://codereview.chromium.org/6144005/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev