From 81c0e9fee67b85d492dec78b28f557f58a8ffab9 Mon Sep 17 00:00:00 2001 From: Nicholas Gates Date: Wed, 4 Oct 2023 15:27:17 +0100 Subject: [PATCH 1/3] Refactor call API --- pydust/src/builtins.zig | 40 ++++++++++++++++++++++++++++++ pydust/src/modules.zig | 6 ++--- pydust/src/types/error.zig | 17 +++---------- pydust/src/types/obj.zig | 50 ++++++++------------------------------ pydust/src/types/slice.zig | 6 ++--- pydust/src/types/type.zig | 2 +- 6 files changed, 60 insertions(+), 61 deletions(-) diff --git a/pydust/src/builtins.zig b/pydust/src/builtins.zig index e788ff28..8ecbba9d 100644 --- a/pydust/src/builtins.zig +++ b/pydust/src/builtins.zig @@ -71,6 +71,46 @@ pub fn callable(object: anytype) bool { return ffi.PyCallable_Check(obj.py) == 1; } +/// Call a callable object with no arguments. +pub fn call0(comptime T: type, object: anytype) !T { + const result = ffi.PyObject_CallNoArgs(py.object(object).py) orelse return PyError.PyRaised; + return try py.as(T, result); +} + +/// Call a callable object with the given arguments. +pub fn call(comptime ReturnType: type, object: anytype, args: anytype, kwargs: anytype) !ReturnType { + const pyobj = py.object(object); + + var argsPy: py.PyTuple = undefined; + if (@typeInfo(@TypeOf(args)) == .Optional and args == null) { + argsPy = try py.PyTuple.new(0); + } else { + argsPy = try py.PyTuple.checked(try py.create(args)); + } + defer argsPy.decref(); + + // TODO(ngates): avoid creating empty dict for kwargs + var kwargsPy: py.PyDict = undefined; + if (@typeInfo(@TypeOf(kwargs)) == .Optional and kwargs == null) { + kwargsPy = try py.PyDict.new(); + } else { + // Annoyingly our trampoline turns an empty kwargs struct into a PyTuple. + // This will be fixed by #94 + const kwobj = try py.create(kwargs); + if (try py.len(kwobj) == 0) { + kwobj.decref(); + kwargsPy = try py.PyDict.new(); + } else { + kwargsPy = try py.PyDict.checked(kwobj); + } + } + defer kwargsPy.decref(); + + // We _must_ return a PyObject to the user to let them handle the lifetime of the object. + const result = ffi.PyObject_Call(pyobj.py, argsPy.obj.py, kwargsPy.obj.py) orelse return PyError.PyRaised; + return try py.as(ReturnType, result); +} + /// Convert an object into a dictionary. Equivalent of Python dict(o). pub fn dict(object: anytype) !py.PyDict { const Dict: py.PyObject = .{ .py = @alignCast(@ptrCast(&ffi.PyDict_Type)) }; diff --git a/pydust/src/modules.zig b/pydust/src/modules.zig index 7a408456..9dd547f3 100644 --- a/pydust/src/modules.zig +++ b/pydust/src/modules.zig @@ -124,11 +124,11 @@ fn Slots(comptime definition: type) type { const pySubmodDef: *ffi.PyModuleDef = @ptrCast((try submodDef.init()).py); // Create a dumb ModuleSpec with a name attribute using types.SimpleNamespace - const SimpleNamespace = try py.importFrom("types", "SimpleNamespace"); - + const types = try py.import("types"); + defer types.decref(); const pyname = try py.PyString.create(name); defer pyname.decref(); - const spec = try SimpleNamespace.call(.{}, .{ .name = pyname }); + const spec = try types.call(py.PyObject, "SimpleNamespace", .{}, .{ .name = pyname }); defer spec.decref(); const submod: py.PyObject = .{ .py = ffi.PyModule_FromDefAndSpec(pySubmodDef, spec.py) orelse return PyError.PyRaised }; diff --git a/pydust/src/types/error.zig b/pydust/src/types/error.zig index 1cf8b98d..72af67e2 100644 --- a/pydust/src/types/error.zig +++ b/pydust/src/types/error.zig @@ -166,25 +166,14 @@ const PyExc = struct { ); defer py.allocator.free(code); - // Compilation should succeed, but execution will fail. - const filename = try py.allocator.dupeZ(u8, line_info.file_name); - defer py.allocator.free(filename); - const compiled = ffi.Py_CompileString(code.ptr, filename.ptr, ffi.Py_file_input) orelse continue; - defer py.decref(compiled); - // Import the compiled code as a module and invoke the failing function - const module_name = try py.allocator.dupeZ(u8, symbol_info.compile_unit_name); - defer py.allocator.free(module_name); - const fake_module: py.PyObject = .{ - .py = ffi.PyImport_ExecCodeModule(module_name.ptr, compiled) orelse continue, - }; + const fake_module = try py.PyModule.fromCode(code, line_info.file_name, symbol_info.compile_unit_name); defer fake_module.decref(); const func_name = try py.allocator.dupeZ(u8, symbol_info.symbol_name); defer py.allocator.free(func_name); - const fake_function = try fake_module.get(func_name); - _ = fake_function.call(.{}, .{}) catch null; - defer fake_function.decref(); + + _ = fake_module.obj.call(void, func_name, .{}, .{}) catch null; // Grab our forced exception info. // We can ignore qtype and qvalue, we just want to get the traceback object. diff --git a/pydust/src/types/obj.zig b/pydust/src/types/obj.zig index 3254a66e..b27987c4 100644 --- a/pydust/src/types/obj.zig +++ b/pydust/src/types/obj.zig @@ -33,45 +33,20 @@ pub const PyObject = extern struct { return name.asSlice(); } - /// Call this object without any arguments. - pub fn call0(self: PyObject) !PyObject { - return .{ .py = ffi.PyObject_CallNoArgs(self.py) orelse return PyError.PyRaised }; - } - /// Call this object with the given args and kwargs. - pub fn call(self: PyObject, args: anytype, kwargs: anytype) !PyObject { - var argsPy: py.PyTuple = undefined; - if (@typeInfo(@TypeOf(args)) == .Optional and args == null) { - argsPy = try py.PyTuple.new(0); - } else { - argsPy = try py.PyTuple.checked(try py.create(args)); - } - defer argsPy.decref(); - - // FIXME(ngates): avoid creating empty dict for kwargs - var kwargsPy: py.PyDict = undefined; - if (@typeInfo(@TypeOf(kwargs)) == .Optional and kwargs == null) { - kwargsPy = try py.PyDict.new(); - } else { - const kwobj = try py.create(kwargs); - if (try py.len(kwobj) == 0) { - kwobj.decref(); - kwargsPy = try py.PyDict.new(); - } else { - kwargsPy = try py.PyDict.checked(kwobj); - } - } - defer kwargsPy.decref(); - - // We _must_ return a PyObject to the user to let them handle the lifetime of the object. - const result = ffi.PyObject_Call(self.py, argsPy.obj.py, kwargsPy.obj.py) orelse return PyError.PyRaised; - return PyObject{ .py = result }; + pub fn call(self: PyObject, comptime T: type, method: [:0]const u8, args: anytype, kwargs: anytype) !T { + const methodObj = try self.get(method); + return py.call(T, methodObj, args, kwargs); } - pub fn get(self: PyObject, attr: [:0]const u8) !PyObject { + pub fn get(self: PyObject, attr: [:0]const u8) !py.PyObject { return .{ .py = ffi.PyObject_GetAttrString(self.py, attr) orelse return PyError.PyRaised }; } + pub fn getAs(self: PyObject, comptime T: type, attr: [:0]const u8) !T { + return try py.as(T, try self.get(attr)); + } + // See: https://docs.python.org/3/c-api/buffer.html#buffer-request-types pub fn getBuffer(self: py.PyObject, flags: c_int) !py.PyBuffer { if (ffi.PyObject_CheckBuffer(self.py) != 1) { @@ -151,11 +126,6 @@ test "call" { defer py.finalize(); const pow = try py.importFrom("math", "pow"); - const result = try pow.call(.{ 2, 3 }, .{}); - - if (py.PyFloat.checkedCast(result)) |f| { - try std.testing.expectEqual(f.as(f32), 8.0); - } - - try std.testing.expectEqual(@as(f32, 8.0), try py.as(f32, result)); + const result = try py.call(f32, pow, .{ 2, 3 }, .{}); + try std.testing.expectEqual(@as(f32, 8.0), result); } diff --git a/pydust/src/types/slice.zig b/pydust/src/types/slice.zig index 10f35f06..a0370004 100644 --- a/pydust/src/types/slice.zig +++ b/pydust/src/types/slice.zig @@ -36,15 +36,15 @@ pub const PySlice = extern struct { } pub fn getStart(self: PySlice, comptime T: type) !T { - return try py.as(T, try self.obj.get("start")); + return try self.obj.getAs(T, "start"); } pub fn getStop(self: PySlice, comptime T: type) !T { - return try py.as(T, try self.obj.get("stop")); + return try try self.obj.getAs(T, "stop"); } pub fn getStep(self: PySlice, comptime T: type) !T { - return try py.as(T, try self.obj.get("step")); + return try self.obj.getAs(T, "step"); } }; diff --git a/pydust/src/types/type.zig b/pydust/src/types/type.zig index 9373823d..d72836cc 100644 --- a/pydust/src/types/type.zig +++ b/pydust/src/types/type.zig @@ -53,7 +53,7 @@ test "PyType" { defer StringIO.decref(); try std.testing.expectEqualSlices(u8, "StringIO", try (try StringIO.name()).asSlice()); - const sio = try StringIO.obj.call0(); + const sio = try py.call0(py.PyObject, StringIO); defer sio.decref(); const sioType = try py.type_(sio); try std.testing.expectEqualSlices(u8, "StringIO", try (try sioType.name()).asSlice()); From 24b2b5896e2645b0df2c8359cdf5e21f05f5f34a Mon Sep 17 00:00:00 2001 From: Nicholas Gates Date: Wed, 4 Oct 2023 15:30:18 +0100 Subject: [PATCH 2/3] Refactor call API --- pydust/src/types/type.zig | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pydust/src/types/type.zig b/pydust/src/types/type.zig index d72836cc..1ec4a63e 100644 --- a/pydust/src/types/type.zig +++ b/pydust/src/types/type.zig @@ -49,8 +49,10 @@ test "PyType" { py.initialize(); defer py.finalize(); - const StringIO = try PyType.checked(try py.importFrom("io", "StringIO")); - defer StringIO.decref(); + const io = try py.import("io"); + defer io.decref(); + + const StringIO = try io.getAs(py.PyType, "StringIO"); try std.testing.expectEqualSlices(u8, "StringIO", try (try StringIO.name()).asSlice()); const sio = try py.call0(py.PyObject, StringIO); From cb8f99e867571fcf1f83b0a8a610926c71d1cb2b Mon Sep 17 00:00:00 2001 From: Nicholas Gates Date: Wed, 4 Oct 2023 16:00:18 +0100 Subject: [PATCH 3/3] Fix resource lak --- pydust/src/builtins.zig | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pydust/src/builtins.zig b/pydust/src/builtins.zig index 8ecbba9d..1dab7f98 100644 --- a/pydust/src/builtins.zig +++ b/pydust/src/builtins.zig @@ -72,12 +72,20 @@ pub fn callable(object: anytype) bool { } /// Call a callable object with no arguments. +/// +/// If the result is a new reference, then as always the caller is responsible for calling decref on it. +/// That means for new references the caller should ask for a return type that they are unable to decref, +/// for example []const u8. pub fn call0(comptime T: type, object: anytype) !T { const result = ffi.PyObject_CallNoArgs(py.object(object).py) orelse return PyError.PyRaised; return try py.as(T, result); } /// Call a callable object with the given arguments. +/// +/// If the result is a new reference, then as always the caller is responsible for calling decref on it. +/// That means for new references the caller should ask for a return type that they are unable to decref, +/// for example []const u8. pub fn call(comptime ReturnType: type, object: anytype, args: anytype, kwargs: anytype) !ReturnType { const pyobj = py.object(object); @@ -106,7 +114,7 @@ pub fn call(comptime ReturnType: type, object: anytype, args: anytype, kwargs: a } defer kwargsPy.decref(); - // We _must_ return a PyObject to the user to let them handle the lifetime of the object. + // Note, the caller is responsible for returning a result type that they are able to decref. const result = ffi.PyObject_Call(pyobj.py, argsPy.obj.py, kwargsPy.obj.py) orelse return PyError.PyRaised; return try py.as(ReturnType, result); }