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

Jump to bottom

Editorial: add a SetFunctionLength abstract operation #1096

Merged
merged 6 commits into from
Apr 19, 2018
Merged

Editorial: add a SetFunctionLength abstract operation #1096

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5136723
Editorial: add a SetFunctionLength abstract operation
tobie Feb 9, 2018
0e5731a
fixup!
tobie Feb 10, 2018
3e54fab
fixup! address feedback
tobie Feb 10, 2018
293548a
fixup! ws
tobie Feb 10, 2018
11c9c07
fixup! address grammar nit and length arg bounding
tobie Feb 12, 2018
0ca337c
fixup! Add "!" back in.
tobie Feb 12, 2018
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
16 changes: 13 additions & 3 deletions spec.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7413,9 +7413,8 @@ <h1>FunctionAllocate ( _functionPrototype_, _strict_, _functionKind_ )</h1>
<h1>FunctionInitialize ( _F_, _kind_, _ParameterList_, _Body_, _Scope_ )</h1>
<p>The abstract operation FunctionInitialize requires the arguments: a function object _F_, _kind_ which is one of (Normal, Method, Arrow), a parameter list Parse Node specified by _ParameterList_, a body Parse Node specified by _Body_, a Lexical Environment specified by _Scope_. FunctionInitialize performs the following steps:</p>
<emu-alg>
1. Assert: _F_ is an extensible object that does not have a `length` own property.
1. Let _len_ be the ExpectedArgumentCount of _ParameterList_.
1. Perform ! DefinePropertyOrThrow(_F_, `"length"`, PropertyDescriptor{[[Value]]: _len_, [[Writable]]: *false*, [[Enumerable]]: *false*, [[Configurable]]: *true*}).
1. Perform ! SetFunctionLength(_F_, _len_).
Copy link
Contributor Author

@tobie tobie Feb 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure whether this should be prefixed with "!". I assume it should, but then SetFunctionName often isn't elsewhere in the spec and I'm not sure why.

Copy link
Collaborator

@jmdyck jmdyck Feb 10, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

If an operation cannot ever return an abrupt completion, then I'd say invocations of it shouldn't be preceded by '!'. It isn't wrong, but it's unnecessary clutter. (This would be more persuasive if you could easily tell from an operation's 'declaration' whether it ever returns an abrupt completion. See issue #253.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So just to clarify my understanding here:

While DefinePropertyOrThrow can throw in some circumstances, it can't in this case, because we know from the first assertion in SetFunctionLength that DefinePropertyOrThrow's first argument is extensible and doesn't have a length own prop which could interfere with setting.

Thus we know that SetFunctionLength won't ever throw.

Preceding an abstract operation call by "!" means that we are in a situation where an abstract operation won't throw, although it might do so in different circumstances.

Operations which never throw should never be prefixed with "!" even though we can't know from looking at the abstract operation's signature or its name that this is the case.

So for now, SetFunctionLength should not be prefixed with anything?

Is my understanding correct?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Your understanding matches mine. Whether it's correct is up to the editors.

Copy link
Member

Choose a reason for hiding this comment

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

I don't agree with this; I think we should use ! everywhere. (I am also not an editor.)

1. Let _Strict_ be _F_.[[Strict]].
1. Set _F_.[[Environment]] to _Scope_.
1. Set _F_.[[FormalParameters]] to _ParameterList_.
Expand Down Expand Up @@ -7535,6 +7534,17 @@ <h1>SetFunctionName ( _F_, _name_ [ , _prefix_ ] )</h1>
</emu-alg>
</emu-clause>

<emu-clause id="sec-setfunctionlength" aoid="SetFunctionLength">
<h1>SetFunctionLength ( _F_, _length_ )</h1>
<p>The abstract operation SetFunctionLength requires a Function argument _F_ and a Number argument _length_. This operation adds a `length` property to _F_ by performing the following steps:</p>
<emu-alg>
1. Assert: _F_ is an extensible object that does not have a `length` own property.
1. Assert: Type(_length_) is Number.
1. Assert: _length_ ≥ 0 and ToInteger(_length_) is equal to _length_.
1. Return ! DefinePropertyOrThrow(_F_, `"length"`, PropertyDescriptor{[[Value]]: _length_, [[Writable]]: *false*, [[Enumerable]]: *false*, [[Configurable]]: *true*}).
</emu-alg>
</emu-clause>

<!-- es6num="9.2.12" -->
<emu-clause id="sec-functiondeclarationinstantiation" aoid="FunctionDeclarationInstantiation">
<h1>FunctionDeclarationInstantiation ( _func_, _argumentsList_ )</h1>
Expand Down Expand Up @@ -24700,7 +24710,7 @@ <h1>Function.prototype.bind ( _thisArg_, ..._args_ )</h1>
1. Let _targetLen_ be ToInteger(_targetLen_).
1. Let _L_ be the larger of 0 and the result of _targetLen_ minus the number of elements of _args_.
1. Else, let _L_ be 0.
1. Perform ! DefinePropertyOrThrow(_F_, `"length"`, PropertyDescriptor {[[Value]]: _L_, [[Writable]]: *false*, [[Enumerable]]: *false*, [[Configurable]]: *true*}).
1. Perform ! SetFunctionLength(_F_, _L_).
1. Let _targetName_ be ? Get(_Target_, `"name"`).
1. If Type(_targetName_) is not String, let _targetName_ be the empty string.
1. Perform SetFunctionName(_F_, _targetName_, `"bound"`).
Expand Down