Incorporated the feedback. Thank you! Martin
http://codereview.chromium.org/6515005/diff/1/src/arm/codegen-arm.cc File src/arm/codegen-arm.cc (right): http://codereview.chromium.org/6515005/diff/1/src/arm/codegen-arm.cc#newcode5852 src/arm/codegen-arm.cc:5852: ASSERT(strict_mode_flag() == kNonStrictMode); On 2011/02/14 10:27:24, Mads Ager wrote:
On 2011/02/14 05:15:22, Martin Maly wrote: > "delete foo" is disallowed in strict mode so we can only reach this
code in
> non-strict mode.
And 'this' rewrites to a slot, right?
Also, could you put in your review comment here as a code comment to
make it
clearer when reading this code later?
Yes, this rewrites to a slot. Added comment. http://codereview.chromium.org/6515005/diff/1/src/arm/full-codegen-arm.cc File src/arm/full-codegen-arm.cc (right): http://codereview.chromium.org/6515005/diff/1/src/arm/full-codegen-arm.cc#newcode3072 src/arm/full-codegen-arm.cc:3072: ASSERT(strict_mode_flag() == kNonStrictMode); On 2011/02/14 10:27:24, Mads Ager wrote:
Maybe duplicate the comment here to explain the assert?
Done. http://codereview.chromium.org/6515005/diff/1/src/objects.cc File src/objects.cc (right): http://codereview.chromium.org/6515005/diff/1/src/objects.cc#newcode2623 src/objects.cc:2623: Object* result = dictionary->DeleteProperty(entry, mode); On 2011/02/14 10:27:24, Mads Ager wrote:
On 2011/02/14 05:15:22, Martin Maly wrote: > DeleteProperty is a wrong place to report the error because it is
missing the
> necessary data to report good error, and it is called from other
places which
> don't even have the relevant information. > Checking for false_value() for strict mode delete felt cleanest.
I think that makes sense. Could you add a comment in the body of the
if
statement that deleting non-configurable properties in strict mode
should throw
exceptions?
Done. http://codereview.chromium.org/6515005/diff/1/test/mjsunit/strict-mode.js File test/mjsunit/strict-mode.js (right): http://codereview.chromium.org/6515005/diff/1/test/mjsunit/strict-mode.js#newcode267 test/mjsunit/strict-mode.js:267: // Delete of an unqialified identifier On 2011/02/14 10:27:24, Mads Ager wrote:
unqialified -> unqualified
Done. http://codereview.chromium.org/6515005/diff/1/test/mjsunit/strict-mode.js#newcode270 test/mjsunit/strict-mode.js:270: CheckStrictMode("function function_name() { delete function_name; }", SyntaxError); On 2011/02/14 10:27:24, Mads Ager wrote:
Make these 80 char lines?
Done. http://codereview.chromium.org/6515005/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
