LIBCXX_ENABLE_FILESYSTEM should represent whether the platform has
support for a filesystem, not just whether we support <filesystem>.
This patch slightly generalizes the setting to also encompass whether
we provide <fstream>, since that only makes sense when a filesystem is
supported.
Details
- Reviewers
philnik phosek ldionne - Group Reviewers
Restricted Project - Commits
- rG66a562d22e70: [libc++] Roll up fstream support into filesystem support
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
With this in place, we really should go ahead with D134912 too, since I'm fairly sure that <fstream> is somewhat widely used on Windows even if <filesystem> is less common.
FWIW, WASI had fstream but not filesystem, so this effectively removes fstream from it. See https://github.com/WebAssembly/wasi-sdk/issues/125
It has some I/O functions available (enough for <fstream>) but not everything required for <filesystem>.
LLVM Embedded Toolchain for Arm also supports <fstream> but not <filesystem>. We have a C library so supporting <fstream> is easy. We don't have POSIX support so supporting <filesystem> is much more challenging.
Please can this change be reverted?
To give some background. Simple file IO can be handled by an embedded toolchain using semihosting IO https://github.com/ARM-software/abi-aa/blob/main/semihosting/semihosting.rst (also supported by RISCV https://github.com/riscv-software-src/riscv-semihosting) the POSIX function in posix_compat.h can't easily be supported using that API.
Thanks for the additional context. So basically, you folks are considering systems where a file system does exist (even in the case of semihosted I/O), but where the platform does not provide the necessary set of library functions to implement all of <filesystem>. In other words, that's a system where LIBCXX_ENABLE_FILESYSTEM is true, but where parts of the filesystem-requiring functionality in libc++ can't be implemented due to incomplete platform support.
If I understand correctly, then I think that's the correct way to think about this, instead of pretending that these platforms don't have support for a filesystem but magically have the <fstream> class and only that one. Can you tell me a bit more about your setups? This didn't come up during our CI testing (which tests all officially supported configurations), so we're not formally aware of your use cases:
- Do you carry downstream patches?
- How did you find out about this change?
- How much of <filesystem> can't be implemented on your platforms?
I'm interested in supporting your use cases, and in particular in making things work well for embedded platforms -- that's part of the reason why we're making all these changes. But I'm also interested in making sure we provide coherent and well-supported ways to subset the library instead of arbitrary ones based on individual platform's needs, so at the moment just re-introducing LIBCXX_ENABLE_FSTREAM doesn't seem like the right approach to me. FWIW, the only reason why I introduced LIBCXX_ENABLE_FSTREAM is that I didn't have time to do this full stack of patches back then. Let's talk.
In other words, that's a system where LIBCXX_ENABLE_FILESYSTEM is true, but where parts of the filesystem-requiring functionality in libc++ can't be implemented due to incomplete platform support.
Yes, that is correct.
Do you carry downstream patches?
At the moment we basically have two kinds of toolchains: one open source toolchain based on picolibc C library (which is in turn based on newlib), it does not need any downstream patches in the libc++ part (just a custom lit configuration), but the C++ support and testing is incomplete; we are currently actively improving it. We also have two proprietary toolchains based on our own C library which do have some downstream changes (but we want to keep them to a minimum). Both kinds rely on the same semihosting API/ABI to implement file system operations in the C library.
How did you find out about this change?
We have CI set up for the proprietary toolchains in which some fstream tests are xfailed (via a separate mechanism, rather than lit "// XFAIL" to reduce downstream changes) because of some missing functionality in the C library. Those started passing unexpectedly because they are now marked as unsupported. We also monitor the stream of libc++ patches (I am subscribed to the libc++ group in Phabricator).
How much of <filesystem> can't be implemented on your platforms?
In the Arm semihosting API, directory operations (such as checking whether a directory exists, creating a directory, and enumerating files in a directory) are not available. Querying and setting permissions, file creation/modification time, space_info, and working with hardlinks and symlinks is also not supported. Basically, the following operations are supported: opening a file by name, seeking, reading, writing, querying the length of an open file, renaming, and removing a file by name.