[libc++][print] Make `<print>` tests require file system support.

[var-const]

Summary

print functions require FILE and stdout to be available and cause
compilation errors on platforms that don't support the file system.

[var-const]

libcxx/utils/libcxx/test/header_information.py

48

Language modes are a drive-by fix.

[philnik]

I would really like it if we could add headers that make these kinds of things break. We should really look into using LLVM libc with the corresponding parts removed to improve the situation here. That being said, I think this patch is OK as-is. Leaving final approval to @Mordante, since he is the author of these utilities.

[var-const]

In D156585#4544020, @philnik wrote:

I would really like it if we could add headers that make these kinds of things break. We should really look into using LLVM libc with the corresponding parts removed to improve the situation here. That being said, I think this patch is OK as-is. Leaving final approval to @Mordante, since he is the author of these utilities.

Yep, I was thinking of the same while working on this patch (e.g. the no-filesystem test should do something to make sure including a header that touches the file system would fail to compile).

[Mordante]

I'm not against this patch, but I would like to investigate a bit further. When FILE is not available we will run into issues building the dylib. src/print.cpp includes <print> which has FILE in its interface. This works now because we build the dylib with C++20, but I expect it will fail when we switch to C++23. I have another patch for print(ostream&...) this patch uses FILE in the dylib, so that is another potential issue before we switch to C++23. Can you test whether changing building the dylib with C++23 works on the platform in question?

I wonder whether no-filesystem is intended to mean no cstdio. If this patch is not urgent I would like to discuss it when Louis is back. If it's urgent we can finish the patch and discuss afterwards.

libcxx/test/std/input.output/iostream.format/print.fun/lit.local.cfg

1

Why do we need this? AFAIK we normally don't do this. Typically we disable all tests manually.

libcxx/utils/libcxx/test/header_information.py

48

Thanks good catch.

[var-const]

In D156585#4544282, @Mordante wrote:

I'm not against this patch, but I would like to investigate a bit further. When FILE is not available we will run into issues building the dylib. src/print.cpp includes <print> which has FILE in its interface. This works now because we build the dylib with C++20, but I expect it will fail when we switch to C++23. I have another patch for print(ostream&...) this patch uses FILE in the dylib, so that is another potential issue before we switch to C++23. Can you test whether changing building the dylib with C++23 works on the platform in question?

I wonder whether no-filesystem is intended to mean no cstdio. If this patch is not urgent I would like to discuss it when Louis is back. If it's urgent we can finish the patch and discuss afterwards.

It's somewhat urgent -- this patch is necessary to fix our internal build.

If I switch to C++23, I get the FILE-related errors while compiling print.cpp like you described.

I was trying to follow the approach previously laid out in https://reviews.llvm.org/D152168 for this patch. D152168 combined the check for support for <filesystem> and <fstream> into a single macro, which makes me feel we shouldn't introduce a separate macro for <cstdio>. There are platforms that don't provide FILE, so I believe we need some macro to express that, the only question is whether we want to combine it with no-filesystem or not.

libcxx/test/std/input.output/iostream.format/print.fun/lit.local.cfg

1

This approach is used heavily in test/std/input.output, mostly to disable tests if localization is not available (there are 10 existing lit.local.cfg files in that directory). I did it this way mostly for consistency.

[Mordante]

In D156585#4548488, @var-const wrote:

In D156585#4544282, @Mordante wrote:

I'm not against this patch, but I would like to investigate a bit further. When FILE is not available we will run into issues building the dylib. src/print.cpp includes <print> which has FILE in its interface. This works now because we build the dylib with C++20, but I expect it will fail when we switch to C++23. I have another patch for print(ostream&...) this patch uses FILE in the dylib, so that is another potential issue before we switch to C++23. Can you test whether changing building the dylib with C++23 works on the platform in question?

I wonder whether no-filesystem is intended to mean no cstdio. If this patch is not urgent I would like to discuss it when Louis is back. If it's urgent we can finish the patch and discuss afterwards.

