Skip to content

dart2js produces wrong values in case of shift operations on negative numbers #42892

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

Closed
sgrekhov opened this issue Jul 31, 2020 · 3 comments
Closed
Labels
area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. closed-as-intended Closed as the reported issue is expected behavior P3 A lower priority bug or feature request web-dart2js

Comments

@sgrekhov
Copy link
Contributor

sgrekhov commented Jul 31, 2020

Compile to dart2js the following program

main() {
  print( -1  << 10);
  print( -1 << 9);
  print( -1 << 8);
  print( -1 << 7);
  print( -1 << 6);
  print( -1 << 5);
  print( -1 << 4);
  print( -1 << 3);
  print( -1 << 2);
  print( -1 << 1);
  print( -1 );
}

Compiled code looks like

  N = {
    main: function() {
      P.print(4294966272);
      P.print(4294966784);
      P.print(4294967040);
      P.print(4294967168);
      P.print(4294967232);
      P.print(4294967264);
      P.print(4294967280);
      P.print(4294967288);
      P.print(4294967292);
      P.print(4294967294);
      P.print(-1);

But these values are wrong. -1 << 10 produces -1024 in both Dart and JavaScript (tried console.log(-1 << 10) in Chrome 84 and got -1024)

In case of rigth shift -4 >> 1 produces 4294967294 ob dart2js and -2 in Dart and JavaScript

@sgrekhov sgrekhov changed the title dart2js produces wrong values in case of left shift of the negative numbers dart2js produces wrong values in case of shift operations on negative numbers Jul 31, 2020
@sigmundch sigmundch added area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. P3 A lower priority bug or feature request web-dart2js labels Jul 31, 2020
@fishythefish
Copy link
Member

dart2js doesn't quite conform to either the VM or JS semantics for bit operations. Our convention is that bit operations are understood to operate on 32-bit unsigned integers. x << y in Dart is essentially lowered to x << y >>> 0 in JS.

/cc @rakudrama for the history behind this decision, but I believe it was a combination of doing what our users wanted/expected while still having an efficient lowering. At this point, a breaking change may be difficult since users likely depend on the existing number semantics.

@rakudrama
Copy link
Member

This is by design. We decided all bit operations operate on unsigned 32 bit values. By 'operate on unsigned values' you can consider the inputs as converted to 32-bit unsigned numbers and if the output fits in a 32-bit unsigned number, it will be the same as the VM (provided the input is converted).

Whatever we do, something will be broken due to JavaScript producing 32-bit signed values.
For example, -6 << 29 is positive in JavaScript but negative on the Dart VM.

We chose to make all bit operations unsigned because it makes more code work than accepting JavaScript's signed behaviour. In particular the following kinds of code often have bugs with signed values: implementations of bit-arrays as an array of 32-bit integers; code that packs bool or int fields into an integer; code that does rotates by combining shifts.

If a signed result is needed it can be obtained via .toSigned(32), e.g. (-1 << 10).toSigned(32).

@lrhn
Copy link
Member

lrhn commented Aug 4, 2020

We made the choice to make JS bit ops unsigned at a point where we had no >>> operator. When we get one again, we may want to reconsider and allow >>> return unsigned, and the other bit-operators return signed values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. closed-as-intended Closed as the reported issue is expected behavior P3 A lower priority bug or feature request web-dart2js
Projects
None yet
Development

No branches or pull requests

5 participants