| Summary: | Add backed intrinsics to private functions exposed with private symbols in global object | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||
| Component: | New Bugs | Assignee: | Yusuke Suzuki <ysuzuki> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | darin, fpizlo, ggaren | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Yusuke Suzuki
2015-05-03 06:20:04 PDT
Created attachment 252267 [details]
Patch
Comment on attachment 252267 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252267&action=review Looks fine. > Source/JavaScriptCore/runtime/JSGlobalObject.cpp:430 > - JSFunction* privateFuncAbs = JSFunction::create(vm, this, 0, String(), globalPrivateFuncAbs); > - JSFunction* privateFuncFloor = JSFunction::create(vm, this, 0, String(), globalPrivateFuncFloor); > + JSFunction* privateFuncAbs = JSFunction::create(vm, this, 1, String(), mathProtoFuncAbs, AbsIntrinsic); > + JSFunction* privateFuncFloor = JSFunction::create(vm, this, 1, String(), mathProtoFuncFloor, FloorIntrinsic); There are irritating inconsistencies here. Why do these functions use String(), but log uses vm.propertyNames->emptyIdentifier.string()? Why are some lengths 0 and other 1 for otherwise identical functions? Besides the change mentioned in change log, this also changes length from 0 to 1. Can we add a test case covering this? Or just wait and do it in a different patch? If it’s right for abs, then it must be right for isFinite as well, for example. Comment on attachment 252267 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252267&action=review >> Source/JavaScriptCore/runtime/JSGlobalObject.cpp:430 >> + JSFunction* privateFuncFloor = JSFunction::create(vm, this, 1, String(), mathProtoFuncFloor, FloorIntrinsic); > > There are irritating inconsistencies here. Why do these functions use String(), but log uses vm.propertyNames->emptyIdentifier.string()? Why are some lengths 0 and other 1 for otherwise identical functions? > > Besides the change mentioned in change log, this also changes length from 0 to 1. Can we add a test case covering this? Or just wait and do it in a different patch? If it’s right for abs, then it must be right for isFinite as well, for example. Yeah! Strictly speaking, these length should be 1. However, fixing it would have no effect on the rest of the JSC. Because nobody looks up the length of private functions. In this patch, I've just fixed the length of touched private functions. But seeing the other functions, there is a lot of inconsistency about it. So in this patch, to keep the patch simle, I'll just fix intrinsic related issues. Committed r183785: <http://trac.webkit.org/changeset/183785> |