diff --git a/docs/advanced/cast/overview.rst b/docs/advanced/cast/overview.rst index b0e32a52f..6341fce6d 100644 --- a/docs/advanced/cast/overview.rst +++ b/docs/advanced/cast/overview.rst @@ -151,6 +151,8 @@ as arguments and return values, refer to the section on binding :ref:`classes`. +------------------------------------+---------------------------+-------------------------------+ | ``std::variant<...>`` | Type-safe union (C++17) | :file:`pybind11/stl.h` | +------------------------------------+---------------------------+-------------------------------+ +| ``std::filesystem::path`` | STL path (C++17) [#]_ | :file:`pybind11/stl.h` | ++------------------------------------+---------------------------+-------------------------------+ | ``std::function<...>`` | STL polymorphic function | :file:`pybind11/functional.h` | +------------------------------------+---------------------------+-------------------------------+ | ``std::chrono::duration<...>`` | STL time duration | :file:`pybind11/chrono.h` | @@ -163,3 +165,7 @@ as arguments and return values, refer to the section on binding :ref:`classes`. +------------------------------------+---------------------------+-------------------------------+ | ``Eigen::SparseMatrix<...>`` | Eigen: sparse matrix | :file:`pybind11/eigen.h` | +------------------------------------+---------------------------+-------------------------------+ + +.. [#] ``std::filesystem::path`` is converted to ``pathlib.Path`` and + ``os.PathLike`` is converted to ``std::filesystem::path``, but this requires + Python 3.6 (for ``__fspath__`` support). diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 0f133c25f..b405a224c 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -610,7 +610,7 @@ protected: bool bad_arg = false; for (; args_copied < args_to_copy; ++args_copied) { const argument_record *arg_rec = args_copied < func.args.size() ? &func.args[args_copied] : nullptr; - if (kwargs_in && arg_rec && arg_rec->name && PyDict_GetItemString(kwargs_in, arg_rec->name)) { + if (kwargs_in && arg_rec && arg_rec->name && dict_getitemstring(kwargs_in, arg_rec->name)) { bad_arg = true; break; } @@ -658,7 +658,7 @@ protected: handle value; if (kwargs_in && arg_rec.name) - value = PyDict_GetItemString(kwargs.ptr(), arg_rec.name); + value = dict_getitemstring(kwargs.ptr(), arg_rec.name); if (value) { // Consume a kwargs value @@ -666,7 +666,9 @@ protected: kwargs = reinterpret_steal(PyDict_Copy(kwargs.ptr())); copied_kwargs = true; } - PyDict_DelItemString(kwargs.ptr(), arg_rec.name); + if (PyDict_DelItemString(kwargs.ptr(), arg_rec.name) == -1) { + throw error_already_set(); + } } else if (arg_rec.value) { value = arg_rec.value; } @@ -2225,7 +2227,7 @@ inline function get_type_override(const void *this_ptr, const type_info *this_ty if (frame && (std::string) str(frame->f_code->co_name) == name && frame->f_code->co_argcount > 0) { PyFrame_FastToLocals(frame); - PyObject *self_caller = PyDict_GetItem( + PyObject *self_caller = dict_getitem( frame->f_locals, PyTuple_GET_ITEM(frame->f_code->co_varnames, 0)); if (self_caller == self.ptr()) return function(); diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index d1d92af6a..80637fb98 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -485,6 +485,43 @@ inline handle get_function(handle value) { return value; } +// Reimplementation of python's dict helper functions to ensure that exceptions +// aren't swallowed (see #2862) + +// copied from cpython _PyDict_GetItemStringWithError +inline PyObject * dict_getitemstring(PyObject *v, const char *key) +{ +#if PY_MAJOR_VERSION >= 3 + PyObject *kv, *rv; + kv = PyUnicode_FromString(key); + if (kv == NULL) { + throw error_already_set(); + } + + rv = PyDict_GetItemWithError(v, kv); + Py_DECREF(kv); + if (rv == NULL && PyErr_Occurred()) { + throw error_already_set(); + } + return rv; +#else + return PyDict_GetItemString(v, key); +#endif +} + +inline PyObject * dict_getitem(PyObject *v, PyObject *key) +{ +#if PY_MAJOR_VERSION >= 3 + PyObject *rv = PyDict_GetItemWithError(v, key); + if (rv == NULL && PyErr_Occurred()) { + throw error_already_set(); + } + return rv; +#else + return PyDict_GetItem(v, key); +#endif +} + // Helper aliases/functions to support implicit casting of values given to python accessors/methods. // When given a pyobject, this simply returns the pyobject as-is; for other C++ type, the value goes // through pybind11::cast(obj) to convert it to an `object`. diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index ca20b7483..2350a5247 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -41,11 +41,21 @@ # include # define PYBIND11_HAS_VARIANT 1 # endif +// std::filesystem::path +# if defined(PYBIND11_CPP17) && __has_include() && \ + PY_VERSION_HEX >= 0x03060000 +# include +# define PYBIND11_HAS_FILESYSTEM 1 +# endif #elif defined(_MSC_VER) && defined(PYBIND11_CPP17) # include # include # define PYBIND11_HAS_OPTIONAL 1 # define PYBIND11_HAS_VARIANT 1 +# if PY_VERSION_HEX >= 0x03060000 +# include +# define PYBIND11_HAS_FILESYSTEM 1 +# endif #endif PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) @@ -377,6 +387,77 @@ template struct type_caster> : variant_caster> { }; #endif +#if defined(PYBIND11_HAS_FILESYSTEM) +template struct path_caster { + +private: + static PyObject* unicode_from_fs_native(const std::string& w) { +#if !defined(PYPY_VERSION) + return PyUnicode_DecodeFSDefaultAndSize(w.c_str(), ssize_t(w.size())); +#else + // PyPy mistakenly declares the first parameter as non-const. + return PyUnicode_DecodeFSDefaultAndSize( + const_cast(w.c_str()), ssize_t(w.size())); +#endif + } + + static PyObject* unicode_from_fs_native(const std::wstring& w) { + return PyUnicode_FromWideChar(w.c_str(), ssize_t(w.size())); + } + +public: + static handle cast(const T& path, return_value_policy, handle) { + if (auto py_str = unicode_from_fs_native(path.native())) { + return module::import("pathlib").attr("Path")(reinterpret_steal(py_str)) + .release(); + } + return nullptr; + } + + bool load(handle handle, bool) { + // PyUnicode_FSConverter and PyUnicode_FSDecoder normally take care of + // calling PyOS_FSPath themselves, but that's broken on PyPy (PyPy + // issue #3168) so we do it ourselves instead. + PyObject* buf = PyOS_FSPath(handle.ptr()); + if (!buf) { + PyErr_Clear(); + return false; + } + PyObject* native = nullptr; + if constexpr (std::is_same_v) { + if (PyUnicode_FSConverter(buf, &native)) { + if (auto c_str = PyBytes_AsString(native)) { + // AsString returns a pointer to the internal buffer, which + // must not be free'd. + value = c_str; + } + } + } else if constexpr (std::is_same_v) { + if (PyUnicode_FSDecoder(buf, &native)) { + if (auto c_str = PyUnicode_AsWideCharString(native, nullptr)) { + // AsWideCharString returns a new string that must be free'd. + value = c_str; // Copies the string. + PyMem_Free(c_str); + } + } + } + Py_XDECREF(native); + Py_DECREF(buf); + if (PyErr_Occurred()) { + PyErr_Clear(); + return false; + } else { + return true; + } + } + + PYBIND11_TYPE_CASTER(T, _("os.PathLike")); +}; + +template<> struct type_caster + : public path_caster {}; +#endif + PYBIND11_NAMESPACE_END(detail) inline std::ostream &operator<<(std::ostream &os, const handle &obj) { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f7be2075b..13556bb94 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -265,6 +265,41 @@ if(Boost_FOUND) endif() endif() +# Check if we need to add -lstdc++fs or -lc++fs or nothing +if(MSVC) + set(STD_FS_NO_LIB_NEEDED TRUE) +else() + file( + WRITE ${CMAKE_CURRENT_BINARY_DIR}/main.cpp + "#include \nint main(int argc, char ** argv) {\n std::filesystem::path p(argv[0]);\n return p.string().length();\n}" + ) + try_compile( + STD_FS_NO_LIB_NEEDED ${CMAKE_CURRENT_BINARY_DIR} + SOURCES ${CMAKE_CURRENT_BINARY_DIR}/main.cpp + COMPILE_DEFINITIONS -std=c++17) + try_compile( + STD_FS_NEEDS_STDCXXFS ${CMAKE_CURRENT_BINARY_DIR} + SOURCES ${CMAKE_CURRENT_BINARY_DIR}/main.cpp + COMPILE_DEFINITIONS -std=c++17 + LINK_LIBRARIES stdc++fs) + try_compile( + STD_FS_NEEDS_CXXFS ${CMAKE_CURRENT_BINARY_DIR} + SOURCES ${CMAKE_CURRENT_BINARY_DIR}/main.cpp + COMPILE_DEFINITIONS -std=c++17 + LINK_LIBRARIES c++fs) +endif() + +if(${STD_FS_NEEDS_STDCXXFS}) + set(STD_FS_LIB stdc++fs) +elseif(${STD_FS_NEEDS_CXXFS}) + set(STD_FS_LIB c++fs) +elseif(${STD_FS_NO_LIB_NEEDED}) + set(STD_FS_LIB "") +else() + message(WARNING "Unknown compiler - not passing -lstdc++fs") + set(STD_FS_LIB "") +endif() + # Compile with compiler warnings turned on function(pybind11_enable_warnings target_name) if(MSVC) @@ -385,6 +420,8 @@ foreach(target ${test_targets}) target_compile_definitions(${target} PRIVATE -DPYBIND11_TEST_BOOST) endif() + target_link_libraries(${target} PRIVATE ${STD_FS_LIB}) + # Always write the output file directly into the 'tests' directory (even on MSVC) if(NOT CMAKE_LIBRARY_OUTPUT_DIRECTORY) set_target_properties(${target} PROPERTIES LIBRARY_OUTPUT_DIRECTORY diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 07899b84e..7183c56b7 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -238,6 +238,12 @@ TEST_SUBMODULE(stl, m) { .def("member_initialized", &opt_exp_holder::member_initialized); #endif +#ifdef PYBIND11_HAS_FILESYSTEM + // test_fs_path + m.attr("has_filesystem") = true; + m.def("parent_path", [](const std::filesystem::path& p) { return p.parent_path(); }); +#endif + #ifdef PYBIND11_HAS_VARIANT static_assert(std::is_same::value, "visitor::result_type is required by boost::variant in C++11 mode"); diff --git a/tests/test_stl.py b/tests/test_stl.py index 330017544..96939257c 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -162,6 +162,25 @@ def test_exp_optional(): assert holder.member_initialized() +@pytest.mark.skipif(not hasattr(m, "has_filesystem"), reason="no ") +def test_fs_path(): + from pathlib import Path + + class PseudoStrPath: + def __fspath__(self): + return "foo/bar" + + class PseudoBytesPath: + def __fspath__(self): + return b"foo/bar" + + assert m.parent_path(Path("foo/bar")) == Path("foo") + assert m.parent_path("foo/bar") == Path("foo") + assert m.parent_path(b"foo/bar") == Path("foo") + assert m.parent_path(PseudoStrPath()) == Path("foo") + assert m.parent_path(PseudoBytesPath()) == Path("foo") + + @pytest.mark.skipif(not hasattr(m, "load_variant"), reason="no ") def test_variant(doc): assert m.load_variant(1) == "int"