From 47079b9e7b81a4b6d93c46a452115d7d8fd3488f Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Thu, 24 Mar 2022 12:57:37 -0400 Subject: [PATCH 1/6] (perf): Add missing move in sp matrix caster and microopt char concats (#3823) * Add missing move in sp matrix caster and microopt char concat * Remove useless move * Add a couple more std::move * Missed one char * Improve error_string * Ensure no temp reallocs in errorString concat * Remove useless move --- include/pybind11/detail/type_caster_base.h | 14 +++++++++----- include/pybind11/eigen.h | 5 +++-- include/pybind11/numpy.h | 6 +++--- include/pybind11/pybind11.h | 12 ++++++------ 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index a4154136a..5b2ec8e1e 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -225,8 +225,8 @@ PYBIND11_NOINLINE detail::type_info *get_type_info(const std::type_index &tp, if (throw_if_missing) { std::string tname = tp.name(); detail::clean_type_id(tname); - pybind11_fail("pybind11::detail::get_type_info: unable to find type info for \"" + tname - + "\""); + pybind11_fail("pybind11::detail::get_type_info: unable to find type info for \"" + + std::move(tname) + '"'); } return nullptr; } @@ -512,9 +512,13 @@ PYBIND11_NOINLINE std::string error_string() { Py_INCREF(f_code); # endif int lineno = PyFrame_GetLineNumber(frame); - errorString += " " + handle(f_code->co_filename).cast() + "(" - + std::to_string(lineno) - + "): " + handle(f_code->co_name).cast() + "\n"; + errorString += " "; + errorString += handle(f_code->co_filename).cast(); + errorString += '('; + errorString += std::to_string(lineno); + errorString += "): "; + errorString += handle(f_code->co_name).cast(); + errorString += '\n'; Py_DECREF(f_code); # if PY_VERSION_HEX >= 0x030900B1 auto *b_frame = PyFrame_GetBack(frame); diff --git a/include/pybind11/eigen.h b/include/pybind11/eigen.h index 930c4f7fa..f658168de 100644 --- a/include/pybind11/eigen.h +++ b/include/pybind11/eigen.h @@ -668,7 +668,7 @@ struct type_caster::value>> { Type::Flags &(Eigen::RowMajor | Eigen::ColMajor), StorageIndex>(shape[0].cast(), shape[1].cast(), - nnz, + std::move(nnz), outerIndices.mutable_data(), innerIndices.mutable_data(), values.mutable_data()); @@ -686,7 +686,8 @@ struct type_caster::value>> { array outerIndices((rowMajor ? src.rows() : src.cols()) + 1, src.outerIndexPtr()); array innerIndices(src.nonZeros(), src.innerIndexPtr()); - return matrix_type(std::make_tuple(data, innerIndices, outerIndices), + return matrix_type(std::make_tuple( + std::move(data), std::move(innerIndices), std::move(outerIndices)), std::make_pair(src.rows(), src.cols())) .release(); } diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index 5e68f6053..07dea21b4 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -940,7 +940,7 @@ protected: void fail_dim_check(ssize_t dim, const std::string &msg) const { throw index_error(msg + ": " + std::to_string(dim) + " (ndim = " + std::to_string(ndim()) - + ")"); + + ')'); } template @@ -1144,11 +1144,11 @@ struct format_descriptor::value> template struct format_descriptor { - static std::string format() { return std::to_string(N) + "s"; } + static std::string format() { return std::to_string(N) + 's'; } }; template struct format_descriptor> { - static std::string format() { return std::to_string(N) + "s"; } + static std::string format() { return std::to_string(N) + 's'; } }; template diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 422d534c1..fa018b509 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -561,14 +561,14 @@ protected: for (auto *it = chain_start; it != nullptr; it = it->next) { if (options::show_function_signatures()) { if (index > 0) { - signatures += "\n"; + signatures += '\n'; } if (chain) { signatures += std::to_string(++index) + ". "; } signatures += rec->name; signatures += it->signature; - signatures += "\n"; + signatures += '\n'; } if (it->doc && it->doc[0] != '\0' && options::show_user_defined_docstrings()) { // If we're appending another docstring, and aren't printing function signatures, @@ -577,15 +577,15 @@ protected: if (first_user_def) { first_user_def = false; } else { - signatures += "\n"; + signatures += '\n'; } } if (options::show_function_signatures()) { - signatures += "\n"; + signatures += '\n'; } signatures += it->doc; if (options::show_function_signatures()) { - signatures += "\n"; + signatures += '\n'; } } } @@ -1055,7 +1055,7 @@ protected: msg += it2->signature; } - msg += "\n"; + msg += '\n'; } msg += "\nInvoked with: "; auto args_ = reinterpret_borrow(args_in); From 146695a904526adc70c97615f7c395c4b3115c18 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Fri, 25 Mar 2022 10:55:13 -0400 Subject: [PATCH 2/6] fix: better exception and error handling for capsules (#3825) * Make capsule errors better match python --- include/pybind11/numpy.h | 3 ++- include/pybind11/pytypes.h | 32 +++++++++++++++++++------------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index 07dea21b4..7624c9fbf 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -1288,7 +1288,8 @@ public: static pybind11::dtype dtype() { list shape; array_info::append_extents(shape); - return pybind11::dtype::from_args(pybind11::make_tuple(base_descr::dtype(), shape)); + return pybind11::dtype::from_args( + pybind11::make_tuple(base_descr::dtype(), std::move(shape))); } }; diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index e54941485..b9ec8af4e 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1574,7 +1574,7 @@ public: void (*destructor)(PyObject *) = nullptr) : object(PyCapsule_New(const_cast(value), name, destructor), stolen_t{}) { if (!m_ptr) { - pybind11_fail("Could not allocate capsule object!"); + throw error_already_set(); } } @@ -1582,34 +1582,42 @@ public: capsule(const void *value, void (*destruct)(PyObject *)) : object(PyCapsule_New(const_cast(value), nullptr, destruct), stolen_t{}) { if (!m_ptr) { - pybind11_fail("Could not allocate capsule object!"); + throw error_already_set(); } } capsule(const void *value, void (*destructor)(void *)) { m_ptr = PyCapsule_New(const_cast(value), nullptr, [](PyObject *o) { auto destructor = reinterpret_cast(PyCapsule_GetContext(o)); + if (destructor == nullptr) { + if (PyErr_Occurred()) { + throw error_already_set(); + } + pybind11_fail("Unable to get capsule context"); + } void *ptr = PyCapsule_GetPointer(o, nullptr); + if (ptr == nullptr) { + throw error_already_set(); + } destructor(ptr); }); - if (!m_ptr) { - pybind11_fail("Could not allocate capsule object!"); - } - - if (PyCapsule_SetContext(m_ptr, (void *) destructor) != 0) { - pybind11_fail("Could not set capsule context!"); + if (!m_ptr || PyCapsule_SetContext(m_ptr, (void *) destructor) != 0) { + throw error_already_set(); } } explicit capsule(void (*destructor)()) { m_ptr = PyCapsule_New(reinterpret_cast(destructor), nullptr, [](PyObject *o) { auto destructor = reinterpret_cast(PyCapsule_GetPointer(o, nullptr)); + if (destructor == nullptr) { + throw error_already_set(); + } destructor(); }); if (!m_ptr) { - pybind11_fail("Could not allocate capsule object!"); + throw error_already_set(); } } @@ -1624,8 +1632,7 @@ public: const auto *name = this->name(); T *result = static_cast(PyCapsule_GetPointer(m_ptr, name)); if (!result) { - PyErr_Clear(); - pybind11_fail("Unable to extract capsule contents!"); + throw error_already_set(); } return result; } @@ -1633,8 +1640,7 @@ public: /// Replaces a capsule's pointer *without* calling the destructor on the existing one. void set_pointer(const void *value) { if (PyCapsule_SetPointer(m_ptr, const_cast(value)) != 0) { - PyErr_Clear(); - pybind11_fail("Could not set capsule pointer"); + throw error_already_set(); } } From 461937d3e545136f48adc08c8cb80dff4c5994cf Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Fri, 25 Mar 2022 12:34:32 -0400 Subject: [PATCH 3/6] ci: test pypy 3.9 (#3789) * ci: test pypy 3.9 * ci: try a use of FindPython with PyPy --- .github/workflows/ci.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3523e82c0..78f39b0c1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,6 +32,7 @@ jobs: - '3.10' - 'pypy-3.7' - 'pypy-3.8' + - 'pypy-3.9' # Items in here will either be added to the build matrix (if not # present), or add new keys to an existing matrix element if all the @@ -45,6 +46,10 @@ jobs: args: > -DPYBIND11_FINDPYTHON=ON -DCMAKE_CXX_FLAGS="-D_=1" + - runs-on: ubuntu-latest + python: 'pypy-3.9' + args: > + -DPYBIND11_FINDPYTHON=ON - runs-on: windows-2019 python: '3.6' args: > From 7742be02d9d1231c7b541c44a32c4f0921c6f7f9 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Fri, 25 Mar 2022 14:54:43 -0400 Subject: [PATCH 4/6] Revert "ci: test pypy 3.9" (#3828) * Revert "ci: test pypy 3.9 (#3789)" This reverts commit 461937d3e545136f48adc08c8cb80dff4c5994cf. * Update ci.yml --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 78f39b0c1..b2fc52bf2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,7 +47,7 @@ jobs: -DPYBIND11_FINDPYTHON=ON -DCMAKE_CXX_FLAGS="-D_=1" - runs-on: ubuntu-latest - python: 'pypy-3.9' + python: 'pypy-3.8' args: > -DPYBIND11_FINDPYTHON=ON - runs-on: windows-2019 From 3a183d4b587841fb868cee23df7bfe0afe893bb8 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Fri, 25 Mar 2022 16:01:34 -0400 Subject: [PATCH 5/6] fix: improve str exceptions and consistency with python (#3826) * Improve str exceptions * Revert macro change just in case * Make clang-tidy happy * Fix one more clang-tidy issue * Refactor duplicate method --- include/pybind11/pytypes.h | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index b9ec8af4e..18cd71580 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1243,8 +1243,8 @@ public: } char *buffer = nullptr; ssize_t length = 0; - if (PYBIND11_BYTES_AS_STRING_AND_SIZE(temp.ptr(), &buffer, &length)) { - pybind11_fail("Unable to extract string contents! (invalid type)"); + if (PyBytes_AsStringAndSize(temp.ptr(), &buffer, &length) != 0) { + throw error_already_set(); } return std::string(buffer, (size_t) length); } @@ -1299,14 +1299,7 @@ public: explicit bytes(const pybind11::str &s); // NOLINTNEXTLINE(google-explicit-constructor) - operator std::string() const { - char *buffer = nullptr; - ssize_t length = 0; - if (PYBIND11_BYTES_AS_STRING_AND_SIZE(m_ptr, &buffer, &length)) { - pybind11_fail("Unable to extract bytes contents!"); - } - return std::string(buffer, (size_t) length); - } + operator std::string() const { return string_op(); } #ifdef PYBIND11_HAS_STRING_VIEW // enable_if is needed to avoid "ambiguous conversion" errors (see PR #3521). @@ -1318,15 +1311,18 @@ public: // valid so long as the `bytes` instance remains alive and so generally should not outlive the // lifetime of the `bytes` instance. // NOLINTNEXTLINE(google-explicit-constructor) - operator std::string_view() const { + operator std::string_view() const { return string_op(); } +#endif +private: + template + T string_op() const { char *buffer = nullptr; ssize_t length = 0; - if (PYBIND11_BYTES_AS_STRING_AND_SIZE(m_ptr, &buffer, &length)) { - pybind11_fail("Unable to extract bytes contents!"); + if (PyBytes_AsStringAndSize(m_ptr, &buffer, &length) != 0) { + throw error_already_set(); } return {buffer, static_cast(length)}; } -#endif }; // Note: breathe >= 4.17.0 will fail to build docs if the below two constructors // are included in the doxygen group; close here and reopen after as a workaround @@ -1337,13 +1333,13 @@ inline bytes::bytes(const pybind11::str &s) { if (PyUnicode_Check(s.ptr())) { temp = reinterpret_steal(PyUnicode_AsUTF8String(s.ptr())); if (!temp) { - pybind11_fail("Unable to extract string contents! (encoding issue)"); + throw error_already_set(); } } char *buffer = nullptr; ssize_t length = 0; - if (PYBIND11_BYTES_AS_STRING_AND_SIZE(temp.ptr(), &buffer, &length)) { - pybind11_fail("Unable to extract string contents! (invalid type)"); + if (PyBytes_AsStringAndSize(temp.ptr(), &buffer, &length) != 0) { + throw error_already_set(); } auto obj = reinterpret_steal(PYBIND11_BYTES_FROM_STRING_AND_SIZE(buffer, length)); if (!obj) { @@ -1355,8 +1351,8 @@ inline bytes::bytes(const pybind11::str &s) { inline str::str(const bytes &b) { char *buffer = nullptr; ssize_t length = 0; - if (PYBIND11_BYTES_AS_STRING_AND_SIZE(b.ptr(), &buffer, &length)) { - pybind11_fail("Unable to extract bytes contents!"); + if (PyBytes_AsStringAndSize(b.ptr(), &buffer, &length) != 0) { + throw error_already_set(); } auto obj = reinterpret_steal(PyUnicode_FromStringAndSize(buffer, length)); if (!obj) { From 42d8593ad4225a634b481cd573f7aeb94de72418 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 28 Mar 2022 17:24:59 -0400 Subject: [PATCH 6/6] style: bump black (#3831) * style: bump black Signed-off-by: Henry Schreiner * fix: pycln broken by typer breakage --- .pre-commit-config.yaml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 01a21e98d..6a00f2fce 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -45,7 +45,7 @@ repos: # Black, the code formatter, natively supports pre-commit - repo: https://github.com/psf/black - rev: "22.1.0" # Keep in sync with blacken-docs + rev: "22.3.0" # Keep in sync with blacken-docs hooks: - id: black @@ -55,7 +55,7 @@ repos: hooks: - id: blacken-docs additional_dependencies: - - black==22.1.0 # keep in sync with black hook + - black==22.3.0 # keep in sync with black hook # Changes tabs to spaces - repo: https://github.com/Lucas-C/pre-commit-hooks @@ -74,6 +74,8 @@ repos: rev: "v1.2.5" hooks: - id: pycln + additional_dependencies: [click<8.1] # Unpin when typer updates + stages: [manual] # Checking for common mistakes - repo: https://github.com/pre-commit/pygrep-hooks @@ -106,7 +108,7 @@ repos: # PyLint has native support - not always usable, but works for us - repo: https://github.com/PyCQA/pylint - rev: "v2.12.2" + rev: "v2.13.2" hooks: - id: pylint files: ^pybind11 @@ -122,7 +124,7 @@ repos: # Check static types with mypy - repo: https://github.com/pre-commit/mirrors-mypy - rev: "v0.941" + rev: "v0.942" hooks: - id: mypy args: [--show-error-codes]