From 84915debe0dbd782e499dfd471284744691b4196 Mon Sep 17 00:00:00 2001 From: Justin Israel Date: Mon, 5 Apr 2021 07:55:27 +1200 Subject: [PATCH] bind: WIP add checks for exception being set in wrapped python functions This commit starts to address the issue of a non-NULL PyObject* being returned when an exception has been set by the wrapped C function. Adds a checking mechanism for PyErr_Occurred. Results in exceptions being propagated to python caller, but may introduce memory leaks for retval not being cleaned up during error. Refs go-python/gopy #254 --- _examples/pyerrors/pyerrors.go | 20 +++++++++++++++++++- _examples/pyerrors/test.py | 14 +++++++++++++- bind/gen.go | 23 ++++++++++++++++++++++- bind/gen_func.go | 4 ++-- main_test.go | 12 +++++++----- 5 files changed, 63 insertions(+), 10 deletions(-) diff --git a/_examples/pyerrors/pyerrors.go b/_examples/pyerrors/pyerrors.go index 76a6dc3..208647d 100644 --- a/_examples/pyerrors/pyerrors.go +++ b/_examples/pyerrors/pyerrors.go @@ -5,7 +5,10 @@ // Package pyerrors holds functions returning an error. package pyerrors -import "errors" +import ( + "errors" + "fmt" +) // Div is a function for detecting errors. func Div(i, j int) (int, error) { @@ -14,3 +17,18 @@ func Div(i, j int) (int, error) { } return i / j, nil } + +type Stringer fmt.Stringer +type MyString string + +func (t MyString) String() string { return string(t) } + +// NewMyString converts a string to a custom MyString type. +// It is an error to pass an empty string value. +func NewMyString(val string) (stringer Stringer, err error) { + if val == "" { + err = errors.New("Empty string value.") + return + } + return MyString(val), nil +} diff --git a/_examples/pyerrors/test.py b/_examples/pyerrors/test.py index 2112332..c729b11 100644 --- a/_examples/pyerrors/test.py +++ b/_examples/pyerrors/test.py @@ -14,7 +14,19 @@ def div(a, b): except Exception as e: print(e) -div(5,0) + +def new_mystring(s): + try: + ms = pyerrors.NewMyString(s) + print('pyerrors.NewMyString("%s") = "%s"'% (s, ms.String())) + except Exception as e: + print(e) + + +div(5,0) # error div(5,2) +new_mystring("") # error +new_mystring("hello") + print("OK") diff --git a/bind/gen.go b/bind/gen.go index 4ebf186..1c819cb 100644 --- a/bind/gen.go +++ b/bind/gen.go @@ -203,9 +203,30 @@ func GoPyMainRun() { # File is generated by gopy. Do not edit. # %[2]s -from pybindgen import retval, param, Module +from pybindgen import retval, param, Function, Module import sys +class CheckedFunction(Function): + def __init__(self, *a, **kw): + super(CheckedFunction, self).__init__(*a, **kw) + self._failure_expression = kw.get('failure_expression', '') + + def set_failure_expression(self, expr): + self._failure_expression = expr + + def generate_call(self): + super(CheckedFunction, self).generate_call() + check = "PyErr_Occurred()" + if self._failure_expression: + check = "{} && {}".format(self._failure_expression, check) + self.before_call.write_error_check(check) + +def add_checked_function(mod, name, retval, params, failure_expression='', *a, **kw): + fn = CheckedFunction(name, retval, params, *a, **kw) + fn.set_failure_expression(failure_expression) + mod._add_function_obj(fn) + return fn + mod = Module('_%[1]s') mod.add_include('"%[1]s_go.h"') mod.add_function('GoPyInit', None, []) diff --git a/bind/gen_func.go b/bind/gen_func.go index 9f619e9..5642e47 100644 --- a/bind/gen_func.go +++ b/bind/gen_func.go @@ -156,14 +156,14 @@ func (g *pyGen) genFuncSig(sym *symbol, fsym *Func) bool { g.gofile.Printf("\n//export %s\n", mnm) g.gofile.Printf("func %s(", mnm) - g.pybuild.Printf("mod.add_function('%s', ", mnm) + g.pybuild.Printf("add_checked_function(mod, '%s', ", mnm) g.pywrap.Printf("def %s(", gname) default: g.gofile.Printf("\n//export %s\n", fsym.ID()) g.gofile.Printf("func %s(", fsym.ID()) - g.pybuild.Printf("mod.add_function('%s', ", fsym.ID()) + g.pybuild.Printf("add_checked_function(mod, '%s', ", fsym.ID()) g.pywrap.Printf("def %s(", gname) } diff --git a/main_test.go b/main_test.go index d3843e2..69bfaae 100644 --- a/main_test.go +++ b/main_test.go @@ -181,7 +181,7 @@ Add(int i, int j) int --- hi.LookupQuestion(42)... Life, the Universe and Everything --- hi.LookupQuestion(12)... -caught: returned a result with an error set +caught: Wrong answer: 12 != 42 --- doc(hi.Person): Person is a simple struct @@ -209,9 +209,9 @@ hi.Person{Name="foo", Age=42} --- p.Name: foo --- p.Work(2)... --- p.Work(24)... -caught: returned a result with an error set +caught: can't work for 24 hours! --- p.Salary(2): 20 ---- p.Salary(24): caught: returned a result with an error set +--- p.Salary(24): caught: can't work for 24 hours! --- Person.__init__ caught: argument 2 must be str, not int | err-type: caught: an integer is required (got type str) | err-type: @@ -545,11 +545,13 @@ func TestPyErrors(t *testing.T) { path := "_examples/pyerrors" testPkg(t, pkg{ path: path, - lang: features[path], // TODO: should print out the error message! + lang: features[path], cmd: "build", extras: nil, - want: []byte(` returned a result with an error set + want: []byte(`Divide by zero. pyerrors.Div(5, 2) = 2 +Empty string value. +pyerrors.NewMyString("hello") = "hello" OK `), })