Bug 215769

Summary: __proto__ in object literal should perform [[SetPrototypeOf]] directly
Product: WebKit Reporter: Alexey Shvayka <ashvayka>
Component: JavaScriptCoreAssignee: Alexey Shvayka <ashvayka>
Status: RESOLVED FIXED    
Severity: Trivial CC: ews-watchlist, keith_miller, mark.lam, msaboff, ross.kirsling, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar, WebExposed
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=215760
Bug Depends on: 142382    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch none

Description Alexey Shvayka 2020-08-24 08:17:35 PDT
Test case: `delete Object.prototype.__proto__; Object.getPrototypeOf({__proto__: null})`
Expected: `null`
Actual: `Object.prototype`

ECMA-262: https://tc39.es/ecma262/#sec-__proto__-property-names-in-object-initializers (step 7.a.i)
Comment 1 Alexey Shvayka 2020-08-25 15:49:44 PDT
Created attachment 407244 [details]
Patch
Comment 2 Alexey Shvayka 2020-08-25 15:50:16 PDT
(In reply to Alexey Shvayka from comment #1)
> Created attachment 407244 [details]
> Patch

Warmed-up runs, --outer 50:

                                                 r265669                    patch

object-literal-underscore-proto-setter       96.5690+-0.8588     ^     77.5015+-0.3901        ^ definitely 1.2460x faster
Comment 3 Ross Kirsling 2020-08-25 17:23:30 PDT
Comment on attachment 407244 [details]
Patch

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

Seems good overall, but is there a test262 case?

> Source/JavaScriptCore/ChangeLog:19
> +        Tweaks globalFuncSetPrototypeDirect() to ignore a prototype value of incorrect type,
> +        which is unobservable for its usages in ClassExprNode::emitBytecode(). Also, removes unused

Does this tweak help with an existing case or is it just removing a restriction that no longer matters?

> Source/JavaScriptCore/parser/Nodes.h:769
> -    class PropertyListNode final : public ExpressionNode {
> +    class PropertyListNode final : public ExpressionNode, public ThrowableExpressionData {

Is this demanded by the emitPutConstantProperty change somehow?
Comment 4 Alexey Shvayka 2020-08-26 03:59:32 PDT
(In reply to Ross Kirsling from comment #3)
> Seems good overall, but is there a test262 case?

Thank you for taking a look!
I am adding the test in https://github.com/tc39/test262/pull/2747, yet the patch is fully covered by LayoutTests/js/script-tests/object-literal-direct-put.js update (ToT fails it).

> Does this tweak help with an existing case or is it just removing a restriction that no longer matters?

It is necessary for this change, as it implements the type check at step 7.a of https://tc39.es/ecma262/#sec-__proto__-property-names-in-object-initializers.
I will make sure to clarify the ChangeLog, "tweak" is rather misleading.

> Is this demanded by the emitPutConstantProperty change somehow?

Yes, it is required to get divot*(), which is needed for OpCall in emitDirectSetPrototypeOf(), even though it never throws.
Comment 5 Ross Kirsling 2020-08-26 14:36:27 PDT
Comment on attachment 407244 [details]
Patch

Cool, r=me given those points of clarification.
Comment 6 Alexey Shvayka 2020-08-26 18:46:23 PDT
Created attachment 407365 [details]
Patch

Clarify ChangeLog, assert that globalFuncSetPrototypeDirect() doesn't throw, and don't extend ThrowableExpressionData.
Comment 7 EWS 2020-08-27 17:27:26 PDT
Committed r266264: <https://trac.webkit.org/changeset/266264>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 407365 [details].
Comment 8 Radar WebKit Bug Importer 2020-08-27 17:28:16 PDT
<rdar://problem/67907341>