Bug 144545 – Add backed intrinsics to private functions exposed with private symbols in global object

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

Add backed intrinsics to private functions exposed with private symbols in global object

[Yusuke Suzuki]

Created attachment 252267 [details]
Patch

[Darin Adler]

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.

[Yusuke Suzuki]

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.

[Yusuke Suzuki]

Committed r183785: <http://trac.webkit.org/changeset/183785>