-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Reland: [libc++][format] P2637R3: Member visit (std::basic_format_arg) #76449 #79032
Reland: [libc++][format] P2637R3: Member visit (std::basic_format_arg) #76449 #79032
Conversation
…_format_arg`) (llvm#76449)"" This reverts commit 02f95b7.
@llvm/pr-subscribers-libcxx Author: Hristo Hristov (H-G-Hristov) ChangesPatch is 51.26 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/79032.diff 15 Files Affected:
diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
index fd882bafe19a517..237a63022d55ff5 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -79,6 +79,7 @@ Implemented Papers
- P1759R6 - Native handles and file streams
- P2868R3 - Remove Deprecated ``std::allocator`` Typedef From C++26
- P2517R1 - Add a conditional ``noexcept`` specification to ``std::apply``
+- P2637R3 - Member ``visit``
- P2447R6 - ``span`` over initializer list
diff --git a/libcxx/docs/Status/Cxx2cPapers.csv b/libcxx/docs/Status/Cxx2cPapers.csv
index f80b1f6b663f045..c45aa3c510072e4 100644
--- a/libcxx/docs/Status/Cxx2cPapers.csv
+++ b/libcxx/docs/Status/Cxx2cPapers.csv
@@ -17,7 +17,7 @@
"`P0792R14 <https://wg21.link/P0792R14>`__","LWG","``function_ref``: a type-erased callable reference","Varna June 2023","","",""
"`P2874R2 <https://wg21.link/P2874R2>`__","LWG","Mandating Annex D Require No More","Varna June 2023","","",""
"`P2757R3 <https://wg21.link/P2757R3>`__","LWG","Type-checking format args","Varna June 2023","","","|format|"
-"`P2637R3 <https://wg21.link/P2637R3>`__","LWG","Member ``visit``","Varna June 2023","|Partial|","18.0",""
+"`P2637R3 <https://wg21.link/P2637R3>`__","LWG","Member ``visit``","Varna June 2023","|Complete|","18.0",""
"`P2641R4 <https://wg21.link/P2641R4>`__","CWG, LWG","Checking if a ``union`` alternative is active","Varna June 2023","","",""
"`P1759R6 <https://wg21.link/P1759R6>`__","LWG","Native handles and file streams","Varna June 2023","|Complete|","18.0",""
"`P2697R1 <https://wg21.link/P2697R1>`__","LWG","Interfacing ``bitset`` with ``string_view``","Varna June 2023","|Complete|","18.0",""
diff --git a/libcxx/docs/Status/FormatIssues.csv b/libcxx/docs/Status/FormatIssues.csv
index 513988d08036ca6..6e58e752191ea5d 100644
--- a/libcxx/docs/Status/FormatIssues.csv
+++ b/libcxx/docs/Status/FormatIssues.csv
@@ -16,7 +16,7 @@ Number,Name,Standard,Assignee,Status,First released version
"`P2693R1 <https://wg21.link/P2693R1>`__","Formatting ``thread::id`` and ``stacktrace``","C++23","Mark de Wever","|In Progress|"
"`P2510R3 <https://wg21.link/P2510R3>`__","Formatting pointers","C++26","Mark de Wever","|Complete|",17.0
"`P2757R3 <https://wg21.link/P2757R3>`__","Type-checking format args","C++26","","",
-"`P2637R3 <https://wg21.link/P2637R3>`__","Member ``visit``","C++26","","",
+"`P2637R3 <https://wg21.link/P2637R3>`__","Member ``visit``","C++26","Hristo Hristov","|Complete|",18.0
"`P2905R2 <https://wg21.link/P2905R2>`__","Runtime format strings","C++26 DR","Mark de Wever","|Complete|",18.0
"`P2918R2 <https://wg21.link/P2918R2>`__","Runtime format strings II","C++26","Mark de Wever","|Complete|",18.0
"`P2909R4 <https://wg21.link/P2909R4>`__","Fix formatting of code units as integers (Dude, where’s my ``char``?)","C++26 DR","Mark de Wever","|Complete|",18.0
diff --git a/libcxx/include/__config b/libcxx/include/__config
index 9a64cdb489119d9..00489d971c296c2 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -995,6 +995,12 @@ typedef __char32_t char32_t;
# define _LIBCPP_DEPRECATED_IN_CXX23
# endif
+# if _LIBCPP_STD_VER >= 26
+# define _LIBCPP_DEPRECATED_IN_CXX26 _LIBCPP_DEPRECATED
+# else
+# define _LIBCPP_DEPRECATED_IN_CXX26
+# endif
+
# if !defined(_LIBCPP_HAS_NO_CHAR8_T)
# define _LIBCPP_DEPRECATED_WITH_CHAR8_T _LIBCPP_DEPRECATED
# else
diff --git a/libcxx/include/__format/format_arg.h b/libcxx/include/__format/format_arg.h
index 10fca15d5a7a94e..02ee3cef7d402b7 100644
--- a/libcxx/include/__format/format_arg.h
+++ b/libcxx/include/__format/format_arg.h
@@ -93,7 +93,7 @@ _LIBCPP_HIDE_FROM_ABI constexpr __arg_t __get_packed_type(uint64_t __types, size
} // namespace __format
-// This function is not user obervable, so it can directly use the non-standard
+// This function is not user observable, so it can directly use the non-standard
// types of the "variant". See __arg_t for more details.
template <class _Visitor, class _Context>
_LIBCPP_HIDE_FROM_ABI decltype(auto) __visit_format_arg(_Visitor&& __vis, basic_format_arg<_Context> __arg) {
@@ -144,6 +144,59 @@ _LIBCPP_HIDE_FROM_ABI decltype(auto) __visit_format_arg(_Visitor&& __vis, basic_
__libcpp_unreachable();
}
+# if _LIBCPP_STD_VER >= 26 && defined(_LIBCPP_HAS_EXPLICIT_THIS_PARAMETER)
+
+template <class _Rp, class _Visitor, class _Context>
+_LIBCPP_HIDE_FROM_ABI _Rp __visit_format_arg(_Visitor&& __vis, basic_format_arg<_Context> __arg) {
+ switch (__arg.__type_) {
+ case __format::__arg_t::__none:
+ return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__monostate_);
+ case __format::__arg_t::__boolean:
+ return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__boolean_);
+ case __format::__arg_t::__char_type:
+ return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__char_type_);
+ case __format::__arg_t::__int:
+ return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__int_);
+ case __format::__arg_t::__long_long:
+ return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__long_long_);
+ case __format::__arg_t::__i128:
+# ifndef _LIBCPP_HAS_NO_INT128
+ return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__i128_);
+# else
+ __libcpp_unreachable();
+# endif
+ case __format::__arg_t::__unsigned:
+ return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__unsigned_);
+ case __format::__arg_t::__unsigned_long_long:
+ return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__unsigned_long_long_);
+ case __format::__arg_t::__u128:
+# ifndef _LIBCPP_HAS_NO_INT128
+ return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__u128_);
+# else
+ __libcpp_unreachable();
+# endif
+ case __format::__arg_t::__float:
+ return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__float_);
+ case __format::__arg_t::__double:
+ return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__double_);
+ case __format::__arg_t::__long_double:
+ return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__long_double_);
+ case __format::__arg_t::__const_char_type_ptr:
+ return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__const_char_type_ptr_);
+ case __format::__arg_t::__string_view:
+ return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__string_view_);
+ case __format::__arg_t::__ptr:
+ return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__ptr_);
+ case __format::__arg_t::__handle:
+ return std::invoke_r<_Rp>(
+ std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__arg.__value_.__handle_});
+ }
+
+ __libcpp_unreachable();
+}
+
+# endif // _LIBCPP_STD_VER >= 26 && defined(_LIBCPP_HAS_EXPLICIT_THIS_PARAMETER)
+
/// Contains the values used in basic_format_arg.
///
/// This is a separate type so it's possible to store the values and types in
@@ -227,6 +280,52 @@ class _LIBCPP_TEMPLATE_VIS basic_format_arg {
_LIBCPP_HIDE_FROM_ABI explicit operator bool() const noexcept { return __type_ != __format::__arg_t::__none; }
+# if _LIBCPP_STD_VER >= 26 && defined(_LIBCPP_HAS_EXPLICIT_THIS_PARAMETER)
+
+ // This function is user facing, so it must wrap the non-standard types of
+ // the "variant" in a handle to stay conforming. See __arg_t for more details.
+ template <class _Visitor>
+ _LIBCPP_HIDE_FROM_ABI decltype(auto) visit(this basic_format_arg __arg, _Visitor&& __vis) {
+ switch (__arg.__type_) {
+# ifndef _LIBCPP_HAS_NO_INT128
+ case __format::__arg_t::__i128: {
+ typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__i128_};
+ return std::invoke(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
+ }
+
+ case __format::__arg_t::__u128: {
+ typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__u128_};
+ return std::invoke(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
+ }
+# endif
+ default:
+ return std::__visit_format_arg(std::forward<_Visitor>(__vis), __arg);
+ }
+ }
+
+ // This function is user facing, so it must wrap the non-standard types of
+ // the "variant" in a handle to stay conforming. See __arg_t for more details.
+ template <class _Rp, class _Visitor>
+ _LIBCPP_HIDE_FROM_ABI _Rp visit(this basic_format_arg __arg, _Visitor&& __vis) {
+ switch (__arg.__type_) {
+# ifndef _LIBCPP_HAS_NO_INT128
+ case __format::__arg_t::__i128: {
+ typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__i128_};
+ return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
+ }
+
+ case __format::__arg_t::__u128: {
+ typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__u128_};
+ return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
+ }
+# endif
+ default:
+ return std::__visit_format_arg<_Rp>(std::forward<_Visitor>(__vis), __arg);
+ }
+ }
+
+# endif // _LIBCPP_STD_VER >= 26 && defined(_LIBCPP_HAS_EXPLICIT_THIS_PARAMETER)
+
private:
using char_type = typename _Context::char_type;
@@ -267,7 +366,11 @@ class _LIBCPP_TEMPLATE_VIS basic_format_arg<_Context>::handle {
// This function is user facing, so it must wrap the non-standard types of
// the "variant" in a handle to stay conforming. See __arg_t for more details.
template <class _Visitor, class _Context>
-_LIBCPP_HIDE_FROM_ABI decltype(auto) visit_format_arg(_Visitor&& __vis, basic_format_arg<_Context> __arg) {
+# if _LIBCPP_STD_VER >= 26 && defined(_LIBCPP_HAS_EXPLICIT_THIS_PARAMETER)
+_LIBCPP_DEPRECATED_IN_CXX26
+# endif
+ _LIBCPP_HIDE_FROM_ABI decltype(auto)
+ visit_format_arg(_Visitor&& __vis, basic_format_arg<_Context> __arg) {
switch (__arg.__type_) {
# ifndef _LIBCPP_HAS_NO_INT128
case __format::__arg_t::__i128: {
@@ -279,7 +382,7 @@ _LIBCPP_HIDE_FROM_ABI decltype(auto) visit_format_arg(_Visitor&& __vis, basic_fo
typename __basic_format_arg_value<_Context>::__handle __h{__arg.__value_.__u128_};
return std::invoke(std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__h});
}
-# endif
+# endif // _LIBCPP_STD_VER >= 26 && defined(_LIBCPP_HAS_EXPLICIT_THIS_PARAMETER)
default:
return std::__visit_format_arg(std::forward<_Visitor>(__vis), __arg);
}
diff --git a/libcxx/include/__format/format_context.h b/libcxx/include/__format/format_context.h
index 5b252b81f691bc1..0beaa84b028beb7 100644
--- a/libcxx/include/__format/format_context.h
+++ b/libcxx/include/__format/format_context.h
@@ -163,20 +163,25 @@ class _LIBCPP_TEMPLATE_VIS basic_format_context<typename __format::__retarget_bu
# endif
__ctx_(std::addressof(__ctx)),
__arg_([](void* __c, size_t __id) {
- return std::visit_format_arg(
- [&](auto __arg) -> basic_format_arg<basic_format_context> {
- if constexpr (same_as<decltype(__arg), monostate>)
- return {};
- else if constexpr (same_as<decltype(__arg), typename basic_format_arg<_Context>::handle>)
- // At the moment it's not possible for formatting to use a re-targeted handle.
- // TODO FMT add this when support is needed.
- std::__throw_format_error("Re-targeting handle not supported");
- else
- return basic_format_arg<basic_format_context>{
- __format::__determine_arg_t<basic_format_context, decltype(__arg)>(),
- __basic_format_arg_value<basic_format_context>(__arg)};
- },
- static_cast<_Context*>(__c)->arg(__id));
+ auto __visitor = [&](auto __arg) -> basic_format_arg<basic_format_context> {
+ if constexpr (same_as<decltype(__arg), monostate>)
+ return {};
+ else if constexpr (same_as<decltype(__arg), typename basic_format_arg<_Context>::handle>)
+ // At the moment it's not possible for formatting to use a re-targeted handle.
+ // TODO FMT add this when support is needed.
+ std::__throw_format_error("Re-targeting handle not supported");
+ else
+ return basic_format_arg<basic_format_context>{
+ __format::__determine_arg_t<basic_format_context, decltype(__arg)>(),
+ __basic_format_arg_value<basic_format_context>(__arg)};
+ };
+# if _LIBCPP_STD_VER >= 26 && defined(_LIBCPP_HAS_EXPLICIT_THIS_PARAMETER)
+ return static_cast<_Context*>(__c)->arg(__id).visit(std::move(__visitor));
+# else
+ _LIBCPP_SUPPRESS_DEPRECATED_PUSH
+ return std::visit_format_arg(std::move(__visitor), static_cast<_Context*>(__c)->arg(__id));
+ _LIBCPP_SUPPRESS_DEPRECATED_POP
+# endif // _LIBCPP_STD_VER >= 26 && defined(_LIBCPP_HAS_EXPLICIT_THIS_PARAMETER)
}) {
}
diff --git a/libcxx/include/format b/libcxx/include/format
index ab9b336d0cdabee..64f6ba1d25284aa 100644
--- a/libcxx/include/format
+++ b/libcxx/include/format
@@ -170,7 +170,7 @@ namespace std {
template<class Context> class basic_format_arg;
template<class Visitor, class Context>
- see below visit_format_arg(Visitor&& vis, basic_format_arg<Context> arg);
+ see below visit_format_arg(Visitor&& vis, basic_format_arg<Context> arg); // Deprecated in C++26
// [format.arg.store], class template format-arg-store
template<class Context, class... Args> struct format-arg-store; // exposition only
diff --git a/libcxx/test/std/utilities/format/format.arguments/format.arg/visit.pass.cpp b/libcxx/test/std/utilities/format/format.arguments/format.arg/visit.pass.cpp
new file mode 100644
index 000000000000000..994ccc70a38daae
--- /dev/null
+++ b/libcxx/test/std/utilities/format/format.arguments/format.arg/visit.pass.cpp
@@ -0,0 +1,333 @@
+//===----------------------------------------------------------------------===//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20, c++23
+// UNSUPPORTED: GCC-ALWAYS_INLINE-FIXME
+// The tested functionality needs deducing this.
+// UNSUPPORTED: clang-16 || clang-17
+// XFAIL: apple-clang
+
+// <format>
+
+// class basic_format_arg;
+
+// template<class Visitor>
+// decltype(auto) visit(this basic_format_arg arg, Visitor&& vis); // since C++26
+
+#include <algorithm>
+#include <cassert>
+#include <format>
+#include <type_traits>
+
+#include "constexpr_char_traits.h"
+#include "make_string.h"
+#include "min_allocator.h"
+#include "test_macros.h"
+
+template <class Context, class To, class From>
+void test(From value) {
+ auto store = std::make_format_args<Context>(value);
+ std::basic_format_args<Context> format_args{store};
+
+ LIBCPP_ASSERT(format_args.__size() == 1);
+ assert(format_args.get(0));
+
+ auto result = format_args.get(0).visit([v = To(value)](auto a) -> To {
+ if constexpr (std::is_same_v<To, decltype(a)>) {
+ assert(v == a);
+ return a;
+ } else {
+ assert(false);
+ return {};
+ }
+ });
+
+ using ct = std::common_type_t<From, To>;
+ assert(static_cast<ct>(result) == static_cast<ct>(value));
+}
+
+// Some types, as an extension, are stored in the variant. The Standard
+// requires them to be observed as a handle.
+template <class Context, class T>
+void test_handle(T value) {
+ auto store = std::make_format_args<Context>(value);
+ std::basic_format_args<Context> format_args{store};
+
+ LIBCPP_ASSERT(format_args.__size() == 1);
+ assert(format_args.get(0));
+
+ format_args.get(0).visit([](auto a) {
+ assert((std::is_same_v<decltype(a), typename std::basic_format_arg<Context>::handle>));
+ });
+}
+
+// Test specific for string and string_view.
+//
+// Since both result in a string_view there's no need to pass this as a
+// template argument.
+template <class Context, class From>
+void test_string_view(From value) {
+ auto store = std::make_format_args<Context>(value);
+ std::basic_format_args<Context> format_args{store};
+
+ LIBCPP_ASSERT(format_args.__size() == 1);
+ assert(format_args.get(0));
+
+ using CharT = typename Context::char_type;
+ using To = std::basic_string_view<CharT>;
+ using V = std::basic_string<CharT>;
+
+ auto result = format_args.get(0).visit([v = V(value.begin(), value.end())](auto a) -> To {
+ if constexpr (std::is_same_v<To, decltype(a)>) {
+ assert(v == a);
+ return a;
+ } else {
+ assert(false);
+ return {};
+ }
+ });
+
+ assert(std::equal(value.begin(), value.end(), result.begin(), result.end()));
+}
+
+template <class CharT>
+void test() {
+ using Context = std::basic_format_context<CharT*, CharT>;
+ std::basic_string<CharT> empty;
+ std::basic_string<CharT> str = MAKE_STRING(CharT, "abc");
+
+ // Test boolean types.
+
+ test<Context, bool>(true);
+ test<Context, bool>(false);
+
+ // Test CharT types.
+
+ test<Context, CharT, CharT>('a');
+ test<Context, CharT, CharT>('z');
+ test<Context, CharT, CharT>('0');
+ test<Context, CharT, CharT>('9');
+
+ // Test char types.
+
+ if (std::is_same_v<CharT, char>) {
+ // char to char -> char
+ test<Context, CharT, char>('a');
+ test<Context, CharT, char>('z');
+ test<Context, CharT, char>('0');
+ test<Context, CharT, char>('9');
+ } else {
+ if (std::is_same_v<CharT, wchar_t>) {
+ // char to wchar_t -> wchar_t
+ test<Context, wchar_t, char>('a');
+ test<Context, wchar_t, char>('z');
+ test<Context, wchar_t, char>('0');
+ test<Context, wchar_t, char>('9');
+ } else if (std::is_signed_v<char>) {
+ // char to CharT -> int
+ // This happens when CharT is a char8_t, char16_t, or char32_t and char
+ // is a signed type.
+ // Note if sizeof(CharT) > sizeof(int) this test fails. If there are
+ // platforms where that occurs extra tests need to be added for char32_t
+ // testing it against a long long.
+ test<Context, int, char>('a');
+ test<Context, int, char>('z');
+ test<Context, int, char>('0');
+ test<Context, int, char>('9');
+ } else {
+ // char to CharT -> unsigned
+ // This happens when CharT is a char8_t, char16_t, or char32_t and char
+ // is an unsigned type.
+ // Note if sizeof(CharT) > sizeof(unsigned) this test fails. If there are
+ // platforms where that occurs extra tests need to be added for char32_t
+ // testing it against an unsigned long long.
+ test<Context, unsigned, char>('a');
+ test<Context, unsigned, char>('z');
+ test<Context, unsigned, char>('0');
+ test<Context, unsigned, char>('9');
+ }
+ }
+
+ // Test signed integer types.
+
+ test<Context, int, signed char>(std::numeric_limits<signed char>::min());
+ test<Context, int, signed char>(0);
+ test<Context, int, signed char>(std::numeric_limits<signed char>::max());
+
+ test<Context, int, short>(std::numeric_limits<short>::min());
+ test<Context, int, short>(std::numeric_limits<signed char>::min());
+ test<Context, int, short>(0);
+ test<Context, int, short>(std::numeric_limits<signed char>::max());
+ test<Context, int, short>(std::numeric_limits<short>::max());
+
+ test<Context, int, int>(std::numeric_limits<int>::min());
+ test<Context, int, int>(std::numeric_limits<short>::min());
+ test<Context, int, int>(std::numeric_limits<signed char>::min());
+ test<Context, int, int>(0);
+ test<Context, int, int>(std::numeric_limits<signed char>::max());
+ test<Context, int, int>(std::numeric_limits<short>::max());
+ test<Context, int, int>(std::numeric_limits<int>::max());
+
+ using LongToType = std::conditional_t<sizeof(long) == sizeof(int), int, long long>;
+
+ test<Context, LongToType, long>(std::numeric_limits<long>::min());
+ test<Context, LongToType, long>(std::numeric_limits<int>::min());
+ test<Context, LongToType, long>(std::numeric_limits<short...
[truncated]
|
libcxx/test/std/utilities/format/format.arguments/format.arg/visit.return_type.pass.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@petrhosek the original patches worked in our Windows pre-commit CI. Can you verify this patch works for you.
I'm still happy with the patch. Please wait for @petrhosek's confirmation before landing.
@petrhosek Thank you for reacting. I'm sorry I didn't see your message earlier: #76449 (comment) |
FWIW, the reason for this is that the test has |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@petrhosek the original patches worked in our Windows pre-commit CI.
FWIW, the reason for this is that the test has
UNSUPPORTED: clang-16 || clang-17
, and the Windows pre-commit CI only uses release versions of clang.
Hmm, now that this #78991 has landed we could probably replace the guards with: __has_extension(cxx_explicit_this_parameter)
@H-G-Hristov Would it be possible to update your branch? I sent it to our Windows builders but Clang is segfaulting while building runtimes. |
@petrhosek Thank you for taking a look. I updated the branch. Please note that pre-release Clang 18 used to have a bug related to "deducing this", which is used by this feature. It has been rectified since then: Could the crash be related to the above? |
I noticed some of clang-cl test crashed (cmake + python errors). I rebased again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mordante I guess the PR missed LLVM18 release. I'll update the release notes accordingly. Is that OK?
I tried this branch on our Windows builders although I'm not sure if it's related or not to this patch:
https://logs.chromium.org/logs/fuchsia/led/phosek_google.com/62644843c966785c9dedf065a79d80df8734b4df5ff7703ea3c2a2b5643cec05/+/u/clang/test/stdout is the full log. |
@petrhosek It is not related. It was supposedly fixed by this: #79619 |
@petrhosek According to the log, the tests in this patch pass. Could you please try again. I'll wait a while and if there is no negative feedback I'll reland the patch.
|
This issue was already reported a while back here: #70225 |
Yes. It would have been nice to get this in LLVM 18, but I don't feel it's critical to backport it. (Especially since it's a C++26 only feature and I expect very few people are already using it.) |
Deleted the offending test case.
libcxx/test/std/utilities/format/format.arguments/format.arg/visit.return_type.pass.cpp
lines: 134-135:
Relands: #76449
Reverted in: 02f95b7