Skip to content

Commit

Permalink
addl unit tests for PR #3970 (#3977)
Browse files Browse the repository at this point in the history
* Add test_perf_accessors (to be merged into test_pytypes).

* Python < 3.8 f-string compatibility

* Use thread_local in inc_ref_counter()

* Intentional breakage, brute-force way to quickly find out how many platforms reach the PYBIND11_HANDLE_REF_DEBUG code, with and without threads.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Remove Intentional breakage

* Drop perf test, move inc_refs tests to test_pytypes

* Fold in PR #3970 with `#ifdef`s

* Complete test coverage for all newly added code.

* Condense new unit tests via a simple local helper macro.

* Remove PYBIND11_PR3970 define. See #3977 (comment)

* Move static keyword first (fixes silly oversight).

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
Ralf W. Grosse-Kunstleve and pre-commit-ci[bot] authored May 31, 2022
1 parent b24c5ed commit 9f7b3f7
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 0 deletions.
19 changes: 19 additions & 0 deletions include/pybind11/pytypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,10 @@ class object_api : public pyobject_tag {

PYBIND11_NAMESPACE_END(detail)

#if !defined(PYBIND11_HANDLE_REF_DEBUG) && !defined(NDEBUG)
# define PYBIND11_HANDLE_REF_DEBUG
#endif

/** \rst
Holds a reference to a Python object (no reference counting)
Expand Down Expand Up @@ -209,6 +213,9 @@ class handle : public detail::object_api<handle> {
this function automatically. Returns a reference to itself.
\endrst */
const handle &inc_ref() const & {
#ifdef PYBIND11_HANDLE_REF_DEBUG
inc_ref_counter(1);
#endif
Py_XINCREF(m_ptr);
return *this;
}
Expand Down Expand Up @@ -244,6 +251,18 @@ class handle : public detail::object_api<handle> {

protected:
PyObject *m_ptr = nullptr;

#ifdef PYBIND11_HANDLE_REF_DEBUG
private:
static std::size_t inc_ref_counter(std::size_t add) {
thread_local std::size_t counter = 0;
counter += add;
return counter;
}

public:
static std::size_t inc_ref_counter() { return inc_ref_counter(0); }
#endif
};

/** \rst
Expand Down
49 changes: 49 additions & 0 deletions tests/test_pytypes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,55 @@ TEST_SUBMODULE(pytypes, m) {
return d;
});

m.def("accessor_moves", []() { // See PR #3970
py::list return_list;
#ifdef PYBIND11_HANDLE_REF_DEBUG
py::int_ py_int_0(0);
py::int_ py_int_42(42);
py::str py_str_count("count");

auto tup = py::make_tuple(0);

py::sequence seq(tup);

py::list lst;
lst.append(0);

# define PYBIND11_LOCAL_DEF(...) \
{ \
std::size_t inc_refs = py::handle::inc_ref_counter(); \
__VA_ARGS__; \
inc_refs = py::handle::inc_ref_counter() - inc_refs; \
return_list.append(inc_refs); \
}

PYBIND11_LOCAL_DEF(tup[py_int_0]) // l-value (to have a control)
PYBIND11_LOCAL_DEF(tup[py::int_(0)]) // r-value

PYBIND11_LOCAL_DEF(tup.attr(py_str_count)) // l-value
PYBIND11_LOCAL_DEF(tup.attr(py::str("count"))) // r-value

PYBIND11_LOCAL_DEF(seq[py_int_0]) // l-value
PYBIND11_LOCAL_DEF(seq[py::int_(0)]) // r-value

PYBIND11_LOCAL_DEF(seq.attr(py_str_count)) // l-value
PYBIND11_LOCAL_DEF(seq.attr(py::str("count"))) // r-value

PYBIND11_LOCAL_DEF(lst[py_int_0]) // l-value
PYBIND11_LOCAL_DEF(lst[py::int_(0)]) // r-value

PYBIND11_LOCAL_DEF(lst.attr(py_str_count)) // l-value
PYBIND11_LOCAL_DEF(lst.attr(py::str("count"))) // r-value

auto lst_acc = lst[py::int_(0)];
lst_acc = py::int_(42); // Detaches lst_acc from lst.
PYBIND11_LOCAL_DEF(lst_acc = py_int_42) // l-value
PYBIND11_LOCAL_DEF(lst_acc = py::int_(42)) // r-value
# undef PYBIND11_LOCAL_DEF
#endif
return return_list;
});

// test_constructors
m.def("default_constructors", []() {
return py::dict("bytes"_a = py::bytes(),
Expand Down
9 changes: 9 additions & 0 deletions tests/test_pytypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,15 @@ def func(self, x, *args):
assert d["var"] == 99


def test_accessor_moves():
inc_refs = m.accessor_moves()
if inc_refs:
# To be changed in PR #3970: [1, 0, 1, 0, ...]
assert inc_refs == [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]
else:
pytest.skip("Not defined: PYBIND11_HANDLE_REF_DEBUG")


def test_constructors():
"""C++ default and converting constructors are equivalent to type calls in Python"""
types = [bytes, bytearray, str, bool, int, float, tuple, list, dict, set]
Expand Down

0 comments on commit 9f7b3f7

Please sign in to comment.