Stop calling back into C to raise exceptions

MANIFEST.in

@@ -2,3 +2,4 @@ include go.mod
 include go.sum
 include starlark.c
 include starlark.go
+include starlark.h

src/pystarlark/__init__.py

@@ -11,9 +11,6 @@
 
 
 class Starlark(StarlarkGo):
-    def eval(self, statement: str, _raw: bool = False) -> Any:
+    def eval(self, statement: str) -> Any:
         response = super().eval(statement)
-        if _raw:
-            return response
-        value = json.loads(response)["value"]
-        return literal_eval(value)
+        return literal_eval(response)

starlark.c

@@ -1,45 +1,57 @@
 #define PY_SSIZE_T_CLEAN
+#include "starlark.h"
 #include <Python.h>
 
 /* This stuff is in the Go file */
 unsigned long NewThread();
 void DestroyThread(unsigned long threadId);
-char *Eval(unsigned long threadId, char *stmt);
-int ExecFile(unsigned long threadId, char *data);
-void FreeCString(char *s);
+StarlarkReturn *Eval(unsigned long threadId, char *stmt);
+StarlarkReturn *ExecFile(unsigned long threadId, char *data);
+void FreeStarlarkReturn(StarlarkReturn *retval);
 
 /* Custom exceptions */
 static PyObject *StarlarkError = NULL;
 static PyObject *SyntaxError = NULL;
 static PyObject *EvalError = NULL;
 
 /* Helpers to raise custom exceptions from Go */
