diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d512d88664..2c9fa50bef 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -325,8 +325,8 @@ jobs: # Testing NVCC; forces sources to behave like .cu files cuda: runs-on: ubuntu-latest - name: "🐍 3.8 • CUDA 11.2 • Ubuntu 20.04" - container: nvidia/cuda:11.2.2-devel-ubuntu20.04 + name: "🐍 3.10 • CUDA 11.7 • Ubuntu 22.04" + container: nvidia/cuda:11.7.0-devel-ubuntu22.04 steps: - uses: actions/checkout@v3 @@ -738,6 +738,9 @@ jobs: args: -DCMAKE_CXX_STANDARD=20 - python: 3.8 args: -DCMAKE_CXX_STANDARD=17 + - python: 3.7 + args: -DCMAKE_CXX_STANDARD=14 + name: "🐍 ${{ matrix.python }} • MSVC 2019 • x86 ${{ matrix.args }}" runs-on: windows-2019 diff --git a/.github/workflows/ci_sh_def.yml b/.github/workflows/ci_sh_def.yml index 5189053770..6942e0f5fd 100644 --- a/.github/workflows/ci_sh_def.yml +++ b/.github/workflows/ci_sh_def.yml @@ -342,8 +342,8 @@ jobs: # Testing NVCC; forces sources to behave like .cu files cuda: runs-on: ubuntu-latest - name: "🐍 3.8 • CUDA 11.2 • Ubuntu 20.04" - container: nvidia/cuda:11.2.2-devel-ubuntu20.04 + name: "🐍 3.10 • CUDA 11.7 • Ubuntu 22.04" + container: nvidia/cuda:11.7.0-devel-ubuntu22.04 steps: - uses: actions/checkout@v3 @@ -761,6 +761,9 @@ jobs: args: -DCMAKE_CXX_STANDARD=20 - python: 3.8 args: -DCMAKE_CXX_STANDARD=17 + - python: 3.7 + args: -DCMAKE_CXX_STANDARD=14 + name: "🐍 ${{ matrix.python }} • MSVC 2019 • x86 ${{ matrix.args }}" runs-on: windows-2019 diff --git a/.github/workflows/ci_sh_def.yml.patch b/.github/workflows/ci_sh_def.yml.patch index b0477b7f88..3edd89475d 100644 --- a/.github/workflows/ci_sh_def.yml.patch +++ b/.github/workflows/ci_sh_def.yml.patch @@ -1,5 +1,5 @@ ---- ci.yml 2022-09-15 05:25:58.998556736 -0700 -+++ ci_sh_def.yml 2022-09-15 05:36:11.236234132 -0700 +--- ci.yml 2022-10-09 21:54:56.682792329 -0700 ++++ ci_sh_def.yml 2022-10-09 21:56:30.413425061 -0700 @@ -1,4 +1,16 @@ -name: CI +# PLEASE KEEP THIS GROUP OF FILES IN SYNC AT ALL TIMES: @@ -126,7 +126,7 @@ -DPYTHON_EXECUTABLE=$(python3 -c "import sys; print(sys.executable)") working-directory: /build-tests -@@ -771,6 +794,7 @@ +@@ -774,6 +797,7 @@ -DPYBIND11_WERROR=ON -DDOWNLOAD_CATCH=ON -DDOWNLOAD_EIGEN=ON @@ -134,7 +134,7 @@ ${{ matrix.args }} - name: Build C++11 run: cmake --build build -j 2 -@@ -825,6 +849,7 @@ +@@ -828,6 +852,7 @@ -DPYBIND11_WERROR=ON -DDOWNLOAD_CATCH=ON -DDOWNLOAD_EIGEN=ON @@ -142,7 +142,7 @@ ${{ matrix.args }} - name: Build C++11 run: cmake --build build --config Debug -j 2 -@@ -865,6 +890,7 @@ +@@ -868,6 +893,7 @@ -DDOWNLOAD_CATCH=ON -DDOWNLOAD_EIGEN=ON -DCMAKE_CXX_STANDARD=20 @@ -150,7 +150,7 @@ - name: Build C++20 run: cmake --build build -j 2 -@@ -912,7 +938,7 @@ +@@ -915,7 +941,7 @@ - name: Configure C++11 # LTO leads to many undefined reference like # `pybind11::detail::function_call::function_call(pybind11::detail::function_call&&) @@ -159,7 +159,7 @@ - name: Build C++11 run: cmake --build build -j 2 -@@ -930,7 +956,7 @@ +@@ -933,7 +959,7 @@ run: git clean -fdx - name: Configure C++14 @@ -168,7 +168,7 @@ - name: Build C++14 run: cmake --build build2 -j 2 -@@ -948,7 +974,7 @@ +@@ -951,7 +977,7 @@ run: git clean -fdx - name: Configure C++17 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 47426da4d6..d02650f1fa 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -12,6 +12,12 @@ # # See https://github.com/pre-commit/pre-commit + +ci: + autoupdate_commit_msg: "chore(deps): update pre-commit hooks" + autofix_commit_msg: "style: pre-commit fixes" + autoupdate_schedule: monthly + # third-party content exclude: ^tools/JoinPaths.cmake$ @@ -36,7 +42,7 @@ repos: # Upgrade old Python syntax - repo: https://github.com/asottile/pyupgrade - rev: "v2.37.3" + rev: "v2.38.2" hooks: - id: pyupgrade args: [--py36-plus] @@ -113,7 +119,7 @@ repos: # PyLint has native support - not always usable, but works for us - repo: https://github.com/PyCQA/pylint - rev: "v2.15.2" + rev: "v2.15.3" hooks: - id: pylint files: ^pybind11 @@ -129,7 +135,7 @@ repos: # Check static types with mypy - repo: https://github.com/pre-commit/mirrors-mypy - rev: "v0.971" + rev: "v0.981" hooks: - id: mypy args: [] diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 4f694970f8..7de9197fcb 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1586,7 +1586,7 @@ class unpacking_collector { throw cast_error_unable_to_convert_call_arg(a.name, a.type); #endif } - m_kwargs[a.name] = a.value; + m_kwargs[a.name] = std::move(a.value); } void process(list & /*args_list*/, detail::kwargs_proxy kp) { diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 285c6258ff..df13f630d7 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -1045,12 +1045,7 @@ PYBIND11_NAMESPACE_END(detail) /// - regular: static_cast(&Class::func) /// - sweet: overload_cast(&Class::func) template -# if (defined(_MSC_VER) && _MSC_VER < 1920) /* MSVC 2017 */ \ - || (defined(__clang__) && __clang_major__ == 5) -static constexpr detail::overload_cast_impl overload_cast = {}; -# else -static constexpr detail::overload_cast_impl overload_cast; -# endif +static constexpr detail::overload_cast_impl overload_cast{}; #endif /// Const member function selector for overload_cast diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index a2c1804e51..4a771a857a 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -423,6 +423,8 @@ PYBIND11_NOINLINE internals &get_internals() { // Cannot use py::gil_scoped_acquire here since that constructor calls get_internals. struct gil_scoped_acquire_local { gil_scoped_acquire_local() : state(PyGILState_Ensure()) {} + gil_scoped_acquire_local(const gil_scoped_acquire_local &) = delete; + gil_scoped_acquire_local &operator=(const gil_scoped_acquire_local &) = delete; ~gil_scoped_acquire_local() { PyGILState_Release(state); } const PyGILState_STATE state; } gil; @@ -523,8 +525,13 @@ struct local_internals { /// Works like `get_internals`, but for things which are locally registered. inline local_internals &get_local_internals() { - static local_internals locals; - return locals; + // Current static can be created in the interpreter finalization routine. If the later will be + // destroyed in another static variable destructor, creation of this static there will cause + // static deinitialization fiasco. In order to avoid it we avoid destruction of the + // local_internals static. One can read more about the problem and current solution here: + // https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables + static auto *locals = new local_internals(); + return *locals; } /// Constructs a std::string with the given arguments, stores it in `internals`, and returns its diff --git a/include/pybind11/gil.h b/include/pybind11/gil.h index a0b5de1514..1ef5f0a8c8 100644 --- a/include/pybind11/gil.h +++ b/include/pybind11/gil.h @@ -80,6 +80,9 @@ class gil_scoped_acquire { inc_ref(); } + gil_scoped_acquire(const gil_scoped_acquire &) = delete; + gil_scoped_acquire &operator=(const gil_scoped_acquire &) = delete; + void inc_ref() { ++tstate->gilstate_counter; } PYBIND11_NOINLINE void dec_ref() { @@ -144,6 +147,9 @@ class gil_scoped_release { } } + gil_scoped_release(const gil_scoped_acquire &) = delete; + gil_scoped_release &operator=(const gil_scoped_acquire &) = delete; + /// This method will disable the PyThreadState_DeleteCurrent call and the /// GIL won't be acquired. This method should be used if the interpreter /// could be shutting down when this is called, as thread deletion is not @@ -178,6 +184,8 @@ class gil_scoped_acquire { public: gil_scoped_acquire() { state = PyGILState_Ensure(); } + gil_scoped_acquire(const gil_scoped_acquire &) = delete; + gil_scoped_acquire &operator=(const gil_scoped_acquire &) = delete; ~gil_scoped_acquire() { PyGILState_Release(state); } void disarm() {} }; @@ -187,6 +195,8 @@ class gil_scoped_release { public: gil_scoped_release() { state = PyEval_SaveThread(); } + gil_scoped_release(const gil_scoped_release &) = delete; + gil_scoped_release &operator=(const gil_scoped_acquire &) = delete; ~gil_scoped_release() { PyEval_RestoreThread(state); } void disarm() {} }; diff --git a/include/pybind11/operators.h b/include/pybind11/operators.h index a0c3b78d65..16a88ae171 100644 --- a/include/pybind11/operators.h +++ b/include/pybind11/operators.h @@ -84,6 +84,7 @@ struct op_impl {}; /// Operator implementation generator template struct op_ { + static constexpr bool op_enable_if_hook = true; template void execute(Class &cl, const Extra &...extra) const { using Base = typename Class::type; diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 23dc6a48d5..3f1a1f44e2 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1808,14 +1808,14 @@ class class_ : public detail::generic_type { return *this; } - template - class_ &def(const detail::op_ &op, const Extra &...extra) { + template = 0> + class_ &def(const T &op, const Extra &...extra) { op.execute(*this, extra...); return *this; } - template - class_ &def_cast(const detail::op_ &op, const Extra &...extra) { + template = 0> + class_ &def_cast(const T &op, const Extra &...extra) { op.execute_cast(*this, extra...); return *this; } diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index f4ba459f20..29506b7eaf 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1829,18 +1829,18 @@ class capsule : public object { // guard if destructor called while err indicator is set error_scope error_guard; auto destructor = reinterpret_cast(PyCapsule_GetContext(o)); - if (destructor == nullptr) { - if (PyErr_Occurred()) { - throw error_already_set(); - } - pybind11_fail("Unable to get capsule context"); + if (PyErr_Occurred()) { + throw error_already_set(); } const char *name = get_name_in_error_scope(o); void *ptr = PyCapsule_GetPointer(o, name); if (ptr == nullptr) { throw error_already_set(); } - destructor(ptr); + + if (destructor != nullptr) { + destructor(ptr); + } }); if (!m_ptr || PyCapsule_SetContext(m_ptr, (void *) destructor) != 0) { @@ -2022,14 +2022,20 @@ class list : public object { detail::list_iterator end() const { return {*this, PyList_GET_SIZE(m_ptr)}; } template void append(T &&val) /* py-non-const */ { - PyList_Append(m_ptr, detail::object_or_cast(std::forward(val)).ptr()); + if (PyList_Append(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) != 0) { + throw error_already_set(); + } } template ::value, int> = 0> void insert(const IdxType &index, ValType &&val) /* py-non-const */ { - PyList_Insert( - m_ptr, ssize_t_cast(index), detail::object_or_cast(std::forward(val)).ptr()); + if (PyList_Insert(m_ptr, + ssize_t_cast(index), + detail::object_or_cast(std::forward(val)).ptr()) + != 0) { + throw error_already_set(); + } } }; diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index d73e40ede9..66ff59e517 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -49,17 +49,31 @@ constexpr forwarded_type forward_like(U &&u) { return std::forward>(std::forward(u)); } +// Checks if a container has a STL style reserve method. +// This will only return true for a `reserve()` with a `void` return. +template +using has_reserve_method = std::is_same().reserve(0)), void>; + template struct set_caster { using type = Type; using key_conv = make_caster; +private: + template ::value, int> = 0> + void reserve_maybe(const anyset &s, Type *) { + value.reserve(s.size()); + } + void reserve_maybe(const anyset &, void *) {} + +public: bool load(handle src, bool convert) { if (!isinstance(src)) { return false; } auto s = reinterpret_borrow(src); value.clear(); + reserve_maybe(s, &value); for (auto entry : s) { key_conv conv; if (!conv.load(entry, convert)) { @@ -94,12 +108,21 @@ struct map_caster { using key_conv = make_caster; using value_conv = make_caster; +private: + template ::value, int> = 0> + void reserve_maybe(const dict &d, Type *) { + value.reserve(d.size()); + } + void reserve_maybe(const dict &, void *) {} + +public: bool load(handle src, bool convert) { if (!isinstance(src)) { return false; } auto d = reinterpret_borrow(src); value.clear(); + reserve_maybe(d, &value); for (auto it : d) { key_conv kconv; value_conv vconv; @@ -160,9 +183,7 @@ struct list_caster { } private: - template < - typename T = Type, - enable_if_t().reserve(0)), void>::value, int> = 0> + template ::value, int> = 0> void reserve_maybe(const sequence &s, Type *) { value.reserve(s.size()); } diff --git a/tests/test_class.cpp b/tests/test_class.cpp index b91432a2c1..90fe4d5650 100644 --- a/tests/test_class.cpp +++ b/tests/test_class.cpp @@ -49,6 +49,26 @@ struct SamePointer {}; } // namespace +namespace test_class { +namespace pr4220_tripped_over_this { // PR #4227 + +template +struct SoEmpty {}; + +template +std::string get_msg(const T &) { + return "This is really only meant to exercise successful compilation."; +} + +using Empty0 = SoEmpty<0x0>; + +void bind_empty0(py::module_ &m) { + py::class_(m, "Empty0").def(py::init<>()).def("get_msg", get_msg); +} + +} // namespace pr4220_tripped_over_this +} // namespace test_class + PYBIND11_TYPE_CASTER_BASE_HOLDER(MismatchBase1, std::shared_ptr) PYBIND11_TYPE_CASTER_BASE_HOLDER(MismatchDerived1, std::unique_ptr) PYBIND11_TYPE_CASTER_BASE_HOLDER(MismatchBase2, std::unique_ptr) @@ -530,6 +550,8 @@ TEST_SUBMODULE(class_, m) { py::class_(gt, "OtherDuplicateNested"); py::class_(gt, "YetAnotherDuplicateNested"); }); + + test_class::pr4220_tripped_over_this::bind_empty0(m); } template diff --git a/tests/test_class.py b/tests/test_class.py index ff9196f0f2..47ba450fc2 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -469,3 +469,10 @@ class ClassScope: m.register_duplicate_nested_class_type(ClassScope) expected = 'generic_type: type "YetAnotherDuplicateNested" is already registered!' assert str(exc_info.value) == expected + + +def test_pr4220_tripped_over_this(): + assert ( + m.Empty0().get_msg() + == "This is really only meant to exercise successful compilation." + ) diff --git a/tests/test_class_sh_shared_ptr_copy_move.cpp b/tests/test_class_sh_shared_ptr_copy_move.cpp index a803d49f8f..1d4ec3cdf7 100644 --- a/tests/test_class_sh_shared_ptr_copy_move.cpp +++ b/tests/test_class_sh_shared_ptr_copy_move.cpp @@ -48,7 +48,6 @@ PYBIND11_TYPE_CASTER_BASE_HOLDER(pybind11_tests::FooShPtr, PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::FooSmHld) namespace pybind11_tests { -namespace { TEST_SUBMODULE(class_sh_shared_ptr_copy_move, m) { namespace py = pybind11; @@ -110,5 +109,4 @@ TEST_SUBMODULE(class_sh_shared_ptr_copy_move, m) { }); } -} // namespace } // namespace pybind11_tests diff --git a/tests/test_eigen.cpp b/tests/test_eigen.cpp index 591dacc624..b0c7bdb48a 100644 --- a/tests/test_eigen.cpp +++ b/tests/test_eigen.cpp @@ -197,11 +197,40 @@ TEST_SUBMODULE(eigen, m) { // Return a block of a matrix (gives non-standard strides) m.def("block", - [](const Eigen::Ref &x, - int start_row, - int start_col, - int block_rows, - int block_cols) { return x.block(start_row, start_col, block_rows, block_cols); }); + [m](const py::object &x_obj, + int start_row, + int start_col, + int block_rows, + int block_cols) { + return m.attr("_block")(x_obj, x_obj, start_row, start_col, block_rows, block_cols); + }); + + m.def( + "_block", + [](const py::object &x_obj, + const Eigen::Ref &x, + int start_row, + int start_col, + int block_rows, + int block_cols) { + // See PR #4217 for background. This test is a bit over the top, but might be useful + // as a concrete example to point to when explaining the dangling reference trap. + auto i0 = py::make_tuple(0, 0); + auto x0_orig = x_obj[*i0].cast(); + if (x(0, 0) != x0_orig) { + throw std::runtime_error( + "Something in the type_caster for Eigen::Ref is terribly wrong."); + } + double x0_mod = x0_orig + 1; + x_obj[*i0] = x0_mod; + auto copy_detected = (x(0, 0) != x0_mod); + x_obj[*i0] = x0_orig; + if (copy_detected) { + throw std::runtime_error("type_caster for Eigen::Ref made a copy."); + } + return x.block(start_row, start_col, block_rows, block_cols); + }, + py::keep_alive<0, 1>()); // test_eigen_return_references, test_eigen_keepalive // return value referencing/copying tests: diff --git a/tests/test_eigen.py b/tests/test_eigen.py index 9c6485de3c..713432a815 100644 --- a/tests/test_eigen.py +++ b/tests/test_eigen.py @@ -217,15 +217,24 @@ def test_negative_stride_from_python(msg): ) +def test_block_runtime_error_type_caster_eigen_ref_made_a_copy(): + with pytest.raises(RuntimeError) as excinfo: + m.block(ref, 0, 0, 0, 0) + assert str(excinfo.value) == "type_caster for Eigen::Ref made a copy." + + def test_nonunit_stride_to_python(): assert np.all(m.diagonal(ref) == ref.diagonal()) assert np.all(m.diagonal_1(ref) == ref.diagonal(1)) for i in range(-5, 7): assert np.all(m.diagonal_n(ref, i) == ref.diagonal(i)), f"m.diagonal_n({i})" - assert np.all(m.block(ref, 2, 1, 3, 3) == ref[2:5, 1:4]) - assert np.all(m.block(ref, 1, 4, 4, 2) == ref[1:, 4:]) - assert np.all(m.block(ref, 1, 4, 3, 2) == ref[1:4, 4:]) + # Must be order="F", otherwise the type_caster will make a copy and + # m.block() will return a dangling reference (heap-use-after-free). + rof = np.asarray(ref, order="F") + assert np.all(m.block(rof, 2, 1, 3, 3) == rof[2:5, 1:4]) + assert np.all(m.block(rof, 1, 4, 4, 2) == rof[1:, 4:]) + assert np.all(m.block(rof, 1, 4, 3, 2) == rof[1:4, 4:]) def test_eigen_ref_to_python(): diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index da0dc8f6ba..c95ff8230a 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -289,6 +289,12 @@ TEST_SUBMODULE(pytypes, m) { return capsule; }); + m.def("return_capsule_with_explicit_nullptr_dtor", []() { + py::print("creating capsule with explicit nullptr dtor"); + return py::capsule(reinterpret_cast(1234), + static_cast(nullptr)); // PR #4221 + }); + // test_accessors m.def("accessor_api", [](const py::object &o) { auto d = py::dict(); diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index a34eaa59e8..070849fc55 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -299,6 +299,17 @@ def test_capsule(capture): """ ) + with capture: + a = m.return_capsule_with_explicit_nullptr_dtor() + del a + pytest.gc_collect() + assert ( + capture.unordered + == """ + creating capsule with explicit nullptr dtor + """ + ) + def test_accessors(): class SubTestObject: diff --git a/tests/test_type_caster_odr_guard_1.cpp b/tests/test_type_caster_odr_guard_1.cpp index 6fcb11528f..80deb04781 100644 --- a/tests/test_type_caster_odr_guard_1.cpp +++ b/tests/test_type_caster_odr_guard_1.cpp @@ -82,4 +82,12 @@ TEST_SUBMODULE(type_caster_odr_guard_1, m) { #else false; #endif + + m.attr("CUDACC") = +#if defined(__CUDACC_VER_MAJOR__) + PYBIND11_TOSTRING(__CUDACC_VER_MAJOR__) "." PYBIND11_TOSTRING( + __CUDACC_VER_MINOR__) "." PYBIND11_TOSTRING(__CUDACC_VER_BUILD__); +#else + py::none(); +#endif } diff --git a/tests/test_type_caster_odr_guard_1.py b/tests/test_type_caster_odr_guard_1.py index 2b2d8e3485..a88703bdd5 100644 --- a/tests/test_type_caster_odr_guard_1.py +++ b/tests/test_type_caster_odr_guard_1.py @@ -36,10 +36,15 @@ def test_type_caster_odr_violation_detected_counter(): num_violations = m.type_caster_odr_violation_detected_count() if num_violations is None: pytest.skip("type_caster_odr_violation_detected_count() is None") - elif num_violations == 0 and m.if_defined__NO_INLINE__: - pytest.skip( - "type_caster_odr_violation_detected_count() == 0: %s, %s, __NO_INLINE__" - % (pybind11_tests.compiler_info, pybind11_tests.cpp_std) - ) - else: - assert num_violations == 1 + if num_violations == 0: + if m.if_defined__NO_INLINE__: + pytest.skip( + "type_caster_odr_violation_detected_count() == 0: %s, %s, __NO_INLINE__" + % (pybind11_tests.compiler_info, pybind11_tests.cpp_std) + ) + if m.CUDACC is not None: + pytest.skip( + "type_caster_odr_violation_detected_count() == 0: %s, %s, CUDACC = %s" + % (pybind11_tests.compiler_info, pybind11_tests.cpp_std, m.CUDACC) + ) + assert num_violations == 1