Skip to content

Commit d703fa8

Browse files
b-longb-long
committed
Unskip TestHi for pybind11 and nanobind backends
1 parent cfffb88 commit d703fa8

6 files changed

Lines changed: 232 additions & 31 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -185,17 +185,12 @@ jobs:
185185
- name: Build
186186
run: go build -v ./...
187187

188-
# The skipped test is pybind11's own auto-generated TypeError wording,
189-
# which differs from pybindgen's for one case in TestHi (Person's
190-
# setters given the wrong argument type) -- a message-text difference,
191-
# not a functional one.
192-
#
193188
# -timeout raises go test's own default (10m): each test here compiles
194189
# twice (cgo, then a separate C++ step), against cffi's one, and once
195190
# measured at ~18.5s/test average on windows-latest, 32 tests alone used
196191
# 590s of the default budget.
197192
- name: Test
198-
run: go test -v -timeout=30m -skip '^TestHi$' ./...
193+
run: go test -v -timeout=30m ./...
199194

200195
# Builds and tests the opt-in nanobind backend (GOPY_BACKEND=nanobind).
201196
# Same shape as the pybind11 job above (the two share a cgo shim and a
@@ -240,11 +235,9 @@ jobs:
240235
- name: Build
241236
run: go build -v ./...
242237

243-
# Skipped for the same reason as in the pybind11 job: nanobind words its
244-
# own auto-generated TypeError the same way pybind11 does, not the way
245-
# pybindgen does.
238+
# -timeout: see the pybind11 job
246239
- name: Test
247-
run: go test -v -timeout=30m -skip '^TestHi$' ./...
240+
run: go test -v -timeout=30m ./...
248241

249242
# Compares per-call overhead across backends (see _examples/bench/run.sh):
250243
# not a pass/fail check, just a table uploaded as a build artifact. Runs

