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

Number.isInteger is incorrectly false for a java.lang.Long #774

Closed
kharvd opened this issue Oct 11, 2023 · 3 comments
Closed

Number.isInteger is incorrectly false for a java.lang.Long #774

kharvd opened this issue Oct 11, 2023 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@kharvd
Copy link

kharvd commented Oct 11, 2023

$ cat Main.java
import org.graalvm.polyglot.*;

public class Main {
    public static void main(String[] args) {
        long longValue = 1699603200000L;
        double doubleValue = (double) longValue;

        Context context = Context.newBuilder().build();
        context.getBindings("js").putMember("longValue", longValue);
        context.getBindings("js").putMember("doubleValue", doubleValue);

        System.out.println("Number.isInteger(longValue): " + context.eval("js", "Number.isInteger(longValue)"));
        System.out.println("Number.isInteger(doubleValue): " + context.eval("js", "Number.isInteger(doubleValue)"));
    }
}

Actual result:

$ ~/Downloads/graalvm-community-openjdk-17.0.8+7.1/Contents/Home/bin/javac Main.java
$ ~/Downloads/graalvm-community-openjdk-17.0.8+7.1/Contents/Home/bin/java Main
Number.isInteger(longValue): false
Number.isInteger(doubleValue): true

Expected result:

Number.isInteger(longValue): true
Number.isInteger(doubleValue): true

Version:

$ ~/Downloads/graalvm-community-openjdk-17.0.8+7.1/Contents/Home/bin/java --version
openjdk 17.0.8 2023-07-18
OpenJDK Runtime Environment GraalVM CE 17.0.8+7.1 (build 17.0.8+7-jvmci-23.0-b15)
OpenJDK 64-Bit Server VM GraalVM CE 17.0.8+7.1 (build 17.0.8+7-jvmci-23.0-b15, mixed mode, sharing)
@kharvd kharvd changed the title Number.isInteger is false for a Long passed from Java Number.isInteger is incorrectly false for a Long passed from Java Oct 11, 2023
@kharvd kharvd changed the title Number.isInteger is incorrectly false for a Long passed from Java Number.isInteger is incorrectly false for a java.lang.Long Oct 11, 2023
@kharvd
Copy link
Author

kharvd commented Oct 11, 2023

Fwiw, even Number.isFinite(longNumber) is false.

@woess looks like you worked on something relevant in March, e.g. 28ba85f - is this expected?

@iamstolis
Copy link
Member

e.g. 28ba85f - is this expected?

No, it is not expected that isInteger()/isFinite() return false for long. It is a bug. It is an unfortunate/unexpected consequence of the changeset that you have mentioned i.e. of the fact that JSRuntime.isNumber() does not return true for long anymore.

I assume that the change in JSRuntime.isNumber() is intentional i.e. that this method should return true just for arguments that represent a numeric value that can be converted to JavaScript Number (i.e. to double) without the loss of the precision. This is not the case for long. Unfortunately, the specializations for some nodes were not updated to match this change.

@iamstolis iamstolis self-assigned this Nov 8, 2023
@iamstolis
Copy link
Member

Fixed by 1c51911

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants