feat: call dictu from native code #755

liz3 · 2024-11-13T23:57:00Z

feat: call dictu from native code

This uses a stack loaded piece of code to interrupt the current vm execution in order to invoke a function called by native code.

It creates a new callframe for that and then makes sure the vm loop breaks after that frame was reached again, this way the result is returned to the caller. It then resets the vm state and continues normally.

Type of Change:

Bug fix
New feature
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Housekeeping:

Tests have been updated to reflect the changes done within this PR (if applicable).
Documentation has been updated to reflect the changes done within this PR (if applicable).

Screenshots (If Applicable):

const entry = "test";

def testFunc (a,b=30) {
  print("Hello world {}".format(entry));
  if(a == 30)
    return nil;
   return a+b;
}

print(entry.testThing(
testFunc, 30
));

This uses a stack loaded piece of code to interrupt the current vm execution in order to invoke a function called by native code. It creates a new callframe for that and then makes sure the vm loop breaks after that frame was reached again, this way the result is returned tp the caller. It then resets the vm state and continues normally.

liz3 · 2024-11-13T23:57:24Z

@Jason2605 would be super thrilled to get some feedback here

Jason2605

This seems really promising so far! A couple of comments to think about but this would be huge 😄

Jason2605 · 2024-11-14T01:01:40Z

src/vm/vm.c

+        return NIL_VAL;
+    int currentFrameCount = vm->frameCount;
+    Value* currentStack = vm->stackTop;
+    CallFrame *frame = &vm->frames[vm->frameCount++];


Here we need to be careful we actually have enough frames, we may need to reallocate if not

Jason2605 · 2024-11-14T01:02:11Z

src/vm/vm.c

@@ -2457,3 +2469,21 @@ DictuInterpretResult dictuInterpret(DictuVM *vm, char *moduleName, char *source)

    return result;
 }
