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

Unexpected values for certain Integer literals #530

Open
jvasileff opened this issue Apr 28, 2015 · 14 comments
Open

Unexpected values for certain Integer literals #530

jvasileff opened this issue Apr 28, 2015 · 14 comments
Assignees
Labels
Milestone

Comments

@jvasileff
Copy link
Member

Integer literals of the following forms produce unexpected values:

  1. outside of 64-bit min/max range, or
  2. negative (per 64-bit-signed interpretation) hex or binary literals that are negated
    // one greater than JVM's runtime.maxIntegerValue
    print(9223372036854775808); // -9223372036854776000 on JS

    value x = -#ffff_ffff_ffff_ffff; 
    value y =  #ffff_ffff_ffff_ffff;

    print(x);   // +1 on JVM, -1 on JS 
    print(-y);  // +1

    print(y);   // -1
    print(-x);  // -1 on JVM, +1 on JS
@jvasileff
Copy link
Member Author

My suggestion would be:

  • For decimal literals, disallow values outside of the range -9223372036854775808..9223372036854775807
  • For binary & hex, interpret as a signed 64-bit long, then perform negation if a negative sign is present

This would allow literals that cannot be stored with perfect precision, and would be smaller than the full range of values possible in JS, but to me seems like a reasonable compromise for cross-platform code.

@gavinking gavinking added this to the 1.2 milestone Apr 28, 2015
@gavinking gavinking assigned gavinking and chochos and unassigned gavinking Apr 28, 2015
@gavinking
Copy link
Member

+1, the JVM backend already performs this kind of validation.

@chochos
Copy link
Member

chochos commented Apr 28, 2015

I use a BigInteger to parse natural literals, so I can just check that they are less than 64 bits long.

chochos added a commit to ceylon/ceylon.language that referenced this issue Apr 28, 2015
chochos added a commit that referenced this issue Apr 28, 2015
@jvasileff
Copy link
Member Author

This one is pretty weird:

    // min in hex
    print (-#8000_0000_0000_0000);   // negative, same as Java
    print (-(#8000_0000_0000_0000)); // positive, opposite of Java
    // so...
    print (-#8000_0000_0000_0000 == -(#8000_0000_0000_0000)); // ** false!

In Java, negating min has no effect, but the operation does of course change the sign in JS. If 64-bit hex literals are to be supported in JS, perhaps it makes sense for all of the above numbers to be positive. Conceptually, I guess, the literal would not be considered to include the "-" sign.

@jvasileff
Copy link
Member Author

Negative decimals still produce unexpected results:

    // min
    print (-9223372036854775808);   // ok, good

    // one less than min 64
    print (-9223372036854775809);   // no compile error

    // a lot less than min: -(2^64 - 1)
    print (-18446744073709551615);  // -1

@chochos
Copy link
Member

chochos commented May 2, 2015

Perhaps I should revert to parsing Long directly instead of using BigInteger?

@chochos chochos reopened this May 2, 2015
@gavinking
Copy link
Member

@chochos why, precisely do you need to parse it at all?

@chochos
Copy link
Member

chochos commented May 2, 2015

Because I need to convert it to base 10, and check that it's a value within valid range

@gavinking
Copy link
Member

Ah, so only for binary and hex literals?

@chochos
Copy link
Member

chochos commented May 2, 2015

Binary and hex literals need to be parsed and converted to base 10. Base 10 literals need to be validated against the max range, so I parse all number literals.

chochos added a commit that referenced this issue May 4, 2015
@chochos
Copy link
Member

chochos commented May 4, 2015

@jvasileff can you check if this is working ok now? I've tested all the cases mentioned in this issue and they work the same way as in JVM.

@jvasileff
Copy link
Member Author

@chochos I tried out several things, and going with the 64-bit scheme, I have a couple thoughts on negatives.

  1. For base-10, I believe the negative sign should be considered part of the literal. This is for both internal consistency, and consistency with the JVM. So, we'd have:
print(-9223372036854775808);        // ALLOWED
print(-(9223372036854775808));      // DISALLOWED
print(9223372036854775808.negated); // DISALLOWED

In the disallowed cases, the literal would actually be a positive value that is out of range.

Of course, due to platform differences, we correctly have:

print(-9223372036854775808); // negative on JVM & JS
print(-(-9223372036854775808)); // negative on JVM, positive on JS
  1. For hex & binary, I believe the negative sign should not be considered part of the literal. Since we are using a signed 64-bit representation, support for negatives is already baked in before considering the - operator. And, any use of the JVM's concept of negation will inevitably lead to non-sensical results.

So, I believe all of the following should print the same positive number on JS (even though they are all negative on the JVM):

print(-#8000_0000_0000_0000);
print(#8000_0000_0000_0000.negated);
print(-(#8000_0000_0000_0000));
print(-(-(-#8000_0000_0000_0000)));

And, the following should print the same negative number on both JS & JVM:

print(#8000_0000_0000_0000);
print(-#8000_0000_0000_0000.negated);
print(-(-#8000_0000_0000_0000));

@jvasileff
Copy link
Member Author

Worth noting, an adjacent - operator may not be part of the literal in some cases due to precedence rules. The following is currently disallowed by both compilers, which I presume to be correct:

 print(-9223372036854775808.negated);

@FroMage
Copy link
Member

FroMage commented Oct 14, 2015

Moving to 1.3, that's really edge-case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants