[libc++][print] Adds ostream overloads. #73262
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Mark de Wever (mordante) ChangesFinishes implementation of
Differential Revision: https://reviews.llvm.org/D156609 Patch is 176.10 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/73262.diff 41 Files Affected:
diff --git a/libcxx/docs/FeatureTestMacroTable.rst b/libcxx/docs/FeatureTestMacroTable.rst
index 3b2dff3108b0f60..b7b0bf588d2f6d9 100644
--- a/libcxx/docs/FeatureTestMacroTable.rst
+++ b/libcxx/docs/FeatureTestMacroTable.rst
@@ -344,7 +344,7 @@ Status
--------------------------------------------------- -----------------
``__cpp_lib_out_ptr`` *unimplemented*
--------------------------------------------------- -----------------
- ``__cpp_lib_print`` *unimplemented*
+ ``__cpp_lib_print`` ``202207L``
--------------------------------------------------- -----------------
``__cpp_lib_ranges_as_const`` *unimplemented*
--------------------------------------------------- -----------------
diff --git a/libcxx/docs/ImplementationDefinedBehavior.rst b/libcxx/docs/ImplementationDefinedBehavior.rst
index c1f13d7f1cf160c..cccf8cfbb707830 100644
--- a/libcxx/docs/ImplementationDefinedBehavior.rst
+++ b/libcxx/docs/ImplementationDefinedBehavior.rst
@@ -28,6 +28,21 @@ The Standard allows implementations to automatically update the
This offers a way for users to update the *remote time zone database* and
give them full control over the process.
+
+`[ostream.formatted.print]/3 <http://eel.is/c++draft/ostream.formatted.print#3>`_ A terminal capable of displaying Unicode
+--------------------------------------------------------------------------------------------------------------------------
+
+* First it determines whether the stream's ``rdbuf()`` has an underlying
+ ``FILE*``. This is ``true`` in the following cases:
+
+ * The stream is ``std::cout``, ``std::cerr``, or ``std::clog``.
+
+ * A ``std::basic_filebuf<CharT, Traits>`` derived from ``std::filebuf``.
+
+* The way to determine whether this ``FILE*`` is the same as specified
+ for `void vprint_unicode(FILE* stream, string_view fmt, format_args args);
+ <http://eel.is/c++draft/print.fun#7>`_.
+
Listed in the index of implementation-defined behavior
======================================================
diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
index f223399cd3f0f92..19e19f29708a589 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -41,6 +41,8 @@ What's New in Libc++ 18.0.0?
Implemented Papers
------------------
+- P2093R14 Formatted output
+- P2539R4 Should the output of ``std::print`` to a terminal be synchronized with the underlying stream?
- P2497R0 - Testing for success or failure of ``<charconv>`` functions
- P2697R1 - Interfacing ``bitset`` with ``string_view``
diff --git a/libcxx/docs/Status/Cxx23Papers.csv b/libcxx/docs/Status/Cxx23Papers.csv
index a7a34f474ff575d..7cb05c4002427db 100644
--- a/libcxx/docs/Status/Cxx23Papers.csv
+++ b/libcxx/docs/Status/Cxx23Papers.csv
@@ -59,7 +59,7 @@
"`P1467R9 <https://wg21.link/P1467R9>`__","LWG","Extended ``floating-point`` types and standard names","July 2022","",""
"`P1642R11 <https://wg21.link/P1642R11>`__","LWG","Freestanding ``[utilities]``, ``[ranges]``, and ``[iterators]``","July 2022","",""
"`P1899R3 <https://wg21.link/P1899R3>`__","LWG","``stride_view``","July 2022","","","|ranges|"
-"`P2093R14 <https://wg21.link/P2093R14>`__","LWG","Formatted output","July 2022","","|In Progress|"
+"`P2093R14 <https://wg21.link/P2093R14>`__","LWG","Formatted output","July 2022","","|Complete|","18.0"
"`P2165R4 <https://wg21.link/P2165R4>`__","LWG","Compatibility between ``tuple``, ``pair`` and ``tuple-like`` objects","July 2022","",""
"`P2278R4 <https://wg21.link/P2278R4>`__","LWG","``cbegin`` should always return a constant iterator","July 2022","","","|ranges|"
"`P2286R8 <https://wg21.link/P2286R8>`__","LWG","Formatting Ranges","July 2022","|Complete|","16.0","|format| |ranges|"
@@ -99,7 +99,7 @@
"`P2167R3 <https://wg21.link/P2167R3>`__","LWG", "Improved Proposed Wording for LWG 2114", "November 2022","","",""
"`P2396R1 <https://wg21.link/P2396R1>`__","LWG", "Concurrency TS 2 fixes ", "November 2022","","","|concurrency TS|"
"`P2505R5 <https://wg21.link/P2505R5>`__","LWG", "Monadic Functions for ``std::expected``", "November 2022","|Complete|","17.0",""
-"`P2539R4 <https://wg21.link/P2539R4>`__","LWG", "Should the output of ``std::print`` to a terminal be synchronized with the underlying stream?", "November 2022","|In Progress|","","|format|"
+"`P2539R4 <https://wg21.link/P2539R4>`__","LWG", "Should the output of ``std::print`` to a terminal be synchronized with the underlying stream?", "November 2022","|Complete|","18.0","|format|"
"`P2602R2 <https://wg21.link/P2602R2>`__","LWG", "Poison Pills are Too Toxic", "November 2022","","","|ranges|"
"`P2708R1 <https://wg21.link/P2708R1>`__","LWG", "No Further Fundamentals TSes", "November 2022","|Nothing to do|","",""
"","","","","","",""
diff --git a/libcxx/docs/Status/FormatIssues.csv b/libcxx/docs/Status/FormatIssues.csv
index 5c6463fa97ed2ba..665f252179f3a1f 100644
--- a/libcxx/docs/Status/FormatIssues.csv
+++ b/libcxx/docs/Status/FormatIssues.csv
@@ -5,11 +5,11 @@ Number,Name,Standard,Assignee,Status,First released version
`P1868 <https://wg21.link/P1868>`_,"width: clarifying units of width and precision in std::format (Implements the unicode support.)","C++20",Mark de Wever,|Complete|,14.0
`P2216 <https://wg21.link/P2216>`_,"std::format improvements","C++20",Mark de Wever,|Complete|,15.0
`P2418 <https://wg21.link/P2418>`__,"Add support for ``std::generator``-like types to ``std::format``","C++20",Mark de Wever,|Complete|,15.0
-"`P2093R14 <https://wg21.link/P2093R14>`__","Formatted output","C++23",Mark de Wever,|In Progress|
+"`P2093R14 <https://wg21.link/P2093R14>`__","Formatted output","C++23",Mark de Wever,|Complete|,"18.0"
"`P2286R8 <https://wg21.link/P2286R8>`__","Formatting Ranges","C++23","Mark de Wever","|Complete|",16.0
"`P2508R1 <https://wg21.link/P2508R1>`__","Exposing ``std::basic-format-string``","C++23","Mark de Wever","|Complete|",15.0
"`P2585R0 <https://wg21.link/P2585R0>`__","Improving default container formatting","C++23","Mark de Wever","|Complete|",17.0
-"`P2539R4 <https://wg21.link/P2539R4>`__","Should the output of ``std::print`` to a terminal be synchronized with the underlying stream?","C++23","Mark de Wever","|In Progress|"
+"`P2539R4 <https://wg21.link/P2539R4>`__","Should the output of ``std::print`` to a terminal be synchronized with the underlying stream?","C++23","Mark de Wever","|Complete|","18.0"
"`P2713R1 <https://wg21.link/P2713R1>`__","Escaping improvements in ``std::format``","C++23","Mark de Wever",""
"`P2675R1 <https://wg21.link/P2675R1>`__","``format``'s width estimation is too approximate and not forward compatible","C++23","Mark de Wever","|Complete|",17.0
"`P2572R1 <https://wg21.link/P2572R1>`__","``std::format`` fill character allowances","C++23","Mark de Wever","|Complete|",17.0
diff --git a/libcxx/docs/Status/FormatPaper.csv b/libcxx/docs/Status/FormatPaper.csv
index 0acde337ccafe16..82da54284c73860 100644
--- a/libcxx/docs/Status/FormatPaper.csv
+++ b/libcxx/docs/Status/FormatPaper.csv
@@ -49,4 +49,4 @@ Section,Description,Dependencies,Assignee,Status,First released version
"`P2093R14 <https://wg21.link/P2093R14>`__","Formatted output"
`[print.fun] <https://wg21.link/print.fun>`__,"Output to ``stdout``",,Mark de Wever,|Complete|, 17.0
`[print.fun] <https://wg21.link/print.fun>`__,"Output to ``FILE*``",,Mark de Wever,|Complete|, 17.0
-`[ostream.formatted.print] <https://wg21.link/ostream.formatted.print>`__,"Output to ``ostream``",,Mark de Wever
+`[ostream.formatted.print] <https://wg21.link/ostream.formatted.print>`__,"Output to ``ostream``",,Mark de Wever,|Complete|, 18.0
diff --git a/libcxx/include/__availability b/libcxx/include/__availability
index 99a16c968de3c60..6ccdb12c1a431e9 100644
--- a/libcxx/include/__availability
+++ b/libcxx/include/__availability
@@ -139,6 +139,12 @@
// # define _LIBCPP_AVAILABILITY_HAS_NO_TZDB
# define _LIBCPP_AVAILABILITY_TZDB
+ // This controls the availability of C++23 <print>, which
+ // has a dependency on the built library (it needs access to
+ // the underlying buffer types of std::cout, std::cerr, and std::clog.
+// # define _LIBCPP_AVAILABILITY_HAS_NO_PRINT
+# define _LIBCPP_AVAILABILITY_PRINT
+
// Enable additional explicit instantiations of iostreams components. This
// reduces the number of weak definitions generated in programs that use
// iostreams by providing a single strong definition in the shared library.
@@ -241,6 +247,9 @@
# define _LIBCPP_AVAILABILITY_HAS_NO_TZDB
# define _LIBCPP_AVAILABILITY_TZDB __attribute__((unavailable))
+# define _LIBCPP_AVAILABILITY_HAS_NO_PRINT
+# define _LIBCPP_AVAILABILITY_PRINT __attribute__((unavailable))
+
# if (defined(__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__) && __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ < 120000) || \
(defined(__ENVIRONMENT_IPHONE_OS_VERSION_MIN_REQUIRED__) && __ENVIRONMENT_IPHONE_OS_VERSION_MIN_REQUIRED__ < 150000) || \
(defined(__ENVIRONMENT_TV_OS_VERSION_MIN_REQUIRED__) && __ENVIRONMENT_TV_OS_VERSION_MIN_REQUIRED__ < 150000) || \
diff --git a/libcxx/include/fstream b/libcxx/include/fstream
index cfe1ff82ba24468..1d1a764c3935fc8 100644
--- a/libcxx/include/fstream
+++ b/libcxx/include/fstream
@@ -256,6 +256,8 @@ public:
inline static const char*
__make_mdstring(ios_base::openmode __mode) _NOEXCEPT;
+ _LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI FILE* __file() { return __file_; }
+
protected:
// 27.9.1.5 Overridden virtual functions:
int_type underflow() override;
diff --git a/libcxx/include/ostream b/libcxx/include/ostream
index 469232964d5b79c..1ed060ca889d6f2 100644
--- a/libcxx/include/ostream
+++ b/libcxx/include/ostream
@@ -159,13 +159,24 @@ basic_ostream<wchar_t, traits>& operator<<(basic_ostream<wchar_t, traits>&, cons
template<class traits>
basic_ostream<wchar_t, traits>& operator<<(basic_ostream<wchar_t, traits>&, const char32_t*) = delete; // since C++20
+// [ostream.formatted.print], print functions
+template<class... Args> // since C++23
+ void print(ostream& os, format_string<Args...> fmt, Args&&... args);
+template<class... Args> // since C++23
+ void println(ostream& os, format_string<Args...> fmt, Args&&... args);
+
+void vprint_unicode(ostream& os, string_view fmt, format_args args); // since C++23
+void vprint_nonunicode(ostream& os, string_view fmt, format_args args); // since C++23
} // std
*/
#include <__assert> // all public C++ headers provide the assertion handler
+#include <__availability>
#include <__config>
#include <__exception/operations.h>
+#include <__format/format_args.h>
+#include <__format/format_functions.h>
#include <__fwd/ostream.h>
#include <__memory/shared_ptr.h>
#include <__memory/unique_ptr.h>
@@ -176,10 +187,13 @@ basic_ostream<wchar_t, traits>& operator<<(basic_ostream<wchar_t, traits>&, cons
#include <__type_traits/void_t.h>
#include <__utility/declval.h>
#include <bitset>
+#include <cstdio>
#include <ios>
#include <locale>
#include <new>
+#include <print>
#include <streambuf>
+#include <string_view>
#include <version>
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
@@ -1195,6 +1209,140 @@ extern template class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS basic_ostream<char>;
extern template class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS basic_ostream<wchar_t>;
#endif
+#if _LIBCPP_STD_VER >= 23
+
+template <class = void> // TODO PRINT template or availability markup fires too eagerly (http://llvm.org/PR61563).
+_LIBCPP_HIDE_FROM_ABI inline void
+__vprint_nonunicode(ostream& __os, string_view __fmt, format_args __args, bool __write_nl) {
+ ostream::sentry __s(__os);
+ if (__s) {
+ string __o = vformat(__os.getloc(), __fmt, __args);
+ if (__write_nl)
+ __o += '\n';
+
+ const char* __str = __o.data();
+ size_t __len = __o.size();
+
+# ifndef _LIBCPP_HAS_NO_EXCEPTIONS
+ try {
+# endif // _LIBCPP_HAS_NO_EXCEPTIONS
+ typedef ostreambuf_iterator<char> _Ip;
+ if (std::__pad_and_output(
+ _Ip(__os),
+ __str,
+ (__os.flags() & ios_base::adjustfield) == ios_base::left ? __str + __len : __str,
+ __str + __len,
+ __os,
+ __os.fill())
+ .failed())
+ __os.setstate(ios_base::badbit | ios_base::failbit);
+
+# ifndef _LIBCPP_HAS_NO_EXCEPTIONS
+ } catch (...) {
+ __os.__set_badbit_and_consider_rethrow();
+ }
+# endif // _LIBCPP_HAS_NO_EXCEPTIONS
+ }
+}
+
+template <class = void> // TODO PRINT template or availability markup fires too eagerly (http://llvm.org/PR61563).
+_LIBCPP_HIDE_FROM_ABI inline void vprint_nonunicode(ostream& __os, string_view __fmt, format_args __args) {
+ std::__vprint_nonunicode(__os, __fmt, __args, false);
+}
+
+// Returns the FILE* associated with the __os.
+// Returns a nullptr when no FILE* is associated with __os.
+// This function is in the dylib since the type of the buffer associated
+// with std::cout, std::cerr, and std::clog is only known in the dylib.
+//
+// This function implements part of the implementation-defined behavior
+// of [ostream.formatted.print]/3
+// If the function is vprint_unicode and os is a stream that refers to
+// a terminal capable of displaying Unicode which is determined in an
+// implementation-defined manner, writes out to the terminal using the
+// 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);
+
+# 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
+__vprint_unicode(ostream& __os, string_view __fmt, format_args __args, bool __write_nl) {
+ FILE* __file = std::__get_ostream_file(__os);
+ if (!__file || !__print::__is_terminal(__file))
+ return std::__vprint_nonunicode(__os, __fmt, __args, __write_nl);
+
+ // [ostream.formatted.print]/3
+ // If the function is vprint_unicode and os is a stream that refers to a
+ // terminal capable of displaying Unicode which is determined in an
+ // implementation-defined manner, writes out to the terminal using the
+ // native Unicode API; if out contains invalid code units, the behavior is
+ // undefined and implementations are encouraged to diagnose it. If the
+ // native Unicode API is used, the function flushes os before writing out.
+ //
+ // This is the path for the native API, start with flushing.
+ __os.flush();
+
+# ifndef _LIBCPP_HAS_NO_EXCEPTIONS
+ try {
+# endif // _LIBCPP_HAS_NO_EXCEPTIONS
+ ostream::sentry __s(__os);
+ if (__s) {
+# ifndef _WIN32
+ __print::__vprint_unicode_posix(__file, __fmt, __args, __write_nl, true);
+# elif !defined(_LIBCPP_HAS_NO_WIDE_CHARACTERS)
+ __print::__vprint_unicode_windows(__file, __fmt, __args, __write_nl, true);
+# else
+# error "Windows builds with wchar_t disabled are not supported."
+# endif
+ }
+
+# ifndef _LIBCPP_HAS_NO_EXCEPTIONS
+ } catch (...) {
+ __os.__set_badbit_and_consider_rethrow();
+ }
+# endif // _LIBCPP_HAS_NO_EXCEPTIONS
+}
+
+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
+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
+print(ostream& __os, format_string<_Args...> __fmt, _Args&&... __args) {
+# ifndef _LIBCPP_HAS_NO_UNICODE
+ if constexpr (__print::__use_unicode)
+ std::__vprint_unicode(__os, __fmt.get(), std::make_format_args(__args...), false);
+ else
+ std::__vprint_nonunicode(__os, __fmt.get(), std::make_format_args(__args...), false);
+# else // _LIBCPP_HAS_NO_UNICODE
+ std::__vprint_nonunicode(__os, __fmt.get(), std::make_format_args(__args...), false);
+# endif // _LIBCPP_HAS_NO_UNICODE
+}
+
+template <class... _Args>
+_LIBCPP_AVAILABILITY_PRINT _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
+ // std::format is a std::string which is then copied. This solution
+ // just appends a newline at the end of the output.
+ if constexpr (__print::__use_unicode)
+ std::__vprint_unicode(__os, __fmt.get(), std::make_format_args(__args...), true);
+ else
+ std::__vprint_nonunicode(__os, __fmt.get(), std::make_format_args(__args...), true);
+# else // _LIBCPP_HAS_NO_UNICODE
+ std::__vprint_nonunicode(__os, __fmt.get(), std::make_format_args(__args...), true);
+# endif // _LIBCPP_HAS_NO_UNICODE
+}
+
+#endif // _LIBCPP_STD_VER >= 23
+
_LIBCPP_END_NAMESPACE_STD
#if !defined(_LIBCPP_REMOVE_TRANSITIVE_INCLUDES) && _LIBCPP_STD_VER <= 20
diff --git a/libcxx/include/print b/libcxx/include/print
index d119c8bda749768..2174557794e4dfe 100644
--- a/libcxx/include/print
+++ b/libcxx/include/print
@@ -198,7 +198,11 @@ inline constexpr bool __use_unicode = true;
# endif
_LIBCPP_HIDE_FROM_ABI inline bool __is_terminal(FILE* __stream) {
-# ifdef _WIN32
+ // The macro _LIBCPP_TESTING_PRINT_IS_TERMINAL is used to change
+ // the behavior in the test. This is not part of the public API.
+# ifdef _LIBCPP_TESTING_PRINT_IS_TERMINAL
+ return _LIBCPP_TESTING_PRINT_IS_TERMINAL(__stream);
+# elif defined(_WIN32)
return std::__is_windows_terminal(__stream);
# elif __has_include(<unistd.h>)
return isatty(fileno(__stream));
diff --git a/libcxx/include/version b/libcxx/include/version
index 80f2920128da9d1..3d7298c035a3903 100644
--- a/libcxx/include/version
+++ b/libcxx/include/version
@@ -455,7 +455,7 @@ __cpp_lib_within_lifetime 202306L <type_traits>
# undef __cpp_lib_optional
# define __cpp_lib_optional 202110L
// # define __cpp_lib_out_ptr 202106L
-// # define __cpp_lib_print 202207L
+# define __cpp_lib_print 202207L
// # define __cpp_lib_ranges_as_const 202207L
# define __cpp_lib_ranges_as_rvalue 202207L
// # define __cpp_lib_ranges_chunk 202202L
diff --git a/libcxx/lib/abi/arm64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/arm64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist
index 8daad89f52e6f7c..b51af1bb0f9ef24 100644
--- a/libcxx/lib/abi/arm64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist
+++ b/libcxx/lib/abi/arm64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist
@@ -1465,6 +1465,7 @@
{'is_defined': True, 'name': '__ZNSt3__117moneypunct_bynameIcLb1EE4initEPKc', 'type': 'FUNC'}
{'is_defined': True, 'name': '__ZNSt3__117moneypunct_bynameIwLb0EE4initEPKc', 'type': 'FUNC'}
{'is_defined': True, 'name': '__ZNSt3__117moneypunct_bynameIwLb1EE4initEPKc', 'type': 'FUNC'}
+{'is_defined': True, 'name': '__ZNSt3__118__get_ostream_fileERNS_13basic_ostreamIcNS_11char_traitsIcEEEE', 'type': 'FUNC'}
{'is_defined': True, 'name': '__ZNSt3__118__time_get_storageIcE4initERKNS_5ctypeIcEE', 'type': 'FUNC'}
{'is_defined': True, 'name': '__ZNSt3__118__time_get_storageIcE9__analyzeEcRKNS_5ctypeIcEE', 'type': 'FUNC'}
{'is_defined': True, 'name': '__ZNSt3__118__time_get_storageIcEC1EPKc', 'type': 'FUNC'}
diff --git a/libcxx/lib/abi/powerpc-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/powerpc-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist
index 91976f500539daa..55987d4c913bbe7 100644
--- a/libcxx/lib/abi/powerpc-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist
+++ b/libcxx/lib/abi/powerpc-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist
@@ ...
[truncated]
|
|
✅ With the latest revision this PR passed the Python code formatter. |
mordante
force-pushed
the
GH-ostream_overloads
branch
from
7e66bd9 to
5cede57
Compare
ldionne
requested changes
Nov 28, 2023
| template <class... _Args> | ||
| _LIBCPP_AVAILABILITY_PRINT _LIBCPP_HIDE_FROM_ABI void | ||
| print(ostream& __os, format_string<_Args...> __fmt, _Args&&... __args) { | ||
| # ifndef _LIBCPP_HAS_NO_UNICODE |
There was a problem hiding this comment.
Is there a reason why we have both _LIBCPP_HAS_NO_UNICODE and __print::__use_unicode?
There was a problem hiding this comment.
_LIBCPP_HAS_NO_UNICODE means there platform has no unicode at all.
__use_unicode means the execution charset is not capable of Unicode. Currently this is only used with MSVC. There were (are?) plans to implement this in Clang too.
There was a problem hiding this comment.
I would suggest renaming this to something like __print::__unicode_execution_charset, which IMO would make this somewhat clearer. I am OK if you want to do this in a separate NFC patch.
...ream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_unicode.pass.cpp
Outdated
Show resolved
Hide resolved
...ormat/output.streams/ostream.formatted/ostream.formatted.print/locale-specific_form.pass.cpp
Show resolved
Hide resolved
mordante
force-pushed
the
GH-ostream_overloads
branch
from
5cede57 to
e9e81f3
Compare
You can test this locally with the following command:git-clang-format --diff 17aa5201710325ac4b8ccc95bb7954fea8e14849 8e457406d51a9b8d55f7641213a07c4c207e8137 -- libcxx/src/ostream.cpp libcxx/test/libcxx/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_unicode.pass.cpp libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/locale-specific_form.pass.cpp libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/print.pass.cpp libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/print_tests.h libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/println.pass.cpp libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_nonunicode.pass.cpp libcxx/test/std/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_unicode.pass.cpp libcxx/include/__availability libcxx/include/fstream libcxx/include/ostream libcxx/include/print libcxx/include/version libcxx/modules/std/ostream.inc libcxx/src/std_stream.h libcxx/test/std/language.support/support.limits/support.limits.general/ostream.version.compile.pass.cpp libcxx/test/std/language.support/support.limits/support.limits.general/print.version.compile.pass.cpp libcxx/test/std/language.support/support.limits/support.limits.general/version.version.compile.pass.cppView the diff from clang-format here.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 71dd6c0d08..a86a66d764 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
@@ -41,8 +41,8 @@ bool is_terminal(FILE*);
scoped_test_env env;
std::string filename = env.create_file("output.txt");
-int is_terminal_calls = 0;
-bool is_terminal_result = false;
+int is_terminal_calls = 0;
+bool is_terminal_result = false;
bool is_terminal(FILE*) {
++is_terminal_calls;
return is_terminal_result;
@@ -52,8 +52,8 @@ bool is_terminal(FILE*) {
// considered to be backed by a FILE*. Then the stream should never check
// whether it's a terminal.
static void test_is_terminal_not_a_file_stream() {
- is_terminal_calls = 0;
- is_terminal_result = false;
+ is_terminal_calls = 0;
+ is_terminal_result = false;
{
std::stringstream stream;
std::print(stream, "test");
@@ -68,8 +68,8 @@ static void test_is_terminal_not_a_file_stream() {
// When the stream is a file stream, its FILE* may be a terminal. Validate this
// is tested.
static void test_is_terminal_file_stream() {
- is_terminal_calls = 0;
- is_terminal_result = false;
+ is_terminal_calls = 0;
+ is_terminal_result = false;
{
std::fstream stream(filename);
assert(stream.is_open());
@@ -90,8 +90,8 @@ static void test_is_terminal_file_stream() {
static void test_is_terminal_rdbuf_derived_from_filebuf() {
struct my_filebuf : public std::filebuf {};
- is_terminal_calls = 0;
- is_terminal_result = false;
+ is_terminal_calls = 0;
+ is_terminal_result = false;
my_filebuf buf;
buf.open(filename, std::ios_base::out);
@@ -105,8 +105,8 @@ static void test_is_terminal_rdbuf_derived_from_filebuf() {
// When the stream is cout, clog, or cerr, its FILE* may be a terminal. Validate
// this is tested.
static void test_is_terminal_std_cout_cerr_clog() {
- is_terminal_calls = 0;
- is_terminal_result = false;
+ is_terminal_calls = 0;
+ is_terminal_result = false;
{
std::print(std::cout, "test");
assert(is_terminal_calls == 1);
@@ -138,7 +138,7 @@ static void test_is_terminal_is_flushed() {
}
};
- is_terminal_result = false;
+ is_terminal_result = false;
sync_counter buf;
std::ostream stream(&buf);
|
mordante
force-pushed
the
GH-ostream_overloads
branch
2 times, most recently
from
a8131c6 to
1ef14d3
Compare
mordante
force-pushed
the
GH-ostream_overloads
branch
4 times, most recently
from
f023642 to
c0c1590
Compare
ldionne
requested changes
Dec 12, 2023
libcxx/src/std_stream.h
Outdated
| @@ -286,7 +286,9 @@ class _LIBCPP_HIDDEN __stdoutbuf | |||
|
|
|||
| __stdoutbuf(FILE* __fp, state_type* __st); | |||
|
|
|||
| protected: | |||
| [[nodiscard]] FILE* __file() { return __file_; } | |||
There was a problem hiding this comment.
For this one I don't care whether we use a friend or not, since Hyrum's law is not a problem for private headers like this one.
There was a problem hiding this comment.
For consistency, I think it makes sense to do it here too.
...utput/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/print_tests.h
Outdated
Show resolved
Hide resolved
| "\nFormat string ", fmt.get(), "\nExpected output ", expected, "\nActual output ", out, '\n')); | ||
| }; | ||
|
|
||
| auto test_exception = []<class... Args>(std::string_view, std::string_view, Args&&...) { |
There was a problem hiding this comment.
Is this intended or are we never testing the exception behavior for this function and vprint_unicode?
There was a problem hiding this comment.
Good catch, this is a copy-paste issue.
Comment on lines
+55
to
+56
| }; | ||
| // [ostream.formatted.print]/3.2 |
There was a problem hiding this comment.
Nit: Maybe add a newline? Also applies in other files.
|
|
||
| # 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 |
There was a problem hiding this comment.
In a follow-up, we could use _LIBCPP_AVAILABILITY_HAS_PRINT to check whether we have __get_ostream_file on the current deployment target. If we don't, we could instead assume that !__file and use __vprint_nonunicode. That would make this mostly work for older deployment targets, except for the flush below. But by and far, users could use <print> on older deployment targets with no issues.
We could then even remove the availability annotations on __vprint_unicode and others, since they would basically have no deployment target requirements anymore.
mordante
force-pushed
the
GH-ostream_overloads
branch
from
f2aa35d to
3708940
Compare
|
Please see issue #70142. This PR has similar issue. |
ldionne
approved these changes
Dec 19, 2023
Finishes implementation of
- P2093R14 Formatted output
- P2539R4 Should the output of std::print to a terminal be
synchronized with the underlying stream?
Differential Revision: https://reviews.llvm.org/D156609
Thanks for the info. I hadn't seen that issue before. I'm not yet convinced the issue is correct and I want to look at it in more detail. I don't want to block this review on it, changing it isn't an ABI break. |
mordante
force-pushed
the
GH-ostream_overloads
branch
from
3708940 to
8e45740
Compare
vitalybuka
added a commit
to llvm/llvm-zorg
that referenced
this pull request
Dec 19, 2023
mordante
added a commit
to mordante/llvm-project
that referenced
this pull request
Dec 23, 2023
This is addresses a review comment in llvm#73262.
mordante
added a commit
to mordante/llvm-project
that referenced
this pull request
Dec 23, 2023
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
that referenced
this pull request
Jan 16, 2024
This is addresses a review comment in #73262.
justinfargnoli
pushed a commit
to justinfargnoli/llvm-project
that referenced
this pull request
Jan 28, 2024
This is addresses a review comment in llvm#73262.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Finishes implementation of
Differential Revision: https://reviews.llvm.org/D156609