-void Raise_StarlarkError(const char *error, const char *error_type) {
-  PyGILState_STATE gilstate = PyGILState_Ensure();
-  PyObject *exc_args = Py_BuildValue("ss", error, error_type);
-  PyErr_SetObject(StarlarkError, exc_args);
-  Py_DECREF(exc_args);
-  PyGILState_Release(gilstate);
+static PyObject *PyStarlarkErrorArgs(StarlarkErrorArgs *args) {
+  return Py_BuildValue("ss", args->error, args->error_type);
 }
 
-void Raise_SyntaxError(const char *error, const char *error_type,
-                       const char *msg, const char *filename, const long line,
-                       const long column) {
-  PyGILState_STATE gilstate = PyGILState_Ensure();
-  PyObject *exc_args =
-      Py_BuildValue("ssssll", error, error_type, msg, filename, line, column);
-  PyErr_SetObject(SyntaxError, exc_args);
-  Py_DECREF(exc_args);
-  PyGILState_Release(gilstate);
+static PyObject *PySyntaxErrorArgs(SyntaxErrorArgs *args) {
+  return Py_BuildValue("ssssll", args->error, args->error_type, args->msg,
+                       args->filename, args->line, args->column);
 }
 
-void Raise_EvalError(const char *error, const char *error_type,
-                     const char *backtrace) {
-  PyGILState_STATE gilstate = PyGILState_Ensure();
-  PyObject *exc_args = Py_BuildValue("sss", error, error_type, backtrace);
-  PyErr_SetObject(EvalError, exc_args);
+static PyObject *PyEvalErrorArgs(EvalErrorArgs *args) {
+  return Py_BuildValue("sss", args->error, args->error_type, args->backtrace);
+}
+
+static void HandleStarlarkError(StarlarkReturn *retval) {
+  PyObject *exc_args = NULL;
+  PyObject *exc_type = NULL;
+
+  switch (retval->error_type) {
+  case STARLARK_GENERAL_ERROR:
+    exc_type = StarlarkError;
+    exc_args = PyStarlarkErrorArgs((StarlarkErrorArgs *)retval->error);
+    break;
+  case STARLARK_SYNTAX_ERROR:
+    exc_type = SyntaxError;
+    exc_args = PySyntaxErrorArgs((SyntaxErrorArgs *)retval->error);
+    break;
+  case STARLARK_EVAL_ERROR:
+    exc_type = EvalError;
+    exc_args = PyEvalErrorArgs((EvalErrorArgs *)retval->error);
+    break;
+  default:
+    exc_type = PyExc_RuntimeError;
+    exc_args = PyUnicode_FromString("Unknown StarlarkReturn->error_type");
+  }
+
+  PyErr_SetObject(exc_type, exc_args);
   Py_DECREF(exc_args);
-  PyGILState_Release(gilstate);
 }
 
 /* Starlark object */
@@ -65,8 +77,8 @@ static void StarlarkGo_dealloc(StarlarkGo *self) {
 static PyObject *StarlarkGo_eval(StarlarkGo *self, PyObject *args) {
   PyObject *obj;
   PyObject *stmt;
-  char *cvalue;
-  PyObject *value;
+  StarlarkReturn *retval;
+  PyObject *value = NULL;
 
   if (PyArg_ParseTuple(args, "U", &obj) == 0)
     return NULL;
@@ -75,15 +87,17 @@ static PyObject *StarlarkGo_eval(StarlarkGo *self, PyObject *args) {
   if (stmt == NULL)
     return NULL;
 
-  cvalue = Eval(self->starlark_thread, PyBytes_AsString(stmt));
+  retval = Eval(self->starlark_thread, PyBytes_AsString(stmt));
 
-  if (cvalue == NULL) {
-    value = NULL;
+  if (retval->error != NULL) {
+    HandleStarlarkError(retval);
+  } else if (retval->value == NULL) {
+    PyErr_SetString(PyExc_RuntimeError, "Starlark value is NULL");
   } else {
-    value = PyUnicode_FromString(cvalue);
-    FreeCString(cvalue);
+    value = PyUnicode_FromString(retval->value);
   }
 
+  FreeStarlarkReturn(retval);
   Py_DecRef(stmt);
 
   return value;
@@ -92,7 +106,8 @@ static PyObject *StarlarkGo_eval(StarlarkGo *self, PyObject *args) {
 static PyObject *StarlarkGo_exec(StarlarkGo *self, PyObject *args) {
   PyObject *obj;
   PyObject *data;
-  int rc;
+  StarlarkReturn *retval;
+  int ok = 0;
 
   if (PyArg_ParseTuple(args, "U", &obj) == 0)
     return NULL;
@@ -101,10 +116,17 @@ static PyObject *StarlarkGo_exec(StarlarkGo *self, PyObject *args) {
   if (data == NULL)
     return NULL;
 
-  rc = ExecFile(self->starlark_thread, PyBytes_AsString(data));
+  retval = ExecFile(self->starlark_thread, PyBytes_AsString(data));
+
+  if (retval->error != NULL) {
+    HandleStarlarkError(retval);
+  } else {
+    ok = 1;
+  }
+
   Py_DecRef(data);
 
-  if (!rc)
+  if (!ok)
     return NULL;
 
   Py_RETURN_NONE;

starlark.go

@@ -2,14 +2,11 @@ package main
 
 /*
 #include <stdlib.h>
-void Raise_StarlarkError(const char *error, const char *error_type);
-void Raise_SyntaxError(const char *error, const char *error_type, const char *msg, const char *filename, const long line, const long column);
-void Raise_EvalError(const char *error, const char *error_type, const char *backtrace);
+#include <starlark.h>
 */
 import "C"
 
 import (
-	"encoding/json"
 	"fmt"
 	"math/rand"
 	"reflect"
@@ -38,60 +35,104 @@ func DestroyThread(threadId C.ulong) {
 	delete(GLOBALS, goThreadId)
 }
 
-func raiseStarlarkError(err error) {
-	error_str := C.CString(err.Error())
-	error_type := C.CString(fmt.Sprintf("%s", reflect.TypeOf(err)))
+func makeStarlarkErrorArgs(err error) *C.StarlarkErrorArgs {
+	args := (*C.StarlarkErrorArgs)(C.malloc(C.sizeof_StarlarkErrorArgs))
+	args.error = C.CString(err.Error())
+	args.error_type = C.CString(fmt.Sprintf("%s", reflect.TypeOf(err)))
 
-	C.Raise_StarlarkError(error_str, error_type)
-
-	FreeCString(error_type)
-	FreeCString(error_str)
+	return args
 }
 
-func raiseSyntaxError(err *syntax.Error) {
-	error_str := C.CString(err.Error())
-	error_type := C.CString(fmt.Sprintf("%s", reflect.TypeOf(err)))
-	msg := C.CString(err.Msg)
-	filename := C.CString(err.Pos.Filename())
+func makeSyntaxErrorArgs(err *syntax.Error) *C.SyntaxErrorArgs {
+	args := (*C.SyntaxErrorArgs)(C.malloc(C.sizeof_SyntaxErrorArgs))
+	args.error = C.CString(err.Error())
+	args.error_type = C.CString(fmt.Sprintf("%s", reflect.TypeOf(err)))
+	args.msg = C.CString(err.Msg)
+	args.filename = C.CString(err.Pos.Filename())
+	args.line = C.uint(err.Pos.Line)
+	args.column = C.uint(err.Pos.Col)
+
+	return args
+}
 
-	C.Raise_SyntaxError(error_str, error_type, msg, filename, C.long(err.Pos.Line), C.long(err.Pos.Col))
+func makeEvalErrorArgs(err *starlark.EvalError) *C.EvalErrorArgs {
+	args := (*C.EvalErrorArgs)(C.malloc(C.sizeof_EvalErrorArgs))
+	args.error = C.CString(err.Error())
+	args.error_type = C.CString(fmt.Sprintf("%s", reflect.TypeOf(err)))
+	args.backtrace = C.CString(err.Backtrace())
 
-	FreeCString(filename)
-	FreeCString(msg)
-	FreeCString(error_type)
-	FreeCString(error_str)
+	return args
 }
 
-func raiseEvalError(err *starlark.EvalError) {
-	error_str := C.CString(err.Error())
-	error_type := C.CString(fmt.Sprintf("%s", reflect.TypeOf(err)))
-	backtrace := C.CString(err.Backtrace())
+func makeStarlarkReturn(err error) *C.StarlarkReturn {
+	retval := (*C.StarlarkReturn)(C.malloc(C.sizeof_StarlarkReturn))
+	retval.value = nil
 
-	C.Raise_EvalError(error_str, error_type, backtrace)
+	if err != nil {
+		syntaxErr, ok := err.(syntax.Error)
+		if ok {
+			retval.error_type = C.STARLARK_SYNTAX_ERROR
+			retval.error = unsafe.Pointer(makeSyntaxErrorArgs(&syntaxErr))
+			return retval
+		}
+
+		evalErr, ok := err.(*starlark.EvalError)
+		if ok {
+			retval.error_type = C.STARLARK_EVAL_ERROR
+			retval.error = unsafe.Pointer(makeEvalErrorArgs(evalErr))
+			return retval
+		}
+
+		retval.error_type = C.STARLARK_GENERAL_ERROR
+		retval.error = unsafe.Pointer(makeStarlarkErrorArgs(err))
+		return retval
+	}
 
-	FreeCString(backtrace)
-	FreeCString(error_type)
-	FreeCString(error_str)
+	retval.error_type = C.STARLARK_NO_ERROR
+	retval.error = nil
+
+	return retval
 }
 
-func handleError(err error) {
-	syntaxErr, ok := err.(syntax.Error)
-	if ok {
-		raiseSyntaxError(&syntaxErr)
-		return
+//export FreeStarlarkReturn
+func FreeStarlarkReturn(retval *C.StarlarkReturn) {
+	switch retval.error_type {
+	case C.STARLARK_GENERAL_ERROR:
+		args := (*C.StarlarkErrorArgs)(retval.error)
+		C.free(unsafe.Pointer(args.error))
+		C.free(unsafe.Pointer(args.error_type))
+	case C.STARLARK_SYNTAX_ERROR:
+		args := (*C.SyntaxErrorArgs)(retval.error)
+		C.free(unsafe.Pointer(args.error))
+		C.free(unsafe.Pointer(args.error_type))
+		C.free(unsafe.Pointer(args.msg))
+		C.free(unsafe.Pointer(args.filename))
+	case C.STARLARK_EVAL_ERROR:
+		args := (*C.EvalErrorArgs)(retval.error)
+		C.free(unsafe.Pointer(args.error))
+		C.free(unsafe.Pointer(args.error_type))
+		C.free(unsafe.Pointer(args.backtrace))
+	case C.STARLARK_NO_ERROR:
+		if retval.error != nil {
+			panic("STARLARK_NO_ERROR but error is not nil")
+		}
+	default:
+		panic("unknown error_type")
+	}
+
+	if retval.value != nil {
+		C.free(unsafe.Pointer(retval.value))
 	}
 
-	evalErr, ok := err.(*starlark.EvalError)
-	if ok {
-		raiseEvalError(evalErr)
-		return
+	if retval.error != nil {
+		C.free(unsafe.Pointer(retval.error))
 	}
 
-	raiseStarlarkError(err)
+	C.free(unsafe.Pointer(retval))
 }
 
 //export Eval
-func Eval(threadId C.ulong, stmt *C.char) *C.char {
+func Eval(threadId C.ulong, stmt *C.char) *C.StarlarkReturn {
 	// Cast *char to string
 	goStmt := C.GoString(stmt)
 	goThreadId := uint64(threadId)
@@ -100,57 +141,30 @@ func Eval(threadId C.ulong, stmt *C.char) *C.char {
 	globals := GLOBALS[goThreadId]
 
 	result, err := starlark.Eval(thread, "<expr>", goStmt, globals)
-	if err != nil {
-		handleError(err)
-		return nil
-	}
+	retval := makeStarlarkReturn(err)
 
-	// Convert starlark.Value struct into a JSON blob
-	rawResponse := make(map[string]string)
-	rawResponse["value"] = result.String()
-	rawResponse["type"] = result.Type()
-	response, _ := json.Marshal(rawResponse)
+	if err == nil {
+		retval.value = C.CString(result.String())
+	}
 
-	// Convert JSON blob to string and then CString
-	return C.CString(string(response))
+	return retval
 }
 
 //export ExecFile
-func ExecFile(threadId C.ulong, data *C.char) C.int {
+func ExecFile(threadId C.ulong, data *C.char) *C.StarlarkReturn {
 	// Cast *char to string
 	goData := C.GoString(data)
 	goThreadId := uint64(threadId)
 
 	thread := THREADS[goThreadId]
 	globals, err := starlark.ExecFile(thread, "main.star", goData, starlark.StringDict{})
-	if err != nil {
-		handleError(err)
-		return C.int(0)
+	retval := makeStarlarkReturn(err)
+
+	if err == nil {
+		GLOBALS[goThreadId] = globals
 	}
-	GLOBALS[goThreadId] = globals
-	return C.int(1)
-}
 
-//export FreeCString
-func FreeCString(s *C.char) {
-	C.free(unsafe.Pointer(s))
+	return retval
 }
 
 func main() {}
-
-/*
-func main() {
-	const data = `
-def fibonacci(n=10):
-	res = list(range(n))
-	for i in res[2:]:
-		res[i] = res[i-2] + res[i-1]
-	return res
-`
-	threadId := NewThread()
-	ExecFile(threadId, C.CString(data))
-	r := Eval(threadId, C.CString("fibonacci(25)"))
-	fmt.Printf("%v\n", C.GoString(r))
-	DestroyThread(threadId)
-}
-*/