It's somewhat urgent -- this patch is necessary to fix our internal build.

As discussed on Discord, I'm happy to approve this now and look at a better solution later.

If I switch to C++23, I get the FILE-related errors while compiling print.cpp like you described.

Thanks for the confirmation.

LGTM modulo comments.

libcxx/test/std/input.output/iostream.format/print.fun/lit.local.cfg

1

As discussed on Discord I prefer to use the explicit // UNSUPPORTED: no-filesystem to keep tests self-contained.

[var-const]

Address feedback

[var-const]

Note: I believe the CI issues are unrelated and spurious.

libcxx/test/std/input.output/iostream.format/print.fun/lit.local.cfg

+if "no-filesystem" in config.available_features:
+    config.unsupported = True

libcxx/test/std/language.support/support.limits/support.limits.general/print.version.compile.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // WARNING: This test was generated by generate_feature_test_macro_components.py
 // and should not be edited manually.
 //
 // clang-format off
 
+// UNSUPPORTED: no-filesystem
+
 // <print>
 
 // Test the feature test macros defined by <print>
 
 /*  Constant           Value
     __cpp_lib_print    202207L [C++23]
 */
 

libcxx/utils/generate_feature_test_macro_components.py

lit_markup = {
     "iomanip": ["UNSUPPORTED: no-localization"],
     "ios": ["UNSUPPORTED: no-localization"],
     "iostream": ["UNSUPPORTED: no-localization"],
     "istream": ["UNSUPPORTED: no-localization"],
     "latch": ["UNSUPPORTED: no-threads"],
     "locale": ["UNSUPPORTED: no-localization"],
     "mutex": ["UNSUPPORTED: no-threads"],
     "ostream": ["UNSUPPORTED: no-localization"],
+    "print": ["UNSUPPORTED: no-filesystem"],
     "regex": ["UNSUPPORTED: no-localization"],
     "semaphore": ["UNSUPPORTED: no-threads"],
     "shared_mutex": ["UNSUPPORTED: no-threads"],
     "sstream": ["UNSUPPORTED: no-localization"],
     "stdatomic.h": ["UNSUPPORTED: no-threads"],
     "stop_token": ["UNSUPPORTED: no-threads"],
     "thread": ["UNSUPPORTED: no-threads"],
 }

libcxx/utils/libcxx/test/header_information.py

lit_header_restrictions = {
     "ios": "// UNSUPPORTED: no-localization",
     "iostream": "// UNSUPPORTED: no-localization",
     "istream": "// UNSUPPORTED: no-localization",
     "latch": "// UNSUPPORTED: no-threads, c++03, c++11, c++14, c++17",
     "locale": "// UNSUPPORTED: no-localization",
     "locale.h": "// UNSUPPORTED: no-localization",
     "mutex": "// UNSUPPORTED: no-threads, c++03",
     "ostream": "// UNSUPPORTED: no-localization",
-    "print": "// UNSUPPORTED: availability-fp_to_chars-missing", # TODO PRINT investigate
+    "print": "// UNSUPPORTED: no-filesystem, c++03, c++11, c++14, c++17, c++20, availability-fp_to_chars-missing", # TODO PRINT investigate
     "regex": "// UNSUPPORTED: no-localization",
     "semaphore": "// UNSUPPORTED: no-threads, c++03, c++11, c++14, c++17",
     "shared_mutex": "// UNSUPPORTED: no-threads, c++03, c++11",
     "sstream": "// UNSUPPORTED: no-localization",
     "stdatomic.h": "// UNSUPPORTED: no-threads, c++03, c++11, c++14, c++17, c++20",
     "stop_token": "// UNSUPPORTED: no-threads, c++03, c++11, c++14, c++17",
     "streambuf": "// UNSUPPORTED: no-localization",
     "strstream": "// UNSUPPORTED: no-localization",