Skip to content
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.

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

[libc++][print] Enables it on Apple backdeployment. #76293

Merged

Conversation

mordante
Copy link
Member

As suggested in #73262 this enable the stream printing on Apple backdeployment targets. This omits the check whether the file is a terminal. This is not entirely conforming, but the differences should be minor and are typically not observable.

Fixes #75225

As suggested in llvm#73262 this enable the stream printing on Apple
backdeployment targets. This omits the check whether the file is a
terminal. This is not entirely conforming, but the differences should be
minor and are typically not observable.

Fixes llvm#75225
@mordante mordante marked this pull request as ready for review December 24, 2023 13:41
@mordante mordante requested a review from a team as a code owner December 24, 2023 13:41
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Dec 24, 2023
@llvmbot
Copy link
Member

llvmbot commented Dec 24, 2023

@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)

Changes

As suggested in #73262 this enable the stream printing on Apple backdeployment targets. This omits the check whether the file is a terminal. This is not entirely conforming, but the differences should be minor and are typically not observable.

Fixes #75225


Full diff: https://github.com/llvm/llvm-project/pull/76293.diff

10 Files Affected:

  • (modified) libcxx/include/__availability (+4)
  • (modified) libcxx/include/ostream (+9-5)
  • (modified) libcxx/src/ostream.cpp (+1-1)
  • (modified) libcxx/test/libcxx/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_unicode.pass.cpp (-1)
  • (modified) libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/locale-specific_form.pass.cpp (-1)
  • (modified) libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/print.pass.cpp (-1)
  • (modified) libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/println.pass.cpp (-1)
  • (modified) libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_nonunicode.pass.cpp (-1)
  • (modified) libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_unicode.pass.cpp (-1)
  • (modified) libcxx/utils/libcxx/test/features.py (-9)
diff --git a/libcxx/include/__availability b/libcxx/include/__availability
index 5df2783dd0c768..923533ac530810 100644
--- a/libcxx/include/__availability
+++ b/libcxx/include/__availability
@@ -264,6 +264,10 @@
 #  define _LIBCPP_AVAILABILITY_HAS_TZDB 0
 #  define _LIBCPP_AVAILABILITY_TZDB __attribute__((unavailable))
 
+// Warning: This availability macro works differently as the other macros.
+// The dylib part of print is not needed on Apple platforms. Therefore when
+// the macro is not available the code calling the dylib is commented out.
+// The macro _LIBCPP_AVAILABILITY_PRINT is not used.
 #  define _LIBCPP_AVAILABILITY_HAS_PRINT 0
 #  define _LIBCPP_AVAILABILITY_PRINT __attribute__((unavailable))
 
