From d3b350f460763cd3727c4942fb4519f9642f8dd4 Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Tue, 31 Jan 2023 16:13:00 +0100 Subject: [PATCH] Custom type conversion to double from large long (#4099) When a large long would be passed to a host call expecting a double, it would crash with a ``` Cannot convert ''(language: Java, type: java.lang.Long) to Java type 'double': Invalid or lossy primitive coercion ``` That is unlikely to be expected by users. It also came up in the Statistics examples during Sum. One could workaround it by forcing the conversion manually with `.to_decimal` but it is not a permanent solution. Instead this change adds a custom type mapping from Long to Double that will do it behind the scenes with no user interaction. The mapping kicks in only for really large longs. # Important Notes Note that the _safe_ range is hardcoded in Truffle and it is not accessible in enso packages. Therefore a simple c&p for that max safe long value was necessary. --- CHANGELOG.md | 2 ++ .../enso/languageserver/boot/MainModule.scala | 3 +- .../CompilerBasedDependencyExtractor.scala | 3 +- .../org/enso/polyglot/HostAccessFactory.java | 35 +++++++++++++++++++ .../org/enso/runner/ContextFactory.scala | 3 +- .../Tests/src/Semantic/Java_Interop_Spec.enso | 7 ++++ 6 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 engine/polyglot-api/src/main/java/org/enso/polyglot/HostAccessFactory.java diff --git a/CHANGELOG.md b/CHANGELOG.md index a1bbc06fb40..be94be33f14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -547,6 +547,7 @@ - [Resolve Fully Qualified Names][4056] - [Optimize Atom storage layouts][3862] - [Make instance methods callable like statics for builtin types][4077] +- [Convert large longs to doubles, safely, for host calls][4099] [3227]: https://github.com/enso-org/enso/pull/3227 [3248]: https://github.com/enso-org/enso/pull/3248 @@ -636,6 +637,7 @@ [4049]: https://github.com/enso-org/enso/pull/4049 [4056]: https://github.com/enso-org/enso/pull/4056 [4077]: https://github.com/enso-org/enso/pull/4077 +[4099]: https://github.com/enso-org/enso/pull/4099 # Enso 2.0.0-alpha.18 (2021-10-12) diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/boot/MainModule.scala b/engine/language-server/src/main/scala/org/enso/languageserver/boot/MainModule.scala index 915886ab002..621f8c65f9c 100644 --- a/engine/language-server/src/main/scala/org/enso/languageserver/boot/MainModule.scala +++ b/engine/language-server/src/main/scala/org/enso/languageserver/boot/MainModule.scala @@ -43,7 +43,7 @@ import org.enso.librarymanager.published.PublishedLibraryCache import org.enso.lockmanager.server.LockManagerService import org.enso.logger.masking.Masking import org.enso.loggingservice.{JavaLoggingLogHandler, LogLevel} -import org.enso.polyglot.{RuntimeOptions, RuntimeServerInfo} +import org.enso.polyglot.{HostAccessFactory, RuntimeOptions, RuntimeServerInfo} import org.enso.searcher.sql.{SqlDatabase, SqlSuggestionsRepo, SqlVersionsRepo} import org.enso.text.{ContentBasedVersioning, Sha3_224VersionCalculator} import org.graalvm.polyglot.Context @@ -270,6 +270,7 @@ class MainModule(serverConfig: LanguageServerConfig, logLevel: LogLevel) { val context = Context .newBuilder() .allowAllAccess(true) + .allowHostAccess(new HostAccessFactory().allWithTypeMapping()) .allowExperimentalOptions(true) .option(RuntimeServerInfo.ENABLE_OPTION, "true") .option(RuntimeOptions.INTERACTIVE_MODE, "true") diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/libraries/CompilerBasedDependencyExtractor.scala b/engine/language-server/src/main/scala/org/enso/languageserver/libraries/CompilerBasedDependencyExtractor.scala index d5a58672b60..a891d23affa 100644 --- a/engine/language-server/src/main/scala/org/enso/languageserver/libraries/CompilerBasedDependencyExtractor.scala +++ b/engine/language-server/src/main/scala/org/enso/languageserver/libraries/CompilerBasedDependencyExtractor.scala @@ -5,7 +5,7 @@ import org.enso.libraryupload.DependencyExtractor import org.enso.loggingservice.{JavaLoggingLogHandler, LogLevel} import org.enso.pkg.Package import org.enso.pkg.SourceFile -import org.enso.polyglot.{PolyglotContext, RuntimeOptions} +import org.enso.polyglot.{HostAccessFactory, PolyglotContext, RuntimeOptions} import org.graalvm.polyglot.Context import java.io.File @@ -55,6 +55,7 @@ class CompilerBasedDependencyExtractor(logLevel: LogLevel) .newBuilder() .allowExperimentalOptions(true) .allowAllAccess(true) + .allowHostAccess(new HostAccessFactory().allWithTypeMapping()) .option(RuntimeOptions.PROJECT_ROOT, pkg.root.getCanonicalPath) .option("js.foreign-object-prototype", "true") .option( diff --git a/engine/polyglot-api/src/main/java/org/enso/polyglot/HostAccessFactory.java b/engine/polyglot-api/src/main/java/org/enso/polyglot/HostAccessFactory.java new file mode 100644 index 00000000000..1f9cbbcf2cb --- /dev/null +++ b/engine/polyglot-api/src/main/java/org/enso/polyglot/HostAccessFactory.java @@ -0,0 +1,35 @@ +package org.enso.polyglot; + +import org.graalvm.polyglot.HostAccess; + +/** Utility class for creating HostAccess object. */ +public class HostAccessFactory { + + // Copied from com.oracle.truffle.api.interop.NumberUtils + // since the latter is inaccessible + private static final long LONG_MAX_SAFE_DOUBLE = 9007199254740991L; // 2 ** 53 - 1 + + public HostAccess allWithTypeMapping() { + return HostAccess.newBuilder() + .allowPublicAccess(true) + .allowAllImplementations(true) + .allowAllClassImplementations(true) + .allowArrayAccess(true) + .allowListAccess(true) + .allowBufferAccess(true) + .allowIterableAccess(true) + .allowIteratorAccess(true) + .allowMapAccess(true) + .allowAccessInheritance(true) + .targetTypeMapping( + Long.class, + Double.class, + (v) -> v != null && !longInSafeDoubleRange(v, LONG_MAX_SAFE_DOUBLE), + (v) -> Double.longBitsToDouble(v)) + .build(); + } + + private boolean longInSafeDoubleRange(Long v, Long max) { + return v >= -max && v <= max; + } +} diff --git a/engine/runner/src/main/scala/org/enso/runner/ContextFactory.scala b/engine/runner/src/main/scala/org/enso/runner/ContextFactory.scala index fbd19830de0..0b2b4cf5553 100644 --- a/engine/runner/src/main/scala/org/enso/runner/ContextFactory.scala +++ b/engine/runner/src/main/scala/org/enso/runner/ContextFactory.scala @@ -5,7 +5,7 @@ import org.enso.polyglot.debugger.{ DebugServerInfo, DebuggerSessionManagerEndpoint } -import org.enso.polyglot.{PolyglotContext, RuntimeOptions} +import org.enso.polyglot.{HostAccessFactory, PolyglotContext, RuntimeOptions} import org.graalvm.polyglot.Context import java.io.{InputStream, OutputStream} @@ -46,6 +46,7 @@ class ContextFactory { .newBuilder() .allowExperimentalOptions(true) .allowAllAccess(true) + .allowHostAccess(new HostAccessFactory().allWithTypeMapping()) .option(RuntimeOptions.PROJECT_ROOT, projectRoot) .option(RuntimeOptions.STRICT_ERRORS, strictErrors.toString) .option(RuntimeOptions.WAIT_FOR_PENDING_SERIALIZATION_JOBS, "true") diff --git a/test/Tests/src/Semantic/Java_Interop_Spec.enso b/test/Tests/src/Semantic/Java_Interop_Spec.enso index 21855c753df..804368194f3 100644 --- a/test/Tests/src/Semantic/Java_Interop_Spec.enso +++ b/test/Tests/src/Semantic/Java_Interop_Spec.enso @@ -12,6 +12,7 @@ polyglot java import java.lang.StringBuilder as Java_String_Builder polyglot java import java.util.ArrayList polyglot java import java.time.LocalDate polyglot java import java.time.LocalTime +polyglot java import org.enso.base.statistics.Moments Any.test_me self x = x.is_nothing @@ -43,6 +44,12 @@ spec = x = Integer.valueOf 1 x.test_me x . should_equal False + Test.group "Numeric values" <| + Test.specify "can be passed in host calls without lossy coercion exception" <| + large_long = 6907338656278321365 + moments = Moments.new 1 + moments.add large_long + Test.group "Java/Enso Date" <| Test.specify "Java date has Enso properties" <| april1st = LocalDate.of 2022 04 01