Page MenuHomePhabricator
Differential D138327

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

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
Herald added a subscriber: libcxx-commits. · View Herald Transcript
Harbormaster completed remote builds in B198543: Diff 476583.Nov 18 2022, 1:22 PM
ldionne updated this revision to Diff 476596.Nov 18 2022, 1:52 PM

Generate files

Harbormaster completed remote builds in B198554: Diff 476596.Nov 18 2022, 3:35 PM
philnik added a subscriber: philnik.Nov 19 2022, 5:42 AM

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

ldionne added a comment.Nov 19 2022, 8:46 AM
In D138327#3939014, @philnik wrote:

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.

Harbormaster completed remote builds in B198635: Diff 476696.Nov 19 2022, 12:04 PM
ldionne updated this revision to Diff 476703.Nov 19 2022, 1:58 PM

Ignore non-ASCII inside a new test file.

tschuett added a subscriber: tschuett.Nov 19 2022, 2:01 PM

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

ldionne added a comment.Nov 19 2022, 2:06 PM
In D138327#3939320, @tschuett wrote:

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?

tschuett added a comment.Nov 19 2022, 2:10 PM

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.
Closed by commit rGaf8c49dc1ec4: [libc++] Introduce a setting to remove fstream from the library (authored by ldionne). · Explain Why
This revision was automatically updated to reflect the committed changes.
ldionne added a commit: rGaf8c49dc1ec4: [libc++] Introduce a setting to remove fstream from the library.
Harbormaster completed remote builds in B198639: Diff 476703.Nov 21 2022, 9:35 AM

Revision Contents
Changeset List

PathSize
libcxx/
3 lines
cmake/
caches/
1 line
include/
2 lines
1 line
4 lines
1 line
src/
3 lines
test/
libcxx/
assertions/
2 lines
2 lines
2 lines
2 lines
2 lines
2 lines
2 lines
std/
input.output/
file.streams/
fstreams/
filebuf.virtuals/
2 lines
3 lines
stream.buffers/
streambuf/
streambuf.virtuals/
streambuf.virt.put/
2 lines
localization/
locales/
locale.convenience/
conversions/
conversions.buffer/
118 lines
17 lines
5 lines
20 lines
17 lines
utilities/
charconv/
charconv.msvc/
24 lines
support/
4 lines
utils/
ci/
17 lines
6 lines
2 lines
libcxx/
test/
1 line

Diff 476747

View Options

libcxx/CMakeLists.txt

Loading...
View Options

libcxx/cmake/caches/Generic-no-fstream.cmake

Loading...
View Options

libcxx/include/CMakeLists.txt

Loading...
View Options

libcxx/include/__config_site.in

Loading...
View Options

libcxx/include/fstream

Loading...
View Options

libcxx/include/module.modulemap.in

Loading...
View Options

libcxx/src/ios.instantiations.cpp

Loading...
View Options

libcxx/test/libcxx/assertions/headers_declare_verbose_abort.sh.cpp

Loading...
View Options

libcxx/test/libcxx/clang_tidy.sh.cpp

Loading...
View Options

libcxx/test/libcxx/double_include.sh.cpp

Loading...
View Options

libcxx/test/libcxx/min_max_macros.compile.pass.cpp

Loading...
View Options

libcxx/test/libcxx/modules_include.sh.cpp

Loading...
View Options

libcxx/test/libcxx/nasty_macros.compile.pass.cpp

Loading...
View Options

libcxx/test/libcxx/no_assert_include.compile.pass.cpp

Loading...
View Options

libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp

Loading...
View Options

libcxx/test/std/input.output/file.streams/lit.local.cfg

Loading...
View Options

libcxx/test/std/input.output/stream.buffers/streambuf/streambuf.virtuals/streambuf.virt.put/xsputn.PR14074.pass.cpp

Loading...
View Options

libcxx/test/std/localization/locales/locale.convenience/conversions/conversions.buffer/overflow.pass.cpp

Loading...
View Options

libcxx/test/std/localization/locales/locale.convenience/conversions/conversions.buffer/pbackfail.pass.cpp

Loading...
View Options

libcxx/test/std/localization/locales/locale.convenience/conversions/conversions.buffer/seekoff.pass.cpp

Loading...
View Options

libcxx/test/std/localization/locales/locale.convenience/conversions/conversions.buffer/test.pass.cpp

Loading...
View Options

libcxx/test/std/localization/locales/locale.convenience/conversions/conversions.buffer/underflow.dat

Loading...
View Options

libcxx/test/std/localization/locales/locale.convenience/conversions/conversions.buffer/underflow.pass.cpp

Loading...
View Options

libcxx/test/std/localization/locales/locale.convenience/conversions/conversions.buffer/underflow_utf8.dat

Loading...
View Options

libcxx/test/std/utilities/charconv/charconv.msvc/test.cpp

Loading...
View Options

libcxx/test/support/test_macros.h

Loading...
View Options

libcxx/utils/ci/buildkite-pipeline.yml

Loading...
View Options

libcxx/utils/ci/run-buildbot

Loading...
View Options

libcxx/utils/generate_header_tests.py

Loading...
View Options

libcxx/utils/libcxx/test/features.py

Loading...