diff --git a/libcxx/include/ostream b/libcxx/include/ostream
index 88ee9d93a1d36c..1eaa7a8c266efd 100644
--- a/libcxx/include/ostream
+++ b/libcxx/include/ostream
@@ -1084,12 +1084,15 @@ _LIBCPP_HIDE_FROM_ABI inline void vprint_nonunicode(ostream& __os, string_view _
 //   native Unicode API;
 // Whether the returned FILE* is "a terminal capable of displaying Unicode"
 // is determined in the same way as the print(FILE*, ...) overloads.
-_LIBCPP_AVAILABILITY_PRINT _LIBCPP_EXPORTED_FROM_ABI FILE* __get_ostream_file(ostream& __os);
+_LIBCPP_EXPORTED_FROM_ABI FILE* __get_ostream_file(ostream& __os);
 
 #  ifndef _LIBCPP_HAS_NO_UNICODE
 template <class = void> // TODO PRINT template or availability markup fires too eagerly (http://llvm.org/PR61563).
-_LIBCPP_AVAILABILITY_PRINT _LIBCPP_HIDE_FROM_ABI void
+_LIBCPP_HIDE_FROM_ABI void
 __vprint_unicode(ostream& __os, string_view __fmt, format_args __args, bool __write_nl) {
+#if _LIBCPP_AVAILABILITY_HAS_PRINT == 0
+  return std::__vprint_nonunicode(__os, __fmt, __args, __write_nl);
+#else
   FILE* __file = std::__get_ostream_file(__os);
   if (!__file || !__print::__is_terminal(__file))
     return std::__vprint_nonunicode(__os, __fmt, __args, __write_nl);
@@ -1124,17 +1127,18 @@ __vprint_unicode(ostream& __os, string_view __fmt, format_args __args, bool __wr
     __os.__set_badbit_and_consider_rethrow();
   }
 #    endif // _LIBCPP_HAS_NO_EXCEPTIONS
+#endif // _LIBCPP_AVAILABILITY_HAS_PRINT
 }
 
 template <class = void> // TODO PRINT template or availability markup fires too eagerly (http://llvm.org/PR61563).
-_LIBCPP_AVAILABILITY_PRINT _LIBCPP_HIDE_FROM_ABI inline void
+_LIBCPP_HIDE_FROM_ABI inline void
 vprint_unicode(ostream& __os, string_view __fmt, format_args __args) {
   std::__vprint_unicode(__os, __fmt, __args, false);
 }
 #  endif // _LIBCPP_HAS_NO_UNICODE
 
 template <class... _Args>
-_LIBCPP_AVAILABILITY_PRINT _LIBCPP_HIDE_FROM_ABI void
+_LIBCPP_HIDE_FROM_ABI void
 print(ostream& __os, format_string<_Args...> __fmt, _Args&&... __args) {
 #  ifndef _LIBCPP_HAS_NO_UNICODE
   if constexpr (__print::__use_unicode)
@@ -1147,7 +1151,7 @@ print(ostream& __os, format_string<_Args...> __fmt, _Args&&... __args) {
 }
 
 template <class... _Args>
-_LIBCPP_AVAILABILITY_PRINT _LIBCPP_HIDE_FROM_ABI void
+_LIBCPP_HIDE_FROM_ABI void
 println(ostream& __os, format_string<_Args...> __fmt, _Args&&... __args) {
 #  ifndef _LIBCPP_HAS_NO_UNICODE
   // Note the wording in the Standard is inefficient. The output of
diff --git a/libcxx/src/ostream.cpp b/libcxx/src/ostream.cpp
index bba8e6550710f8..443dce9a390bee 100644
--- a/libcxx/src/ostream.cpp
+++ b/libcxx/src/ostream.cpp
@@ -17,7 +17,7 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-_LIBCPP_AVAILABILITY_PRINT _LIBCPP_EXPORTED_FROM_ABI FILE* __get_ostream_file(ostream& __os) {
+_LIBCPP_EXPORTED_FROM_ABI FILE* __get_ostream_file(ostream& __os) {
   // dynamic_cast requires RTTI, this only affects users whose vendor builds
   // the dylib with RTTI disabled. It does not affect users who build with RTTI
   // disabled but use a dylib where the RTTI is enabled.
diff --git a/libcxx/test/libcxx/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_unicode.pass.cpp b/libcxx/test/libcxx/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_unicode.pass.cpp
index 71dd6c0d08d620..5438d31bca33f8 100644
--- a/libcxx/test/libcxx/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_unicode.pass.cpp
+++ b/libcxx/test/libcxx/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_unicode.pass.cpp
@@ -10,7 +10,6 @@
 // UNSUPPORTED: GCC-ALWAYS_INLINE-FIXME
 
 // XFAIL: availability-fp_to_chars-missing
-// XFAIL: availability-print-missing
 
 // Clang modules do not work with the definiton of _LIBCPP_TESTING_PRINT_IS_TERMINAL
 // XFAIL: clang-modules-build
diff --git a/libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/locale-specific_form.pass.cpp b/libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/locale-specific_form.pass.cpp
index 7e1156acf8335b..6b62e2f1754de5 100644
--- a/libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/locale-specific_form.pass.cpp
+++ b/libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/locale-specific_form.pass.cpp
@@ -13,7 +13,6 @@
 // UNSUPPORTED: no-filesystem
 
 // XFAIL: availability-fp_to_chars-missing
-// XFAIL: availability-print-missing
 
 // Bionic has minimal locale support, investigate this later.
 // XFAIL: LIBCXX-ANDROID-FIXME
diff --git a/libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/print.pass.cpp b/libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/print.pass.cpp
index 0831ef71076629..6abfb30265e1e1 100644
--- a/libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/print.pass.cpp
+++ b/libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/print.pass.cpp
@@ -12,7 +12,6 @@
 // UNSUPPORTED: no-filesystem
 
 // XFAIL: availability-fp_to_chars-missing
-// XFAIL: availability-print-missing
 
 // <ostream>
 
diff --git a/libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/println.pass.cpp b/libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/println.pass.cpp
index deb262d2fb627e..479a3de0a93c8d 100644
--- a/libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/println.pass.cpp
+++ b/libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/println.pass.cpp
@@ -12,7 +12,6 @@
 // UNSUPPORTED: no-filesystem
 
 // XFAIL: availability-fp_to_chars-missing
-// XFAIL: availability-print-missing
 
 // <ostream>
 
diff --git a/libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_nonunicode.pass.cpp b/libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_nonunicode.pass.cpp
index 350e20387b01c3..054ea36a04c7ee 100644
--- a/libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_nonunicode.pass.cpp
+++ b/libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_nonunicode.pass.cpp
@@ -12,7 +12,6 @@
 // UNSUPPORTED: no-filesystem
 
 // XFAIL: availability-fp_to_chars-missing
-// XFAIL: availability-print-missing
 
 // <ostream>
 
diff --git a/libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_unicode.pass.cpp b/libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_unicode.pass.cpp
index 9b14c429bdb45f..b04a745b348f56 100644
--- a/libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_unicode.pass.cpp
+++ b/libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_unicode.pass.cpp
@@ -12,7 +12,6 @@
 // UNSUPPORTED: no-filesystem
 
 // XFAIL: availability-fp_to_chars-missing
-// XFAIL: availability-print-missing
 
 // <ostream>
 
diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index 77efbdf1310349..461e134f165fc8 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -586,13 +586,4 @@ def check_gdb(cfg):
             cfg.available_features,
         ),
     ),
-    # Tests that require support for <print> and std::print in <ostream> in the built library.
-    Feature(
-        name="availability-print-missing",
-        when=lambda cfg: BooleanExpression.evaluate(
-            # TODO(ldionne) Please provide the correct value.
-            "stdlib=apple-libc++ && target={{.+}}-apple-macosx{{(10.9|10.10|10.11|10.12|10.13|10.14|10.15|11.0|12.0|13.0)(.0)?}}",
-            cfg.available_features,
-        ),
-    ),
 ]

@ldionne ldionne self-assigned this Jan 16, 2024
@mordante mordante merged commit 4a87db6 into llvm:main Jan 16, 2024
8 of 9 checks passed
@mordante mordante deleted the enables_ostream_print_on_apple_backdeployment branch January 16, 2024 18:14
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
As suggested in llvm#73262 this enable the stream printing on Apple
backdeployment targets. This omits the check whether the file is a
terminal. This is not entirely conforming, but the differences should be
minor and are typically not observable.

Fixes llvm#75225
mordante added a commit to mordante/llvm-project that referenced this pull request Feb 2, 2024
Having the test in the header requires including unistd.h on POSIX
platforms. This header has other declarations which may conflict with
code that uses named declarations provided by this header. For example
code using "int pipe;" would conflict with the function pipe in this
header.

Moving the code to the dylib means std::print would not be available on
Apple backdeployment targets. On POSIX platforms there is no transcoding
required so a not Standard conforming implementation is still a useful
and the observable differences are minimal. This behaviour has been done
for print before llvm#76293.

Note questions have been raised in LWG4044 "Confusing requirements for
std::print on POSIX platforms", whether or not the isatty check on POSIX
platforms is required. When this LWG issue is resolved the
backdeployment targets could become Standard compliant.

This patch is intended to be backported to the LLVM-18 branch.

Fixes: llvm#79782
mordante added a commit to mordante/llvm-project that referenced this pull request Feb 3, 2024
Having the test in the header requires including unistd.h on POSIX
platforms. This header has other declarations which may conflict with
code that uses named declarations provided by this header. For example
code using "int pipe;" would conflict with the function pipe in this
header.

Moving the code to the dylib means std::print would not be available on
Apple backdeployment targets. On POSIX platforms there is no transcoding
required so a not Standard conforming implementation is still a useful
and the observable differences are minimal. This behaviour has been done
for print before llvm#76293.

Note questions have been raised in LWG4044 "Confusing requirements for
std::print on POSIX platforms", whether or not the isatty check on POSIX
platforms is required. When this LWG issue is resolved the
backdeployment targets could become Standard compliant.

This patch is intended to be backported to the LLVM-18 branch.

Fixes: llvm#79782
mordante added a commit to mordante/llvm-project that referenced this pull request Feb 9, 2024
Having the test in the header requires including unistd.h on POSIX
platforms. This header has other declarations which may conflict with
code that uses named declarations provided by this header. For example
code using "int pipe;" would conflict with the function pipe in this
header.

Moving the code to the dylib means std::print would not be available on
Apple backdeployment targets. On POSIX platforms there is no transcoding
required so a not Standard conforming implementation is still a useful
and the observable differences are minimal. This behaviour has been done
for print before llvm#76293.

Note questions have been raised in LWG4044 "Confusing requirements for
std::print on POSIX platforms", whether or not the isatty check on POSIX
platforms is required. When this LWG issue is resolved the
backdeployment targets could become Standard compliant.

This patch is intended to be backported to the LLVM-18 branch.

Fixes: llvm#79782
mordante added a commit to mordante/llvm-project that referenced this pull request Feb 10, 2024
Having the test in the header requires including unistd.h on POSIX
platforms. This header has other declarations which may conflict with
code that uses named declarations provided by this header. For example
code using "int pipe;" would conflict with the function pipe in this
header.

Moving the code to the dylib means std::print would not be available on
Apple backdeployment targets. On POSIX platforms there is no transcoding
required so a not Standard conforming implementation is still a useful
and the observable differences are minimal. This behaviour has been done
for print before llvm#76293.

Note questions have been raised in LWG4044 "Confusing requirements for
std::print on POSIX platforms", whether or not the isatty check on POSIX
platforms is required. When this LWG issue is resolved the
backdeployment targets could become Standard compliant.

This patch is intended to be backported to the LLVM-18 branch.

Fixes: llvm#79782
mordante added a commit that referenced this pull request Feb 10, 2024
Having the test in the header requires including unistd.h on POSIX
platforms. This header has other declarations which may conflict with
code that uses named declarations provided by this header. For example
code using "int pipe;" would conflict with the function pipe in this
header.

Moving the code to the dylib means std::print would not be available on
Apple backdeployment targets. On POSIX platforms there is no transcoding
required so a not Standard conforming implementation is still a useful
and the observable differences are minimal. This behaviour has been done
for print before #76293.

Note questions have been raised in LWG4044 "Confusing requirements for
std::print on POSIX platforms", whether or not the isatty check on POSIX
platforms is required. When this LWG issue is resolved the
backdeployment targets could become Standard compliant.

This patch is intended to be backported to the LLVM-18 branch.

Fixes: #79782
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Feb 11, 2024
Having the test in the header requires including unistd.h on POSIX
platforms. This header has other declarations which may conflict with
code that uses named declarations provided by this header. For example
code using "int pipe;" would conflict with the function pipe in this
header.

Moving the code to the dylib means std::print would not be available on
Apple backdeployment targets. On POSIX platforms there is no transcoding
required so a not Standard conforming implementation is still a useful
and the observable differences are minimal. This behaviour has been done
for print before llvm#76293.

Note questions have been raised in LWG4044 "Confusing requirements for
std::print on POSIX platforms", whether or not the isatty check on POSIX
platforms is required. When this LWG issue is resolved the
backdeployment targets could become Standard compliant.

This patch is intended to be backported to the LLVM-18 branch.

Fixes: llvm#79782
(cherry picked from commit 4fb7b33)
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 16, 2024
Having the test in the header requires including unistd.h on POSIX
platforms. This header has other declarations which may conflict with
code that uses named declarations provided by this header. For example
code using "int pipe;" would conflict with the function pipe in this
header.

Moving the code to the dylib means std::print would not be available on
Apple backdeployment targets. On POSIX platforms there is no transcoding
required so a not Standard conforming implementation is still a useful
and the observable differences are minimal. This behaviour has been done
for print before llvm#76293.

Note questions have been raised in LWG4044 "Confusing requirements for
std::print on POSIX platforms", whether or not the isatty check on POSIX
platforms is required. When this LWG issue is resolved the
backdeployment targets could become Standard compliant.

This patch is intended to be backported to the LLVM-18 branch.

Fixes: llvm#79782
(cherry picked from commit 4fb7b33)
blueboxd pushed a commit to blueboxd/libcxx that referenced this pull request May 4, 2024
Having the test in the header requires including unistd.h on POSIX
platforms. This header has other declarations which may conflict with
code that uses named declarations provided by this header. For example
code using "int pipe;" would conflict with the function pipe in this
header.

Moving the code to the dylib means std::print would not be available on
Apple backdeployment targets. On POSIX platforms there is no transcoding
required so a not Standard conforming implementation is still a useful
and the observable differences are minimal. This behaviour has been done
for print before llvm/llvm-project#76293.

Note questions have been raised in LWG4044 "Confusing requirements for
std::print on POSIX platforms", whether or not the isatty check on POSIX
platforms is required. When this LWG issue is resolved the
backdeployment targets could become Standard compliant.

This patch is intended to be backported to the LLVM-18 branch.

Fixes: llvm/llvm-project#79782
NOKEYCHECK=True
GitOrigin-RevId: 4fb7b3301bfbd439eb3d30d6a36c7cdb26941a0d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[libc++] Make <print> work without deployment target requirement
3 participants