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

Definition of integer is broken #1962

Closed
waldemarhorwat opened this issue Apr 22, 2020 · 6 comments
Closed

Definition of integer is broken #1962

waldemarhorwat opened this issue Apr 22, 2020 · 6 comments

Comments

@waldemarhorwat
Copy link

The change to explicitly note mathematical values changed the definition of integer in the spec to:

When the term integer is used in this specification, it refers to a Number value whose mathematical value is in the set of integers, unless otherwise stated: when the term mathematical integer is used in this specification, it refers to a mathematical value which is in the set of integers. As shorthand, integer t can be used to refer to either of the two, as determined by t.

This is a very confusing definition, since "integer" is generally understood to be what this calls a "mathematical integer". Readers of the spec are unlikely to be intimately familiar with this section when reading other parts of the spec. The confusion extends to ourselves: many places in the spec rely on the common meaning of "integer" and are incorrect or invalid if "integer" is limited to the set of "Number values whose mathematical values are in the set of integers" (heh — even this definition uses the common meaning of "integer" and is trivially recursive if read literally). Other places in the spec are incorrect because they don't account for the existence of two "integers" equal to zero: +0 and -0, so there are actually 257 "integers" between 0 inclusive and 256 exclusive.

My suggestion is to retain the well-known meanings of "integer", "real number", "rational number" (at least with that lower-case spelling) and use a more explicit label to represent Numbers that happen to have integral mathematical values.

Fixing this will take a sweep though the entire spec and some discussion. In some places it's relatively apparent what kind of integer is wanted, even though the spec uses the wrong one (hint: when the spec uses "integer" in formulas or to talk about BigNum values, it's probably a bad idea to interpret that to mean "Number values whose mathematical values are in the set of integers"). In other places I can't tell which kind of integer should be used and more discussion is needed.

@michaelficarra michaelficarra added the editor call to be discussed in the next editor call label Apr 22, 2020
@michaelficarra
Copy link
Member

I've wanted to clean up uses of "integer"/"integral" for a while now. I will bring it up during the editor call today.

@michaelficarra michaelficarra removed the editor call to be discussed in the next editor call label Apr 22, 2020
@michaelficarra
Copy link
Member

The editor group would like to make a clear distinction between mathematical integers ("integers") and integral Number values (phrasing TBD).

@waldemarhorwat
Copy link
Author

waldemarhorwat commented Apr 22, 2020

@michaelficarra I agree that they need to be distinct concepts. The word "integer" used to mean an integer. #1135 changed that to mean an integral Number value, which introduced lots of bugs. We should revert that back to the commonly understood definition of an integer and use some other phrasing for integral Number values. If we don't, it will get even worse when we start having integral Decimal values.

@ljharb
Copy link
Member

ljharb commented Apr 22, 2020

To be clear, BigInt is specified on top of #1135, so the work required to conceptually revert it is much more than git revert - it would potentially require specifying all of BigInt differently as well.

@waldemarhorwat
Copy link
Author

@ljharb: the changes to adjust BigInt are small compared to the changes to the entire spec needed to fix #1135's bugs.

@bakkot
Copy link
Contributor

bakkot commented Oct 13, 2020

Fixed by #2007.

@bakkot bakkot closed this as completed Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants