diff --git a/libcxx/docs/ReleaseNotes.rst b/libcxx/docs/ReleaseNotes.rst --- a/libcxx/docs/ReleaseNotes.rst +++ b/libcxx/docs/ReleaseNotes.rst @@ -111,6 +111,9 @@ - The classes ``strstreambuf`` , ``istrstream``, ``ostrstream``, and ``strstream`` have been deprecated. They have been deprecated in the Standard since C++98, but were never marked as deprecated in libc++. +- LWG3631 ``basic_format_arg(T&&) should use remove_cvref_t throughout`` removed + support for ``volatile`` qualified formatters. + Upcoming Deprecations and Removals ---------------------------------- diff --git a/libcxx/docs/Status/Cxx23Issues.csv b/libcxx/docs/Status/Cxx23Issues.csv --- a/libcxx/docs/Status/Cxx23Issues.csv +++ b/libcxx/docs/Status/Cxx23Issues.csv @@ -278,7 +278,7 @@ "`3867 `__","Should ``std::basic_osyncstream``'s move assignment operator be ``noexcept``?","February 2023","","","" "`3441 `__","Misleading note about calls to customization points","February 2023","","","" "`3622 `__","Misspecified transitivity of equivalence in ยง[unord.req.general]","February 2023","","","" -"`3631 `__","``basic_format_arg(T&&)`` should use ``remove_cvref_t`` throughout","February 2023","|Complete|","15.0","" +"`3631 `__","``basic_format_arg(T&&)`` should use ``remove_cvref_t`` throughout","February 2023","|Complete|","17.0","" "`3645 `__","``resize_and_overwrite`` is overspecified to call its callback with lvalues","February 2023","|Complete|","14.0","" "`3655 `__","The ``INVOKE`` operation and union types","February 2023","","","" "`3723 `__","``priority_queue::push_range`` needs to ``append_range``","February 2023","","","|ranges|" diff --git a/libcxx/docs/Status/Cxx2cIssues.csv b/libcxx/docs/Status/Cxx2cIssues.csv --- a/libcxx/docs/Status/Cxx2cIssues.csv +++ b/libcxx/docs/Status/Cxx2cIssues.csv @@ -11,7 +11,7 @@ "`3912 `__","``enumerate_view::iterator::operator-`` should be ``noexcept``","Varna June 2023","","","|ranges|" "`3914 `__","Inconsistent template-head of ``ranges::enumerate_view``","Varna June 2023","","","|ranges|" "`3915 `__","Redundant paragraph about expression variations","Varna June 2023","","","|ranges|" -"`3925 `__","Concept ``formattable``'s definition is incorrect","Varna June 2023","|In Progress|","","|format|" +"`3925 `__","Concept ``formattable``'s definition is incorrect","Varna June 2023","|Complete|","17.0","|format|" "`3927 `__","Unclear preconditions for ``operator[]`` for sequence containers","Varna June 2023","|Nothing To Do|","","" "`3935 `__","``template constexpr complex& operator=(const complex&)`` has no specification","Varna June 2023","|Complete|","Clang 3.4","" "`3938 `__","Cannot use ``std::expected`` monadic ops with move-only ``error_type``","Varna June 2023","","","" diff --git a/libcxx/include/__format/concepts.h b/libcxx/include/__format/concepts.h --- a/libcxx/include/__format/concepts.h +++ b/libcxx/include/__format/concepts.h @@ -16,6 +16,7 @@ #include <__format/format_fwd.h> #include <__format/format_parse_context.h> #include <__type_traits/is_specialization.h> +#include <__type_traits/remove_const.h> #include <__utility/pair.h> #include @@ -43,17 +44,21 @@ template using __fmt_iter_for = _CharT*; +template >> +concept __formattable_with = + semiregular<_Formatter> && + requires(_Formatter& __f, + const _Formatter& __cf, + _Tp&& __t, + _Context __fc, + basic_format_parse_context __pc) { + { __f.parse(__pc) } -> same_as; + { __cf.format(__t, __fc) } -> same_as; + }; + template concept __formattable = - (semiregular, _CharT>>) && - requires(formatter, _CharT> __f, - const formatter, _CharT> __cf, - _Tp __t, - basic_format_context<__fmt_iter_for<_CharT>, _CharT> __fc, - basic_format_parse_context<_CharT> __pc) { - { __f.parse(__pc) } -> same_as::iterator>; - { __cf.format(__t, __fc) } -> same_as<__fmt_iter_for<_CharT>>; - }; + __formattable_with, basic_format_context<__fmt_iter_for<_CharT>, _CharT>>; # if _LIBCPP_STD_VER >= 23 template diff --git a/libcxx/include/__format/format_arg.h b/libcxx/include/__format/format_arg.h --- a/libcxx/include/__format/format_arg.h +++ b/libcxx/include/__format/format_arg.h @@ -13,6 +13,7 @@ #include <__assert> #include <__concepts/arithmetic.h> #include <__config> +#include <__format/concepts.h> #include <__format/format_error.h> #include <__format/format_fwd.h> #include <__format/format_parse_context.h> @@ -158,18 +159,14 @@ /// Contains the implementation for basic_format_arg::handle. struct __handle { template - _LIBCPP_HIDE_FROM_ABI explicit __handle(_Tp&& __v) noexcept + _LIBCPP_HIDE_FROM_ABI explicit __handle(_Tp& __v) noexcept : __ptr_(_VSTD::addressof(__v)), __format_([](basic_format_parse_context<_CharT>& __parse_ctx, _Context& __ctx, const void* __ptr) { - using _Dp = remove_cvref_t<_Tp>; - using _Formatter = typename _Context::template formatter_type<_Dp>; - constexpr bool __const_formattable = - requires { _Formatter().format(std::declval(), std::declval<_Context&>()); }; - using _Qp = conditional_t<__const_formattable, const _Dp, _Dp>; + using _Dp = remove_const_t<_Tp>; + using _Qp = conditional_t<__formattable_with, const _Dp, _Dp>; + static_assert(__formattable_with<_Qp, _Context>, "Mandated by [format.arg]/10"); - static_assert(__const_formattable || !is_const_v>, "Mandated by [format.arg]/18"); - - _Formatter __f; + typename _Context::template formatter_type<_Dp> __f; __parse_ctx.advance_to(__f.parse(__parse_ctx)); __ctx.advance_to(__f.format(*const_cast<_Qp*>(static_cast(__ptr)), __ctx)); }) {} @@ -221,9 +218,7 @@ _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(basic_string_view<_CharT> __value) noexcept : __string_view_(__value) {} _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(const void* __value) noexcept : __ptr_(__value) {} - _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(__handle __value) noexcept - // TODO FMT Investigate why it doesn't work without the forward. - : __handle_(std::forward<__handle>(__value)) {} + _LIBCPP_HIDE_FROM_ABI __basic_format_arg_value(__handle&& __value) noexcept : __handle_(std::move(__value)) {} }; template diff --git a/libcxx/include/__format/format_arg_store.h b/libcxx/include/__format/format_arg_store.h --- a/libcxx/include/__format/format_arg_store.h +++ b/libcxx/include/__format/format_arg_store.h @@ -22,6 +22,7 @@ #include <__type_traits/conditional.h> #include <__type_traits/extent.h> #include <__type_traits/is_same.h> +#include <__type_traits/remove_const.h> #include <__utility/forward.h> #include #include @@ -157,11 +158,18 @@ return __arg_t::__none; } +// Pseudo constuctor for basic_format_arg +// +// Modeled after template explicit basic_format_arg(T& v) noexcept; +// [format.arg]/4-6 template -_LIBCPP_HIDE_FROM_ABI basic_format_arg<_Context> __create_format_arg(_Tp&& __value) noexcept { - constexpr __arg_t __arg = __determine_arg_t<_Context, remove_cvref_t<_Tp>>(); +_LIBCPP_HIDE_FROM_ABI basic_format_arg<_Context> __create_format_arg(_Tp& __value) noexcept { + using _Dp = remove_const_t<_Tp>; + constexpr __arg_t __arg = __determine_arg_t<_Context, _Dp>(); static_assert(__arg != __arg_t::__none, "the supplied type is not formattable"); + static_assert(__formattable_with<_Tp, _Context>); + // Not all types can be used to directly initialize the // __basic_format_arg_value. First handle all types needing adjustment, the // final else requires no adjustment. @@ -179,9 +187,9 @@ return basic_format_arg<_Context>{__arg, static_cast(__value)}; else if constexpr (__arg == __arg_t::__string_view) // Using std::size on a character array will add the NUL-terminator to the size. - if constexpr (is_array_v>) + if constexpr (is_array_v<_Dp>) return basic_format_arg<_Context>{ - __arg, basic_string_view{__value, extent_v> - 1}}; + __arg, basic_string_view{__value, extent_v<_Dp> - 1}}; else // When the _Traits or _Allocator are different an implicit conversion will // fail. @@ -190,8 +198,7 @@ else if constexpr (__arg == __arg_t::__ptr) return basic_format_arg<_Context>{__arg, static_cast(__value)}; else if constexpr (__arg == __arg_t::__handle) - return basic_format_arg<_Context>{ - __arg, typename __basic_format_arg_value<_Context>::__handle{_VSTD::forward<_Tp>(__value)}}; + return basic_format_arg<_Context>{__arg, typename __basic_format_arg_value<_Context>::__handle{__value}}; else return basic_format_arg<_Context>{__arg, __value}; } diff --git a/libcxx/include/__format/formatter_string.h b/libcxx/include/__format/formatter_string.h --- a/libcxx/include/__format/formatter_string.h +++ b/libcxx/include/__format/formatter_string.h @@ -115,7 +115,8 @@ using _Base = __formatter_string<_CharT>; template - _LIBCPP_HIDE_FROM_ABI typename _FormatContext::iterator format(_CharT __str[_Size], _FormatContext& __ctx) const { + _LIBCPP_HIDE_FROM_ABI typename _FormatContext::iterator + format(const _CharT (&__str)[_Size], _FormatContext& __ctx) const { return _Base::format(basic_string_view<_CharT>(__str, _Size), __ctx); } }; diff --git a/libcxx/test/std/utilities/format/format.formattable/concept.formattable.compile.pass.cpp b/libcxx/test/std/utilities/format/format.formattable/concept.formattable.compile.pass.cpp --- a/libcxx/test/std/utilities/format/format.formattable/concept.formattable.compile.pass.cpp +++ b/libcxx/test/std/utilities/format/format.formattable/concept.formattable.compile.pass.cpp @@ -54,7 +54,14 @@ template void assert_is_not_formattable() { - static_assert(!std::formattable); + // clang-format off + static_assert(!std::formattable< T , CharT>); + static_assert(!std::formattable< T& , CharT>); + static_assert(!std::formattable< T&& , CharT>); + static_assert(!std::formattable); + static_assert(!std::formattable); + static_assert(!std::formattable); + // clang-format on } template @@ -66,9 +73,16 @@ #ifndef TEST_HAS_NO_WIDE_CHARACTERS || std::same_as #endif - ) - static_assert(std::formattable); - else + ) { + // clang-format off + static_assert(std::formattable< T , CharT>); + static_assert(std::formattable< T& , CharT>); + static_assert(std::formattable< T&& , CharT>); + static_assert(std::formattable); + static_assert(std::formattable); + static_assert(std::formattable); + // clang-format on + } else assert_is_not_formattable(); } @@ -243,6 +257,27 @@ test_P2286_vector_bool>>(); } +// Tests volatile quified objects are no longer formattable. +template +void test_LWG3631() { + assert_is_not_formattable(); + + assert_is_not_formattable(); + + assert_is_not_formattable(); + assert_is_not_formattable(); + + assert_is_not_formattable(); + assert_is_not_formattable, CharT>(); + assert_is_not_formattable(); + + assert_is_not_formattable, CharT>(); + + assert_is_not_formattable, CharT>(); + assert_is_not_formattable, CharT>(); + assert_is_not_formattable, CharT>(); +} + class c { void f(); void fc() const; @@ -335,12 +370,40 @@ assert_is_not_formattable, CharT>(); } +struct abstract { + virtual ~abstract() = 0; +}; + +template + requires std::same_as +#ifndef TEST_HAS_NO_WIDE_CHARACTERS + || std::same_as +#endif +struct std::formatter { + template + constexpr typename ParseContext::iterator parse(ParseContext& parse_ctx) { + return parse_ctx.begin(); + } + + template + typename FormatContext::iterator format(const abstract&, FormatContext& ctx) const { + return ctx.out(); + } +}; + +template +void test_abstract_class() { + assert_is_formattable(); +} + template void test() { test_P0645(); test_P1361(); test_P1636(); test_P2286(); + test_LWG3631(); + test_abstract_class(); test_disabled(); } diff --git a/libcxx/test/std/utilities/format/format.tuple/format.functions.tests.h b/libcxx/test/std/utilities/format/format.tuple/format.functions.tests.h --- a/libcxx/test/std/utilities/format/format.tuple/format.functions.tests.h +++ b/libcxx/test/std/utilities/format/format.tuple/format.functions.tests.h @@ -351,23 +351,17 @@ test_escaping(check, std::make_pair(CharT('*'), STR(""))); test_escaping(check, std::make_tuple(CharT('*'), STR(""))); - // Test cvref-qualified types. + // Test const ref-qualified types. // clang-format off - check(SV("(42)"), SV("{}"), std::tuple< int >{42}); - check(SV("(42)"), SV("{}"), std::tuple{42}); - check(SV("(42)"), SV("{}"), std::tuple< volatile int >{42}); - check(SV("(42)"), SV("{}"), std::tuple{42}); + check(SV("(42)"), SV("{}"), std::tuple< int >{42}); + check(SV("(42)"), SV("{}"), std::tuple{42}); int answer = 42; - check(SV("(42)"), SV("{}"), std::tuple< int& >{answer}); - check(SV("(42)"), SV("{}"), std::tuple{answer}); - check(SV("(42)"), SV("{}"), std::tuple< volatile int& >{answer}); - check(SV("(42)"), SV("{}"), std::tuple{answer}); - - check(SV("(42)"), SV("{}"), std::tuple< int&&>{42}); - check(SV("(42)"), SV("{}"), std::tuple{42}); - check(SV("(42)"), SV("{}"), std::tuple< volatile int&&>{42}); - check(SV("(42)"), SV("{}"), std::tuple{42}); + check(SV("(42)"), SV("{}"), std::tuple< int& >{answer}); + check(SV("(42)"), SV("{}"), std::tuple{answer}); + + check(SV("(42)"), SV("{}"), std::tuple< int&&>{42}); + check(SV("(42)"), SV("{}"), std::tuple{42}); // clang-format on }