+Value callFunction(DictuVM* vm, Value function, int argCount, Value* args) {
+    if(!IS_FUNCTION(function) && !IS_CLOSURE(function))


Here it would be nice to return an EMPTY val and throw a runtime error (happy you're just testing so may not have put that in yet etc)

Also be nice to handle natives here too, that should be a lot easier as we don't need to handle call frames

Jason2605 · 2024-11-14T01:07:24Z

src/vm/vm.c

+    for(int i = argCount -1; i >= 0; i--) {
+        push(vm, args[i]);
+    }
+    runWithBreakFrame(vm, currentFrameCount+1);


Here we would need to handle a runtime error as at the minute (without testing) I'm assuming this will probably dump

liz3 · 2024-11-14T01:43:38Z

@Jason2605 I've addressed the review points.
The next step will be adding tests but i am yet to decide how to do that.

I am very thankful you do not seam against this approach because i was very uncertain whether this would have other side effects but it seams that it does give a good way to callback into dictu code from natives, the overhead is manageable i hope.

liz3 · 2024-11-14T04:20:12Z

added tests!

Jason2605 · 2024-11-14T23:05:07Z

I think we're almost there! I think the runtime stack trace does segfault out though if a runtime error is actually raised, so we will need to have a look into that

liz3 · 2024-11-14T23:57:10Z

@Jason2605 ive fixed the issue, i had to read the error logic to make sense of the issue but after that it appeared logical.
I intentionally set the closure to NULL and then have a special case within the error stack trace, should there be more "types of callframes" in the future we might need to change the approach but for now this should work

Jason2605 · 2024-11-15T00:47:15Z

I actually wonder if it's worth silencing it for the synthetic callframe. I'm not sure it gives the end user a whole lot of benefit and may cause confusion, what do you think?

liz3 · 2024-11-15T01:01:08Z

Silencing should be fine too, it is not real code afterall.

liz3 · 2024-11-15T01:06:31Z

http test service returning 502 🫠

Jason2605 · 2024-11-15T01:08:28Z

Aha yeah the tests rely on an external service (not great). I’ve re-ran them now

Jason2605 · 2024-11-15T01:10:03Z

It’s 1am for me right now so I’ll give this a proper look over tomorrow! Thank you again for this :)

Jason2605

We have the setUp method we can use here that is implicitly called before each test run!

https://dictu-lang.com/docs/standard-lib/unittest/#setup

Jason2605 · 2024-11-15T19:32:02Z

tests/ffi/ffi.du

@@ -38,18 +38,51 @@ import FFI;
 import Path;

 class TestFFIModule < UnitTest {
+    var mod = nil;
+    doSetup() {


Suggested change

doSetup() {

setUp() {

Jason2605 · 2024-11-15T19:32:08Z

tests/ffi/ffi.du

-            FFI.suffix
-        ));
-        const mod = FFI.load(path);
+        this.doSetup();


Suggested change

this.doSetup();

Jason2605 · 2024-11-15T19:32:15Z

tests/ffi/ffi.du

        this.assertEquals(mod.test, "Dictu!");
        for(var i = 0; i < 40; i+=2) {
            this.assertEquals(mod.dictuFFITestAdd(i, i), i+i);
        }
        this.assertEquals(mod.dictuFFITestStr(), "Hello From Dictu FFI module!");
    }
+    testFFIModuleCallback() {
+        this.doSetup();


Suggested change

this.doSetup();

liz3 · 2024-11-15T20:17:41Z

We have the setUp method we can use here that is implicitly called before each test run!

https://dictu-lang.com/docs/standard-lib/unittest/#setup

done

Jason2605 · 2024-11-15T20:20:56Z

God damn HTTP tests 😠

Jason2605 · 2024-11-15T20:23:12Z

It's getting merged. Good find with this! Thank you for the PR 😃

fix: rename and add entry for ffi

70d1083

Jason2605 reviewed Nov 14, 2024

View reviewed changes

liz3 added 2 commits November 14, 2024 02:35

fix: address comments

5415b63

add native handler

9d73540

liz3 added 4 commits November 14, 2024 02:54

this is not reversed

285e4d0

add tests for callback based ffi entries

7f3c839

fix: remove dummy function

9b16ab0

remove clangd file

b07a823

fix: add error handling for sythentic call frame

da8129f

fix: continue on synthetic frame

a6f331a

liz3 added 2 commits November 15, 2024 02:22

fix: remove unneeded changes

69e10a4

fix: remove useless change

4391dd4

Jason2605 reviewed Nov 15, 2024

View reviewed changes

use unit tests: setUp

1a6764a

Jason2605 merged commit ac22490 into dictu-lang:develop Nov 15, 2024
7 of 9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: call dictu from native code #755

feat: call dictu from native code #755

liz3 commented Nov 13, 2024 •

edited

Loading

liz3 commented Nov 13, 2024

Jason2605 left a comment

Jason2605 Nov 14, 2024

Jason2605 Nov 14, 2024

Jason2605 Nov 14, 2024

Jason2605 Nov 14, 2024

liz3 commented Nov 14, 2024

liz3 commented Nov 14, 2024

Jason2605 commented Nov 14, 2024

liz3 commented Nov 14, 2024

Jason2605 commented Nov 15, 2024

liz3 commented Nov 15, 2024 •

edited

Loading

liz3 commented Nov 15, 2024 •

edited

Loading

Jason2605 commented Nov 15, 2024

Jason2605 commented Nov 15, 2024

Jason2605 left a comment

Jason2605 Nov 15, 2024

Jason2605 Nov 15, 2024

Jason2605 Nov 15, 2024

liz3 commented Nov 15, 2024

Jason2605 commented Nov 15, 2024

Jason2605 commented Nov 15, 2024

feat: call dictu from native code #755

feat: call dictu from native code #755

Conversation

liz3 commented Nov 13, 2024 • edited Loading

feat: call dictu from native code

Type of Change:

Housekeeping:

Screenshots (If Applicable):

liz3 commented Nov 13, 2024

Jason2605 left a comment

Choose a reason for hiding this comment

Jason2605 Nov 14, 2024

Choose a reason for hiding this comment

Jason2605 Nov 14, 2024

Choose a reason for hiding this comment

Jason2605 Nov 14, 2024

Choose a reason for hiding this comment

Jason2605 Nov 14, 2024

Choose a reason for hiding this comment

liz3 commented Nov 14, 2024

liz3 commented Nov 14, 2024

Jason2605 commented Nov 14, 2024

liz3 commented Nov 14, 2024

Jason2605 commented Nov 15, 2024

liz3 commented Nov 15, 2024 • edited Loading

liz3 commented Nov 15, 2024 • edited Loading

Jason2605 commented Nov 15, 2024

Jason2605 commented Nov 15, 2024

Jason2605 left a comment

Choose a reason for hiding this comment

Jason2605 Nov 15, 2024

Choose a reason for hiding this comment

Jason2605 Nov 15, 2024

Choose a reason for hiding this comment

Jason2605 Nov 15, 2024

Choose a reason for hiding this comment

liz3 commented Nov 15, 2024

Jason2605 commented Nov 15, 2024

Jason2605 commented Nov 15, 2024

liz3 commented Nov 13, 2024 •

edited

Loading

liz3 commented Nov 15, 2024 •

edited

Loading

liz3 commented Nov 15, 2024 •

edited

Loading