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

Quirks of methods with float parameters in interop #272

Closed
tporeba opened this issue Apr 15, 2020 · 4 comments
Closed

Quirks of methods with float parameters in interop #272

tporeba opened this issue Apr 15, 2020 · 4 comments
Assignees
Labels
interop Polyglot Language Interoperability

Comments

@tporeba
Copy link

tporeba commented Apr 15, 2020

I have a method:

    public static void floatArg (float arg2){
        System.out.println("floatArg"+arg2);
    };

and I try to call it from graal.js via interop:

Util.floatArg(1); // OK
Util.floatArg(2.0); // OK
Util.floatArg(16777215.0); // OK
Util.floatArg(1.1); // UnsupportedTypeException, why?
Util.floatArg(16777216.0); // UnsupportedTypeException, why?
Util.floatArg(21474836); // UnsupportedTypeException, why?
Util.floatArg(9223372036854775800); // call succeeds, despite a lossy conversion!

I don't understand what is the logic behind throwing UnsupportedTypeException - why some values are accepted and others don't? I thought it might be something with IEEE754 spec and only calls for values that can be losslessly converted from double to float are allowed. But this hypothesis doesn't seem to hold, see last example.

@tporeba tporeba changed the title Quicks of methods with float parameters in interop Quirks of methods with float parameters in interop Apr 15, 2020
@wirthi wirthi added the interop Polyglot Language Interoperability label Apr 15, 2020
@wirthi
Copy link
Member

wirthi commented Apr 21, 2020

Hi @tporeba

in general, we are very wary of (potential) lossy conversions. However, you uncover some cases where I need to take an additional look.

You have to take care what the parser actually makes from your code. Even though 16777215.0 is written as a double value, the engine converts it to a java.lang.Integer immediately. This is the reason why 2.0 and 16777215.0 work fine.

We have a problem with 16777216.0 and 21474836 though. There seems to be a bug in our implementation that fails to convert them properly to float - although it works fine if I force them into a double type!

9223372036854775800 is another problem: this loses some digits already when parsed, even without any Java involved. I still need to check why this can be converted to float, as there seems to be another lossy conversion.

Best,
Christian

@wirthi
Copy link
Member

wirthi commented Apr 21, 2020

BTW, the UnsupportedTypeException will print a more readable error message in the future. This is already fixed on the master branch and will be in 20.1.0.

TypeError: invokeMember (floatArg) on package.Util failed due to: Cannot convert '1.1'(language: Java, type: java.lang.Double) to Java type 'float': Invalid or lossy primitive coercion.

@wirthi
Copy link
Member

wirthi commented Apr 22, 2020

Some confusion might come from the fact that we dynamically check the lossy-ness of a value.

  • For Integers, we compare whether they are larger than 2^24-1. Both 16777216.0 and 21474836 are, so not convertable to float.
  • For Doubles, we actually convert them to a float and check the result, basically d == (float)d. If that is true, we allow the conversion. That is the case for 9223372036854775800, but not for 1.1 (try in Java: double d = 1.1; d==(float)d;). Also, if you force the above integer values into a double value, than they ARE convertable.

To me, this differentiation makes sense from a Java point of view, but is confusing for JavaScript, as we somewhat mix Doubles and Integers here.

@wirthi
Copy link
Member

wirthi commented May 7, 2020

The inconsistency between integer and double is fixed by
oracle/graal@2664180 - in the future, we allow the conversion to float is without loss according to Java's == operator.

This will result in your 5th and 6th test to pass as well. The 4th test (1.1) will still fail, and the 7th test will still pass (see my first comment).

This will only be part of GraalVM JavaScript as of version 20.2. unfortunately.

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

No branches or pull requests

4 participants