From b4e4adab5eb8fdd085d34c6af11bb9d3b4a4112a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=83=C2=96R=C3=83=C2=96K=20Attila?= Date: Tue, 4 Jul 2017 01:05:44 +0200 Subject: [PATCH 1/2] First iteration of consume call policy. Noisy, ugly, and incomplete. some cleanup --- include/pybind11/attr.h | 11 ++++++++ include/pybind11/pybind11.h | 25 +++++++++++++++++++ tests/CMakeLists.txt | 1 + tests/test_consume.cpp | 50 +++++++++++++++++++++++++++++++++++++ tests/test_consume.py | 42 +++++++++++++++++++++++++++++++ 5 files changed, 129 insertions(+) create mode 100644 tests/test_consume.cpp create mode 100644 tests/test_consume.py diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index dce875a6b9..014fe75716 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -44,6 +44,9 @@ template struct base { /// Keep patient alive while nurse lives template struct keep_alive { }; +/// Give ownership of parameter to the called function +template struct consume { }; + /// Annotation indicating that a class is involved in a multiple inheritance relationship struct multiple_inheritance { }; @@ -117,6 +120,7 @@ enum op_type : int; struct undefined_t; template struct op_; inline void keep_alive_impl(size_t Nurse, size_t Patient, function_call &call, handle ret); +inline void consume_impl(size_t Consumed, function_call &call); /// Internal data structure which holds metadata about a keyword argument struct argument_record { @@ -450,6 +454,13 @@ template struct process_attribute struct process_attribute> : public process_attribute_default> { + template = 0> + static void precall(function_call &call) { + consume_impl(Consumed, call); + } +}; + /// Recursively iterate over variadic template arguments template struct process_attributes { static void init(const Args&... args, function_record *r) { diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 0383988eaf..525a031664 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1508,6 +1508,31 @@ PYBIND11_NOINLINE inline void keep_alive_impl(size_t Nurse, size_t Patient, func keep_alive_impl(get_arg(Nurse), get_arg(Patient)); } +// template // ??? +inline void consume_impl(handle consumed) { + if (!consumed) + pybind11_fail("Could not activate consume!"); + + if (consumed.is_none()) + return; /* Nothing to consume */ + + // auto tinfo = all_type_info(Py_TYPE(consumed.ptr())); // ??? + + //consumed.dec_ref(); ??? + + // del the name of consumed from the Python scope somehow ... + + auto inst = reinterpret_cast(consumed.ptr()); + auto &holder = values_and_holders(inst).begin()->holder< std::unique_ptr >(); // holder_type ??? + holder.release(); +} + +PYBIND11_NOINLINE inline void consume_impl(size_t Consumed, function_call &call) { + consume_impl( + Consumed == 0 ? handle() : Consumed <= call.args.size() ? call.args[Consumed - 1] : handle() + ); +} + inline std::pair all_type_info_get_cache(PyTypeObject *type) { auto res = get_internals().registered_types_py #ifdef __cpp_lib_unordered_map_try_emplace diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 8f2f300ef7..916e5fd2bf 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -29,6 +29,7 @@ set(PYBIND11_TEST_FILES test_buffers.cpp test_builtin_casters.cpp test_call_policies.cpp + test_consume.cpp test_callbacks.cpp test_chrono.cpp test_class.cpp diff --git a/tests/test_consume.cpp b/tests/test_consume.cpp new file mode 100644 index 0000000000..85ed109a5d --- /dev/null +++ b/tests/test_consume.cpp @@ -0,0 +1,50 @@ +/* + tests/test_consume.cpp -- consume call policy + + Copyright (c) 2016 Wenzel Jakob + Copyright (c) 2017 Attila Török + + All rights reserved. Use of this source code is governed by a + BSD-style license that can be found in the LICENSE file. +*/ + +#include "pybind11_tests.h" + +class Box { + int size; + static int num_boxes; + +public: + Box(int size): size(size) { py::print("Box created."); ++num_boxes; } + ~Box() { py::print("Box destroyed."); --num_boxes; } + + int get_size() { return size; } + static int get_num_boxes() { return num_boxes; } +}; + +int Box::num_boxes = 0; + +class Filter { + int threshold; + +public: + Filter(int threshold): threshold(threshold) { py::print("Filter created."); } + ~Filter() { py::print("Filter destroyed."); } + + void process(Box *box) { // ownership of box is taken + py::print("Box is processed by Filter."); + if (box->get_size() > threshold) + delete box; + // otherwise the box is leaked + }; +}; + +test_initializer consume([](py::module &m) { + py::class_(m, "Box") + .def(py::init()) + .def_static("get_num_boxes", &Box::get_num_boxes); + + py::class_(m, "Filter") + .def(py::init()) + .def("process", &Filter::process, py::consume<2>()); +}); diff --git a/tests/test_consume.py b/tests/test_consume.py new file mode 100644 index 0000000000..28fca7a7d9 --- /dev/null +++ b/tests/test_consume.py @@ -0,0 +1,42 @@ +import pytest + + +def test_consume_argument(capture): + from pybind11_tests import Box, Filter + + with capture: + filt = Filter(4) + assert capture == "Filter created." + with capture: + box_1 = Box(1) + box_8 = Box(8) + assert capture == """ + Box created. + Box created. + """ + + assert Box.get_num_boxes() == 2 + + with capture: + filt.process(box_1) # box_1 is not big enough, but process() leaks it + assert capture == "Box is processed by Filter." + + assert Box.get_num_boxes() == 2 + + with capture: + filt.process(box_8) # box_8 is destroyed by process() of filt + assert capture == """ + Box is processed by Filter. + Box destroyed. + """ + + assert Box.get_num_boxes() == 1 # box_1 still exists somehow, but we can't access it + + with capture: + del filt + del box_1 + del box_8 + pytest.gc_collect() + assert capture == "Filter destroyed." + + assert Box.get_num_boxes() == 1 # 1 box is leaked, and we can't do anything From 689cb12b8a23fbce59e6dfdc301c2c33bfd837f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=96R=C3=96K=20Attila?= Date: Tue, 26 Dec 2017 00:05:31 +0100 Subject: [PATCH 2/2] better? --- include/pybind11/cast.h | 1 + include/pybind11/pybind11.h | 15 +++++++++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 0aa632601e..99c1768ce1 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1371,6 +1371,7 @@ template class type_caster> template struct holder_helper { static auto get(const T &p) -> decltype(p.get()) { return p.get(); } + static void release(T &p) { p.release(); } }; /// Type caster for holder types like std::shared_ptr, etc. diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 525a031664..d19532dae1 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1516,15 +1516,18 @@ inline void consume_impl(handle consumed) { if (consumed.is_none()) return; /* Nothing to consume */ - // auto tinfo = all_type_info(Py_TYPE(consumed.ptr())); // ??? + auto inst = reinterpret_cast(consumed.ptr()); + auto value_and_holder = values_and_holders(inst).begin(); - //consumed.dec_ref(); ??? - // del the name of consumed from the Python scope somehow ... - auto inst = reinterpret_cast(consumed.ptr()); - auto &holder = values_and_holders(inst).begin()->holder< std::unique_ptr >(); // holder_type ??? - holder.release(); + auto &holder = value_and_holder->holder< std::unique_ptr >(); // holder_type ??? + + + holder_helper>::release(holder); + + value_and_holder->set_holder_constructed(false); + inst->owned = false; } PYBIND11_NOINLINE inline void consume_impl(size_t Consumed, function_call &call) {