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

Consistently use floating-point numbers everywhere #1802

Merged
merged 8 commits into from Sep 21, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 25 additions & 0 deletions CHANGELOG.md
@@ -1,10 +1,35 @@
## 1.55.0

* **Potentially breaking bug fix:** Sass numbers are now universally stored as
64-bit floating-point numbers, rather than sometimes being stored as integers.
This will generally make arithmetic with very large numbers more reliable and
more consistent across platforms, but it does mean that numbers between nine
quadrillion and nine quintillion will no longer be represented with full
accuracy when compiling Sass on the Dart VM.

* **Potentially breaking bug fix:** Sass equality is now properly [transitive].
Two numbers are now considered equal (after doing unit conversions) if they
round to the same `1e-11`th. Previously, numbers were considered equal if they
were within `1e-11` of one another, which led to some circumstances where `$a
== $b` and `$b == $c` but `$a != $b`.

[transitive]: https://en.wikipedia.org/wiki/Transitive_property

* **Potentially breaking bug fix:** Various functions in `sass:math` no longer
treat floating-point numbers that are very close (but not identical) to
integers as integers. Instead, these functions now follow the floating-point
specification exactly. For example, `math.pow(0.000000000001, -1)` now returns
`1000000000000` instead of `Infinity`.

### Dart API

* Add an optional `argumentName` parameter to `SassScriptException()` to make it
easier to throw exceptions associated with particular argument names.

* Most APIs that previously returned `num` now return `double`. All APIs
continue to _accept_ `num`, although in Dart 2.0.0 most of these APIs will be
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have any insight about which APIs may be excluded here (the reason for saying most of in this sentence) ?

