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

Enso numbers polyglot API inconsistency #11672

Closed
Akirathan opened this issue Nov 26, 2024 · 2 comments
Closed

Enso numbers polyglot API inconsistency #11672

Akirathan opened this issue Nov 26, 2024 · 2 comments
Labels

Comments

@Akirathan
Copy link
Member

While working on #11468, I revealed an inconsistency in how we respond to some polyglot messages: https://github.com/enso-org/enso/pull/11468/files#r1859020769

The issue is in the following newly added tests (all of which are passing on develop):

@Test
public void everyValueSmallerThanIntegerMaxVal_IsPrimitiveInt() {
var almostMaxInt = Integer.toString(Integer.MAX_VALUE - 1);
var intVal = ContextUtils.evalModule(ctx, "main = " + almostMaxInt);
assertThat("Is a number", intVal.isNumber(), is(true));
assertThat("Fits in int", intVal.fitsInInt(), is(true));
assertThat("Fits in long", intVal.fitsInLong(), is(true));
assertThat("Fits in double", intVal.fitsInDouble(), is(true));
assertThat("Fits in big int", intVal.fitsInBigInteger(), is(true));
}
@Test
public void everyValueSmallerThanLongMaxVal_IsPrimitiveLong() {
var almostMaxLong = Long.toString(Long.MAX_VALUE - 1);
var longVal = ContextUtils.evalModule(ctx, "main = " + almostMaxLong);
assertThat("Is a number", longVal.isNumber(), is(true));
assertThat("Does not fit in int", longVal.fitsInInt(), is(false));
assertThat("Fits in long", longVal.fitsInLong(), is(true));
assertThat("Does not fit in double (but could)", longVal.fitsInDouble(), is(false));
assertThat("Fits in big int", longVal.fitsInBigInteger(), is(true));
}
@Test
public void everyValueBiggerThanLongMaxVal_IsEnsoBigInt() {
// This number is bigger than Long.MAX_VALUE, but smaller than Double.MAX_VALUE
// so it could technically fit in double, but in Enso, it is automatically converted
// to EnsoBigInteger
var bigIntVal = ContextUtils.evalModule(ctx, "main = 9223372036854775808");
assertThat("Is a number", bigIntVal.isNumber(), is(true));
assertThat("Does not fit in int", bigIntVal.fitsInInt(), is(false));
assertThat("Does not fit in long", bigIntVal.fitsInLong(), is(false));
assertThat("Does not fit in double (but could)", bigIntVal.fitsInDouble(), is(false));
assertThat("Fits in big int", bigIntVal.fitsInBigInteger(), is(true));
}

Revealed inconsistencies

According to Enso polyglot API:

The aforementioned semantics work differently than host polyglot API. Long value should fit in long. A value smaller than Double.MAX_VALUE and bigger than Long.MAX_VALUE should fit in double. Am I right?

@Akirathan
Copy link
Member Author

This inconsistency is blocking progress on #11468. We first need to resolve how the numbers should behave.

@Akirathan
Copy link
Member Author

This is no inconsistency. I have just not correctly understood the definition of double and long. The tests will remain in the code base, and I have added comments in - 1826dde . Closing this issue.

@github-project-automation github-project-automation bot moved this from ❓New to 🟢 Accepted in Issues Board Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 🟢 Accepted
Development

No branches or pull requests

1 participant