From f2907651fadc54c89492d4da03a120faa7b9260d Mon Sep 17 00:00:00 2001 From: Boris Dalstein Date: Sat, 12 Oct 2024 05:33:13 +0200 Subject: [PATCH 1/3] Fix #5399: iterator increment operator does not skip first item (#5400) * Fix #5399: iterator increment operator does not skip first item * Fix postfix increment operator: init() must be called before copying *this --- include/pybind11/pytypes.h | 20 +++++++++++++++----- tests/test_pytypes.cpp | 12 ++++++++++++ tests/test_pytypes.py | 5 +++++ 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 7aafab6dcc..027e36098b 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1470,11 +1470,17 @@ class iterator : public object { PYBIND11_OBJECT_DEFAULT(iterator, object, PyIter_Check) iterator &operator++() { + init(); advance(); return *this; } iterator operator++(int) { + // Note: We must call init() first so that rv.value is + // the same as this->value just before calling advance(). + // Otherwise, dereferencing the returned iterator may call + // advance() again and return the 3rd item instead of the 1st. + init(); auto rv = *this; advance(); return rv; @@ -1482,15 +1488,12 @@ class iterator : public object { // NOLINTNEXTLINE(readability-const-return-type) // PR #3263 reference operator*() const { - if (m_ptr && !value.ptr()) { - auto &self = const_cast(*this); - self.advance(); - } + init(); return value; } pointer operator->() const { - operator*(); + init(); return &value; } @@ -1513,6 +1516,13 @@ class iterator : public object { friend bool operator!=(const iterator &a, const iterator &b) { return a->ptr() != b->ptr(); } private: + void init() const { + if (m_ptr && !value.ptr()) { + auto &self = const_cast(*this); + self.advance(); + } + } + void advance() { value = reinterpret_steal(PyIter_Next(m_ptr)); if (value.ptr() == nullptr && PyErr_Occurred()) { diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 19f65ce7eb..8df4cdd3f6 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -150,6 +150,18 @@ TEST_SUBMODULE(pytypes, m) { m.def("get_iterator", [] { return py::iterator(); }); // test_iterable m.def("get_iterable", [] { return py::iterable(); }); + m.def("get_first_item_from_iterable", [](const py::iterable &iter) { + // This tests the postfix increment operator + py::iterator it = iter.begin(); + py::iterator it2 = it++; + return *it2; + }); + m.def("get_second_item_from_iterable", [](const py::iterable &iter) { + // This tests the prefix increment operator + py::iterator it = iter.begin(); + ++it; + return *it; + }); m.def("get_frozenset_from_iterable", [](const py::iterable &iter) { return py::frozenset(iter); }); m.def("get_list_from_iterable", [](const py::iterable &iter) { return py::list(iter); }); diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 39d0b619b8..9fd24b34f1 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -52,6 +52,11 @@ def test_from_iterable(pytype, from_iter_func): def test_iterable(doc): assert doc(m.get_iterable) == "get_iterable() -> Iterable" + lins = [1, 2, 3] + i = m.get_first_item_from_iterable(lins) + assert i == 1 + i = m.get_second_item_from_iterable(lins) + assert i == 2 def test_float(doc): From 077e49fcd6d6e38bbde63b3b824b02487209e9fa Mon Sep 17 00:00:00 2001 From: cyyever Date: Sat, 12 Oct 2024 11:36:41 +0800 Subject: [PATCH 2/3] Export libc++ exceptions (#5390) * Export libc++ exceptions * Remove emscripten limit * Remove __apple_build_version__ condition from PYBIND11_EXPORT_EXCEPTION * Add a comment --- include/pybind11/detail/common.h | 19 +++++++++++-------- tests/test_exceptions.py | 2 +- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index fa47199221..c974d89959 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -164,14 +164,6 @@ # endif #endif -#if !defined(PYBIND11_EXPORT_EXCEPTION) -# if defined(__apple_build_version__) -# define PYBIND11_EXPORT_EXCEPTION PYBIND11_EXPORT -# else -# define PYBIND11_EXPORT_EXCEPTION -# endif -#endif - // For CUDA, GCC7, GCC8: // PYBIND11_NOINLINE_FORCED is incompatible with `-Wattributes -Werror`. // When defining PYBIND11_NOINLINE_FORCED, it is best to also use `-Wno-attributes`. @@ -329,6 +321,17 @@ PYBIND11_WARNING_POP # endif #endif +// For libc++, the exceptions should be exported, +// otherwise, the exception translation would be incorrect. +// IMPORTANT: This code block must stay BELOW the #include above (see PR #5390). +#if !defined(PYBIND11_EXPORT_EXCEPTION) +# if defined(_LIBCPP_EXCEPTION) +# define PYBIND11_EXPORT_EXCEPTION PYBIND11_EXPORT +# else +# define PYBIND11_EXPORT_EXCEPTION +# endif +#endif + // Must be after including or one of the other headers specified by the standard #if defined(__cpp_lib_char8_t) && __cpp_lib_char8_t >= 201811L # define PYBIND11_HAS_U8STRING diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 21449d58ce..47214a7029 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -76,7 +76,7 @@ def test_cross_module_exceptions(msg): # TODO: FIXME @pytest.mark.xfail( - "env.MACOS and (env.PYPY or pybind11_tests.compiler_info.startswith('Homebrew Clang')) or sys.platform.startswith('emscripten')", + "env.MACOS and env.PYPY", raises=RuntimeError, reason="See Issue #2847, PR #2999, PR #4324", ) From f7e14e985be167ca158fd3ee2fe5d8a4f175fa87 Mon Sep 17 00:00:00 2001 From: Francesco Ballarin Date: Sat, 12 Oct 2024 20:19:50 +0200 Subject: [PATCH 3/3] Address regression introduced in #5381 (#5396) * Incomplete attempt to address regression introduced in #5381 * style: pre-commit fixes * Revert "style: pre-commit fixes" This reverts commit 9d107d2f751d76b2c26e90cd08d2ac163022c873. * Revert "Incomplete attempt to address regression introduced in #5381" This reverts commit 8cf1cdbc96ac326ff6d64a36ea291472f74c016f. * Simpler fix for the regression introduced in #5381 * style: pre-commit fixes * Added if constexpr workaround This can probably be done better but at least this is a start. * style: pre-commit fixes * Replace if constexpr with template struct if constexpr was not added until C++ 17. I think this should do the same thing as before. * style: pre-commit fixes * Made comment clearer * Added test cases * style: pre-commit fixes * Fixed is_same_or_base_of reference * style: pre-commit fixes * Added static assert messages * style: pre-commit fixes * Replaced typedef with using * style: pre-commit fixes * Back out `ForwardClassPtr` (to be discussed separately). Tested locally with clang-tidy. * Shuffle new `static_assert()` and leave error messages blank (they are more distracting than helpful here). --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: gentlegiantJGC Co-authored-by: Ralf W. Grosse-Kunstleve --- include/pybind11/cast.h | 15 ++++++++++++--- tests/test_class.cpp | 16 ++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index c44ff29d3d..f6a7e83be8 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1564,15 +1564,24 @@ struct function_call { handle init_self; }; +// See PR #5396 for the discussion that led to this +template +struct is_same_or_base_of : std::is_same {}; + +// Only evaluate is_base_of if Derived is complete. +// is_base_of raises a compiler error if Derived is incomplete. +template +struct is_same_or_base_of + : any_of, std::is_base_of> {}; + /// Helper class which loads arguments for C++ functions called from Python template class argument_loader { using indices = make_index_sequence; - template - using argument_is_args = std::is_base_of>; + using argument_is_args = is_same_or_base_of>; template - using argument_is_kwargs = std::is_base_of>; + using argument_is_kwargs = is_same_or_base_of>; // Get kwargs argument position, or -1 if not present: static constexpr auto kwargs_pos = constexpr_last(); diff --git a/tests/test_class.cpp b/tests/test_class.cpp index 9001d86b19..cb84c327a0 100644 --- a/tests/test_class.cpp +++ b/tests/test_class.cpp @@ -52,8 +52,24 @@ void bind_empty0(py::module_ &m) { } } // namespace pr4220_tripped_over_this + +namespace pr5396_forward_declared_class { +class ForwardClass; +class Args : public py::args {}; +} // namespace pr5396_forward_declared_class + } // namespace test_class +static_assert(py::detail::is_same_or_base_of::value, ""); +static_assert( + py::detail::is_same_or_base_of::value, + ""); +static_assert(!py::detail::is_same_or_base_of< + py::args, + test_class::pr5396_forward_declared_class::ForwardClass>::value, + ""); + TEST_SUBMODULE(class_, m) { m.def("obj_class_name", [](py::handle obj) { return py::detail::obj_class_name(obj.ptr()); });