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.

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

Function implicit lengths should not include optional arguments #264

Closed
ljharb opened this Issue Dec 17, 2015 · 10 comments

Comments

Projects
None yet
5 participants
@ljharb
Member

ljharb commented Dec 17, 2015

http://tc39.github.io/ecma262/#sec-ecmascript-standard-built-in-objects

Every built-in Function object, including constructors, has a length property whose value is an integer. Unless otherwise specified, this value is equal to the largest number of named arguments shown in the subclause headings for the function description, including optional parameters. However, rest parameters shown using the form “...name” are not included in the default argument count.

Most builtin functions that don't mention an explicit length (except for legacy ones like Object and Date) do not include optional parameters in the function's length property. In addition, all of the explicit lengths seem to follow the same guideline.

In addition, function foo(a, b = 3) {}.length gives 1 - default arguments are not included.

The above note should be changed to say: "not including optional parameters.", and if this would meaningfully affect the implicit lengths of any existing functions, then those legacy functions should have explicit lengths added. This change can also remove any explicit lengths that match the changed text.

If everyone is in agreement, I'll be happy to prepare a PR.

@domenic

This comment has been minimized.

Show comment
Hide comment
@domenic

domenic Dec 17, 2015

Member

It seems better to have explicit lengths everywhere and remove this guidance.

Member

domenic commented Dec 17, 2015

It seems better to have explicit lengths everywhere and remove this guidance.

@ljharb

This comment has been minimized.

Show comment
Hide comment
@ljharb

ljharb Dec 17, 2015

Member

I would be 1000% in favor of that! However, it's gotten some pushback previously in the category of "we prefer to avoid redundancy".

Member

ljharb commented Dec 17, 2015

I would be 1000% in favor of that! However, it's gotten some pushback previously in the category of "we prefer to avoid redundancy".

@ljharb

This comment has been minimized.

Show comment
Hide comment
@ljharb

ljharb Dec 17, 2015

Member

(It's also worth noting that implementors have often gotten the implicit length wrong, and have even been confused by the current wording, assuming the change I recommend here as fact.)

Member

ljharb commented Dec 17, 2015

(It's also worth noting that implementors have often gotten the implicit length wrong, and have even been confused by the current wording, assuming the change I recommend here as fact.)

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Dec 17, 2015

Member

I don't care how we resolve the redundancy really. If people want to make everything explicit I'm fine with that.

The issue here, to be clear, is about what the length of, say, Map is, which has one optional parameter and no explicit length specification. The current language says it should be 1, but it should probably be 0. Edge and Chrome implement 0. I think the current language is just a bug.

Member

bterlson commented Dec 17, 2015

I don't care how we resolve the redundancy really. If people want to make everything explicit I'm fine with that.

The issue here, to be clear, is about what the length of, say, Map is, which has one optional parameter and no explicit length specification. The current language says it should be 1, but it should probably be 0. Edge and Chrome implement 0. I think the current language is just a bug.

@ljharb

This comment has been minimized.

Show comment
Hide comment
@ljharb

ljharb Dec 17, 2015

Member

Firefox and Safari also implement 0 for Map.

Member

ljharb commented Dec 17, 2015

Firefox and Safari also implement 0 for Map.

@anba

This comment has been minimized.

Show comment
Hide comment
@anba

anba Dec 17, 2015

Contributor

I don't mind if the default length computation is switched from "including optional parameters." to "not including optional parameters." in Ch.17. But explicit length definitions everywhere should not be applied for the reasons outlined in #104.

Contributor

anba commented Dec 17, 2015

I don't mind if the default length computation is switched from "including optional parameters." to "not including optional parameters." in Ch.17. But explicit length definitions everywhere should not be applied for the reasons outlined in #104.

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Dec 18, 2015

Member

The value of the length property of Map is specified as 0 in 23.1.2

Member

allenwb commented Dec 18, 2015

The value of the length property of Map is specified as 0 in 23.1.2

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Dec 18, 2015

Member

The intent was that new built-ins would follow the same length determination rules as used for ES defined functions, except if the function signature was very similar was very similar to some pre-ES6 built-in that had a different explicitly assigned length value.

For a while in ES6 draft we tried to use ES-style signatures (default values indicating options and rest) for all built-ins but it seemed a poor match to the pseudo-code and there were too many length exceptions for pre-ES6 functions. So we reverted to ES3/5 signature conventions.

Not that even with you give every built-in an explicit length you still probably want to state the default rule somewhere so that future spec. writer have some guidance when assigning lengths for new built-ins.

Member

allenwb commented Dec 18, 2015

The intent was that new built-ins would follow the same length determination rules as used for ES defined functions, except if the function signature was very similar was very similar to some pre-ES6 built-in that had a different explicitly assigned length value.

For a while in ES6 draft we tried to use ES-style signatures (default values indicating options and rest) for all built-ins but it seemed a poor match to the pseudo-code and there were too many length exceptions for pre-ES6 functions. So we reverted to ES3/5 signature conventions.

Not that even with you give every built-in an explicit length you still probably want to state the default rule somewhere so that future spec. writer have some guidance when assigning lengths for new built-ins.

@ljharb

This comment has been minimized.

Show comment
Hide comment
@ljharb

ljharb Dec 18, 2015

Member

So it sounds like there is complete consensus that the language in Chapter 17 should change to exclude optional parameters from the length by default - and there is not consensus about whether everything should have an explicit length or not.

If this seems accurate and there are no objections, I'll prepare a PR that updates the language in Chapter 17, and I'll go through every function in the spec in an attempt to minimize explicit length references while matching implementations (the current state).

Member

ljharb commented Dec 18, 2015

So it sounds like there is complete consensus that the language in Chapter 17 should change to exclude optional parameters from the length by default - and there is not consensus about whether everything should have an explicit length or not.

If this seems accurate and there are no objections, I'll prepare a PR that updates the language in Chapter 17, and I'll go through every function in the spec in an attempt to minimize explicit length references while matching implementations (the current state).

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Dec 18, 2015

Member

@ljharb I would accept such a PR :) Depending on how much normatively changes (I suspect not much) we may or may not want to get committee feedback before pulling it in. But I suspect the normative changes will be minimal.

Member

bterlson commented Dec 18, 2015

@ljharb I would accept such a PR :) Depending on how much normatively changes (I suspect not much) we may or may not want to get committee feedback before pulling it in. But I suspect the normative changes will be minimal.

