Bug 82046

Summary: The JSC code generator doesn't generate correct code for Constructors
Product: WebKit Reporter: Tommy Widenflycht <tommyw>
Component: JavaScriptCoreAssignee: Tommy Widenflycht <tommyw>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, haraken, japhet, ojan, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 81657    
Attachments:
Description Flags
Patch
none
Patch none

Tommy Widenflycht
Reported 2012-03-23 04:27:48 PDT
The main bulk of code uses the name jsConstructor for the created object, and then calls GenerateParametersCheck which generates code that uses the name castedThis.
Attachments
Patch (3.08 KB, patch)
2012-03-23 04:39 PDT, Tommy Widenflycht 		no flags
DetailsFormatted DiffDiff
Patch (13.69 KB, patch)
2012-03-23 05:48 PDT, Tommy Widenflycht 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Tommy Widenflycht
Comment 1 2012-03-23 04:39:27 PDT
Created attachment 133461 [details] Patch
Kentaro Hara
Comment 2 2012-03-23 04:54:09 PDT
Comment on attachment 133461 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133461&action=review > Source/WebCore/ChangeLog:3 > + The JSC code generator doesn't generate correct code for Constructors This patch looks OK, but it is just renaming a variable. "[JSC] Rename jsConstructor to castedThis for naming consistency in CodeGeneratorJS.pm" would be a better title.
Tommy Widenflycht
Comment 3 2012-03-23 05:12:28 PDT
It's more than just a rename, without this rename the generated code doesn't compile if the constructor takes parameters.
Kentaro Hara
Comment 4 2012-03-23 05:14:05 PDT
Comment on attachment 133461 [details] Patch The change will affect exisiting run-bindings-tests. Would you update them?
Tommy Widenflycht
Comment 5 2012-03-23 05:14:53 PDT
Sure
Tommy Widenflycht
Comment 6 2012-03-23 05:46:14 PDT
Just for documentation purposes this is the generated code before my patch: EncodedJSValue JSC_HOST_CALL JSTestObjConstructor::constructJSTestObj(ExecState* exec) { JSTestObjConstructor* jsConstructor = static_cast<JSTestObjConstructor*>(exec->callee()); if (exec->argumentCount() < 1) return throwVMError(exec, createTypeError(exec, "Not enough arguments")); if (exec->argumentCount() <= 0 || !exec->argument(0).isFunction()) { setDOMException(exec, TYPE_MISMATCH_ERR); return JSValue::encode(jsUndefined()); } RefPtr<TestCallback> testCallback = JSTestCallback::create(asObject(exec->argument(0)), castedThis->globalObject()); RefPtr<TestObj> object = TestObj::create(testCallback); return JSValue::encode(asObject(toJS(exec, jsConstructor->globalObject(), object.get()))); }
Tommy Widenflycht
Comment 7 2012-03-23 05:48:02 PDT
Created attachment 133467 [details] Patch
Kentaro Hara
Comment 8 2012-03-23 05:50:22 PDT
Comment on attachment 133467 [details] Patch OK. Thanks for the patch.
WebKit Review Bot
Comment 9 2012-03-23 06:28:14 PDT
Comment on attachment 133467 [details] Patch Clearing flags on attachment: 133467 Committed r111856: <http://trac.webkit.org/changeset/111856>
WebKit Review Bot
Comment 10 2012-03-23 06:28:20 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.