Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normative: Wrapped function should reuse target name and length #339

Merged
merged 7 commits into from
Jan 19, 2022
Merged

Normative: Wrapped function should reuse target name and length #339

Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b920761
Editorial: Rename targetFunction to Target
leobalter Nov 2, 2021
4671b87
Editorial: Rename obj to wrapped
leobalter Nov 2, 2021
66470ac
Normative: Wrapped function should reuse target name and length
leobalter Nov 2, 2021
bc5195c
Normalize target name only acceptable as a string
leobalter Nov 2, 2021
c712d70
Move WrappedFunctionCreate to the abstractions section
leobalter Nov 2, 2021
6fcb0a5
Nit pick, flex abstraction return
leobalter Nov 2, 2021
2239a3e
Apply review feedback
leobalter Nov 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 74 additions & 16 deletions spec.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,29 +120,87 @@ <h1>[[Call]] ( _thisArgument_, _argumentsList_ )</h1>
In the case of an abrupt ~throw~ completion, the type of error to be created should match the type of the abrupt throw completion record. This could be revisited when merging into the main specification. Additionally, in the case of a ~break~ or ~continue~ completion, since those are not supported, a TypeError is expected.
</emu-note>
</emu-clause>

<emu-clause id="sec-wrappedfunctioncreate" aoid="WrappedFunctionCreate">
<h1>WrappedFunctionCreate ( _callerRealm_, _targetFunction_ )</h1>
<p>The abstract operation WrappedFunctionCreate takes arguments _callerRealm_ and _targetFunction_. It is used to specify the creation of new wrapped function exotic objects. It performs the following steps when called:</p>
<emu-alg>
1. Assert: _callerRealm_ is a Realm Record.
1. Assert: IsCallable(_targetFunction_) is *true*.
1. Let _internalSlotsList_ be the internal slots listed in <emu-xref href="#table-internal-slots-of-wrapped-function-exotic-objects"></emu-xref>, plus [[Prototype]] and [[Extensible]].
1. Let _obj_ be ! MakeBasicObject(_internalSlotsList_).
1. Set _obj_.[[Prototype]] to _callerRealm_.[[Intrinsics]].[[%Function.prototype%]].
1. Set _obj_.[[Call]] as described in <emu-xref href="#sec-wrapped-function-exotic-objects-call-thisargument-argumentslist"></emu-xref>.
1. Set _obj_.[[WrappedTargetFunction]] to _targetFunction_.
1. Set _obj_.[[Realm]] to _callerRealm_.
1. Return _obj_.
</emu-alg>
</emu-clause>
</emu-clause>

<emu-clause id="sec-shadowrealm-objects">
<h1>ShadowRealm Objects</h1>
<emu-clause id="sec-shadowrealm-abstracts">
<h1>ShadowRealm Abstract Operations</h1>

<emu-clause id="sec-wrappedfunctioncreate" aoid="WrappedFunctionCreate">
<h1>WrappedFunctionCreate ( _callerRealm_, _Target_ )</h1>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<h1>WrappedFunctionCreate ( _callerRealm_, _Target_ )</h1>
<h1>
WrappedFunctionCreate (
_callerRealm_: a Realm Record,
_Target_: an ECMAScript language value,
)
</h1>

and then the assertion on line 134 can be removed.

<p>The abstract operation WrappedFunctionCreate takes arguments _callerRealm_ and _Target_. It is used to specify the creation of new wrapped function exotic objects. It performs the following steps when called:</p>
<emu-alg>
1. Assert: _callerRealm_ is a Realm Record.
1. Assert: IsCallable(_Target_) is *true*.
1. Let _internalSlotsList_ be the internal slots listed in <emu-xref href="#table-internal-slots-of-wrapped-function-exotic-objects"></emu-xref>, plus [[Prototype]] and [[Extensible]].
1. Let _wrapped_ be ! MakeBasicObject(_internalSlotsList_).
1. Set _wrapped_.[[Prototype]] to _callerRealm_.[[Intrinsics]].[[%Function.prototype%]].
1. Set _wrapped_.[[Call]] as described in <emu-xref href="#sec-wrapped-function-exotic-objects-call-thisargument-argumentslist"></emu-xref>.
1. Set _wrapped_.[[WrappedTargetFunction]] to _Target_.
1. Set _wrapped_.[[Realm]] to _callerRealm_.
1. Let _result_ be CopyNameAndLength(_wrapped_, _Target_, *"wrapped"*).
1. If _result_ is an Abrupt Completion, throw a TypeError exception.
Comment on lines +145 to +146
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
1. Let _result_ be CopyNameAndLength(_wrapped_, _Target_, *"wrapped"*).
1. If _result_ is an Abrupt Completion, throw a TypeError exception.
1. Perform ? CopyNameAndLength(_wrapped_, _Target_, *"wrapped"*).