‎bind/cxx_args.inc‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
// Argument conversion shared by the pybind11 and nanobind backends, spliced
2+
// into each one's generated .cpp in place of @ARG_HELPERS@ (see
3+
// cxxArgHelpers in pybind11.go). Each binding takes its arguments as raw
4+
// python objects and converts them here, the same way pybindgen's generated
5+
// code does (PyArg_ParseTuple, with the same format code per C type, plus
6+
// pybindgen's own range checks), so that a wrongly-typed argument raises the
7+
// same exception, with the same message, under every backend -- rather than
8+
// pybind11's or nanobind's own "incompatible function arguments" TypeError.
9+
// argn is the argument's 1-based position, as PyArg_ParseTuple counts it.
10+
//
11+
// gopy_raise must be defined first: it throws the C++ exception that hands
12+
// the python error already set back to python (pybind11 and nanobind each
13+
// spell that differently). Note that this text is spliced into a python
14+
// string literal in build.py, so it must not contain any backslashes.
15+
16+
// the type name PyArg_ParseTuple reports for o
17+
static const char* gopy_arg_type(PyObject* o) {
18+
return o == Py_None ? "None" : Py_TYPE(o)->tp_name;
19+
}
20+
21+
static void gopy_arg_error(PyObject* exc, const char* msg) {
22+
PyErr_SetString(exc, msg);
23+
gopy_raise();
24+
}
25+
26+
// PyArg_ParseTuple's "s"
27+
static const char* gopy_arg_str(PyObject* o, int argn) {
28+
if (!PyUnicode_Check(o)) {
29+
PyErr_Format(PyExc_TypeError, "argument %d must be str, not %.50s", argn, gopy_arg_type(o));
30+
gopy_raise();
31+
}
32+
Py_ssize_t n;
33+
const char* s = PyUnicode_AsUTF8AndSize(o, &n);
34+
if (!s) {
35+
gopy_raise();
36+
}
37+
if ((Py_ssize_t)strlen(s) != n) {
38+
gopy_arg_error(PyExc_ValueError, "embedded null character");
39+
}
40+
return s; // borrowed from o, which outlives the call it is passed to
41+
}
42+
43+
// PyArg_ParseTuple's "L"
44+
static long long gopy_arg_L(PyObject* o) {
45+
long long v = PyLong_AsLongLong(o);
46+
if (v == -1 && PyErr_Occurred()) {
47+
gopy_raise();
48+
}
49+
return v;
50+
}
51+
52+
// PyArg_ParseTuple's "K"
53+
static unsigned long long gopy_arg_K(PyObject* o, int argn) {
54+
if (!PyLong_Check(o)) {
55+
PyErr_Format(PyExc_TypeError, "argument %d must be int, not %.50s", argn, gopy_arg_type(o));
56+
gopy_raise();
57+
}
58+
return PyLong_AsUnsignedLongLongMask(o);
59+
}
60+
61+
// PyArg_ParseTuple's "i"
62+
static int gopy_arg_i(PyObject* o) {
63+
long v = PyLong_AsLong(o);
64+
if (v == -1 && PyErr_Occurred()) {
65+
gopy_raise();
66+
}
67+
if (v > INT_MAX) {
68+
gopy_arg_error(PyExc_OverflowError, "signed integer is greater than maximum");
69+
}
70+
if (v < INT_MIN) {
71+
gopy_arg_error(PyExc_OverflowError, "signed integer is less than minimum");
72+
}
73+
return (int)v;
74+
}
75+
76+
// PyArg_ParseTuple's "i", then pybindgen's own check, for the C types it
77+
// parses that way despite being narrower than int (it checks only the upper
78+
// bound).
79+
static int gopy_arg_i_max(PyObject* o, int max) {
80+
int v = gopy_arg_i(o);
81+
if (v > max) {
82+
gopy_arg_error(PyExc_ValueError, "Out of range");
83+
}
84+
return v;
85+
}
86+
87+
// PyArg_ParseTuple's "I"
88+
static unsigned int gopy_arg_I(PyObject* o) {
89+
unsigned long v = PyLong_AsUnsignedLongMask(o);
90+
if (v == (unsigned long)-1 && PyErr_Occurred()) {
91+
gopy_raise();
92+
}
93+
return (unsigned int)v;
94+
}
95+
96+
// PyArg_ParseTuple's "d" (and, narrowed, "f")
97+
static double gopy_arg_d(PyObject* o) {
98+
double v = PyFloat_AsDouble(o);
99+
if (v == -1.0 && PyErr_Occurred()) {
100+
gopy_raise();
101+
}
102+
return v;
103+
}
104+
105+
// pybindgen's bool: PyArg_ParseTuple's "O", then PyObject_IsTrue
106+
static char gopy_arg_bool(PyObject* o) {
107+
int v = PyObject_IsTrue(o);
108+
if (v < 0) {
109+
gopy_raise();
110+
}
111+
return (char)v;
112+
}
113+
114+
// as the cffi backend checks a callback argument (pybindgen can't take one)
115+
static void gopy_arg_callable(PyObject* o, int argn) {
116+
if (!PyCallable_Check(o)) {
117+
PyErr_Format(PyExc_TypeError, "argument %d must be callable, not %.50s", argn, Py_TYPE(o)->tp_name);
118+
gopy_raise();
119+
}
120+
}

‎bind/nanobind.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,6 @@ func (g *pyGen) nanobindBuildPreamble() string {
2525
"@NAME@", g.cfg.Name,
2626
"@CMD@", g.cfg.Cmd,
2727
"@VERSION@", g.cfg.Version,
28+
"@ARG_HELPERS@", cxxArgHelpers,
2829
).Replace(nanobindBuildPy)
2930
}

‎bind/nanobind_build.py‎

