[webkit-reviews] review granted: [Bug 16901] Convert remaining JS function objects to use the new PrototypeFunction class : [Attachment 18490] patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Sam Weinig <sam at webkit.org>'s request
for review:
Bug 16901: Convert remaining JS function objects to use the new
PrototypeFunction class
http://bugs.webkit.org/show_bug.cgi?id=16901

Attachment 18490: patch
http://bugs.webkit.org/attachment.cgi?id=18490&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
 104	 if (args.size() > 0)
 105	     b = args[0]->toBoolean(exec);
 106	 else
 107	     b = false;

I know this is not new code, but the check of args.size is entirely unnecessary
because undefined already turns into false.

 116	 if (args.isEmpty())
 117	     return jsBoolean(false);
122118	   return jsBoolean(args[0]->toBoolean(exec)); /* TODO: optimize for
bool case */

Here too.

 48	// Functions
 49	JSValue* booleanProtoFuncToString(ExecState*, JSObject*, const List&);
 50	JSValue* booleanProtoFuncValueOf(ExecState*, JSObject*, const List&);

 49	// Functions
 50	JSValue* numberProtoFuncToString(ExecState*, JSObject*, const List&);
 51	JSValue* numberProtoFuncToLocaleString(ExecState*, JSObject*, const
List&);
 52	JSValue* numberProtoFuncValueOf(ExecState*, JSObject*, const List&);
 53	JSValue* numberProtoFuncToFixed(ExecState*, JSObject*, const List&);
 54	JSValue* numberProtoFuncToExponential(ExecState*, JSObject*, const
List&);
 55	JSValue* numberProtoFuncToPrecision(ExecState*, JSObject*, const
List&);

 39	JSValue* regExpProtoFuncTest(ExecState*, JSObject*, const List&);
 40	JSValue* regExpProtoFuncExec(ExecState*, JSObject*, const List&);
 41	JSValue* regExpProtoFuncCompile(ExecState*, JSObject*, const List&);
 42	JSValue* regExpProtoFuncToString(ExecState*, JSObject*, const List&);

These need not be declared in the headers. I suggest the functions be given
internal linkage.

 830 JSValue* globalFuncUnEscape(ExecState* exec, JSObject*, const List& args)

This should not have a capital "E". The word "unescape" is a single word.

 159	 double parseIntOverflow(const char* s, int length, int radix);

Please omit the "s" here.

 91	    UString pattern = args.isEmpty() ? UString("") :
arg0->toString(exec);

In the future we should investigate if this can be changed to
arg0->isUndefined() instead of args.isEmpty().

r=me