Skip to content

feature: support compilers that use std::experimental::filesystem #3840

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Apr 1, 2022
Merged

feature: support compilers that use std::experimental::filesystem #3840

Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
505bf23
feature: support compilers that use std::experimental::filesystem suc…
Mar 31, 2022
9e7d07b
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 31, 2022
ceb1fa4
only use this feature if cpp17
Mar 31, 2022
2de5c5e
Merge branch 'support-experimental-filesystem' of github.com:Llcoolso…
Mar 31, 2022
d2c4f56
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 31, 2022
c4e3cdf
more specific namespace alias + style
Mar 31, 2022
235d88b
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 31, 2022
9dd08b3
move to pybind11 namespace
Mar 31, 2022
6c63632
no namespace alias
Apr 1, 2022
761717a
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 21 additions & 7 deletions include/pybind11/stl/filesystem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,28 @@
#include <string>

#ifdef __has_include
# if defined(PYBIND11_CPP17) && __has_include(<filesystem>)
# include <filesystem>
# define PYBIND11_HAS_FILESYSTEM 1
# if defined(PYBIND11_CPP17)
# if __has_include(<filesystem>) && \
PY_VERSION_HEX >= 0x03060000
# include <filesystem>
# define PYBIND11_HAS_FILESYSTEM 1
# elif __has_include(<experimental/filesystem>)
# include <experimental/filesystem>
# define PYBIND11_HAS_EXPERIMENTAL_FILESYSTEM 1
# endif
# endif
#endif

#if !defined(PYBIND11_HAS_FILESYSTEM) && !defined(PYBIND11_HAS_FILESYSTEM_IS_OPTIONAL)
#if !defined(PYBIND11_HAS_FILESYSTEM) && !defined(PYBIND11_HAS_EXPERIMENTAL_FILESYSTEM) \
&& !defined(PYBIND11_HAS_FILESYSTEM_IS_OPTIONAL)
# error \
"#include <filesystem> is not available. (Use -DPYBIND11_HAS_FILESYSTEM_IS_OPTIONAL to ignore.)"
"Neither #include <filesystem> nor #include <experimental/filesystem is available. (Use -DPYBIND11_HAS_FILESYSTEM_IS_OPTIONAL to ignore.)"
#endif

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
PYBIND11_NAMESPACE_BEGIN(detail)
Copy link
Collaborator

@Skylion007 Skylion007 Mar 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

detail namespace begins here. Can you not just move the include logic after line 38?

Copy link
Collaborator

@Skylion007 Skylion007 Mar 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If not, it's fine as is.

Copy link
Collaborator

@Skylion007 Skylion007 Mar 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we even need the namespace btw? It's only used on the type caster right? Just mimic what we do for experimental optional and spell it out twice with an IFDEF?

Copy link
Contributor Author

@Llcoolsouder Llcoolsouder Apr 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in the latest commit.

Since we're here though, what is the point of the PYBIND11_NAMESPACE_BEGIN and PYBIND11_NAMESPACE_END macros? It looks like we're just replacing the language syntax with a lesser known syntax.

Copy link
Collaborator

@rwgk rwgk Apr 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From memory: in some environments the macros insert "visibility hidden" attributes.


#if defined(PYBIND11_HAS_FILESYSTEM)
#if defined(PYBIND11_HAS_FILESYSTEM) || defined(PYBIND11_HAS_EXPERIMENTAL_FILESYSTEM)
template <typename T>
struct path_caster {

Expand Down Expand Up @@ -94,9 +101,16 @@ struct path_caster {
PYBIND11_TYPE_CASTER(T, const_name("os.PathLike"));
};

#endif // PYBIND11_HAS_FILESYSTEM || defined(PYBIND11_HAS_EXPERIMENTAL_FILESYSTEM)

#if defined(PYBIND11_HAS_FILESYSTEM)
template <>
struct type_caster<std::filesystem::path> : public path_caster<std::filesystem::path> {};
#endif // PYBIND11_HAS_FILESYSTEM
#elif defined(PYBIND11_HAS_EXPERIMENTAL_FILESYSTEM)
template <>
struct type_caster<std::experimental::filesystem::path>
: public path_caster<std::experimental::filesystem::path> {};
#endif

PYBIND11_NAMESPACE_END(detail)
PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)