currently this is catching the almost-certainly-TypeError and throwing a TypeError, which would be masking the actual cause

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the issue here is not with TypeError, is not leaking cross realms error objects.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, i see. shouldn't the message, or the cause, or the type (if it's a builtin) be preserved if possible?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is no definition for the message, same as in PerformShadowRealmEval.

There is no logic yet for the cause, and I'd prefer to do that in a separate PR and tackle all these points throwing TypeError.

1. Return _wrapped_.
</emu-alg>
</emu-clause>

<emu-clause id="sec-copynameandlength" aoid="CopyNameAndLength">
<h1>CopyNameAndLength ( _F_, _Target_, _prefix_ [ , _argCount_ ] )</h1>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<h1>CopyNameAndLength ( _F_, _Target_, _prefix_ [ , _argCount_ ] )</h1>
<h1>
CopyNameAndLength (
_F_: a function object,
_Target_: a function object,
_prefix_: a String,
optional _argCount_: a Number,
)
</h1>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this a new notation?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the entire spec uses it for AOs since tc39/ecma262#545 was merged in July.

<emu-alg>
1. Assert: Type(_prefix_) is String.
1. If _argCount_ is *undefined*, then set _argCount_ to 0.
1. Let _L_ be 0.
1. Let _targetHasLength_ be ? HasOwnProperty(_Target_, *"length"*).
1. If _targetHasLength_ is *true*, then
1. Let _targetLen_ be ? Get(_Target_, *"length"*).
1. If Type(_targetLen_) is Number, then
1. If _targetLen_ is *+&infin;*<sub>𝔽</sub>, set _L_ to +&infin;.
1. Else if _targetLen_ is *-&infin;*<sub>𝔽</sub>, set _L_ to 0.
1. Else,
1. Let _targetLenAsInt_ be ! ToIntegerOrInfinity(_targetLen_).
1. Assert: _targetLenAsInt_ is finite.
1. Set _L_ to max(_targetLenAsInt_ - _argCount_, 0).
1. Perform ! SetFunctionLength(_F_, _L_).
1. Let _targetName_ be ? Get(_Target_, *"name"*).
1. If Type(_targetName_) is not String, set _targetName_ to the empty String.
1. Perform ! SetFunctionName(_F_, _targetName_, _prefix_).
</emu-alg>

<emu-note type=editor>
Function.prototype.bind can replace steps 4 to 10 with this abstraction.
</emu-note>

<emu-clause id="sec-function.prototype.bind">
<h1>Function.prototype.bind ( _thisArg_, ..._args_ )</h1>
<p>When the `bind` method is called with argument _thisArg_ and zero or more _args_, it performs the following steps:</p>
<emu-alg>
1. Let _Target_ be the *this* value.
1. If IsCallable(_Target_) is *false*, throw a *TypeError* exception.
1. Let _F_ be ? BoundFunctionCreate(_Target_, _thisArg_, _args_).
1. <ins>Let _argCount_ be the number of elements in _args_.
1. <ins>Perform ? CopyNameAndLength(_F_, _Target_, *"bound"*, _argCount_).
1. <del>Let _L_ be 0.
1. <del>Let _targetHasLength_ be ? HasOwnProperty(_Target_, *"length"*).
1. <del>If _targetHasLength_ is *true*, then
1. <del>Let _targetLen_ be ? Get(_Target_, *"length"*).
1. <del>If Type(_targetLen_) is Number, then
1. <del>If _targetLen_ is *+&infin;*<sub>𝔽</sub>, set _L_ to +&infin;.
1. <del>Else if _targetLen_ is *-&infin;*<sub>𝔽</sub>, set _L_ to 0.
1. <del>Else,
1. <del>Let _targetLenAsInt_ be ! ToIntegerOrInfinity(_targetLen_).
1. <del>Assert: _targetLenAsInt_ is finite.
1. <del>Let _argCount_ be the number of elements in _args_.
1. <del>Set _L_ to max(_targetLenAsInt_ - _argCount_, 0).
1. <del>Perform ! SetFunctionLength(_F_, _L_).
1. <del>Let _targetName_ be ? Get(_Target_, *"name"*).
1. <del>If Type(_targetName_) is not String, set _targetName_ to the empty String.
1. <del>Perform SetFunctionName(_F_, _targetName_, *"bound"*).</del>
1. Return _F_.
</emu-alg>
</emu-clause>
</emu-clause>

<emu-clause id="sec-performshadowrealmeval" aoid="PerformShadowRealmEval">
<h1>PerformShadowRealmEval ( _sourceText_, _callerRealm_, _evalRealm_ )</h1>
<emu-alg>
Expand Down