Lines changed: 55 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,30 @@ def add_checked_function(mod, name, retval, params, failure_expression="", *a, *
5959
add_checked_string_function = add_checked_function
6060

6161

62+
# How wrapper converts an argument of each C type: as pybindgen's generated
63+
# code would, for the same exception on a wrongly-typed argument under every
64+
# backend (see the gopy_arg_* helpers in cxx_args.inc). %(o)s is the
65+
# argument's PyObject*, %(n)d its 1-based position. Identical in
66+
# pybind11_build.py and nanobind_build.py.
67+
ARG_CONV = {
68+
"int64_t": "gopy_arg_L(%(o)s)",
69+
"long long": "gopy_arg_L(%(o)s)",
70+
"uint64_t": "gopy_arg_K(%(o)s, %(n)d)",
71+
"int": "gopy_arg_i(%(o)s)",
72+
"int32_t": "gopy_arg_i(%(o)s)",
73+
"uint32_t": "gopy_arg_I(%(o)s)",
74+
"unsigned int": "gopy_arg_I(%(o)s)",
75+
"int16_t": "(int16_t)gopy_arg_i_max(%(o)s, 0x7fff)",
76+
"uint16_t": "(uint16_t)gopy_arg_i_max(%(o)s, 0xffff)",
77+
"int8_t": "(int8_t)gopy_arg_i_max(%(o)s, 0x7f)",
78+
"uint8_t": "(uint8_t)gopy_arg_i_max(%(o)s, 0xff)",
79+
"double": "gopy_arg_d(%(o)s)",
80+
"float": "(float)gopy_arg_d(%(o)s)",
81+
"bool": "gopy_arg_bool(%(o)s)",
82+
"char*": "const_cast<char*>(gopy_arg_str(%(o)s, %(n)d))",
83+
}
84+
85+
6286
def wrapper(name, ret, params, callback_kinds):
6387
"""Returns the m.def(...) call binding name, or "" if its signature
6488
isn't supported yet (a raw PyObject*): the .cpp simply never binds it, so
@@ -73,23 +97,25 @@ def wrapper(name, ret, params, callback_kinds):
7397
args = []
7498
setup = []
7599
call_args = []
76-
for ctype, pname in params:
77-
if ctype == "char*":
78-
args.append("const std::string& " + pname)
79-
call_args.append("const_cast<char*>(%s.c_str())" % pname)
80-
elif ctype == "bool":
81-
args.append("bool " + pname)
82-
call_args.append("(char)(%s ? 1 : 0)" % pname)
100+
for i, (ctype, pname) in enumerate(params):
101+
if ctype in ARG_CONV:
102+
# converted up front, in order, while the GIL is still held
103+
args.append("nb::handle " + pname)
104+
setup.append(
105+
"auto _a_%s = %s;" % (pname, ARG_CONV[ctype] % {"o": pname + ".ptr()", "n": i + 1})
106+
)
107+
call_args.append("_a_" + pname)
83108
elif ctype in ("complex64", "complex128"):
84109
cxxfloat = "float" if ctype == "complex64" else "double"
85110
args.append("std::complex<%s> %s" % (cxxfloat, pname))
86111
call_args.append("%s.real(), %s.imag()" % (pname, pname))
87112
elif ctype.startswith("callback:"):
88113
i = callback_kinds.setdefault(ctype, len(callback_kinds))
89-
args.append("nb::callable " + pname)
114+
args.append("nb::handle " + pname)
90115
setup.append(
91-
"int64_t _h_%s = gopy_cb_register(%s);\n"
92-
" GopyCBGuard _g_%s{_h_%s};" % (pname, pname, pname, pname)
116+
"gopy_arg_callable(%s.ptr(), %d);\n"
117+
" int64_t _h_%s = gopy_cb_register(nb::borrow<nb::callable>(%s));\n"
118+
" GopyCBGuard _g_%s{_h_%s};" % (pname, i + 1, pname, pname, pname, pname)
93119
)
94120
call_args.append("_h_%s" % pname)
95121
else:
@@ -134,11 +160,20 @@ def wrapper(name, ret, params, callback_kinds):
134160
cpptype = ret
135161
if setup:
136162
body = "\n ".join(setup) + "\n " + body
137-
return ' m.def("%s", [](%s) -> %s {\n %s\n });' % (
163+
# Unlike pybind11, nanobind refuses None for an nb::handle parameter
164+
# unless told otherwise, before the gopy_arg_* helper that should report
165+
# it (as pybindgen would) ever runs; telling it so takes an nb::arg for
166+
# every parameter, not just those.
167+
annotations = "".join(
168+
', nb::arg("%s")%s' % (a.rsplit(" ", 1)[1], ".none()" if a.startswith("nb::handle ") else "")
169+
for a in args
170+
)
171+
return ' m.def("%s", [](%s) -> %s {\n %s\n }%s);' % (
138172
name,
139173
", ".join(args),
140174
cpptype,
141175
body,
176+
annotations,
142177
)
143178

144179

@@ -205,9 +240,11 @@ def cxxparam(t):
205240

206241
MODULE_TEMPLATE = '''// python bindings for package @NAME@ using nanobind.
207242
// File is generated by gopy version @VERSION@. Do not edit.
243+
#include <climits>
208244
#include <complex>
209245
#include <cstdint>
210246
#include <cstdlib>
247+
#include <cstring>
211248
#include <mutex>
212249
#include <string>
213250
#include <unordered_map>
@@ -222,6 +259,13 @@ def cxxparam(t):
222259
#include "@HEADER@"
223260
}
224261
262+
// Hands the python error already set back to python, from C++.
263+
[[noreturn]] static void gopy_raise() {
264+
throw nb::python_error();
265+
}
266+
267+
@ARG_HELPERS@
268+
225269
// Raises the exception, if any, that the last Go call on this thread
226270
// recorded (see gopySetError/GopyTakeError, shared with the cffi backend).
227271
static inline void _check() {

‎bind/pybind11.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,19 @@ import (
2222
//go:embed pybind11_build.py
2323
var pybind11BuildPy string
2424

25+
// cxxArgHelpers is the argument conversion code shared by the pybind11 and
26+
// nanobind backends' generated .cpp, which each build.py has in place of
27+
// @ARG_HELPERS@.
28+
//
29+
//go:embed cxx_args.inc
30+
var cxxArgHelpers string
31+
2532
// pybind11BuildPreamble returns the start of build.py: the pybind11 recorder.
2633
func (g *pyGen) pybind11BuildPreamble() string {
2734
return strings.NewReplacer(
2835
"@NAME@", g.cfg.Name,
2936
"@CMD@", g.cfg.Cmd,
3037
"@VERSION@", g.cfg.Version,
38+
"@ARG_HELPERS@", cxxArgHelpers,
3139
).Replace(pybind11BuildPy)
3240
}

‎bind/pybind11_build.py‎

Lines changed: 45 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,30 @@ def add_checked_function(mod, name, retval, params, failure_expression="", *a, *
5757
add_checked_string_function = add_checked_function
5858

5959

60+
# How wrapper converts an argument of each C type: as pybindgen's generated
61+
# code would, for the same exception on a wrongly-typed argument under every
62+
# backend (see the gopy_arg_* helpers in cxx_args.inc). %(o)s is the
63+
# argument's PyObject*, %(n)d its 1-based position. Identical in
64+
# pybind11_build.py and nanobind_build.py.
65+
ARG_CONV = {
66+
"int64_t": "gopy_arg_L(%(o)s)",
67+
"long long": "gopy_arg_L(%(o)s)",
68+
"uint64_t": "gopy_arg_K(%(o)s, %(n)d)",
69+
"int": "gopy_arg_i(%(o)s)",
70+
"int32_t": "gopy_arg_i(%(o)s)",
71+
"uint32_t": "gopy_arg_I(%(o)s)",
72+
"unsigned int": "gopy_arg_I(%(o)s)",
73+
"int16_t": "(int16_t)gopy_arg_i_max(%(o)s, 0x7fff)",
74+
"uint16_t": "(uint16_t)gopy_arg_i_max(%(o)s, 0xffff)",
75+
"int8_t": "(int8_t)gopy_arg_i_max(%(o)s, 0x7f)",
76+
"uint8_t": "(uint8_t)gopy_arg_i_max(%(o)s, 0xff)",
77+
"double": "gopy_arg_d(%(o)s)",
78+
"float": "(float)gopy_arg_d(%(o)s)",
79+
"bool": "gopy_arg_bool(%(o)s)",
80+
"char*": "const_cast<char*>(gopy_arg_str(%(o)s, %(n)d))",
81+
}
82+
83+
6084
def wrapper(name, ret, params, callback_kinds):
6185
"""Returns the m.def(...) call binding name, or "" if its signature
6286
isn't supported yet (a raw PyObject*): the .cpp simply never binds it, so
@@ -71,23 +95,25 @@ def wrapper(name, ret, params, callback_kinds):
7195
args = []
7296
setup = []
7397
call_args = []
74-
for ctype, pname in params:
75-
if ctype == "char*":
76-
args.append("const std::string& " + pname)
77-
call_args.append("const_cast<char*>(%s.c_str())" % pname)
78-
elif ctype == "bool":
79-
args.append("bool " + pname)
80-
call_args.append("(char)(%s ? 1 : 0)" % pname)
98+
for i, (ctype, pname) in enumerate(params):
99+
if ctype in ARG_CONV:
100+
# converted up front, in order, while the GIL is still held
101+
args.append("py::handle " + pname)
102+
setup.append(
103+
"auto _a_%s = %s;" % (pname, ARG_CONV[ctype] % {"o": pname + ".ptr()", "n": i + 1})
104+
)
105+
call_args.append("_a_" + pname)
81106
elif ctype in ("complex64", "complex128"):
82107
cxxfloat = "float" if ctype == "complex64" else "double"
83108
args.append("std::complex<%s> %s" % (cxxfloat, pname))
84109
call_args.append("%s.real(), %s.imag()" % (pname, pname))
85110
elif ctype.startswith("callback:"):
86111
i = callback_kinds.setdefault(ctype, len(callback_kinds))
87-
args.append("py::function " + pname)
112+
args.append("py::handle " + pname)
88113
setup.append(
89-
"int64_t _h_%s = gopy_cb_register(%s);\n"
90-
" GopyCBGuard _g_%s{_h_%s};" % (pname, pname, pname, pname)
114+
"gopy_arg_callable(%s.ptr(), %d);\n"
115+
" int64_t _h_%s = gopy_cb_register(py::reinterpret_borrow<py::function>(%s));\n"
116+
" GopyCBGuard _g_%s{_h_%s};" % (pname, i + 1, pname, pname, pname, pname)
91117
)
92118
call_args.append("_h_%s" % pname)
93119
else:
@@ -203,9 +229,11 @@ def cxxparam(t):
203229

204230
MODULE_TEMPLATE = '''// python bindings for package @NAME@ using pybind11.
205231
// File is generated by gopy version @VERSION@. Do not edit.
232+
#include <climits>
206233
#include <complex>
207234
#include <cstdint>
208235
#include <cstdlib>
236+
#include <cstring>
209237
#include <mutex>
210238
#include <string>
211239
#include <unordered_map>
@@ -220,6 +248,13 @@ def cxxparam(t):
220248
#include "@HEADER@"
221249
}
222250
251+
// Hands the python error already set back to python, from C++.
252+
[[noreturn]] static void gopy_raise() {
253+
throw py::error_already_set();
254+
}
255+
256+
@ARG_HELPERS@
257+
223258
// Raises the exception, if any, that the last Go call on this thread
224259
// recorded (see gopySetError/GopyTakeError, shared with the cffi backend).
225260
static inline void _check() {

0 commit comments

Comments
 (0)