Merge branch 'master' into smart_holder

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<T>``       | 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).

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<dict>(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();

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`.

include/pybind11/stl.h

@@ -41,11 +41,21 @@
 #    include <variant>
 #    define PYBIND11_HAS_VARIANT 1
 #  endif
+// std::filesystem::path
+#  if defined(PYBIND11_CPP17) && __has_include(<filesystem>) && \
+      PY_VERSION_HEX >= 0x03060000
+#    include <filesystem>
+#    define PYBIND11_HAS_FILESYSTEM 1
+#  endif
 #elif defined(_MSC_VER) && defined(PYBIND11_CPP17)
 #  include <optional>
 #  include <variant>
 #  define PYBIND11_HAS_OPTIONAL 1
 #  define PYBIND11_HAS_VARIANT 1
+#  if PY_VERSION_HEX >= 0x03060000
+#    include <filesystem>
+#    define PYBIND11_HAS_FILESYSTEM 1
+#  endif
 #endif
 
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
@@ -377,6 +387,77 @@ template <typename... Ts>
 struct type_caster<std::variant<Ts...>> : variant_caster<std::variant<Ts...>> { };
 #endif
 
+#if defined(PYBIND11_HAS_FILESYSTEM)
+template<typename T> 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<char*>(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<object>(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<typename T::value_type, char>) {
+            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<typename T::value_type, wchar_t>) {
+            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<std::filesystem::path>
+    : public path_caster<std::filesystem::path> {};
+#endif
+
 PYBIND11_NAMESPACE_END(detail)
 
 inline std::ostream &operator<<(std::ostream &os, const handle &obj) {

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 <filesystem>\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

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<py::detail::variant_caster_visitor::result_type, py::handle>::value,
                   "visitor::result_type is required by boost::variant in C++11 mode");

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 <filesystem>")
+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 <variant>")
 def test_variant(doc):
     assert m.load_variant(1) == "int"