Addressing PR comments

enso-org · Dec 13, 2023 · fd7a132 · fd7a132
1 parent 5068975
commit fd7a132
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 21 deletions.
diff --git a/build.sbt b/build.sbt
@@ -1386,9 +1386,11 @@ lazy val `runtime-language-arrow` =
   (project in file("engine/runtime-language-arrow"))
     .settings(
       inConfig(Compile)(truffleRunOptionsSettings),
-      instrumentationSettings
+      instrumentationSettings,
+      libraryDependencies ++= Seq(
+        "org.graalvm.truffle" % "truffle-api" % graalMavenPackagesVersion % "provided",
+      )
     )
-    .dependsOn(`polyglot-api`)
 
 lazy val runtime = (project in file("engine/runtime"))
   .configs(Benchmark)

diff --git a/engine/runtime-language-arrow/src/main/java/org/enso/interpreter/arrow/ArrowContext.java b/engine/runtime-language-arrow/src/main/java/org/enso/interpreter/arrow/ArrowContext.java
@@ -2,7 +2,7 @@
 
 import com.oracle.truffle.api.TruffleLanguage;
 
-public class ArrowContext {
+final class ArrowContext {
   private final TruffleLanguage.Env env;
 
   public ArrowContext(TruffleLanguage.Env env) {

diff --git a/engine/runtime-language-arrow/src/main/java/org/enso/interpreter/arrow/ArrowLanguage.java b/engine/runtime-language-arrow/src/main/java/org/enso/interpreter/arrow/ArrowLanguage.java
@@ -14,7 +14,7 @@
 public class ArrowLanguage extends TruffleLanguage<ArrowContext> {
 
   public static final String ID = "arrow";
-  public static final String MIME = "application/arrow";
+  public static final String MIME = "application/vnd.apache.arrow.file";
 
   public ArrowLanguage() {}
 

diff --git a/engine/runtime-language-arrow/src/main/java/org/enso/interpreter/arrow/ArrowParser.java b/engine/runtime-language-arrow/src/main/java/org/enso/interpreter/arrow/ArrowParser.java
@@ -4,7 +4,9 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-public class ArrowParser {
+public final class ArrowParser {
+
+  private ArrowParser() {}
 
   public static class Result {
 
@@ -33,7 +35,7 @@ public Mode getAction() {
 
   public static Result parse(Source source) {
     String src = source.getCharacters().toString();
-    Matcher m = ArrayPattern.matcher(src);
+    Matcher m = ARRAY_PATTERN.matcher(src);
     if (m.find()) {
       try {
         var layout = LogicalLayout.valueOf(m.group(1));
@@ -44,7 +46,7 @@ public static Result parse(Source source) {
       }
     }
 
-    m = CastPattern.matcher(src);
+    m = CAST_PATTERN.matcher(src);
     if (m.find()) {
       try {
         var layout = LogicalLayout.valueOf(m.group(1));
@@ -57,8 +59,8 @@ public static Result parse(Source source) {
     return null;
   }
 
-  private static final Pattern ArrayPattern = Pattern.compile("new\\[(.+)\\]");
-  private static final Pattern CastPattern = Pattern.compile("cast\\[(.+)\\]");
+  private static final Pattern ARRAY_PATTERN = Pattern.compile("new\\[(.+)\\]");
+  private static final Pattern CAST_PATTERN = Pattern.compile("cast\\[(.+)\\]");
 
   public enum PhysicalLayout {
     Primitive,

diff --git a/...-language-arrow/src/main/java/org/enso/interpreter/arrow/runtime/ArrowFixedArrayDate.java b/...-language-arrow/src/main/java/org/enso/interpreter/arrow/runtime/ArrowFixedArrayDate.java
@@ -19,7 +19,7 @@
 import java.time.ZonedDateTime;
 
 @ExportLibrary(InteropLibrary.class)
-public class ArrowFixedArrayDate implements TruffleObject {
+public final class ArrowFixedArrayDate implements TruffleObject {
   private final long size;
   private final ByteBuffer buffer;
 
@@ -74,7 +74,9 @@ static void doDay(
         @Cached.Shared("interop") @CachedLibrary(limit = "1") InteropLibrary iop)
         throws UnsupportedMessageException {
       // TODO: Needs null bitmap
-      assert iop.isDate(value);
+      if (!iop.isDate(value)) {
+        throw UnsupportedMessageException.create();
+      }
       var at = index * receiver.unit.sizeInBytes();
       var time = iop.asDate(value).toEpochDay();
       receiver.buffer.putInt((int) at, (int) time);
@@ -87,7 +89,9 @@ static void doMilliseconds(
         Object value,
         @Cached.Shared("interop") @CachedLibrary(limit = "1") InteropLibrary iop)
         throws UnsupportedMessageException {
-      assert iop.isDate(value) && iop.isTime(value);
+      if (!iop.isDate(value) || !iop.isTime(value)) {
+        throw UnsupportedMessageException.create();
+      }
 
       var at = index * receiver.unit.sizeInBytes();
       if (iop.isTimeZone(value)) {

diff --git a/...e-language-arrow/src/main/java/org/enso/interpreter/arrow/runtime/ArrowFixedArrayInt.java b/...e-language-arrow/src/main/java/org/enso/interpreter/arrow/runtime/ArrowFixedArrayInt.java
@@ -13,7 +13,7 @@
 import java.nio.ByteBuffer;
 
 @ExportLibrary(InteropLibrary.class)
-public class ArrowFixedArrayInt implements TruffleObject {
+public final class ArrowFixedArrayInt implements TruffleObject {
   private final long size;
   private final ByteBuffer buffer;
 
@@ -68,7 +68,9 @@ public static void doByte(
         Object value,
         @Cached.Shared("interop") @CachedLibrary(limit = "1") InteropLibrary iop)
         throws UnsupportedMessageException {
-      assert (iop.fitsInByte(value));
+      if (!iop.fitsInByte(value)) {
+        throw UnsupportedMessageException.create();
+      }
       receiver.buffer.put((int) index, (iop.asByte(value)));
     }
 
@@ -79,7 +81,9 @@ public static void doShort(
         Object value,
         @Cached.Shared("interop") @CachedLibrary(limit = "1") InteropLibrary iop)
         throws UnsupportedMessageException {
-      assert (iop.fitsInShort(value));
+      if (!iop.fitsInShort(value)) {
+        throw UnsupportedMessageException.create();
+      }
       receiver.buffer.putShort((int) index, (iop.asShort(value)));
     }
 
@@ -90,7 +94,9 @@ public static void doInt(
         Object value,
         @Cached.Shared("interop") @CachedLibrary(limit = "1") InteropLibrary iop)
         throws UnsupportedMessageException {
-      assert (iop.fitsInInt(value));
+      if (!iop.fitsInInt(value)) {
+        throw UnsupportedMessageException.create();
+      }
       receiver.buffer.putInt((int) index, (iop.asInt(value)));
     }
 
@@ -101,7 +107,9 @@ public static void doLong(
         Object value,
         @Cached.Shared("interop") @CachedLibrary(limit = "1") InteropLibrary iop)
         throws UnsupportedMessageException {
-      assert (iop.fitsInLong(value));
+      if (!iop.fitsInLong(value)) {
+        throw UnsupportedMessageException.create();
+      }
       receiver.buffer.putLong((int) index, (iop.asLong(value)));
     }
   }

diff --git a/engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/VerifyArrowTest.java b/engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/VerifyArrowTest.java
@@ -63,8 +63,6 @@ public void arrowDate32() {
     Value date32Array = date32Constr.newInstance((long) 10);
     assertNotNull("allocated value should not be null", date32Array);
     assertTrue("allocated value should be an array", date32Array.hasArrayElements());
-    // TODO false?
-    // assertTrue("allocated value should be an array", interop.hasArrayElements(date32Array));
     var startDate = LocalDate.now();
     populateArrayWithConsecutiveDays(date32Array, startDate);
     var rawDayPlus2 = date32Array.getArrayElement(2);
@@ -87,8 +85,6 @@ public void arrowDate64() {
     Value date64Array = date64Constr.newInstance((long) 10);
     assertNotNull("allocated value should not be null", date64Array);
     assertTrue("allocated value should be an array", date64Array.hasArrayElements());
-    // TODO false?
-    // assertTrue("allocated value should be an array", interop.hasArrayElements(date64Array));
     var startDate = ZonedDateTime.now(ZoneId.of("Europe/Paris"));
     var startDateZone = startDate.getZone();
     populateArrayWithConsecutiveDays(date64Array, startDate);