Bug 16901 - Convert remaining JS function objects to use the new PrototypeFunction class
Summary: Convert remaining JS function objects to use the new PrototypeFunction class
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-01-16 22:17 PST by Sam Weinig
Modified: 2008-01-17 11:28 PST (History)
0 users

See Also:


Attachments
patch (76.76 KB, patch)
2008-01-16 22:33 PST, Sam Weinig
darin: review+
Details | Formatted Diff | Diff
perhaps easier to review with No WS changes (66.39 KB, patch)
2008-01-16 22:34 PST, Sam Weinig
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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]
patch
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]
patch

 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
Comment 4 Sam Weinig 2008-01-17 11:28:22 PST
Landed in r29588.