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

Refactor and improve numbers #81

Merged
merged 9 commits into from
Nov 8, 2020
Merged

Refactor and improve numbers #81

merged 9 commits into from
Nov 8, 2020

Conversation

kaj
Copy link
Owner

@kaj kaj commented Nov 3, 2020

  • Encapuslate the difference between a Ratio<isize> and a Ratio<BigInt> inside the Number type, so their use outside of number.rs is identical.
  • Add a third case, the Number may be a f64. This allows semi-correct handling of Infinity, -Infinity and NaN.
  • For value comparisons, we consider NaN == NaN, Infinity == Infinity and -Infinity == -Infinity. This differs from sane and normal float handling in Rust, where each NaN or infinity is unique. Therefore, f64 is encapsulated in a type that implements Ord and Eq (and not just their partials) actually treat float value with normal PartialOrd behaviour.
  • css::Value, sass::Value and lots of related types no longer implement Ord. PartialOrd is enough.
  • Power and trigonometric functions in sass:math is simplified by the fact that a Number can now be a f64.

Instead, Number now uses an internal NumValue which might contain a
Ratio<isizze>, a Ratio<BigInt> or an f64.

This also fixes handling of division by zero and other Infinite and
NaN results.
Use NumValue and not a local struct in parser.
Different kinds of NumValue should order according to value,
not according to implementation.
Neither css::Value nor sass::Value, nor lots of related types, should
actually impl Ord, but only PartialOrd.  This means a NumValue should
not impl Ord either, and we can accept the usual and sane rules for
floating-point comparions (i.e. NaN.partial_cmp(NaN) == None).
@kaj kaj merged commit 7b5450d into master Nov 8, 2020
@kaj kaj deleted the refactor-numbers branch November 8, 2020 21:10
kaj added a commit that referenced this pull request Nov 10, 2020
Progress: 2718 of 5840 tests passed in dart-sass compatiblilty mode.

### Breaking changes

* `SourceName` and `sass::Item::Import` was changed by #62.
* `Use` was added to the `sass::Item` enum by #80.
* Changes to `sass::Value` and `css::Vaue` by #81.

### Improvements

* PR #80: Implement `@use` for standard sass modules.  All standard
  modules are at least partially supported, only `sass:string` is
  fully supported.  See issue #60 for for module support status.
* PR 82: Implement hwb support in `sass:color`.
  Also, provide `From<Rgba>` for `css::Value` and deprecate color
  value constructors.
* Provide `From<bool>` for `css::Value` (and deprecate `Value::bool(v)`).
* PR #62: Improve `ParseError`, `SourcePos`, and `SourceName` by using
  located spans by [nom_locate](https://lib.rs/crates/nom_locate) in
  the parser.  A `sass::Item::Import` now handles where each file is
  imported from, to improve error reporting.
* PR #81: Improved number handlig.  Now `Value::Number` handles both
  machine-sized rationals, bignum rationals and floats internally and
  `Value::NumberBig` is removed.  Also, `Value` no longer implemnts
  `Ord` but only `PartialOrd`, to handle f64 NaN an infinite values
  correctly.
* Improve parsing of `@else` clauses.
* Update spec to 2020-10-29.

Tested with rustc 1.47.0 (18bf6b4f0 2020-10-07),
1.44.1 (c7087fe00 2020-06-17), 1.42.0 (b8cedc004 2020-03-09),
1.40.0 (73528e339 2019-12-16), 1.38.0 (625451e37 2019-09-23),
1.48.0-beta.8 (121901459 2020-11-08), and
1.49.0-nightly (25f6938da 2020-11-09),
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.

1 participant