dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.12k stars 1.57k forks source link

[dart2js] Negated integer zero sometimes prints as `-0.0` and other times prints as `0`. #49870

Open darkstarx opened 2 years ago

darkstarx commented 2 years ago

Code 1

void main() async
{
  print(-5.0.compareTo(5.0));
}

Output:

-0.0

Code 2

void main() async
{
  print(-5.0.compareTo(5.0));
   final int i = 0;
   print(-i);
}

Output:

0
0

The first line of the main function is identical in both code examples but the results of its evaluating are different!

Based on Flutter 3.0.5 Dart SDK 2.17.6

parlough commented 2 years ago

Interesting, thanks for opening an issue. This seems to only be occurring with dart2js (the web production compiler). 0.0 and -0.0 are roughly equal and identical on the web platform according to Numbers in Dart, so this may be expected.

For future reference, potential issues related to the Dart SDK, such as the web compilers, should go on the SDK repository. Thanks for opening the issue though!

I don't have permission to move this to there, but is this side effect expected @sigmundch? It seems to go away if --disable-type-inference is specified.

sigmundch commented 2 years ago

@parlough - thanks for flagging this for us.

At first, I thought that this was indeed as expected. I even thought that we may have a constant propagation step causing this discrepancy. Upon closer inspection, that's not the case. The generated code indeed shows -0.0 in both print statements. Instead, there is a bug in our reasoning of numbers in our global type propagation. We then incorrectly optimize the print method in a way that changes how the string is emitted. Disabling the global phase as you suggested shows the right value being printed. Also adding a third call to print with a String value does the trick too:

void main() async {
  print(-5.0.compareTo(5.0));
  final int i = 0;
  print(-i);
  print('foo');
}

(this prints -0.0 again)

rakudrama commented 10 months ago

The bug is that we are inconsistent in how we optimize x.toString() depending on what we know about x. If we know x must be an int value, dart2js uses JavaScript's formatting, which is fast and small, e.g. ""+x. JavaScript evaluates ""+(-0.0) to "0". If we don't know, we call the method (JSNumber_methods.toString$1(x)), or the interpolation stringify helper (S(x)) and those paths format -0.0 as "-0.0".

The difficulty for the programmer is that what we know about x is what dart2js infers about the value of x at the call size. The optimization would be more reliable if it was based on the static type.

The most simple fix is to turn off the optimization. This has downsides:

  1. The string -0.0 will start to appear in places where 0 used to appear, especially in interpolations. Fixing the bug is a breaking change.
  2. Interpolation like "Page $page of $pages" would compile to "Page " + S(page) + " of " + S(pages) instead of the smaller and faster "Page " + page + " of " + pages".

We already print 2.0 as "2" because it would be silly to produce the string "Page 2.0 of 9.0". The decision to format -0.0 as "-0.0" was motivated by some tests, and the observation that the programmer could change the sign easily, either by replacing negation with subtraction (print(0-i)) or adding zero somewhere.

The way the programmer can get "2.0" is to use one of the methods like double.toStringAsFixed or double.toStringAsPrecision. Perhaps we should have never produced "-0.0" by default, letting programmers use these methods for negative zero if they care. I think this is a lesser breaking change than fixing the bug - changing the methods to produce results so that the buggy optimization is no longer a bug.

Another option is to use the static type to determine the operation. Prefer producing integers for toString (and interpolation) unless the static type of the receiver is double, and then produce 0.0, -0.0, 1.0 etc. JavaScript Numbers are doing double duty as int and double values, so let the programmer tell us which. The inconsistency with the static type approach is that inside print the static type is Object? and inside List<E>.toString the static type of the element is E, so both would be 'preferred integer'. This gives

double x = 1.0;
print(x);               // "1"
print(x.toString());    // "1.0"
print('$x');            // "1.0"
print([x]);             //  "[1]"