fix error handler

c_bindings/pocketpy_c.cpp

@@ -1,3 +1,4 @@
+#include "error.h"
 #include "pocketpy.h"
 #include "pocketpy_c.h"
 
@@ -5,33 +6,17 @@ using namespace pkpy;
 
 #define PKPY_STACK_SIZE 32
 
-#define SAFEGUARD_OPEN try { \
+#define ERRHANDLER_OPEN \
-
-#define SAFEGUARD_CLOSE \
-    } catch(std::exception& e) { \
-        std::cerr << "ERROR: a std::exception " \
-        << "this probably means pocketpy itself has a bug!\n" \
-        << e.what() << "\n"; \
-        exit(2); \
-    } catch(...) { \
-        std::cerr << "ERROR: a unknown exception was thrown from " << __func__ \
-        << "\nthis probably means pocketpy itself has a bug!\n"; \
-        exit(2); \
-    }
-
-
-#define ERRHANDLER_OPEN SAFEGUARD_OPEN \
-    try { \
     if (vm->c_data->size() > 0 && vm->c_data->top() == nullptr) \
         return false; \
+    try {
 
 #define ERRHANDLER_CLOSE \
-    } catch( Exception e ) { \
+    } catch(Exception& e ) { \
         vm->c_data->push(py_var(vm, e)); \
         vm->c_data->push(NULL); \
         return false; \
-    } \
+    }
-    SAFEGUARD_CLOSE \
 
 
 
@@ -47,6 +32,23 @@ class CVM : public VM {
         c_data->clear();
         delete c_data;
     }
+
+    struct TempStack{
+        CVM* cvm;
+        ValueStackImpl<PKPY_STACK_SIZE>* prev;
+        TempStack(CVM* cvm, ValueStackImpl<PKPY_STACK_SIZE>* new_data) : cvm(cvm) {
+            prev = cvm->c_data;
+            cvm->c_data = new_data;
+        }
+
+        ~TempStack() { restore(); }
+
+        void restore(){
+            if(prev == nullptr) return;
+            cvm->c_data = prev;
+            prev = nullptr;
+        }
+    };
 };
 
 
