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

Better define how numbers work in Sass #2902

Closed
wants to merge 10 commits into from

Conversation

connorskees
Copy link
Contributor

@connorskees connorskees commented Sep 5, 2020

Resolves #2892

This serves to better define the edge cases regarding numbers in Sass and formally moves all numbers to be represented as 64-bit floating point real numbers.

This is largely a description of how numbers currently work in dart-sass; however, this document suggests denying NaN, Infinity, and -Infinity as map keys. These values are currently de facto denied as map keys in dart-sass due to a crash, but libsass does allow them.

Interestingly, libsass will also treat NaN and Infinity as equal to themselves, causing (1/0) == (2/0) to be true and making it possible to use NaN to retrieve values from maps. Irrespective of this document, this behavior is considered a bug (or at the very least a deviation from dart-sass). This behavior is tracked in sass/libsass#3126.

Units are deliberately left out of this document, though could certainly be taken into consideration.

Copy link
Member

@jathak jathak left a comment

Choose a reason for hiding this comment

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

Thanks for writing this up!

A few high-level notes notes (@nex3 may also want to take a look):

  • it probably makes sense to tie much of this to existing specifications for numbers in CSS, ECMAScript, and/or IEEE-754 to ensure that our handling of edge cases matches other parts of the web ecosystem as much as possible
  • since we're formalizing the internal representation of numbers and how to format them, we should probably also formalize the syntax for number literals here as well (the CSS spec's syntax is a good starting place)

proposal/numbers.md Show resolved Hide resolved
proposal/numbers.md Show resolved Hide resolved
Copy link
Contributor

@nex3 nex3 left a comment

Choose a reason for hiding this comment

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

I'll echo Jen in saying thanks for writing this up! This has been on our to-do list for a while but it's hard to get around to it with everything else we have on our plates 😄.

proposal/numbers.md Show resolved Hide resolved
proposal/numbers.md Outdated Show resolved Hide resolved
proposal/numbers.md Outdated Show resolved Hide resolved
proposal/numbers.md Outdated Show resolved Hide resolved
proposal/numbers.md Outdated Show resolved Hide resolved
proposal/numbers.md Outdated Show resolved Hide resolved
proposal/numbers.md Outdated Show resolved Hide resolved
proposal/numbers.md Show resolved Hide resolved
- Remove trailing zeros
- If the number is negative and non-zero, emit a negative sign, `-`
- If in compressed mode:
- If the whole number is zero, do nothing
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what "the whole number" refers to here. It would probably be clearer if you used some variables; see other specs for examples.

proposal/numbers.md Outdated Show resolved Hide resolved
proposal/numbers.md Outdated Show resolved Hide resolved
proposal/numbers.md Outdated Show resolved Hide resolved
@connorskees connorskees requested a review from nex3 July 11, 2021 22:36
Copy link
Contributor

@nex3 nex3 left a comment

Choose a reason for hiding this comment

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

Sorry it's taken me so long to circle back to this!

It's become apparent in the course of writing other specs that it would be useful to be able to refer explicitly to the fuzzy equality and comparison operations Sass uses, so could you add a Procedures section that defines fuzzy-equals and fuzzy-comparison operations, and then refer to those in the spec text?

Comment on lines +97 to +99
In order to support numbers as map keys, a Sass implementation must raise an error when
trying to construct a map with, insert into (`map-insert`), retrieve (`map-get`) or remove (`map-remove`)
from a map any of `NaN`, `Infinity`, or `-Infinity`.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a breaking change. That doesn't necessarily mean we can't do it, but I'd like to see some discussion in the spec of:

  • Why we're making this an error rather than just having these values produce map entries that can't be retrieved with map-get().

  • What the deprecation process will look like for moving users onto this new behavior.

It's also worth noting that this would be an idiosyncratic way to handle these numbers in maps. JavaScript and C# both allow NaN as a key that can be accessed later with another NaN value. Ruby, Python, and Dart all allow it as a key but don't allow it to be looked up. I don't know of any language that throws an error when it's used as a key.

Comment on lines +65 to +66
> 1.0000000010 == 1.0000000020
> ---------^ ---------^
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: these diagrams aren't rendering correctly. You'll need to make them code blocks.

Comment on lines +73 to +76
> These two numbers differ only *beyond* the 10th decimal place, so they should be considered
> equal
> 1.00000000001 == 1.00000000002
> ---------^ ---------^
Copy link
Contributor

Choose a reason for hiding this comment

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

This is misleading; only differing beyond the 10th decimal place is essentially the current behavior, since it implies that 1.00000000001 == 1.00000000009. It's worth making clear here that 1.00000000001 != 1.00000000009 but 1.00000000001 == 0.99999999995.

Comment on lines +53 to +57
Numbers in Sass are symmetrically and transitively equal. That is,
if `a == b`, then `b == a` and if `a == b` and `b == c`, then `a == c`.

Numbers in Sass are *not* necessarily reflexively equal. `NaN == NaN`,
`Infinity == Infinity`, and `-Infinity == -Infinity` will always be false.
Copy link
Contributor

Choose a reason for hiding this comment

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

These are more emergent properties of the spec than specifications themselves, so they should be in non-normative quote blocks.

Numbers in Sass are *not* necessarily reflexively equal. `NaN == NaN`,
`Infinity == Infinity`, and `-Infinity == -Infinity` will always be false.

Two numbers are considered equal if they do no differ when rounded to 10 decimal places.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Two numbers are considered equal if they do no differ when rounded to 10 decimal places.
Two numbers are considered equal if they do not differ when rounded to 10 decimal places.

### Ordering

Numbers in Sass do not have total ordering. Any comparison to `NaN` will return false.
Otherwise, ordering follows what would be expected when comparing two numbers.
Copy link
Contributor

Choose a reason for hiding this comment

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

"what would be expected" is a bit vague, I'd say instead "the natural numerical ordering".

Numbers in Sass do not have total ordering. Any comparison to `NaN` will return false.
Otherwise, ordering follows what would be expected when comparing two numbers.

Numbers with more than 10 decimal places are rounded to 10 places before comparison.
Copy link
Contributor

Choose a reason for hiding this comment

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

Give some (non-normative) examples here.


The non-reflexive nature of number equality in Sass makes it difficult to
use floating point numbers as keys inside maps. This property means
that `hash(a)` doesn't necessarily equal `hash(b)` even if `a == b`.
Copy link
Contributor

Choose a reason for hiding this comment

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

For what numbers would a == b and hash(a) != hash(b)?

trying to construct a map with, insert into (`map-insert`), retrieve (`map-get`) or remove (`map-remove`)
from a map any of `NaN`, `Infinity`, or `-Infinity`.

### Formatting
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to keep the way we specify output a bit more general, to leave room for implementations to produce different output that's still CSS-identical. In this case, I think it makes sense to describe the output in terms of CSS parsing:

To serialize a number number:

  • If number is fuzzy-equal to an integer, return text for which the Consume a number algorithm will return that integer with the type "integer".
  • Otherwise, return text with at most ten decimal places for which the Consume a number algorithm will return a number that's fuzzy-equal to number.

@stof
Copy link
Contributor

stof commented Sep 1, 2022

@nex3 should this be closed in favor of #3378 ?

@nex3
Copy link
Contributor

nex3 commented Sep 2, 2022

Yes, good call.

@nex3 nex3 closed this Sep 2, 2022
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

Successfully merging this pull request may close these issues.

Use floating-point numbers universally
4 participants