Closed farmaazon closed 9 months ago
farmaazon
commented
9 months ago @hubertp @4e6 do you think this is hard/long to implement? If yes, we'd like to do some workarounds for constructors in GUI. If not, then we'll just wait for proper solution.
4e6
commented
9 months ago I can see that the engine does not return the function schema for the partially applied constructors (while it does for the methods). I need to check what is the reason for that.
4e6
commented
9 months ago There's not enough runtime information to build a method pointer for partially applied constructors. They are represented as closures in runtime. I need to check the codegen and see if I can supply the required info.
hubertp
commented
9 months ago @hubertp @4e6 do you think this is hard/long to implement? If yes, we'd like to do some workarounds for constructors in GUI. If not, then we'll just wait for proper solution.
To answer your question - we don't know yet. It's hard to estimate this change atm. But Dmitry is looking into it.
4e6
commented
9 months ago In #6957 the FunctionSchema was added to the response to provide information about the returned function.
Below there is a recap of what's working and what doesn't in case of constructors.
Given the type definition.
type T
A x yT.A - a call to constructor
ExpressionUpdate(
type = Function,
methodCall = MethodCall(MethodPointer(Main, Main.T, A), [0, 1]),
payload = Value(
functionSchema = MethodCall(MethodPointer(Main, Main.T, A), [0, 1])
)
)ExpressionUpdate(
type = Function,
methodCall = MethodCall(MethodPointer(Main, Main.T, A), []),
payload = Value()
)T.A 1 - a call to a partially applied constructor
ExpressionUpdate(
type = Function,
methodCall = MethodCall(MethodPointer(Main, Main.T, A), [1]),
payload = Value(
functionSchema = MethodCall(MethodPointer(Main, Main.T, A), [1])
)
)ExpressionUpdate(
type = Function,
methodCall = MethodCall(MethodPointer(Main, Main.T, A), []),
payload = Value()
)T.A 1 2 - a call to a fully applied constructor
ExpressionUpdate(
type = Main.T,
methodCall = MethodCall(MethodPointer(Main, Main.T, A), []),
payload = Value()
)ExpressionUpdate(
type = Main.T,
methodCall = MethodCall(MethodPointer(Main, Main.T, A), []),
payload = Value()
)
enso-bot[bot]
commented
9 months ago Dmitry Bushev reports a new STANDUP for yesterday (2024-01-08):
Progress: Started working on the task. Identified the issue. Started analyzing different examples of the application of the constructor function. Reviewed how the partial application in the ordinary functions is implemented. It should be finished by 2024-01-12.
Next Day: Next day I will be working on the #8663 task. Continue working on the task
enso-bot[bot]
commented
9 months ago Dmitry Bushev reports a new STANDUP for today (2024-01-09):
Progress: Continue working on the task. Identified the issue. Finally got the idea of what info is missing from the updates about the constructor calls. Got an idea of how to implement the fix without changing the runtime representation. It should be finished by 2024-01-12.
Next Day: Next day I will be working on the #8663 task. Continue working on the task
JaroslavTulach
commented
9 months ago I believe it is good idea to start with a test. If I understand @4e6 correctly, the problem is in FunctionPointer.fromFunction method. Let's see how it behaves for different kinds of functions:
diff --git engine/runtime/src/test/java/org/enso/interpreter/test/instrument/FunctionPointerTest.java engine/runtime/src/test/java/org/enso/interpreter/test/instrument/FunctionPointerTest.java
new file mode 100644
index 0000000000..9dbd4fb997
--- /dev/null
+++ engine/runtime/src/test/java/org/enso/interpreter/test/instrument/FunctionPointerTest.java
@@ -0,0 +1,174 @@
+package org.enso.interpreter.test.instrument;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertNotNull;
+
+import java.nio.file.Paths;
+import java.util.Map;
+import java.util.logging.Level;
+
+import org.enso.interpreter.runtime.callable.atom.AtomConstructor;
+import org.enso.interpreter.runtime.callable.function.Function;
+import org.enso.polyglot.RuntimeOptions;
+import org.graalvm.polyglot.Context;
+import org.graalvm.polyglot.Language;
+import org.graalvm.polyglot.Source;
+import org.graalvm.polyglot.io.IOAccess;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import org.enso.interpreter.service.ExecutionService.FunctionPointer;
+import org.enso.interpreter.test.TestBase;
+
+public class FunctionPointerTest extends TestBase {
+
+ private Context context;
+
+ @Before
+ public void initContext() {
+ context =
+ Context.newBuilder()
+ .allowExperimentalOptions(true)
+ .option(
+ RuntimeOptions.LANGUAGE_HOME_OVERRIDE,
+ Paths.get("../../distribution/component").toFile().getAbsolutePath())
+ .option(RuntimeOptions.LOG_LEVEL, Level.WARNING.getName())
+ .logHandler(System.err)
+ .allowExperimentalOptions(true)
+ .allowIO(IOAccess.ALL)
+ .allowAllAccess(true)
+ .build();
+
+ var engine = context.getEngine();
+ Map<String, Language> langs = engine.getLanguages();
+ Assert.assertNotNull("Enso found: " + langs, langs.get("enso"));
+ }
+
+ @After
+ public void disposeContext() {
+ context.close();
+ }
+
+ @Test
+ public void moduleFunctionPointer() throws Exception {
+ var rawCode = """
+ from Standard.Base import all
+
+ run a b = a + b
+ """;
+ var src = Source.newBuilder("enso", rawCode, "TestFunctionPointer.enso").build();
+ var module = context.eval(src);
+ var res = module.invokeMember("eval_expression", "run");
+
+ assertTrue("fn: " + res, res.canExecute());
+ var rawRes = TestBase.unwrapValue(context, res);
+ assertTrue("function: " + rawRes, rawRes instanceof Function);
+ var c = FunctionPointer.fromFunction((Function) rawRes);
+ assertNotNull(c);
+ assertEquals("TestFunctionPointer", c.moduleName().toString());
+ assertEquals("TestFunctionPointer", c.typeName().toString());
+ assertEquals("run", c.functionName().toString());
+ }
+
+ @Test
+ public void typeStaticMethodPointer() throws Exception {
+ var rawCode = """
+ from Standard.Base import all
+
+ type X
+ run a b = a + b
+ """;
+ var src = Source.newBuilder("enso", rawCode, "StaticMethodPointer.enso").build();
+ var module = context.eval(src);
+ var res = module.invokeMember("eval_expression", "X.run");
+
+ assertTrue("fn: " + res, res.canExecute());
+ var rawRes = TestBase.unwrapValue(context, res);
+ assertTrue("function: " + rawRes, rawRes instanceof Function);
+ var c = FunctionPointer.fromFunction((Function) rawRes);
+ assertNotNull(c);
+ assertEquals("StaticMethodPointer", c.moduleName().toString());
+ assertEquals("StaticMethodPointer.X", c.typeName().toString());
+ assertEquals("run", c.functionName().toString());
+
+ var apply = res.execute(1);
+ assertTrue("fn: " + apply, apply.canExecute());
+ var rawApply = TestBase.unwrapValue(context, res);
+ assertTrue("function: " + rawApply, rawApply instanceof Function);
+ var a = FunctionPointer.fromFunction((Function) rawApply);
+ assertNotNull(a);
+ assertEquals("StaticMethodPointer", a.moduleName().toString());
+ assertEquals("StaticMethodPointer.X", a.typeName().toString());
+ assertEquals("run", a.functionName().toString());
+ }
+
+ @Test
+ public void typeInstanceMethodPointer() throws Exception {
+ var rawCode = """
+ from Standard.Base import all
+
+ type X
+ run self b c = [self, b, c]
+ """;
+ var src = Source.newBuilder("enso", rawCode, "InstanceMethodPointer.enso").build();
+ var module = context.eval(src);
+ var res = module.invokeMember("eval_expression", "X.run");
+
+ assertTrue("fn: " + res, res.canExecute());
+ var rawRes = TestBase.unwrapValue(context, res);
+ assertTrue("function: " + rawRes, rawRes instanceof Function);
+ var c = FunctionPointer.fromFunction((Function) rawRes);
+ assertNotNull(c);
+ assertEquals("InstanceMethodPointer", c.moduleName().toString());
+ assertEquals("InstanceMethodPointer.X", c.typeName().toString());
+ assertEquals("run", c.functionName().toString());
+
+ var apply = res.execute(1);
+ assertTrue("fn: " + apply, apply.canExecute());
+ var rawApply = TestBase.unwrapValue(context, res);
+ assertTrue("function: " + rawApply, rawApply instanceof Function);
+ var a = FunctionPointer.fromFunction((Function) rawApply);
+ assertNotNull(a);
+ assertEquals("InstanceMethodPointer", a.moduleName().toString());
+ assertEquals("InstanceMethodPointer.X", a.typeName().toString());
+ assertEquals("run", a.functionName().toString());
+ }
+
+ @Test
+ public void typeConstructorPointer() throws Exception {
+ var rawCode = """
+ from Standard.Base import all
+
+ type X
+ Run a b
+ """;
+ var src = Source.newBuilder("enso", rawCode, "ConstructorPointer.enso").build();
+ var module = context.eval(src);
+ var res = module.invokeMember("eval_expression", "X.Run");
+
+ assertTrue("fn: " + res, res.canInstantiate());
+ var rawRes = TestBase.unwrapValue(context, res);
+ assertTrue("function: " + rawRes.getClass(), rawRes instanceof AtomConstructor);
+ var rawFn = ((AtomConstructor) rawRes).getConstructorFunction();
+ var c = FunctionPointer.fromFunction((Function) rawFn);
+ assertNotNull("We should get a pointer for " + rawFn, c);
+ /*
+ assertEquals("ConstructorPointer", c.moduleName().toString());
+ assertEquals("ConstructorPointer.X", c.typeName().toString());
+ assertEquals("run", c.functionName().toString());
+
+ var apply = res.execute(1);
+ assertTrue("fn: " + apply, apply.canExecute());
+ var rawApply = TestBase.unwrapValue(context, res);
+ assertTrue("function: " + rawApply, rawApply instanceof Function);
+ var a = FunctionPointer.fromFunction((Function) rawApply);
+ assertNotNull(a);
+ assertEquals("ConstructorPointer", a.moduleName().toString());
+ assertEquals("ConstructorPointer.X", a.typeName().toString());
+ assertEquals("run", a.functionName().toString());
+ */
+ }
+}The last line before /* block */ fails. E.g. FunctionPointer.fromFunction really returns null for AtomConstructor.getConstructorFunction().
enso-bot[bot]
commented
9 months ago Dmitry Bushev reports a new STANDUP for yesterday (2024-01-10):
Progress: Continue working on the task. Updated the update handling logic to use the method pointer detected during the call if it fails to build the function schema for the result function. This is suboptimal because I cannot be sure that it holds for any function. Got another idea to create a separate root node for a constructor function. It should be finished by 2024-01-12.
Next Day: Next day I will be working on the #8663 task. Continue working on the task
enso-bot[bot]
commented
9 months ago Dmitry Bushev reports a new STANDUP for today (2024-01-11):
Progress: Continue working on the task. End up re-using the method root node for the constructor function instead of creating a separate one. Fixed the NPE in the function pointer building logic. Fixed the runtime tests. Created the PR. It should be finished by 2024-01-12.
Next Day: Next day I will be working on the #8663 task. Continue working on the task
enso-bot[bot]
commented
9 months ago Dmitry Bushev reports a new STANDUP for yesterday (2024-01-12):
Progress: Finisnhing working on the task. Was working on review comments. Refactored the function pointer logic. Added the tests for the function pointer. It should be finished by 2024-01-12.
Next Day: Next day I will be working on the #8663 task. Continue working on the task
To display proper placeholders for constructor's argument everywhere, we need to get similar information as for method calls in execution updates. It could be even exactly the same
methodCallfield.