http://codereview.chromium.org/5676005/diff/1/src/date.js
File src/date.js (right):

http://codereview.chromium.org/5676005/diff/1/src/date.js#newcode989
src/date.js:989: ':' + PadInt(this.getUTCMinutes(), 2) + ':' +
PadInt(this.getUTCSeconds(), 2) +
Not your change, but while we are here, there is a long line here

http://codereview.chromium.org/5676005/diff/1/src/date.js#newcode1003
src/date.js:1003: return CheckJSONPrimitive(this.toISOString());
Two things;
1. If "this" is a custom object, and it has a toISOString method that
returns an object this will throw an error, but it should not. It should
only throw an error if there is no toISOString method.

2. If you agree to the above and change the error message we should
delete the TypeError result_not_primitive since this is the only
callsite. Also, we can delete CheckJSONPrimitive since this is the only
place we use it.

http://codereview.chromium.org/5676005/diff/1/test/mjsunit/json.js
File test/mjsunit/json.js (right):

http://codereview.chromium.org/5676005/diff/1/test/mjsunit/json.js#newcode368
test/mjsunit/json.js:368:
assertEquals('[37,null,1,"foo","37","true",null,"has toJSON",null,"has
toJSON"]',
Long line + space after every comma

http://codereview.chromium.org/5676005/diff/1/test/mjsunit/json.js#newcode400
test/mjsunit/json.js:400: assertEquals('"42"',
JSON.stringify(falseNum));
We should also test the case where toJSON is set as a method on an
object with a toISOSting method

http://codereview.chromium.org/5676005/

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

Reply via email to