ljharb added a commit to ljharb/ecma262 that referenced this issue Dec 19, 2015

[Normative] change default function length value to not include optio…
…nal arguments.

Fixes tc39#264.

 - A few legacy methods that have deviating lengths are documented explicitly.
 - Any unneeded explicit lengths are removed.
 - Needed legacy explicit lengths are added.
 - Spacing inside optional argument brackets is made consistent
 - Fixed a bug with a `DataView` note having the wrong argument name

The DataView prototype get/set methods have inconsistent lengths across all browsers, so rather than select one of them as legacy, I decided to let them fall into the default category, which hopefully implementations will align with.

ljharb added a commit to ljharb/ecma262 that referenced this issue Dec 19, 2015

[Normative] change default function length value to not include optio…
…nal arguments.

Fixes tc39#264.

 - A few legacy methods that have deviating lengths are documented explicitly.
 - Any unneeded explicit lengths are removed.
 - Needed legacy explicit lengths are added.
 - Spacing inside optional argument brackets is made consistent
 - Fixed a bug with a `DataView` note having the wrong argument name

The DataView prototype get/set methods have inconsistent lengths across all browsers, so rather than select one of them as legacy, I decided to let them fall into the default category, which hopefully implementations will align with.

ljharb added a commit to ljharb/ecma262 that referenced this issue Dec 20, 2015

[Normative] change default function length value to not include optio…
…nal arguments.

Fixes tc39#264.

 - A few legacy methods that have deviating lengths are documented explicitly.
 - Any unneeded explicit lengths are removed.
 - Needed legacy explicit lengths are added.
 - Spacing inside optional argument brackets is made consistent
 - Fixed a bug with a `DataView` note having the wrong argument name

The DataView prototype get/set methods have inconsistent lengths across all browsers, so rather than select one of them as legacy, I decided to let them fall into the default category, which hopefully implementations will align with.

ljharb added a commit to ljharb/ecma262 that referenced this issue Dec 20, 2015

[Normative] change default function length value to not include optio…
…nal arguments.

Fixes tc39#264.

 - A few legacy methods that have deviating lengths are documented explicitly.
 - Any unneeded explicit lengths are removed.
 - Needed legacy explicit lengths are added.
 - Spacing inside optional argument brackets is made consistent
 - Fixed a bug with a `DataView` note having the wrong argument name

The DataView prototype get/set methods have inconsistent lengths across all browsers, so rather than select one of them as legacy, I decided to let them fall into the default category, which hopefully implementations will align with.

ljharb added a commit to ljharb/ecma262 that referenced this issue Dec 20, 2015

Normative: change default function length value to not include option…
…al arguments.

Fixes tc39#264.

 - A few legacy methods that have deviating lengths are documented explicitly.
 - Any unneeded explicit lengths are removed.
 - Needed legacy explicit lengths are added.
 - Spacing inside optional argument brackets is made consistent
 - Fixed a bug with a `DataView` note having the wrong argument name

The DataView prototype get/set methods have inconsistent lengths across all browsers, so rather than select one of them as legacy, I decided to let them fall into the default category, which hopefully implementations will align with.

Make optional argument spacing a bit more consistent

ljharb added a commit to ljharb/ecma262 that referenced this issue Dec 20, 2015

