Skip to content

Commit

Permalink
Accept Array-like objects seamlessly in builtins (#3817)
Browse files Browse the repository at this point in the history
Most of the time, rather than defining the type of the parameter of the builtin, we want to accept every Array-like object i.e. Vector, Array, polyglot Array etc.
Rather than writing all possible combinations, and likely causing bugs on the way anyway as we already saw, one should use `CoerceArrayNode` to convert to Java's `Object[]`.
Added various test cases to illustrate the problem.
  • Loading branch information
hubertp authored Oct 25, 2022
1 parent bc09c7b commit f6379fb
Show file tree
Hide file tree
Showing 13 changed files with 234 additions and 61 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@
- [Fix performance of method calls on polyglot arrays][3781]
- [Missing foreign language generates proper Enso error][3798]
- [Made Vector performance to be on par with Array][3811]
- [Accept Array-like object seamlessly in builtins][3817]

[3227]: https://github.com/enso-org/enso/pull/3227
[3248]: https://github.com/enso-org/enso/pull/3248
Expand Down Expand Up @@ -455,6 +456,7 @@
[3781]: https://github.com/enso-org/enso/pull/3781
[3798]: https://github.com/enso-org/enso/pull/3798
[3811]: https://github.com/enso-org/enso/pull/3811
[3817]: https://github.com/enso-org/enso/pull/3817

# Enso 2.0.0-alpha.18 (2021-10-12)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.oracle.truffle.api.nodes.Node;
import org.enso.interpreter.dsl.*;
import org.enso.interpreter.epb.node.CoercePrimitiveNode;
import org.enso.interpreter.node.expression.builtin.mutable.CoerceArrayNode;
import org.enso.interpreter.node.expression.foreign.CoerceNothing;
import org.enso.interpreter.runtime.Context;
import org.enso.interpreter.runtime.data.Array;
Expand All @@ -19,9 +20,6 @@
name = "from_array",
description = "Creates a Vector by copying Array content.")
public abstract class FromArrayBuiltinVectorNode extends Node {
private @Child CoercePrimitiveNode coercePrimitiveNode = CoercePrimitiveNode.build();
private @Child CoerceNothing coerceNothingNode = CoerceNothing.build();

static FromArrayBuiltinVectorNode build() {
return FromArrayBuiltinVectorNodeGen.create();
}
Expand All @@ -34,24 +32,11 @@ Vector fromVector(Vector arr) {
}

@Specialization(guards = "interop.hasArrayElements(arr)")
Vector fromArrayLikeObject(Object arr, @CachedLibrary(limit = "3") InteropLibrary interop) {
try {
long length = interop.getArraySize(arr);
Object[] target = new Object[(int) length];
for (int i = 0; i < length; i++) {
try {
var value = interop.readArrayElement(arr, i);
target[i] = coerceNothingNode.execute(coercePrimitiveNode.execute(value));
} catch (InvalidArrayIndexException ex) {
var ctx = Context.get(this);
var err = ctx.getBuiltins().error().makeInvalidArrayIndexError(arr, i);
throw new PanicException(err, this);
}
}
return Vector.fromArray(new Array(target));
} catch (UnsupportedMessageException ex) {
throw unsupportedException(arr);
}
Vector fromArrayLikeObject(
Object arr,
@Cached CoerceArrayNode coerce,
@CachedLibrary(limit = "3") InteropLibrary interop) {
return Vector.fromArray(new Array(coerce.execute(arr)));
}

@Fallback
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.enso.interpreter.node.expression.builtin.interop.generic;

import com.oracle.truffle.api.dsl.Cached;
import com.oracle.truffle.api.dsl.Specialization;
import com.oracle.truffle.api.interop.ArityException;
import com.oracle.truffle.api.interop.InteropLibrary;
import com.oracle.truffle.api.interop.UnsupportedMessageException;
Expand All @@ -9,22 +11,30 @@
import org.enso.interpreter.Constants;
import org.enso.interpreter.dsl.BuiltinMethod;
import org.enso.interpreter.node.expression.builtin.interop.syntax.HostValueToEnsoNode;
import org.enso.interpreter.node.expression.builtin.mutable.CoerceArrayNode;
import org.enso.interpreter.runtime.data.Array;
import org.enso.interpreter.runtime.error.PanicException;

@BuiltinMethod(
type = "Polyglot",
name = "execute",
description = "Executes a polyglot function object (e.g. a lambda).")
public class ExecuteNode extends Node {
public abstract class ExecuteNode extends Node {
private @Child InteropLibrary library =
InteropLibrary.getFactory().createDispatched(Constants.CacheSizes.BUILTIN_INTEROP_DISPATCH);
private @Child HostValueToEnsoNode hostValueToEnsoNode = HostValueToEnsoNode.build();
private final BranchProfile err = BranchProfile.create();

Object execute(Object callable, Array arguments) {
static ExecuteNode build() {
return ExecuteNodeGen.create();
}

abstract Object execute(Object callable, Object arguments);

@Specialization
Object doExecute(Object callable, Object arguments, @Cached("build()") CoerceArrayNode coerce) {
try {
return hostValueToEnsoNode.execute(library.execute(callable, arguments.getItems()));
return hostValueToEnsoNode.execute(library.execute(callable, coerce.execute(arguments)));
} catch (UnsupportedMessageException | ArityException | UnsupportedTypeException e) {
err.enter();
throw new PanicException(e.getMessage(), this);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.enso.interpreter.node.expression.builtin.interop.generic;

import com.oracle.truffle.api.dsl.Cached;
import com.oracle.truffle.api.dsl.Specialization;
import com.oracle.truffle.api.interop.ArityException;
import com.oracle.truffle.api.interop.InteropLibrary;
import com.oracle.truffle.api.interop.UnsupportedMessageException;
Expand All @@ -8,22 +10,30 @@
import com.oracle.truffle.api.profiles.BranchProfile;
import org.enso.interpreter.Constants;
import org.enso.interpreter.dsl.BuiltinMethod;
import org.enso.interpreter.runtime.data.Array;
import org.enso.interpreter.node.expression.builtin.mutable.CoerceArrayNode;
import org.enso.interpreter.runtime.error.PanicException;

@BuiltinMethod(
type = "Polyglot",
name = "new",
description = "Instantiates a polyglot constructor.")
public class InstantiateNode extends Node {
public abstract class InstantiateNode extends Node {

private @Child InteropLibrary library =
InteropLibrary.getFactory().createDispatched(Constants.CacheSizes.BUILTIN_INTEROP_DISPATCH);
private final BranchProfile err = BranchProfile.create();

Object execute(Object constructor, Array arguments) {
static InstantiateNode build() {
return InstantiateNodeGen.create();
}

abstract Object execute(Object constructor, Object arguments);

@Specialization
Object doExecute(
Object constructor, Object arguments, @Cached("build()") CoerceArrayNode coerce) {
try {
return library.instantiate(constructor, arguments.getItems());
return library.instantiate(constructor, coerce.execute(arguments));
} catch (UnsupportedMessageException | ArityException | UnsupportedTypeException e) {
err.enter();
throw new PanicException(e.getMessage(), this);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,27 +1,42 @@
package org.enso.interpreter.node.expression.builtin.interop.generic;

import com.oracle.truffle.api.interop.*;
import com.oracle.truffle.api.dsl.Cached;
import com.oracle.truffle.api.dsl.Specialization;
import com.oracle.truffle.api.interop.ArityException;
import com.oracle.truffle.api.interop.InteropLibrary;
import com.oracle.truffle.api.interop.UnknownIdentifierException;
import com.oracle.truffle.api.interop.UnsupportedMessageException;
import com.oracle.truffle.api.interop.UnsupportedTypeException;
import com.oracle.truffle.api.nodes.Node;
import com.oracle.truffle.api.profiles.BranchProfile;
import org.enso.interpreter.Constants;
import org.enso.interpreter.dsl.BuiltinMethod;
import org.enso.interpreter.node.expression.builtin.mutable.CoerceArrayNode;
import org.enso.interpreter.node.expression.builtin.text.util.ExpectStringNode;
import org.enso.interpreter.runtime.data.Array;
import org.enso.interpreter.runtime.error.PanicException;

@BuiltinMethod(
type = "Polyglot",
name = "invoke",
description = "Invokes a polyglot method by name, dispatching by the target argument.")
public class InvokeNode extends Node {
public abstract class InvokeNode extends Node {
private @Child InteropLibrary library =
InteropLibrary.getFactory().createDispatched(Constants.CacheSizes.BUILTIN_INTEROP_DISPATCH);
private @Child ExpectStringNode expectStringNode = ExpectStringNode.build();
private final BranchProfile err = BranchProfile.create();

Object execute(Object target, Object name, Array arguments) {
static InvokeNode build() {
return InvokeNodeGen.create();
}

abstract Object execute(Object target, Object name, Object arguments);

@Specialization
Object doExecute(
Object target, Object name, Object arguments, @Cached("build()") CoerceArrayNode coerce) {
try {
return library.invokeMember(target, expectStringNode.execute(name), arguments.getItems());
return library.invokeMember(
target, expectStringNode.execute(name), coerce.execute(arguments));
} catch (UnsupportedMessageException
| ArityException
| UnsupportedTypeException
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,27 @@
package org.enso.interpreter.node.expression.builtin.meta;

import com.oracle.truffle.api.dsl.Cached;
import com.oracle.truffle.api.dsl.Specialization;
import com.oracle.truffle.api.nodes.Node;
import org.enso.interpreter.dsl.BuiltinMethod;
import org.enso.interpreter.node.expression.builtin.mutable.CoerceArrayNode;
import org.enso.interpreter.runtime.callable.atom.Atom;
import org.enso.interpreter.runtime.callable.atom.AtomConstructor;
import org.enso.interpreter.runtime.data.Array;

@BuiltinMethod(
type = "Meta",
name = "new_atom",
description = "Creates a new atom with given constructor and fields.")
public class NewAtomInstanceNode extends Node {
Atom execute(AtomConstructor constructor, Array fields) {
return constructor.newInstance(fields.getItems());
public abstract class NewAtomInstanceNode extends Node {

static NewAtomInstanceNode build() {
return NewAtomInstanceNodeGen.create();
}

abstract Atom execute(AtomConstructor constructor, Object fields);

@Specialization
Atom doExecute(AtomConstructor constructor, Object fields, @Cached CoerceArrayNode coerce) {
return constructor.newInstance(coerce.execute(fields));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package org.enso.interpreter.node.expression.builtin.mutable;

import com.oracle.truffle.api.dsl.Cached;
import com.oracle.truffle.api.dsl.Fallback;
import com.oracle.truffle.api.dsl.Specialization;
import com.oracle.truffle.api.interop.InteropLibrary;
import com.oracle.truffle.api.interop.InvalidArrayIndexException;
import com.oracle.truffle.api.interop.UnsupportedMessageException;
import com.oracle.truffle.api.library.CachedLibrary;
import com.oracle.truffle.api.nodes.ExplodeLoop;
import com.oracle.truffle.api.nodes.Node;
import org.enso.interpreter.node.expression.builtin.interop.syntax.HostValueToEnsoNode;
import org.enso.interpreter.runtime.Context;
import org.enso.interpreter.runtime.builtin.Builtins;
import org.enso.interpreter.runtime.callable.atom.Atom;
import org.enso.interpreter.runtime.data.Vector;
import org.enso.interpreter.runtime.data.Array;
import org.enso.interpreter.runtime.error.PanicException;

public abstract class CoerceArrayNode extends Node {
private @Child InteropLibrary library = InteropLibrary.getFactory().createDispatched(10);

public static CoerceArrayNode build() {
return CoerceArrayNodeGen.create();
}

public abstract Object[] execute(Object value);

@Specialization
Object[] doArray(Array arr) {
return arr.getItems();
}

@Specialization
Object[] doVector(Vector arr, @Cached HostValueToEnsoNode hostValueToEnsoNode) {
try {
return convertToArray(arr, hostValueToEnsoNode);
} catch (UnsupportedMessageException e) {
Builtins builtins = Context.get(this).getBuiltins();
Atom err = builtins.error().makeTypeError(builtins.array(), arr, "arr");
throw new PanicException(err, this);
} catch (InvalidArrayIndexException e) {
Builtins builtins = Context.get(this).getBuiltins();
throw new PanicException(
builtins.error().makeInvalidArrayIndexError(arr, e.getInvalidIndex()), this);
}
}

@Specialization(guards = "interop.hasArrayElements(arr)")
Object[] doArrayLike(
Object arr,
@CachedLibrary(limit = "5") InteropLibrary interop,
@Cached HostValueToEnsoNode hostValueToEnsoNode) {
try {
return convertToArray(arr, hostValueToEnsoNode);
} catch (UnsupportedMessageException e) {
Builtins builtins = Context.get(this).getBuiltins();
Atom err = builtins.error().makeTypeError(builtins.array(), arr, "arr");
throw new PanicException(err, this);
} catch (InvalidArrayIndexException e) {
Builtins builtins = Context.get(this).getBuiltins();
throw new PanicException(
builtins.error().makeInvalidArrayIndexError(arr, e.getInvalidIndex()), this);
}
}

private Object[] convertToArray(Object arr, HostValueToEnsoNode hostValueToEnsoNode)
throws UnsupportedMessageException, InvalidArrayIndexException {
int argsLength = (int) library.getArraySize(arr);
Object[] arr1 = new Object[argsLength];
for (int i = 0; i < argsLength; i++) {
arr1[i] = hostValueToEnsoNode.execute(library.readArrayElement(arr, i));
}
return arr1;
}

@Fallback
Object[] doOther(Object arr) {
Builtins builtins = Context.get(this).getBuiltins();
Atom error = builtins.error().makeTypeError("array", arr, "arr");
throw new PanicException(error, this);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.dsl.Cached;
import com.oracle.truffle.api.io.TruffleProcessBuilder;
import com.oracle.truffle.api.nodes.ExplodeLoop;
import org.apache.commons.lang3.SystemUtils;
import org.enso.interpreter.dsl.Builtin;
import org.enso.interpreter.node.expression.builtin.mutable.CoerceArrayNode;
import org.enso.interpreter.node.expression.builtin.text.util.ExpectStringNode;
import org.enso.interpreter.runtime.Context;
import org.enso.interpreter.runtime.callable.atom.Atom;
import org.enso.interpreter.runtime.data.Array;
import org.enso.interpreter.runtime.data.text.Text;
import org.enso.interpreter.runtime.error.PanicException;

Expand Down Expand Up @@ -53,20 +54,23 @@ public static void exit(long code) {
@Builtin.WrapException(from = IOException.class, to = PanicException.class)
@Builtin.WrapException(from = InterruptedException.class, to = PanicException.class)
@CompilerDirectives.TruffleBoundary
@ExplodeLoop
public static Atom createProcess(
Context ctx,
Object command,
Array arguments,
Object arguments,
Object input,
boolean redirectIn,
boolean redirectOut,
boolean redirectErr,
@Cached CoerceArrayNode coerce,
@Cached ExpectStringNode expectStringNode)
throws IOException, InterruptedException {
String[] cmd = new String[arguments.getItems().length + 1];
Object[] arrArguments = coerce.execute(arguments);
String[] cmd = new String[arrArguments.length + 1];
cmd[0] = expectStringNode.execute(command);
for (int i = 1; i <= arguments.getItems().length; i++) {
cmd[i] = expectStringNode.execute(arguments.getItems()[i - 1]);
for (int i = 1; i <= arrArguments.length; i++) {
cmd[i] = expectStringNode.execute(arrArguments[i - 1]);
}
TruffleProcessBuilder pb = ctx.getEnvironment().newProcessBuilder(cmd);

Expand Down
26 changes: 26 additions & 0 deletions test/Tests/src/Data/Polyglot_Spec.enso
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
from Standard.Base import all

from Standard.Test import Test, Test_Suite

polyglot java import java.time.LocalDate
polyglot java import java.lang.String
polyglot java import java.util.function.Function

spec = Test.group "Polyglot" <|
Test.specify "should be able to invoke a polyglot method by name and pass arguments" <|
poly_date = LocalDate.now
date = Date.now.to_date_time

Polyglot.invoke poly_date "atStartOfDay" [] . should_equal date
Polyglot.invoke poly_date "atStartOfDay" [].to_array . should_equal date

Test.specify "should be able to create a new polyglot object using the constructor" <|
Polyglot.new String ["42"] . should_equal "42"
Polyglot.new String ["42"].to_array . should_equal "42"

Test.specify "should be able to execute a polyglot function object along with corresponding arguments" <|
fun = Function.identity
Polyglot.execute fun ["42"] . should_equal "42"
Polyglot.execute fun ["42"].to_array . should_equal "42"

main = Test_Suite.run_main spec
2 changes: 2 additions & 0 deletions test/Tests/src/Main.enso
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import project.Semantic.Deep_Export.Spec as Deep_Export_Spec
import project.Semantic.Error_Spec
import project.Semantic.Import_Loop.Spec as Import_Loop_Spec
import project.Semantic.Meta_Spec
import project.Semantic.Meta_Location_Spec
import project.Semantic.Names_Spec
import project.Semantic.Runtime_Spec
import project.Semantic.Warnings_Spec
Expand Down Expand Up @@ -101,6 +102,7 @@ main = Test_Suite.run_main <|
Map_Spec.spec
Maybe_Spec.spec
Meta_Spec.spec
Meta_Location_Spec.spec
Names_Spec.spec
Noise_Generator_Spec.spec
Noise_Spec.spec
Expand Down
Loading

0 comments on commit f6379fb

Please sign in to comment.