As I'm working on version 2 of scssphp (to make it fully spec compliant and easier to maintain), I think it would make sense that scssphp would use float (PHP's name for the double type) for arguments instead of float|int (which is the equivalent of Dart's num type), as we don't have existing consumers of those APIs yet for which we need to provide BC. So I'd like to know if there are some places for which I should not do the migration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You should be safe to just use float everywhere. I'll update this to remove "most of".

changed to accept only `double`.

### JS API

* Fix a bug in which certain warning spans would not have their properties
Expand Down
8 changes: 5 additions & 3 deletions lib/src/ast/sass/expression/number.dart
Expand Up @@ -6,6 +6,7 @@ import 'package:meta/meta.dart';
import 'package:source_span/source_span.dart';

import '../../../visitor/interface/expression.dart';
import '../../../value/number.dart';
import '../expression.dart';

/// A number literal.
Expand All @@ -14,17 +15,18 @@ import '../expression.dart';
@sealed
class NumberExpression implements Expression {
/// The numeric value.
final num value;
final double value;

/// The number's unit, or `null`.
final String? unit;

final FileSpan span;

NumberExpression(this.value, this.span, {this.unit});
NumberExpression(num value, this.span, {this.unit})
: value = value.toDouble();

T accept<T>(ExpressionVisitor<T> visitor) =>
visitor.visitNumberExpression(this);

String toString() => "$value${unit ?? ''}";
String toString() => SassNumber(value, unit).toString();
}
3 changes: 1 addition & 2 deletions lib/src/callable/built_in.dart
Expand Up @@ -72,8 +72,7 @@ class BuiltInCallable implements Callable, AsyncBuiltInCallable {
///
/// If passed, [url] is the URL of the module in which the function is
/// defined.
BuiltInCallable.overloadedFunction(
this.name, Map<String, Callback> overloads,
BuiltInCallable.overloadedFunction(this.name, Map<String, Callback> overloads,
{Object? url})
: _overloads = [
for (var entry in overloads.entries)
Expand Down
2 changes: 1 addition & 1 deletion lib/src/exception.dart
Expand Up @@ -171,7 +171,7 @@ class SassScriptException {
/// triggered this exception. If it's not null, it's automatically included in
/// [message].
SassScriptException(String message, [String? argumentName])
: message = argumentName == null ? message : "\$$argumentName: $message";
: message = argumentName == null ? message : "\$$argumentName: $message";

String toString() => "$message\n\nBUG: This should include a source span!";
}
Expand Down
18 changes: 9 additions & 9 deletions lib/src/functions/color.dart
Expand Up @@ -10,8 +10,8 @@ import '../callable.dart';
import '../evaluation_context.dart';
import '../exception.dart';
import '../module/built_in.dart';
import '../util/number.dart';
import '../util/nullable.dart';
import '../util/number.dart';
import '../utils.dart';
import '../value.dart';

Expand Down Expand Up @@ -452,7 +452,7 @@ SassColor _updateComponents(List<Value> arguments,
///
/// [max] should be 255 for RGB channels, 1 for the alpha channel, and 100
/// for saturation, lightness, whiteness, and blackness.
num? getParam(String name, num max,
double? getParam(String name, num max,
{bool checkPercent = false, bool assertPercent = false}) {
var number = keywords.remove(name)?.assertNumber(name);
if (number == null) return null;
Expand Down Expand Up @@ -500,15 +500,15 @@ SassColor _updateComponents(List<Value> arguments,
}

/// Updates [current] based on [param], clamped within [max].
num updateValue(num current, num? param, num max) {
double updateValue(double current, double? param, num max) {
if (param == null) return current;
if (change) return param;
if (adjust) return (current + param).clamp(0, max);
if (adjust) return (current + param).clamp(0, max).toDouble();
return current + (param > 0 ? max - current : current) * (param / 100);
}

int updateRgb(int current, num? param) =>
fuzzyRound(updateValue(current, param, 255));
int updateRgb(int current, double? param) =>
fuzzyRound(updateValue(current.toDouble(), param, 255));

if (hasRgb) {
return color.changeRgb(
Expand Down Expand Up @@ -789,8 +789,8 @@ bool _isVarSlash(Value value) =>
/// within `0` and [max]. Otherwise, this throws a [SassScriptException].
///
/// [name] is used to identify the argument in the error message.
num _percentageOrUnitless(SassNumber number, num max, String name) {
num value;
double _percentageOrUnitless(SassNumber number, num max, String name) {
double value;
if (!number.hasUnits) {
value = number.value;
} else if (number.hasUnit("%")) {
Expand All @@ -800,7 +800,7 @@ num _percentageOrUnitless(SassNumber number, num max, String name) {
'\$$name: Expected $number to have no units or "%".');
}

return value.clamp(0, max);
return value.clamp(0, max).toDouble();
}

/// Returns [color1] and [color2], mixed together and weighted by [weight].
Expand Down
145 changes: 49 additions & 96 deletions lib/src/functions/math.dart
Expand Up @@ -11,7 +11,6 @@ import '../callable.dart';
import '../evaluation_context.dart';
import '../exception.dart';
import '../module/built_in.dart';
import '../util/number.dart';
import '../value.dart';

/// The global definitions of Sass math functions.
Expand All @@ -30,13 +29,18 @@ final module = BuiltInModule("math", functions: [
], variables: {
"e": SassNumber(math.e),
"pi": SassNumber(math.pi),
"epsilon": SassNumber(2.220446049250313e-16),
"max-safe-integer": SassNumber(9007199254740991),
"min-safe-integer": SassNumber(-9007199254740991),
"max-number": SassNumber(double.maxFinite),
"min-number": SassNumber(double.minPositive),
});

///
/// Bounding functions
///

final _ceil = _numberFunction("ceil", (value) => value.ceil());
final _ceil = _numberFunction("ceil", (value) => value.ceil().toDouble());

final _clamp = _function("clamp", r"$min, $number, $max", (arguments) {
var min = arguments[0].assertNumber("min");
Expand All @@ -55,7 +59,7 @@ final _clamp = _function("clamp", r"$min, $number, $max", (arguments) {
return number;
});

final _floor = _numberFunction("floor", (value) => value.floor());
final _floor = _numberFunction("floor", (value) => value.floor().toDouble());

final _max = _function("max", r"$numbers...", (arguments) {
SassNumber? max;
Expand All @@ -77,7 +81,7 @@ final _min = _function("min", r"$numbers...", (arguments) {
throw SassScriptException("At least one argument must be passed.");
});

final _round = _numberFunction("round", fuzzyRound);
final _round = _numberFunction("round", (number) => number.round().toDouble());

///
/// Distance functions
Expand Down Expand Up @@ -112,20 +116,16 @@ final _log = _function("log", r"$number, $base: null", (arguments) {
var number = arguments[0].assertNumber("number");
if (number.hasUnits) {
throw SassScriptException("\$number: Expected $number to have no units.");
} else if (arguments[1] == sassNull) {
return SassNumber(math.log(number.value));
}

var numberValue = _fuzzyRoundIfZero(number.value);
if (arguments[1] == sassNull) return SassNumber(math.log(numberValue));

var base = arguments[1].assertNumber("base");
if (base.hasUnits) {
throw SassScriptException("\$base: Expected $base to have no units.");
} else {
return SassNumber(math.log(number.value) / math.log(base.value));
}

var baseValue = fuzzyEquals(base.value, 1)
? fuzzyRound(base.value)
: _fuzzyRoundIfZero(base.value);
return SassNumber(math.log(numberValue) / math.log(baseValue));
});

final _pow = _function("pow", r"$base, $exponent", (arguments) {
Expand All @@ -136,45 +136,18 @@ final _pow = _function("pow", r"$base, $exponent", (arguments) {
} else if (exponent.hasUnits) {
throw SassScriptException(
"\$exponent: Expected $exponent to have no units.");
} else {
return SassNumber(math.pow(base.value, exponent.value));
}

// Exponentiating certain real numbers leads to special behaviors. Ensure that
// these behaviors are consistent for numbers within the precision limit.
var baseValue = _fuzzyRoundIfZero(base.value);
var exponentValue = _fuzzyRoundIfZero(exponent.value);
if (fuzzyEquals(baseValue.abs(), 1) && exponentValue.isInfinite) {
return SassNumber(double.nan);
} else if (fuzzyEquals(baseValue, 0)) {
if (exponentValue.isFinite) {
var intExponent = fuzzyAsInt(exponentValue);
if (intExponent != null && intExponent % 2 == 1) {
exponentValue = fuzzyRound(exponentValue);
}
}
} else if (baseValue.isFinite &&
fuzzyLessThan(baseValue, 0) &&
exponentValue.isFinite &&
fuzzyIsInt(exponentValue)) {
exponentValue = fuzzyRound(exponentValue);
} else if (baseValue.isInfinite &&
fuzzyLessThan(baseValue, 0) &&
exponentValue.isFinite) {
var intExponent = fuzzyAsInt(exponentValue);
if (intExponent != null && intExponent % 2 == 1) {
exponentValue = fuzzyRound(exponentValue);
}
}
return SassNumber(math.pow(baseValue, exponentValue));
});

final _sqrt = _function("sqrt", r"$number", (arguments) {
var number = arguments[0].assertNumber("number");
if (number.hasUnits) {
throw SassScriptException("\$number: Expected $number to have no units.");
} else {
return SassNumber(math.sqrt(number.value));
}

var numberValue = _fuzzyRoundIfZero(number.value);
return SassNumber(math.sqrt(numberValue));
});

///
Expand All @@ -185,75 +158,60 @@ final _acos = _function("acos", r"$number", (arguments) {
var number = arguments[0].assertNumber("number");
if (number.hasUnits) {
throw SassScriptException("\$number: Expected $number to have no units.");
} else {
return SassNumber.withUnits(math.acos(number.value) * 180 / math.pi,
numeratorUnits: ['deg']);
}

var numberValue = fuzzyEquals(number.value.abs(), 1)
? fuzzyRound(number.value)
: number.value;
var acos = math.acos(numberValue) * 180 / math.pi;
return SassNumber.withUnits(acos, numeratorUnits: ['deg']);
});

final _asin = _function("asin", r"$number", (arguments) {
var number = arguments[0].assertNumber("number");
if (number.hasUnits) {
throw SassScriptException("\$number: Expected $number to have no units.");
} else {
return SassNumber.withUnits(math.asin(number.value) * 180 / math.pi,
numeratorUnits: ['deg']);
}

var numberValue = fuzzyEquals(number.value.abs(), 1)
? fuzzyRound(number.value)
: _fuzzyRoundIfZero(number.value);
var asin = math.asin(numberValue) * 180 / math.pi;
return SassNumber.withUnits(asin, numeratorUnits: ['deg']);
});

final _atan = _function("atan", r"$number", (arguments) {
var number = arguments[0].assertNumber("number");
if (number.hasUnits) {
throw SassScriptException("\$number: Expected $number to have no units.");
} else {
return SassNumber.withUnits(math.atan(number.value) * 180 / math.pi,
numeratorUnits: ['deg']);
}

var numberValue = _fuzzyRoundIfZero(number.value);
var atan = math.atan(numberValue) * 180 / math.pi;
return SassNumber.withUnits(atan, numeratorUnits: ['deg']);
});

final _atan2 = _function("atan2", r"$y, $x", (arguments) {
var y = arguments[0].assertNumber("y");
var x = arguments[1].assertNumber("x");

var xValue = _fuzzyRoundIfZero(x.convertValueToMatch(y, 'x', 'y'));
var yValue = _fuzzyRoundIfZero(y.value);
var atan2 = math.atan2(yValue, xValue) * 180 / math.pi;
return SassNumber.withUnits(atan2, numeratorUnits: ['deg']);
});

final _cos = _function("cos", r"$number", (arguments) {
var value =
arguments[0].assertNumber("number").coerceValueToUnit("rad", "number");
return SassNumber(math.cos(value));
});

final _sin = _function("sin", r"$number", (arguments) {
var value = _fuzzyRoundIfZero(
arguments[0].assertNumber("number").coerceValueToUnit("rad", "number"));
return SassNumber(math.sin(value));
return SassNumber.withUnits(
math.atan2(y.value, x.convertValueToMatch(y, 'x', 'y')) * 180 / math.pi,
numeratorUnits: ['deg']);
});

final _tan = _function("tan", r"$number", (arguments) {
var value =
arguments[0].assertNumber("number").coerceValueToUnit("rad", "number");
var asymptoteInterval = 0.5 * math.pi;
var tanPeriod = 2 * math.pi;
if (fuzzyEquals((value - asymptoteInterval) % tanPeriod, 0)) {
return SassNumber(double.infinity);
} else if (fuzzyEquals((value + asymptoteInterval) % tanPeriod, 0)) {
return SassNumber(double.negativeInfinity);
} else {
var numberValue = _fuzzyRoundIfZero(value);
return SassNumber(math.tan(numberValue));
}
});
final _cos = _function(
"cos",
r"$number",
(arguments) => SassNumber(math.cos(arguments[0]
.assertNumber("number")
.coerceValueToUnit("rad", "number"))));

final _sin = _function(
"sin",
r"$number",
(arguments) => SassNumber(math.sin(arguments[0]
.assertNumber("number")
.coerceValueToUnit("rad", "number"))));

final _tan = _function(
"tan",
r"$number",
(arguments) => SassNumber(math.tan(arguments[0]
.assertNumber("number")
.coerceValueToUnit("rad", "number"))));

///
/// Unit functions
Expand Down Expand Up @@ -329,14 +287,9 @@ final _div = _function("div", r"$number1, $number2", (arguments) {
/// Helpers
///

num _fuzzyRoundIfZero(num number) {
if (!fuzzyEquals(number, 0)) return number;
return number.isNegative ? -0.0 : 0;
}

/// Returns a [Callable] named [name] that transforms a number's value
/// using [transform] and preserves its units.
BuiltInCallable _numberFunction(String name, num transform(num value)) {
BuiltInCallable _numberFunction(String name, double transform(double value)) {
return _function(name, r"$number", (arguments) {
var number = arguments[0].assertNumber("number");
return SassNumber.withUnits(transform(number.value),
Expand Down