Page MenuHomePhabricator

[libc++] Introduce a setting to remove fstream from the library
ClosedPublic

Authored by ldionne on Nov 18 2022, 1:17 PM.

Details

Reviewers
None
Group Reviewers
Restricted Project
Commits
rGaf8c49dc1ec4: [libc++] Introduce a setting to remove fstream from the library
Summary

This allows porting the library to platforms that are able to support
<iostream> but that do not have a notion of a filesystem, and where it
hence doesn't make sense to support std::fstream (and never will).

Also, remove reliance on <fstream> in various tests that didn't
actually need it.

Diff Detail

Event Timeline

ldionne created this revision.Nov 18 2022, 1:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 18 2022, 1:17 PM
Herald added a subscriber: arichardson. · View Herald Transcript
ldionne requested review of this revision.Nov 18 2022, 1:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 18 2022, 1:17 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne updated this revision to Diff 476596.Nov 18 2022, 1:52 PM

Generate files

Why does this need it's own flag, and isn't just under _LIBCPP_HAS_NO_FILESYSTEM?

Why does this need it's own flag, and isn't just under _LIBCPP_HAS_NO_FILESYSTEM?

Because _LIBCPP_HAS_NO_FILESYSTEM has been used to control whether we ship <filesystem>, and is for example disabled by default on Windows. See the comment in libcxx/CMakeLists.txt.

Rolling this setting into _LIBCPP_HAS_NO_FILESYSTEM will need to wait for https://reviews.llvm.org/D134912 to land and then I'll have to make sure that no other users of _LIBCPP_HAS_NO_FILESYSTEM are broken by also disabling fstream in that configuration, which will require a bit more time. So I am pursuing both at the same time because I don't want to wait that long to get this configuration, but notice how I did not document this new flag (this is on purpose so that people don't start depending on it).

ldionne updated this revision to Diff 476696.Nov 19 2022, 12:00 PM
ldionne edited the summary of this revision. (Show Details)

Fix various tests that used <fstream> as an implementation detail. The only one I gave up on is libcxx/test/std/localization/locales/locale.convenience/conversions/conversions.buffer/seekoff.pass.cpp.

ldionne updated this revision to Diff 476703.Nov 19 2022, 1:58 PM

Ignore non-ASCII inside a new test file.

The LLVM libc has a porting guide for new targets: https://libc.llvm.org/porting.html. libcxx seems to hide these options in CMake.

The LLVM libc has a porting guide for new targets: https://libc.llvm.org/porting.html. libcxx seems to hide these options in CMake.

Can you please expand? Are you suggesting that we handle these configuration options differently? How?

IDK. Maybe a document how to port libcxx on a new C library.

ldionne accepted this revision as: Restricted Project.Nov 20 2022, 7:51 AM

AIX is stuck, everything else is green.

This revision was not accepted when it landed; it landed in state Needs Review.Nov 20 2022, 7:51 AM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.