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

Reply via email to