@@ -76,8 +78,6 @@ static void unpack_return(CVM* vm, PyObject* ret) {
 
 bool pkpy_clear_error(pkpy_vm* vm_handle, char** message) {
     CVM* vm = (CVM*) vm_handle;
-    SAFEGUARD_OPEN
-
     if (vm->c_data->size() == 0 || vm->c_data->top() != nullptr) 
         return false;
 
@@ -92,8 +92,6 @@ bool pkpy_clear_error(pkpy_vm* vm_handle, char** message) {
     vm->callstack.clear();
     vm->s_data.clear(); 
     return true;
-
-    SAFEGUARD_CLOSE
 }
 
 void gc_marker_ex(CVM* vm) {
@@ -149,27 +147,6 @@ void pkpy_vm_destroy(pkpy_vm* vm_handle) {
     delete vm;
 }
 
-static void propagate_if_errored(CVM* vm, ValueStackImpl<PKPY_STACK_SIZE>* stored_stack) {
-    try {
-        if (vm->c_data->size() == 0 || vm->c_data->top() != nullptr) 
-            return;
-
-        vm->c_data->pop();
-        Exception& e = py_cast<Exception&>(vm, vm->c_data->top());
-        vm->c_data->pop();
-
-        vm->c_data = stored_stack;
-
-        throw e;
-    } catch(Exception& e) {
-        throw;
-    } catch(...) {
-        std::cerr << "ERROR: a non pocketpy exeception was thrown " 
-            << "this probably means pocketpy itself has a bug!\n"; 
-        exit(2); 
-    }
-}
-
 PyObject* c_function_wrapper(VM* vm, ArgsView args) {
     LuaStyleFuncC f;
     if(args[-1] != PY_NULL){
@@ -185,13 +162,19 @@ PyObject* c_function_wrapper(VM* vm, ArgsView args) {
     for (int i = 0; i < args.size(); i++)
         local_stack.push(args[i]);
     
-    ValueStackImpl<PKPY_STACK_SIZE>* stored_stack = cvm->c_data;
+    // tmp is controlled by RAII
-    cvm->c_data = &local_stack;
+    auto tmp = CVM::TempStack(cvm, &local_stack);
-
     int retc = f(cvm);
 
-    propagate_if_errored(cvm, stored_stack);
+    // propagate_if_errored
-    cvm->c_data = stored_stack;
+    if (!cvm->c_data->empty() && cvm->c_data->top() == nullptr){
+        cvm->c_data->pop();     // pop nullptr
+        Exception& e = _py_cast<Exception&>(vm, cvm->c_data->popx());
+        tmp.restore();
+        // throw e;
+        vm->_error(e);
+    }
+    tmp.restore();
 
     PyObject* ret = cvm->None;
 
@@ -315,7 +298,7 @@ bool pkpy_get_global(pkpy_vm* vm_handle, const char* name) {
     if (o == nullptr) {
         o = vm->builtins->attr().try_get(name);
         if (o == nullptr)
-            throw Exception("NameError", "could not find requested global");
+            throw Exception("NameError", name);
     }
 
     vm->c_data->push(o);
@@ -535,7 +518,6 @@ bool pkpy_is_none(pkpy_vm* vm_handle, int index) {
 
 bool pkpy_check_global(pkpy_vm* vm_handle, const char* name) {
     CVM* vm = (CVM*) vm_handle;
-    SAFEGUARD_OPEN
     PyObject* o = vm->_main->attr().try_get(name);
     if (o == nullptr) {
         o = vm->builtins->attr().try_get(name);
@@ -543,17 +525,13 @@ bool pkpy_check_global(pkpy_vm* vm_handle, const char* name) {
             return false;
     }
     return true;
-
-    SAFEGUARD_CLOSE
 }
 
 bool pkpy_check_error(pkpy_vm* vm_handle) {
     CVM* vm = (CVM*) vm_handle;
-    SAFEGUARD_OPEN
     if (vm->c_data->size() > 0 && vm->c_data->top() == nullptr) 
         return true; 
     return false;
-    SAFEGUARD_CLOSE
 }
 
 
@@ -582,11 +560,15 @@ bool pkpy_push(pkpy_vm* vm_handle, int index) {
 }
 
 
-bool pkpy_error(pkpy_vm* vm_handle, const char* message) {
+bool pkpy_error(pkpy_vm* vm_handle, const char* name, const char* message) {
     CVM* vm = (CVM*) vm_handle;
-    ERRHANDLER_OPEN
+    // already in error state
-    throw Exception("CBindingError", message);
+    if (vm->c_data->size() > 0 && vm->c_data->top() == nullptr) {
-    ERRHANDLER_CLOSE
+        return false;
+    }
+    vm->c_data->push(py_var(vm, Exception(name, message)));
+    vm->c_data->push(nullptr);
+    return false;
 }
 
 bool pkpy_getattr(pkpy_vm* vm_handle, const char* name) {

c_bindings/pocketpy_c.h

@@ -27,7 +27,7 @@ PK_EXPORT bool pkpy_clear_error(pkpy_vm*, char** message);
 //when queried
 //note that at the moment this is more like a panic than throwing an error
 //the user will not be able to catch it with python code
-PK_EXPORT bool pkpy_error(pkpy_vm*, const char* message);
+PK_EXPORT bool pkpy_error(pkpy_vm*, const char* name, const char* message);
 
 PK_EXPORT pkpy_vm* pkpy_vm_create(bool use_stdio, bool enable_os);
 PK_EXPORT bool pkpy_vm_run(pkpy_vm*, const char* source);

c_bindings/test.c

@@ -26,7 +26,7 @@ void fail_impl(pkpy_vm* vm, bool result, int lineno) {
     } else {
         char* message;
         if (pkpy_clear_error(vm, &message)) {
-            printf("actually errored!\n");
+            printf("actually errored! line %i\n", lineno);
             free(message);
             exit(1);
         }
@@ -83,14 +83,9 @@ int test_nested_error(pkpy_vm* vm) {
 
 pkpy_vm* vm;
 
-void cleanup(void) {
-    pkpy_vm_destroy(vm);
-}
-
 int main(int argc, char** argv) {
 
     vm = pkpy_vm_create(true, true);
-    atexit(cleanup);
 
     //test run
     check(pkpy_vm_run(vm, "print('hello world!')"));
@@ -308,16 +303,16 @@ int main(int argc, char** argv) {
 
     pkpy_vm_run(vm, "test_error_propagate()");
     check(pkpy_check_error(vm));
-    fprintf(stderr, "testing code going to standard error, can ignore next error\n");
+    // testing code going to standard error, can ignore next error
     pkpy_clear_error(vm, NULL);
 
     //with the current way execptions are handled, this will fail and pass the
     //error clean through, ignoring the python handling
     //
     //maybe worth fixing someday, but for now it is functionating as implemented
-    error(pkpy_vm_run(vm, "try : test_error_propagate(); except NameError : pass"));
+    check(pkpy_vm_run(vm, "try : test_error_propagate(); except NameError : pass"));
 
-    error(pkpy_error(vm, "test direct error mechanism"));
+    error(pkpy_error(vm, "_", "test direct error mechanism"));
 
 
     //more complicated error handling
@@ -334,10 +329,10 @@ int main(int argc, char** argv) {
     //at such a time this interferes with a real world use case of the bindings
     //we can revisit it
     //
-    //check(pkpy_vm_run(vm, "def error_from_python() : raise NotImplementedError()"));
+    check(pkpy_vm_run(vm, "def error_from_python() : raise NotImplementedError()"));
-    //check(pkpy_push_function(vm, test_nested_error));
+    check(pkpy_push_function(vm, test_nested_error));
-    //check(pkpy_set_global(vm, "test_nested_error"));
+    check(pkpy_set_global(vm, "test_nested_error"));
-    //fail(pkpy_vm_run(vm, "test_nested_error()"));
+    error(pkpy_vm_run(vm, "test_nested_error()"));
 
     check(pkpy_vm_run(vm, "import math"));
     check(pkpy_get_global(vm, "math"));
@@ -356,5 +351,7 @@ int main(int argc, char** argv) {
     check(pkpy_eval(vm, "math"));
     check(pkpy_setattr(vm, "pi"));
     check(pkpy_vm_run(vm, "print(math.pi)"));
+
+    pkpy_vm_destroy(vm);
     return 0;
 }

c_bindings/test_answers.txt

@@ -1,7 +1,7 @@
 hello world!
 successfully errored with this message: 
 Traceback (most recent call last):
-NameError: could not find requested global
+NameError: nonexistatn
 
 testing int methods
 11
@@ -52,7 +52,9 @@ testing pushing functions
 12
 successfully errored with this message: 
 Traceback (most recent call last):
-NameError: could not find requested global
+  File "<c-bound>", line 1
+    test_error_propagate()
+NameError: does not exist
 successfully errored with this message: 
 Traceback (most recent call last):
   File "<c-bound>", line 1
@@ -60,10 +62,14 @@ Traceback (most recent call last):
 NameError: testing error throwing from python
 successfully errored with this message: 
 Traceback (most recent call last):
-NameError: could not find requested global
+_: test direct error mechanism
 successfully errored with this message: 
 Traceback (most recent call last):
-CBindingError: test direct error mechanism
+  File "<c-bound>", line 1
+    test_nested_error()
+  File "<c-bound>", line 1
+    def error_from_python() : raise NotImplementedError()
+NotImplementedError
 pi: 3.14
 pi: 3.14
 2

run_c_binding_test.sh

@@ -1,10 +1,10 @@
 python3 preprocess.py
 
 echo "compiling c++ lib"
-clang++ -c -o pocketpy_c.o c_bindings/pocketpy_c.cpp -Wfatal-errors --std=c++17 -O2 -Wall -Wno-sign-compare -Wno-unused-variable -fno-rtti -stdlib=libc++ -I src/ -g
+clang++ -c -o pocketpy_c.o c_bindings/pocketpy_c.cpp -Wfatal-errors -O1 --std=c++17 -Wall -Wno-sign-compare -Wno-unused-variable -fno-rtti -stdlib=libc++ -I src/ -g
 
 echo "compiling c executable" 
-clang -c -o test.o c_bindings/test.c -Wfatal-errors -O2 -Wall -Wno-sign-compare -Wno-unused-variable -I src/ -g
+clang -c -o test.o c_bindings/test.c -Wfatal-errors -Wall -O1 -Wno-sign-compare -Wno-unused-variable -I src/ -g
 echo "linking"
 clang++ -o c_binding_test test.o pocketpy_c.o -stdlib=libc++ -g
 ./c_binding_test > binding_test_scratch
@@ -13,6 +13,8 @@ diff -q -s  binding_test_scratch c_bindings/test_answers.txt
 if [ $? -eq 1 ]
 then
     echo "ERROR: c binding test failed"
+    rm pocketpy_c.o
+    rm test.o
     exit 1
 fi