Normative: Change default function length value to not include option…
…al arguments.

Fixes tc39#264.

 - A few legacy methods that have deviating lengths are documented explicitly.
 - Any unneeded explicit lengths are removed.
 - Needed legacy explicit lengths are added.
 - Spacing inside optional argument brackets is made consistent
 - Fixed a bug with a `DataView` note having the wrong argument name

The DataView prototype get/set methods have inconsistent lengths across all browsers, so rather than select one of them as legacy, I decided to let them fall into the default category, which hopefully implementations will align with.

Make optional argument spacing a bit more consistent

ljharb added a commit to ljharb/ecma262 that referenced this issue Dec 20, 2015

Normative: Change default function length value to not include option…
…al arguments.

Fixes tc39#264.

 - A few legacy methods that have deviating lengths are documented explicitly.
 - Any unneeded explicit lengths are removed.
 - Needed legacy explicit lengths are added.
 - Spacing inside optional argument brackets is made consistent
 - Fixed a bug with a `DataView` note having the wrong argument name

The DataView prototype get/set methods have inconsistent lengths across all browsers, so rather than select one of them as legacy, I decided to let them fall into the default category, which hopefully implementations will align with.

ljharb added a commit to ljharb/ecma262 that referenced this issue Dec 20, 2015

Normative: Change default function length value to not include option…
…al arguments.

Fixes tc39#264.

 - A few legacy methods that have deviating lengths are documented explicitly.
 - Any unneeded explicit lengths are removed.
 - Needed legacy explicit lengths are added.
 - Spacing inside optional argument brackets is made consistent
 - Fixed a bug with a `DataView` note having the wrong argument name

The DataView prototype get/set methods have inconsistent lengths across all browsers, so rather than select one of them as legacy, I decided to let them fall into the default category, which hopefully implementations will align with. Here are the changes:
 - DataView.prototype.getFloat32 (sometimes 0 or 2, now 1)
 - DataView.prototype.getFloat64 (sometimes 0 or 2, now 1)
 - DataView.prototype.getInt16 (sometimes 0 or 2, now 1)
 - DataView.prototype.getInt32 (sometimes 0 or 2, now 1)
 - DataView.prototype.getUint16 (sometimes 0 or 2, now 1)
 - DataView.prototype.getUint32 (sometimes 0 or 2, now 1)
 - DataView.prototype.setFloat32 (sometimes 0 or 3, now 2)
 - DataView.prototype.setFloat64 (sometimes 0 or 3, now 2)
 - DataView.prototype.setInt16 (sometimes 0 or 3, now 2)
 - DataView.prototype.setInt32 (sometimes 0 or 3, now 2)
 - DataView.prototype.setUint16 (sometimes 0 or 3, now 2)
 - DataView.prototype.setUint32 (sometimes 0 or 3, now 2)

Note that the "int 8" methods had no optional arguments, and thus were always 2 - this brings all the DataView get and set methods into alignment with 1 for get, and 2 for set - since they all have the same number of required arguments respectively.

ljharb added a commit to ljharb/ecma262 that referenced this issue Dec 20, 2015

Normative: Change default function length value to not include option…
…al arguments.

Fixes tc39#264.

 - A few legacy methods that have deviating lengths are documented explicitly.
 - Any unneeded explicit lengths are removed.
 - Needed legacy explicit lengths are added.
 - Spacing inside optional argument brackets is made consistent
 - Fixed a bug with a `DataView` note having the wrong argument name

The DataView prototype get/set methods have inconsistent lengths across all browsers, so rather than select one of them as legacy, I decided to let them fall into the default category, which hopefully implementations will align with. Here are the changes:
 - DataView.prototype.getFloat32 (sometimes 0 or 2, now 1)
 - DataView.prototype.getFloat64 (sometimes 0 or 2, now 1)
 - DataView.prototype.getInt16 (sometimes 0 or 2, now 1)
 - DataView.prototype.getInt32 (sometimes 0 or 2, now 1)
 - DataView.prototype.getUint16 (sometimes 0 or 2, now 1)
 - DataView.prototype.getUint32 (sometimes 0 or 2, now 1)
 - DataView.prototype.setFloat32 (sometimes 0 or 3, now 2)
 - DataView.prototype.setFloat64 (sometimes 0 or 3, now 2)
 - DataView.prototype.setInt16 (sometimes 0 or 3, now 2)
 - DataView.prototype.setInt32 (sometimes 0 or 3, now 2)
 - DataView.prototype.setUint16 (sometimes 0 or 3, now 2)
 - DataView.prototype.setUint32 (sometimes 0 or 3, now 2)

Note that the "int 8" methods had no optional arguments, and thus were always 1 or 2 - this brings all the DataView get and set methods into alignment with 1 for get, and 2 for set - since they all have the same number of required arguments respectively.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment