Bug 210862 – Remove revoked Proxy checks from ProxyCreate

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Alexey Shvayka]

Remove revoked Proxy checks from ProxyCreate

[Alexey Shvayka]

Created attachment 397205 [details]
Patch

[Alexey Shvayka]

Created attachment 397222 [details]
Patch

Adjust tests.

[Ross Kirsling]

Comment on attachment 397222 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=397222&action=review

> JSTests/ChakraCore.yaml:1849
> -  cmd: runChakra :skip, "NoException", "arraywithproxy.baseline", []
> +  cmd: runChakra :skipDueToOutdatedOrBadTest, "NoException", "", []

Whoooa, I didn't know this setting existed. Pretty sure I could have used it in the past! :D

> Source/JavaScriptCore/ChangeLog:11
> +        Also cleans up ProxyObject creation by using isFunction() instead of
> +        isCallable(), which are identical.

Hmm, I wonder if this should be cleaned up for redundancy (in another patch of course)...

It's odd that isCallable still checks getCallData for JSFunctionType, when it doesn't look like this could ever be CallType::None.

[Alexey Shvayka]

Comment on attachment 397222 [details]
Patch

(In reply to Ross Kirsling from comment #3)

Thank you for review!

> Whoooa, I didn't know this setting existed. Pretty sure I could have used it
> in the past! :D

I've discovered it by accident: it was used for /test/es6/proxyenumbug.js, which is totally outdated. JSC made some significant progress on spec compliance since ChakraCore tests were last updated, and I believe tests are up to date with ECMA-262 and don't have print('key' + Symbol()) anymore. It would be great to re-sync them.

> It's odd that isCallable still checks getCallData for JSFunctionType, when
> it doesn't look like this could ever be CallType::None.

This would be a great improvement! With this patch, isCallable() has 1-2 call sites as isFunction() seems to be preferred. Also there are a few usages of getCallData() which ignore the CallData.

Do you mind if I land this after test262 sync?

[Alexey Shvayka]

Created attachment 397425 [details]
Patch

Set reviewer and rebase patch.

[EWS]

Committed r260621: <https://trac.webkit.org/changeset/260621>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 397425 [details].

[Radar WebKit Bug Importer]

<rdar://problem/62282842>