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.

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

Editorial: Replace terms "integer index" and "array index" #1623

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jmdyck
Copy link
Collaborator

@jmdyck jmdyck commented Jul 13, 2019

The problem with "integer index" and "array index" (defined in 6.1.7 The Object Type) is that they are terms for String values, and yet pretty much everything else in the spec (and a lot of usage outside the spec) assumes that an index is an integer value.

The term "integer index" is worse, because both words mislead the reader to think that this is an integer value. And there's a danger that someone can use the term in the natural sense of "an index that's an integer" without realizing that that conflicts with the given definition. In fact, this has happened in the current spec, where about half of the uses of "integer index" actually refer to an integer value: see step 14.c.i in String.prototype.split and the descriptions of all the [[FooNextIndex]] slots.

So I propose the following renaming:

  • "integer index" -> "Uint53-string"
  • "array index" -> "Uint32-string"

Rationale for the new terms:

  • It's clear we're talking about a String value.
  • But it's also fairly clear that they are related to unsigned integers, based on the "Uint32" terminology the spec already uses.
  • And the "53" vs "32" makes explicit the distinction between two: the range of permissible values (which "integer" vs "array" doesn't really convey).

(Of course, I didn't convert the uses of "integer index" that weren't using it in the defined sense.)


The spec also used the terms "integer-indexed" and "array-indexed", but didn't define them or use them entirely consistently, so I added definitions for "Uint53-indexed" and "Uint32-indexed" and tried to be consistent in their usage.

(But I left "Integer-Indexed exotic objects".)


Effect on downstream specs:

  • "array index":

    • WebIDL uses "array index" in ES's sense, but defines its own abstract operation for "property name P is an array index", so it wouldn't have to change due to this PR. (It could if its editors wanted.)

    • HTML uses the term "array index property name", but refers to WebIDL's definition.

    • Intl doesn't use "array index".

  • "integer index":

    • Neither WebIDL nor HTML use "integer index".

    • Intl has 2 occurrences of "integer index", but not with this meaning, and not linking to ES.

@ljharb
Copy link
Member

ljharb commented Jul 13, 2019

These terms seem more precise, to be sure, but not more clear - why not “integer index String” vs “integer index Number”, etc?

@jmdyck
Copy link
Collaborator Author

jmdyck commented Oct 14, 2019

(force-pushed to resolve a merge conflict)

@jmdyck

This comment has been minimized.

@bakkot bakkot added the editor call to be discussed in the next editor call label Nov 6, 2019
@syg syg removed the editor call to be discussed in the next editor call label Nov 13, 2019
@syg
Copy link
Contributor

syg commented Nov 14, 2019

The conclusion from the editor call is that Uint53-string and Uint32-string are not good names. While more clarity could be useful here, until better names are proposed, the churn here is not worth it.

That said, 90f7d8f is independent of renaming and is useful in itself. @jmdyck would you like to make a new PR with that commit or morph this PR into just that commit?

jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Nov 15, 2019
... that refer to the index of an index
rather than the index of an element.

[Cherry-picked from PR tc39#1623.]
@jmdyck
Copy link
Collaborator Author

jmdyck commented Nov 15, 2019

While more clarity could be useful here, until better names are proposed, the churn here is not worth it.

Any guidance on your criteria for good names? (These still strike me as good names, so I'm not going to find better ones without a different metric for goodness.)

That said, 90f7d8f is independent of renaming and is useful in itself. @jmdyck would you like to make a new PR with that commit or morph this PR into just that commit?

Done (new PR).

@jmdyck

This comment has been minimized.

@jmdyck

This comment has been minimized.

spec.html Outdated
@@ -2053,7 +2053,8 @@ <h1>The Object Type</h1>
</li>
</ul>
<p>Properties are identified using key values. A property key value is either an ECMAScript String value or a Symbol value. All String and Symbol values, including the empty String, are valid as property keys. A <dfn id="property-name">property name</dfn> is a property key that is a String value.</p>
<p>An <dfn id="integer-index">integer index</dfn> is a String-valued property key that is a canonical numeric String (see <emu-xref href="#sec-canonicalnumericindexstring"></emu-xref>) and whose numeric value is either *+0* or a positive integer &le; 2<sup>53</sup> - 1. An <dfn id="array-index">array index</dfn> is an integer index whose numeric value _i_ is in the range <emu-eqn>+0 &le; _i_ &lt; 2<sup>32</sup> - 1</emu-eqn>.</p>
<p>A <dfn id="uint53-string" oldids="integer-index">Uint53-string</dfn> is a String-valued property key that is a canonical numeric String (see <emu-xref href="#sec-canonicalnumericindexstring"></emu-xref>) and whose numeric value is either *+0* or a positive integer &le; 2<sup>53</sup> - 1. A <dfn id="uint32-string" oldids="array-index">Uint32-string</dfn> is a Uint53-string whose numeric value _i_ is in the range <emu-eqn>+0 &le; _i_ &lt; 2<sup>32</sup> - 1</emu-eqn>.</p>
Copy link
Member

@michaelficarra michaelficarra Feb 6, 2020

Choose a reason for hiding this comment

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

Suggested change
<p>A <dfn id="uint53-string" oldids="integer-index">Uint53-string</dfn> is a String-valued property key that is a canonical numeric String (see <emu-xref href="#sec-canonicalnumericindexstring"></emu-xref>) and whose numeric value is either *+0* or a positive integer &le; 2<sup>53</sup> - 1. A <dfn id="uint32-string" oldids="array-index">Uint32-string</dfn> is a Uint53-string whose numeric value _i_ is in the range <emu-eqn>+0 &le; _i_ &lt; 2<sup>32</sup> - 1</emu-eqn>.</p>
<p>A <dfn id="uint53-string" oldids="integer-index">Uint53-string</dfn> is a String-valued property key that is a canonical numeric String (see <emu-xref href="#sec-canonicalnumericindexstring"></emu-xref>) and whose numeric value _i_ is an integer in the range <emu-eqn>+0 &le; _i_ &lt; 2<sup>53</sup>. A <dfn id="uint32-string" oldids="array-index">Uint32-string</dfn> is a Uint53-string whose numeric value _i_ is in the range <emu-eqn>+0 &le; _i_ &lt; 2<sup>32</sup> - 1</emu-eqn>.</p>

Copy link
Collaborator Author

@jmdyck jmdyck Feb 6, 2020

Choose a reason for hiding this comment

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

The change from &le; to &lt; excludes the value 2^53 - 1, which is currently included. I'm guessing you didn't intend that bit.

Copy link
Member

Choose a reason for hiding this comment

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

Nope, just suggesting to use the same style of description as below.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, that aside, the thing about the suggested change is that it would admit the value "-0" as a Uint53-string and Uint32-string (because -0 is an integer and +0 &le; -0 is true), which the current definition doesn't. So I think that would be backwards-incompatible.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe it's best to just specify in terms of mathematical values then?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It might be, but this PR is just about replacing some terms, not about reformulating definitions.

Copy link
Member

@michaelficarra michaelficarra left a comment

Choose a reason for hiding this comment

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

LGTM otherwise.

@michaelficarra michaelficarra added the editor call to be discussed in the next editor call label Feb 6, 2020
@jmdyck

This comment has been minimized.

@michaelficarra michaelficarra removed the editor call to be discussed in the next editor call label Apr 8, 2020
@jmdyck

This comment has been minimized.

@jmdyck

This comment has been minimized.

@jmdyck

This comment has been minimized.

@jmdyck

This comment has been minimized.

@ljharb ljharb force-pushed the master branch 3 times, most recently from 3d0c24c to 7a79833 Compare June 29, 2021 02:21
The prose for Arguments Exotic Object
says that it maps "array-indexed properties"
to formal parameter bindings.

And yet the Note refers to "integer-indexed data properties".

Now, "array-indexed" and "integer-indexed" aren't defined,
but "array index" and "integer index" are,
so this could be taken as a disagreement.
Change the Note to agree with the normative prose.

(This use of "integer-indexed" was introduced before "integer index"
had a specific definition, so I think it just didn't get cleaned up
when "integer index" was introduced.)
Also, define "Uint53-indexed" and use it to replace "integer-indexed".

(Remaining occurrences of "integer index" and "integer-indexed"
are using the phrases not as the terms were defined.
That is, they are talking about an integer value
that's an index into some sequence.)

(Oh, and I'm also leaving the term "Integer-Indexed exotic object".)
Also, define "Uint32-indexed" and use it to replace "array-indexed".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants