From abe59a9c2a1fe7f70df09f9c8dd7050bc2ea98d1 Mon Sep 17 00:00:00 2001 From: Qifan Lu Date: Thu, 24 Dec 2020 23:48:46 -0800 Subject: [PATCH 1/3] Do not set docstring for function when it's empty --- include/pybind11/pybind11.h | 13 ++++++++++--- tests/test_docstring_options.cpp | 8 ++++++++ tests/test_docstring_options.py | 3 +++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 104f32206d..7a567b4789 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -439,9 +439,14 @@ class cpp_function : public function { /* Install docstring */ auto *func = (PyCFunctionObject *) m_ptr; - if (func->m_ml->ml_doc) + // Free and reset previous docstring if it exists + if (func->m_ml->ml_doc) { std::free(const_cast(func->m_ml->ml_doc)); - func->m_ml->ml_doc = strdup(signatures.c_str()); + func->m_ml->ml_doc = nullptr; + } + // Then install new one if it's non-empty (when at least one option is enabled) + if (!signatures.empty()) + func->m_ml->ml_doc = strdup(signatures.c_str()); if (rec->is_method) { m_ptr = PYBIND11_INSTANCE_METHOD_NEW(m_ptr, rec->scope.ptr()); @@ -472,7 +477,9 @@ class cpp_function : public function { arg.value.dec_ref(); } if (rec->def) { - std::free(const_cast(rec->def->ml_doc)); + if (rec->def->ml_doc) { + std::free(const_cast(rec->def->ml_doc)); + } // Python 3.9.0 decref's these in the wrong order; rec->def // If loaded on 3.9.0, let these leak (use Python 3.9.1 at runtime to fix) // See https://github.com/python/cpython/pull/22670 diff --git a/tests/test_docstring_options.cpp b/tests/test_docstring_options.cpp index 8c8f79fd5f..8a97af55fc 100644 --- a/tests/test_docstring_options.cpp +++ b/tests/test_docstring_options.cpp @@ -45,6 +45,14 @@ TEST_SUBMODULE(docstring_options, m) { m.def("test_function7", [](int, int) {}, py::arg("a"), py::arg("b"), "A custom docstring"); + { + py::options options; + options.disable_user_defined_docstrings(); + options.disable_function_signatures(); + + m.def("test_function8", []() {}); + } + { py::options options; options.disable_user_defined_docstrings(); diff --git a/tests/test_docstring_options.py b/tests/test_docstring_options.py index 87d80d2df6..8ee6613884 100644 --- a/tests/test_docstring_options.py +++ b/tests/test_docstring_options.py @@ -34,6 +34,9 @@ def test_docstring_options(): assert m.test_function7.__doc__.startswith("test_function7(a: int, b: int) -> None") assert m.test_function7.__doc__.endswith("A custom docstring\n") + # when all options are disabled, no docstring (instead of an empty one) should be generated + assert m.test_function8.__doc__ is None + # Suppression of user-defined docstrings for non-function objects assert not m.DocstringTestFoo.__doc__ assert not m.DocstringTestFoo.value_prop.__doc__ From 7cf6ffc5afc215b76401b6966e55f5202e25a05a Mon Sep 17 00:00:00 2001 From: Qifan Lu Date: Fri, 25 Dec 2020 17:15:11 -0800 Subject: [PATCH 2/3] No need to check pointer for `free` --- include/pybind11/pybind11.h | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 7a567b4789..6d9860def9 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -439,12 +439,9 @@ class cpp_function : public function { /* Install docstring */ auto *func = (PyCFunctionObject *) m_ptr; - // Free and reset previous docstring if it exists - if (func->m_ml->ml_doc) { - std::free(const_cast(func->m_ml->ml_doc)); - func->m_ml->ml_doc = nullptr; - } - // Then install new one if it's non-empty (when at least one option is enabled) + std::free(const_cast(func->m_ml->ml_doc)); + func->m_ml->ml_doc = nullptr; + // Install docstring if it's non-empty (when at least one option is enabled) if (!signatures.empty()) func->m_ml->ml_doc = strdup(signatures.c_str()); @@ -477,9 +474,7 @@ class cpp_function : public function { arg.value.dec_ref(); } if (rec->def) { - if (rec->def->ml_doc) { - std::free(const_cast(rec->def->ml_doc)); - } + std::free(const_cast(rec->def->ml_doc)); // Python 3.9.0 decref's these in the wrong order; rec->def // If loaded on 3.9.0, let these leak (use Python 3.9.1 at runtime to fix) // See https://github.com/python/cpython/pull/22670 From e4ce12fd8fe996f00a0eb5cd92360ccc154c1ce1 Mon Sep 17 00:00:00 2001 From: Qifan Lu Date: Sat, 26 Dec 2020 16:21:09 -0800 Subject: [PATCH 3/3] Use ternary operator to conditionally set `ml_doc` --- include/pybind11/pybind11.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 6d9860def9..70ba635ebe 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -440,10 +440,8 @@ class cpp_function : public function { /* Install docstring */ auto *func = (PyCFunctionObject *) m_ptr; std::free(const_cast(func->m_ml->ml_doc)); - func->m_ml->ml_doc = nullptr; // Install docstring if it's non-empty (when at least one option is enabled) - if (!signatures.empty()) - func->m_ml->ml_doc = strdup(signatures.c_str()); + func->m_ml->ml_doc = signatures.empty() ? nullptr : strdup(signatures.c_str()); if (rec->is_method) { m_ptr = PYBIND11_INSTANCE_METHOD_NEW(m_ptr, rec->scope.ptr());