Bug 16901

Summary: Convert remaining JS function objects to use the new PrototypeFunction class
Product: WebKit Reporter: Sam Weinig <sam>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Macintosh   
OS: OS X 10.4   
Description Flags
darin: review+
perhaps easier to review with No WS changes none

Description Sam Weinig 2008-01-16 22:17:08 PST
There are a few remaining uses of custom JS function classes that should be converted to use the new static function based PrototypeFunction class.  The uniformity should help when making further optimizations of the function architecture.
Comment 1 Sam Weinig 2008-01-16 22:33:26 PST
Created attachment 18490 [details]
Comment 2 Sam Weinig 2008-01-16 22:34:24 PST
Created attachment 18491 [details]
perhaps easier to review with No WS changes
Comment 3 Darin Adler 2008-01-17 09:17:54 PST
Comment on attachment 18490 [details]

 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().

Comment 4 Sam Weinig 2008-01-17 11:28:22 PST
Landed in r29588.