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.

Sign up for GitHub

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

Execute foreign function and check autoscoped constructor result #10187

Merged
merged 9 commits into from
Jun 6, 2024
+112 −25
Merged

Execute foreign function and check autoscoped constructor result #10187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
aa05145
Test that yields: Not_Invokable.Error (a, b) => a + b
JaroslavTulach Jun 5, 2024
890cba4
Support execute on TruffleObject
JaroslavTulach Jun 5, 2024
ab0724c
Check if autoscoped constructor really produces Atom
JaroslavTulach Jun 6, 2024
55dce2f
Pick prototype's scope to begin with
JaroslavTulach Jun 6, 2024
4584c66
Merge remote-tracking branch 'origin/develop' into wip/jtulach/Foreig…
JaroslavTulach Jun 6, 2024
fe14b8d
Execute JavaScript with insufficient number of arguments
JaroslavTulach Jun 6, 2024
98fff4d
There is no such thing as partially applied autoscope constructor any…
JaroslavTulach Jun 6, 2024
48af079
add: runtime test
4e6 Jun 6, 2024
84f4198
misc: runtime test cleanup
4e6 Jun 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 24 additions & 19 deletions ...gration-tests/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1307,7 +1307,7 @@ class RuntimeServerTest
)
}

it should "send method pointer updates of partially applied autoscope constructors" in {
JaroslavTulach marked this conversation as resolved.
Show resolved Hide resolved
it should "send error updates for partially applied autoscope constructors" in {
val contextId = UUID.randomUUID()
val requestId = UUID.randomUUID()
val moduleName = "Enso_Test.Test.Main"
Expand Down Expand Up @@ -1361,7 +1361,7 @@ class RuntimeServerTest
)
)
)
context.receiveN(4) should contain theSameElementsAs Seq(
context.receiveN(3) should contain theSameElementsAs Seq(
Api.Response(requestId, Api.PushContextResponse(contextId)),
TestMessages.update(
contextId,
Expand All @@ -1382,26 +1382,31 @@ class RuntimeServerTest
)
)
),
TestMessages.update(
contextId,
id_x_1,
ConstantsGen.FUNCTION_BUILTIN,
methodCall = Some(
Api.MethodCall(
Api.MethodPointer(moduleName, s"$moduleName.T", "A"),
Vector(1)
)
),
payload = Api.ExpressionUpdate.Payload.Value(
functionSchema = Some(
Api.FunctionSchema(
Api.MethodPointer(moduleName, s"$moduleName.T", "A"),
Vector(1)
Api.Response(
Api.ExecutionFailed(
contextId,
Api.ExecutionResult.Diagnostic.error(
"Type_Error.Error",
Some(mainFile),
Some(model.Range(model.Position(8, 0), model.Position(8, 12))),
None,
Vector(
Api.StackTraceElement(
"Main.test",
Some(mainFile),
Some(model.Range(model.Position(8, 0), model.Position(8, 12))),
None
),
Api.StackTraceElement(
"Main.main",
Some(mainFile),
Some(model.Range(model.Position(4, 10), model.Position(4, 18))),
None
)
)
)
)
),
context.executionComplete(contextId)
)
)
}

Expand Down
1 change: 1 addition & 0 deletions engine/runtime/src/main/java/org/enso/interpreter/node/EnsoRootNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ protected EnsoRootNode(
super(language, buildFrameDescriptor(localScope));
Objects.requireNonNull(language);
Objects.requireNonNull(localScope);
Objects.requireNonNull(moduleScope);
this.name = name;
this.localScope = localScope;
this.moduleScope = moduleScope;
Expand Down
45 changes: 44 additions & 1 deletion engine/runtime/src/main/java/org/enso/interpreter/node/callable/InvokeCallableNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,23 @@
package org.enso.interpreter.node.callable;

import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.dsl.Cached;
import com.oracle.truffle.api.dsl.Cached.Shared;
import com.oracle.truffle.api.dsl.Fallback;
import com.oracle.truffle.api.dsl.Specialization;
import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.interop.ArityException;
import com.oracle.truffle.api.interop.InteropLibrary;
import com.oracle.truffle.api.interop.UnsupportedMessageException;
import com.oracle.truffle.api.interop.UnsupportedTypeException;
import com.oracle.truffle.api.library.CachedLibrary;
import com.oracle.truffle.api.nodes.Node;
import com.oracle.truffle.api.source.SourceSection;
import java.util.UUID;
import java.util.concurrent.locks.Lock;
import org.enso.interpreter.Constants;
import org.enso.interpreter.node.BaseNode;
import org.enso.interpreter.node.BaseNode.TailStatus;
import org.enso.interpreter.node.callable.dispatch.InvokeFunctionNode;
import org.enso.interpreter.node.callable.thunk.ThunkExecutorNode;
import org.enso.interpreter.runtime.EnsoContext;
Expand All @@ -29,6 +35,7 @@
import org.enso.interpreter.runtime.error.Warning;
import org.enso.interpreter.runtime.error.WarningsLibrary;
import org.enso.interpreter.runtime.error.WithWarnings;
import org.enso.interpreter.runtime.library.dispatch.TypesLibrary;
import org.enso.interpreter.runtime.state.State;

/**
Expand Down Expand Up @@ -278,7 +285,7 @@ public Object invokeWarnings(
VirtualFrame callerFrame,
State state,
Object[] arguments,
@CachedLibrary(limit = "3") WarningsLibrary warnings) {
@Shared("warnings") @CachedLibrary(limit = "3") WarningsLibrary warnings) {

Warning[] extracted;
Object callable;
Expand Down Expand Up @@ -323,6 +330,42 @@ public Object invokeWarnings(
}
}

@Specialization(
guards = {
"!warnings.hasWarnings(self)",
"!types.hasType(self)",
"!types.hasSpecialDispatch(self)",
"iop.isExecutable(self)",
})
Object doPolyglot(
Object self,
VirtualFrame frame,
State state,
Object[] arguments,
@CachedLibrary(limit = "3") InteropLibrary iop,
@Shared("warnings") @CachedLibrary(limit = "3") WarningsLibrary warnings,
@CachedLibrary(limit = "3") TypesLibrary types,
@Cached ThunkExecutorNode thunkNode) {
var errors = EnsoContext.get(this).getBuiltins().error();
try {
for (int i = 0; i < arguments.length; i++) {
arguments[i] = thunkNode.executeThunk(frame, arguments[i], state, TailStatus.NOT_TAIL);
}
return iop.execute(self, arguments);
} catch (UnsupportedTypeException ex) {
var err = errors.makeUnsupportedArgumentsError(ex.getSuppliedValues(), ex.getMessage());
throw new PanicException(err, this);
} catch (ArityException ex) {
var err =
errors.makeArityError(
ex.getExpectedMinArity(), ex.getExpectedMaxArity(), arguments.length);
throw new PanicException(err, this);
} catch (UnsupportedMessageException ex) {
var err = errors.makeNotInvokable(self);
throw new PanicException(err, this);
}
}

@Fallback
public Object invokeGeneric(
Object callable, VirtualFrame callerFrame, State state, Object[] arguments) {
Expand Down
16 changes: 12 additions & 4 deletions ...ne/runtime/src/main/java/org/enso/interpreter/runtime/callable/UnresolvedConstructor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@
import org.enso.interpreter.runtime.callable.function.Function;
import org.enso.interpreter.runtime.data.EnsoObject;
import org.enso.interpreter.runtime.data.Type;
import org.enso.interpreter.runtime.data.atom.Atom;
import org.enso.interpreter.runtime.data.atom.AtomConstructor;
import org.enso.interpreter.runtime.error.PanicException;
import org.enso.interpreter.runtime.library.dispatch.TypesLibrary;
import org.enso.interpreter.runtime.scope.ModuleScope;
import org.enso.interpreter.runtime.state.State;

/**
Expand Down Expand Up @@ -171,7 +172,8 @@ public SourceSection getSourceSection() {
static DirectCallNode buildApplication(UnresolvedConstructor prototype) {
UUID id = null;
SourceSection section = null;
ModuleScope scope = null;
var scope =
prototype.where.getRootNode() instanceof EnsoRootNode root ? root.getModuleScope() : null;
for (var where = prototype.where; where != null; where = where.getParent()) {
if (where instanceof ExpressionNode withId && withId.getId() != null) {
id = withId.getId();
Expand Down Expand Up @@ -235,7 +237,7 @@ Object instantiateUncached(
return invokeConstructor(c, unresolved.asPrototype(), unresolved, state, callNode);
}

private static Object invokeConstructor(
private Object invokeConstructor(
AtomConstructor c,
UnresolvedConstructor prototype,
UnresolvedConstructor unresolved,
Expand All @@ -254,7 +256,13 @@ private static Object invokeConstructor(
args[0] = fn;
var helper = Function.ArgumentsHelper.buildArguments(fn, null, state, args);
var r = callNode.call(helper);
return r;
if (r instanceof Atom) {
return r;
} else {
var ctx = EnsoContext.get(this);
var err = ctx.getBuiltins().error().makeTypeError(c.getType(), r, prototype.toString());
throw new PanicException(err, this);
}
}

private static Object checkSingleton(Type c, UnresolvedConstructor unresolved) {
Expand Down
17 changes: 16 additions & 1 deletion test/Base_Tests/src/Data/Polyglot_Spec.enso
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from Standard.Base import all

import Standard.Base.Errors.Common.Not_Invokable
from Standard.Test import all


Expand Down Expand Up @@ -34,6 +34,18 @@ add_specs suite_builder = suite_builder.group "Polyglot" group_builder->
group_builder.specify "access Integer field from Polyglot object" <|
js_meaning.meaning . should_equal 42

group_builder.specify "Execute JavaScript function" <|
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be nice to also see what happens if I try to specify more or less arguments to the function. Can you do, e.g.,

curried = js_plus 3
curried 5 . should_equal 8

?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Invoking:

curried = js_plus 3

main =
    curried

foreign js js_plus = """
    return (a, b) => a + b;

yields NaN as 3 + undefined is NaN.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd expect it to behave the same way as if js_plus was an enso function. I.e. main should return a function

Copy link
Member Author

@JaroslavTulach JaroslavTulach Jun 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested in fe14b8d

I'd expect it to behave the same way as if js_plus was an enso function

You might expect it, but there is no chance to achieve such behavior. JavaScript functions don't have a fixed arity. You can invoke them with any number of arguments. If you want that to be changed, then you need to beg for a change of the ECMAScript specification ;-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I didn't think about that

js_plus 3 5 . should_equal 8

group_builder.specify "Execute JavaScript with insufficient number of arguments" <|
r = js_plus 3
r.is_nan . should_be_true

group_builder.specify "Cannot Execute JavaScript number" <|
fourty_two = js_meaning.meaning
Test.expect_panic Not_Invokable <|
fourty_two "Cannot invoke"
radeusgd marked this conversation as resolved.
Show resolved Hide resolved

group_builder.specify "use Integer obtained from a call" <|
Java_Integer.parseInt "42" . should_equal 42

Expand All @@ -50,6 +62,9 @@ add_specs suite_builder = suite_builder.group "Polyglot" group_builder->
foreign js js_meaning = """
return { meaning : 6 * 7 };

foreign js js_plus = """
return (a, b) => a + b;

main filter=Nothing =
suite = Test.build suite_builder->
add_specs suite_builder
Expand Down
15 changes: 15 additions & 0 deletions test/Base_Tests/src/Semantic/Conversion_Spec.enso
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,21 @@ add_specs suite_builder =
foo = v:Foo
Foo.Value 10 . should_equal foo

group_builder.specify "Foo.Value constructor is not autoscoped" <|

v = ..Value
err = Test.expect_panic Type_Error <|
f = v:Foo
f

msg = err.to_text

msg . should_contain "Type error:"
msg . should_contain "Expected `..Value` to be Foo"
msg . should_contain "got Foo.Value["
msg . should_contain "foo=_"
msg . should_contain "Try to apply foo argument"
Copy link
Member Author

@JaroslavTulach JaroslavTulach Jun 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running a.enso:

from Standard.Base import all

type Foo
    Value a

main =
    v = ..Value
    f = v:Foo
    f

yields

Type error: Expected `..Value` to be Foo, but got Foo.Value[a.enso:4:5-11] a=_.
Try to apply a argument.
        at <enso> a.main(a.enso:8:9-9)


group_builder.specify "..False can be autoscoped" <|

bool b:Boolean = b
Expand Down
Loading