[libcxx-commits] [libcxx] 48abcf1 - [libc++][format] Adds formattable-with concept.
Mark de Wever via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Jun 20 23:06:18 PDT 2023
Author: Mark de Wever
Date: 2023-06-21T08:05:33+02:00
New Revision: 48abcf11ada1e0b300d907dcdb3cf7b191fdc85e
URL: https://github.com/llvm/llvm-project/commit/48abcf11ada1e0b300d907dcdb3cf7b191fdc85e
DIFF: https://github.com/llvm/llvm-project/commit/48abcf11ada1e0b300d907dcdb3cf7b191fdc85e.diff
LOG: [libc++][format] Adds formattable-with concept.
This change has a few additional effects:
- Abstract classes are now formattable.
- Volatile objects are no longer formattable.
Implements
- LWG3631 basic_format_arg(T&&) should use remove_cvref_t<T> throughout
- LWG3925 Concept formattable's definition is incorrect
Reviewed By: #libc, ldionne
Differential Revision: https://reviews.llvm.org/D152092
Added:
Modified:
libcxx/docs/ReleaseNotes.rst
libcxx/docs/Status/Cxx23Issues.csv
libcxx/docs/Status/Cxx2cIssues.csv
libcxx/include/__format/concepts.h
libcxx/include/__format/format_arg.h
libcxx/include/__format/format_arg_store.h
libcxx/include/__format/formatter_string.h
libcxx/test/std/utilities/format/format.formattable/concept.formattable.compile.pass.cpp
libcxx/test/std/utilities/format/format.tuple/format.functions.tests.h
Removed:
################################################################################
diff --git a/libcxx/docs/ReleaseNotes.rst b/libcxx/docs/ReleaseNotes.rst
index a3ecb8e41de87..d951f14f8ad31 100644
--- a/libcxx/docs/ReleaseNotes.rst
+++ b/libcxx/docs/ReleaseNotes.rst
@@ -111,6 +111,9 @@ Deprecations and Removals
- 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<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
index 1b7c74bcc3972..f8f7259877819 100644
--- a/libcxx/docs/Status/Cxx23Issues.csv
+++ b/libcxx/docs/Status/Cxx23Issues.csv
@@ -278,7 +278,7 @@
"`3867 <https://wg21.link/LWG3867>`__","Should ``std::basic_osyncstream``'s move assignment operator be ``noexcept``?","February 2023","","",""
"`3441 <https://wg21.link/LWG3441>`__","Misleading note about calls to customization points","February 2023","","",""
"`3622 <https://wg21.link/LWG3622>`__","Misspecified transitivity of equivalence in ยง[unord.req.general]","February 2023","","",""
-"`3631 <https://wg21.link/LWG3631>`__","``basic_format_arg(T&&)`` should use ``remove_cvref_t<T>`` throughout","February 2023","|Complete|","15.0",""
+"`3631 <https://wg21.link/LWG3631>`__","``basic_format_arg(T&&)`` should use ``remove_cvref_t<T>`` throughout","February 2023","|Complete|","17.0",""
"`3645 <https://wg21.link/LWG3645>`__","``resize_and_overwrite`` is overspecified to call its callback with lvalues","February 2023","|Complete|","14.0",""
"`3655 <https://wg21.link/LWG3655>`__","The ``INVOKE`` operation and union types","February 2023","","",""
"`3723 <https://wg21.link/LWG3723>`__","``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
index 44b6a4c7f0ce7..daf12b7f59384 100644
--- a/libcxx/docs/Status/Cxx2cIssues.csv
+++ b/libcxx/docs/Status/Cxx2cIssues.csv
@@ -11,7 +11,7 @@
"`3912 <https://wg21.link/LWG3912>`__","``enumerate_view::iterator::operator-`` should be ``noexcept``","Varna June 2023","","","|ranges|"
"`3914 <https://wg21.link/LWG3914>`__","Inconsistent template-head of ``ranges::enumerate_view``","Varna June 2023","","","|ranges|"
"`3915 <https://wg21.link/LWG3915>`__","Redundant paragraph about expression variations","Varna June 2023","","","|ranges|"
-"`3925 <https://wg21.link/LWG3925>`__","Concept ``formattable``'s definition is incorrect","Varna June 2023","|In Progress|","","|format|"
+"`3925 <https://wg21.link/LWG3925>`__","Concept ``formattable``'s definition is incorrect","Varna June 2023","|Complete|","17.0","|format|"
"`3927 <https://wg21.link/LWG3927>`__","Unclear preconditions for ``operator[]`` for sequence containers","Varna June 2023","|Nothing To Do|","",""
"`3935 <https://wg21.link/LWG3935>`__","``template<class X> constexpr complex& operator=(const complex<X>&)`` has no specification","Varna June 2023","|Complete|","Clang 3.4",""
"`3938 <https://wg21.link/LWG3938>`__","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
index 62552f8270583..ae96b6a198118 100644
--- 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 <tuple>
@@ -43,17 +44,21 @@ concept __fmt_char_type =
template <class _CharT>
using __fmt_iter_for = _CharT*;
+template <class _Tp, class _Context, class _Formatter = typename _Context::template formatter_type<remove_const_t<_Tp>>>
+concept __formattable_with =
+ semiregular<_Formatter> &&
+ requires(_Formatter& __f,
+ const _Formatter& __cf,
+ _Tp&& __t,
+ _Context __fc,
+ basic_format_parse_context<typename _Context::char_type> __pc) {
+ { __f.parse(__pc) } -> same_as<typename decltype(__pc)::iterator>;
+ { __cf.format(__t, __fc) } -> same_as<typename _Context::iterator>;
+ };
+
template <class _Tp, class _CharT>
concept __formattable =
- (semiregular<formatter<remove_cvref_t<_Tp>, _CharT>>) &&
- requires(formatter<remove_cvref_t<_Tp>, _CharT> __f,
- const formatter<remove_cvref_t<_Tp>, _CharT> __cf,
- _Tp __t,
- basic_format_context<__fmt_iter_for<_CharT>, _CharT> __fc,
- basic_format_parse_context<_CharT> __pc) {
- { __f.parse(__pc) } -> same_as<typename basic_format_parse_context<_CharT>::iterator>;
- { __cf.format(__t, __fc) } -> same_as<__fmt_iter_for<_CharT>>;
- };
+ __formattable_with<remove_reference_t<_Tp>, basic_format_context<__fmt_iter_for<_CharT>, _CharT>>;
# if _LIBCPP_STD_VER >= 23
template <class _Tp, class _CharT>
diff --git a/libcxx/include/__format/format_arg.h b/libcxx/include/__format/format_arg.h
index a27da8d74cc37..271404bdd703b 100644
--- 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 @@ class __basic_format_arg_value {
/// Contains the implementation for basic_format_arg::handle.
struct __handle {
template <class _Tp>
- _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<const _Dp&>(), 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, _Context>, const _Dp, _Dp>;
+ static_assert(__formattable_with<_Qp, _Context>, "Mandated by [format.arg]/10");
- static_assert(__const_formattable || !is_const_v<remove_reference_t<_Tp>>, "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<const _Dp*>(__ptr)), __ctx));
}) {}
@@ -221,9 +218,7 @@ class __basic_format_arg_value {
_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 <class _Context>
diff --git a/libcxx/include/__format/format_arg_store.h b/libcxx/include/__format/format_arg_store.h
index b9a36e58930ec..669b7477acd70 100644
--- 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 <string>
#include <string_view>
@@ -157,11 +158,18 @@ consteval __arg_t __determine_arg_t() {
return __arg_t::__none;
}
+// Pseudo constuctor for basic_format_arg
+//
+// Modeled after template<class T> explicit basic_format_arg(T& v) noexcept;
+// [format.arg]/4-6
template <class _Context, class _Tp>
-_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 @@ _LIBCPP_HIDE_FROM_ABI basic_format_arg<_Context> __create_format_arg(_Tp&& __val
return basic_format_arg<_Context>{__arg, static_cast<unsigned long long>(__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<remove_cvref_t<_Tp>>)
+ if constexpr (is_array_v<_Dp>)
return basic_format_arg<_Context>{
- __arg, basic_string_view<typename _Context::char_type>{__value, extent_v<remove_cvref_t<_Tp>> - 1}};
+ __arg, basic_string_view<typename _Context::char_type>{__value, extent_v<_Dp> - 1}};
else
// When the _Traits or _Allocator are
diff erent an implicit conversion will
// fail.
@@ -190,8 +198,7 @@ _LIBCPP_HIDE_FROM_ABI basic_format_arg<_Context> __create_format_arg(_Tp&& __val
else if constexpr (__arg == __arg_t::__ptr)
return basic_format_arg<_Context>{__arg, static_cast<const void*>(__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
index 25a9e8ee4920c..142e1d3626f6e 100644
--- a/libcxx/include/__format/formatter_string.h
+++ b/libcxx/include/__format/formatter_string.h
@@ -115,7 +115,8 @@ struct _LIBCPP_TEMPLATE_VIS formatter<_CharT[_Size], _CharT>
using _Base = __formatter_string<_CharT>;
template <class _FormatContext>
- _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
index 71faa828f4048..aafff368527ba 100644
--- 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 <class T, class CharT>
void assert_is_not_formattable() {
- static_assert(!std::formattable<T, CharT>);
+ // 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<const T , CharT>);
+ static_assert(!std::formattable<const T& , CharT>);
+ static_assert(!std::formattable<const T&& , CharT>);
+ // clang-format on
}
template <class T, class CharT>
@@ -66,9 +73,16 @@ void assert_is_formattable() {
#ifndef TEST_HAS_NO_WIDE_CHARACTERS
|| std::same_as<CharT, wchar_t>
#endif
- )
- static_assert(std::formattable<T, CharT>);
- 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<const T , CharT>);
+ static_assert(std::formattable<const T& , CharT>);
+ static_assert(std::formattable<const T&& , CharT>);
+ // clang-format on
+ } else
assert_is_not_formattable<T, CharT>();
}
@@ -243,6 +257,27 @@ void test_P2286() {
test_P2286_vector_bool<CharT, std::vector<bool, min_allocator<bool>>>();
}
+// Tests volatile quified objects are no longer formattable.
+template <class CharT>
+void test_LWG3631() {
+ assert_is_not_formattable<volatile CharT, CharT>();
+
+ assert_is_not_formattable<volatile bool, CharT>();
+
+ assert_is_not_formattable<volatile signed int, CharT>();
+ assert_is_not_formattable<volatile unsigned int, CharT>();
+
+ assert_is_not_formattable<volatile std::chrono::microseconds, CharT>();
+ assert_is_not_formattable<volatile std::chrono::sys_time<std::chrono::microseconds>, CharT>();
+ assert_is_not_formattable<volatile std::chrono::day, CharT>();
+
+ assert_is_not_formattable<std::array<volatile int, 42>, CharT>();
+
+ assert_is_not_formattable<std::pair<volatile int, int>, CharT>();
+ assert_is_not_formattable<std::pair<int, volatile int>, CharT>();
+ assert_is_not_formattable<std::pair<volatile int, volatile int>, CharT>();
+}
+
class c {
void f();
void fc() const;
@@ -335,12 +370,40 @@ void test_disabled() {
assert_is_not_formattable<std::variant<c>, CharT>();
}
+struct abstract {
+ virtual ~abstract() = 0;
+};
+
+template <class CharT>
+ requires std::same_as<CharT, char>
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+ || std::same_as<CharT, wchar_t>
+#endif
+struct std::formatter<abstract, CharT> {
+ template <class ParseContext>
+ constexpr typename ParseContext::iterator parse(ParseContext& parse_ctx) {
+ return parse_ctx.begin();
+ }
+
+ template <class FormatContext>
+ typename FormatContext::iterator format(const abstract&, FormatContext& ctx) const {
+ return ctx.out();
+ }
+};
+
+template <class CharT>
+void test_abstract_class() {
+ assert_is_formattable<abstract, CharT>();
+}
+
template <class CharT>
void test() {
test_P0645<CharT>();
test_P1361<CharT>();
test_P1636<CharT>();
test_P2286<CharT>();
+ test_LWG3631<CharT>();
+ test_abstract_class<CharT>();
test_disabled<CharT>();
}
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
index 6ebc17d4e8eae..2c21f6cdd78ed 100644
--- 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 @@ void run_tests(TestFunction check, ExceptionTest check_exception) {
test_escaping<CharT>(check, std::make_pair(CharT('*'), STR("")));
test_escaping<CharT>(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<const int >{42});
- check(SV("(42)"), SV("{}"), std::tuple< volatile int >{42});
- check(SV("(42)"), SV("{}"), std::tuple<const volatile int >{42});
+ check(SV("(42)"), SV("{}"), std::tuple< int >{42});
+ check(SV("(42)"), SV("{}"), std::tuple<const int >{42});
int answer = 42;
- check(SV("(42)"), SV("{}"), std::tuple< int& >{answer});
- check(SV("(42)"), SV("{}"), std::tuple<const int& >{answer});
- check(SV("(42)"), SV("{}"), std::tuple< volatile int& >{answer});
- check(SV("(42)"), SV("{}"), std::tuple<const volatile int& >{answer});
-
- check(SV("(42)"), SV("{}"), std::tuple< int&&>{42});
- check(SV("(42)"), SV("{}"), std::tuple<const int&&>{42});
- check(SV("(42)"), SV("{}"), std::tuple< volatile int&&>{42});
- check(SV("(42)"), SV("{}"), std::tuple<const volatile int&&>{42});
+ check(SV("(42)"), SV("{}"), std::tuple< int& >{answer});
+ check(SV("(42)"), SV("{}"), std::tuple<const int& >{answer});
+
+ check(SV("(42)"), SV("{}"), std::tuple< int&&>{42});
+ check(SV("(42)"), SV("{}"), std::tuple<const int&&>{42});
// clang-format on
}
More information about the libcxx-commits
mailing list