http://codereview.chromium.org/7014019/diff/8001/src/extensions/experimental/datetime-format.cc File src/extensions/experimental/datetime-format.cc (right):
http://codereview.chromium.org/7014019/diff/8001/src/extensions/experimental/datetime-format.cc#newcode79 src/extensions/experimental/datetime-format.cc:79: millis = v8::Script::Compile(v8::String::New("eval('new Date()')"))->Run() Since we don't have a Date::New() method not taking a double this is the right way to get the current date as a date object. I would split this over a couple of lines with named variables instead of having one big line. More seriously, creating a date can throw an exception, you need to handle that here. Consider the case where a user overwrites Date: function Date() { throw 42; } http://codereview.chromium.org/7014019/diff/8001/src/extensions/experimental/datetime-format.cc#newcode172 src/extensions/experimental/datetime-format.cc:172: CreateDateTimeFormat(args[0]->ToString(), args[1]->ToString()); String conversions can throw exceptions. If you expect the arguments to be strings you should check before converting. If not, you need to check for exceptions and deal with the exception case. V8 version 3.3.7 (candidate) > var o = { valueOf: function() { throw 42; } } > o + '' The string conversion of 'o' throws. http://codereview.chromium.org/7014019/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
