LGTM

http://codereview.chromium.org/6691003/diff/1/src/arm/codegen-arm.cc
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/6691003/diff/1/src/arm/codegen-arm.cc#newcode3118
src/arm/codegen-arm.cc:3118: !function_info->strict_mode()) {  // Strict
mode functions use slow path.
We'll need a fast path for strict mode functions too, eventually.

http://codereview.chromium.org/6691003/diff/1/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/6691003/diff/1/src/objects.cc#newcode5549
src/objects.cc:5549: map() ==
context()->global_context()->function_map_strict()) ||
Try splitting it into two asserts:
 ASSERT(!shared()->strict_mode() || map == ....->function_map_strict());
 ASSERT(shared()->strict_mode() || map == ....->function_map());
It's easier to see which one fails, if it does.

(Why isn't logical implication an operator :)

http://codereview.chromium.org/6691003/diff/1/test/mjsunit/strict-mode.js
File test/mjsunit/strict-mode.js (right):

http://codereview.chromium.org/6691003/diff/1/test/mjsunit/strict-mode.js#newcode991
test/mjsunit/strict-mode.js:991:
How about an inherited strictness:
  var athird = (function() { "use strict"; return function(){};})();
or do we consider it well tested that this generates a strict function?

http://codereview.chromium.org/6691003/diff/1/test/mjsunit/strict-mode.js#newcode996
test/mjsunit/strict-mode.js:996: assertThrows(pill, TypeError);
Should you check whether it has a .prototype property?

http://codereview.chromium.org/6691003/diff/1/test/mjsunit/strict-mode.js#newcode1004
test/mjsunit/strict-mode.js:1004: assertEquals(false,
descriptor.configurable);
I think we have an assertFalse function.

http://codereview.chromium.org/6691003/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to