Splitting out pybind11/stl/filesystem.h. (#3077)

* Splitting out pybind11/stl/filesystem.h. To solve breakages like: https://github.com/deepmind/open_spiel/runs/2999582108 Mostly following the suggestion here: #2730 (comment) Except using pybind11/stl/filesystem.h instead of pybind11/stlfs.h, as decided via chat. stl.h restored to the exact state before merging PR #2730 via: ``` git checkout 733f8de stl.h ``` * Properly including new stl subdirectory in pip wheel config. This now passes interactively: ``` pytest tests/extra_python_package/ ``` * iwyu cleanup. iwyuh.py -c -std=c++17 -DPYBIND11_TEST_BOOST -Ipybind11/include -I/usr/include/python3.9 -I/usr/include/eigen3 include/pybind11/stl/filesystem.h * Adding PYBIND11_HAS_FILESYSTEM_IS_OPTIONAL. * Eliminating else after return.
pybind · Jul 8, 2021 · 6d1b197 · 6d1b197
1 parent c090c8c
commit 6d1b197
Show file tree

Hide file tree

Showing 7 changed files with 121 additions and 84 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -125,7 +125,8 @@ set(PYBIND11_HEADERS
     include/pybind11/pybind11.h
     include/pybind11/pytypes.h
     include/pybind11/stl.h
-    include/pybind11/stl_bind.h)
+    include/pybind11/stl_bind.h
+    include/pybind11/stl/filesystem.h)
 
 # Compare with grep and warn if mismatched
 if(PYBIND11_MASTER_PROJECT AND NOT CMAKE_VERSION VERSION_LESS 3.12)

diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h
@@ -41,21 +41,11 @@
 #    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)
@@ -387,77 +377,6 @@ 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) {

diff --git a/include/pybind11/stl/filesystem.h b/include/pybind11/stl/filesystem.h
@@ -0,0 +1,103 @@
+// Copyright (c) 2021 The Pybind Development Team.
+// All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+#pragma once
+
+#include "../cast.h"
+#include "../pybind11.h"
+#include "../pytypes.h"
+
+#include "../detail/common.h"
+#include "../detail/descr.h"
+
+#include <string>
+
+#ifdef __has_include
+#  if defined(PYBIND11_CPP17) && __has_include(<filesystem>) && \
+      PY_VERSION_HEX >= 0x03060000
+#    include <filesystem>
+#    define PYBIND11_HAS_FILESYSTEM 1
+#  endif
+#endif
+
+#if !defined(PYBIND11_HAS_FILESYSTEM) && !defined(PYBIND11_HAS_FILESYSTEM_IS_OPTIONAL)
+#    error                                                                                        \
+        "#include <filesystem> is not available. (Use -DPYBIND11_HAS_FILESYSTEM_IS_OPTIONAL to ignore.)"
+#endif
+
+PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
+PYBIND11_NAMESPACE_BEGIN(detail)
+
+#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;
+        }
+        return true;
+    }
+
+    PYBIND11_TYPE_CASTER(T, _("os.PathLike"));
+};
+
+template<> struct type_caster<std::filesystem::path>
+    : public path_caster<std::filesystem::path> {};
+#endif // PYBIND11_HAS_FILESYSTEM
+
+PYBIND11_NAMESPACE_END(detail)
+PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)
diff --git a/tests/extra_python_package/test_files.py b/tests/extra_python_package/test_files.py
@@ -46,6 +46,10 @@
     "include/pybind11/detail/typeid.h",
 }
 
+stl_headers = {
+    "include/pybind11/stl/filesystem.h",
+}
+
 cmake_files = {
     "share/cmake/pybind11/FindPythonLibsNew.cmake",
     "share/cmake/pybind11/pybind11Common.cmake",
@@ -67,7 +71,7 @@
     "setup_helpers.pyi",
 }
 
-headers = main_headers | detail_headers
+headers = main_headers | detail_headers | stl_headers
 src_files = headers | cmake_files
 all_files = src_files | py_files
 
@@ -77,6 +81,7 @@
     "pybind11/include",
     "pybind11/include/pybind11",
     "pybind11/include/pybind11/detail",
+    "pybind11/include/pybind11/stl",
     "pybind11/share",
     "pybind11/share/cmake",
     "pybind11/share/cmake/pybind11",

diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp
@@ -11,6 +11,11 @@
 #include "constructor_stats.h"
 #include <pybind11/stl.h>
 
+#ifndef PYBIND11_HAS_FILESYSTEM_IS_OPTIONAL
+#define PYBIND11_HAS_FILESYSTEM_IS_OPTIONAL
+#endif
+#include <pybind11/stl/filesystem.h>
+
 #include <vector>
 #include <string>
 

diff --git a/tools/setup_global.py.in b/tools/setup_global.py.in
@@ -33,8 +33,9 @@ class InstallHeadersNested(install_headers):
 
 main_headers = glob.glob("pybind11/include/pybind11/*.h")
 detail_headers = glob.glob("pybind11/include/pybind11/detail/*.h")
+stl_headers = glob.glob("pybind11/include/pybind11/stl/*.h")
 cmake_files = glob.glob("pybind11/share/cmake/pybind11/*.cmake")
-headers = main_headers + detail_headers
+headers = main_headers + detail_headers + stl_headers
 
 cmdclass = {"install_headers": InstallHeadersNested}
 $extra_cmd
@@ -58,6 +59,7 @@ setup(
         (base + "share/cmake/pybind11", cmake_files),
         (base + "include/pybind11", main_headers),
         (base + "include/pybind11/detail", detail_headers),
+        (base + "include/pybind11/stl", stl_headers),
     ],
     cmdclass=cmdclass,
 )
diff --git a/tools/setup_main.py.in b/tools/setup_main.py.in
@@ -16,12 +16,14 @@ setup(
         "pybind11",
         "pybind11.include.pybind11",
         "pybind11.include.pybind11.detail",
+        "pybind11.include.pybind11.stl",
         "pybind11.share.cmake.pybind11",
     ],
     package_data={
         "pybind11": ["py.typed", "*.pyi"],
         "pybind11.include.pybind11": ["*.h"],
         "pybind11.include.pybind11.detail": ["*.h"],
+        "pybind11.include.pybind11.stl": ["*.h"],
         "pybind11.share.cmake.pybind11": ["*.cmake"],
     },
     extras_require={