LGTM. Let's try to commit it and see the impact!
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", 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; Can be written (parent_ != NULL) && parent_->strict_mode_ Probably doesn't make any difference, though. http://codereview.chromium.org/6144005/diff/32001/src/parser.cc#newcode1101 src/parser.cc:1101: temp_scope_->EnableStrictMode(); 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. 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 ++) { Perhaps ASSERT that both name and parameter(i)->name() are symbols. They must be, but better safe than sorry. 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: } How about adding a case where there are more directive prologues than "use strict", and it's neither first nor last? http://codereview.chromium.org/6144005/diff/32001/test/mjsunit/strict-mode.js#newcode88 test/mjsunit/strict-mode.js:88: CheckStrictMode("var arguments;", SyntaxError) Test that we catches: var o = {set foo(eval) {}}; and likewise for "arguments". (It uses ParseFunctionLiteral, so it should just work). http://codereview.chromium.org/6144005/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
