[libcxx-commits] [libcxx] ffe262a - [libc++][format] Improve pointer formatters.
Mark de Wever via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Jun 28 23:39:46 PDT 2022
Author: Mark de Wever
Date: 2022-06-29T08:39:42+02:00
New Revision: ffe262a198a9f9030991df6d3ddd812e74fa3523
URL: https://github.com/llvm/llvm-project/commit/ffe262a198a9f9030991df6d3ddd812e74fa3523
DIFF: https://github.com/llvm/llvm-project/commit/ffe262a198a9f9030991df6d3ddd812e74fa3523.diff
LOG: [libc++][format] Improve pointer formatters.
This changes the implementation of the formatter. Instead of inheriting
from a specialized parser all formatters will use the same generic
parser. This reduces the binary size.
The new parser contains some additional fields only used in the chrono
formatting. Since this doesn't change the size of the parser the fields
are in the generic parser. The parser is designed to fit in 128-bit,
making it cheap to pass by value.
The new format function is a const member function. This isn't required
by the Standard yet, but it will be after LWG-3636 is accepted.
Additionally P2286 adds a formattable concept which requires the member
function to be const qualified in C++23. This paper is likely to be
accepted in the 2022 July plenary.
This is based on D125606. That commit did the groundwork and did similar
changes for the string formatters.
Depends on D128139.
Reviewed By: #libc, ldionne
Differential Revision: https://reviews.llvm.org/D128671
Added:
Modified:
libcxx/include/__format/formatter_pointer.h
libcxx/include/__format/parser_std_format_spec.h
Removed:
libcxx/test/libcxx/utilities/format/format.string/format.string.std/std_format_spec_pointer.pass.cpp
################################################################################
diff --git a/libcxx/include/__format/formatter_pointer.h b/libcxx/include/__format/formatter_pointer.h
index 9448d79f5d248..3cd4c9bba9602 100644
--- a/libcxx/include/__format/formatter_pointer.h
+++ b/libcxx/include/__format/formatter_pointer.h
@@ -10,16 +10,14 @@
#ifndef _LIBCPP___FORMAT_FORMATTER_POINTER_H
#define _LIBCPP___FORMAT_FORMATTER_POINTER_H
-#include <__algorithm/copy.h>
-#include <__assert>
#include <__availability>
#include <__config>
-#include <__format/format_error.h>
#include <__format/format_fwd.h>
+#include <__format/format_parse_context.h>
#include <__format/formatter.h>
#include <__format/formatter_integral.h>
+#include <__format/formatter_output.h>
#include <__format/parser_std_format_spec.h>
-#include <__iterator/access.h>
#include <cstddef>
#include <cstdint>
@@ -31,35 +29,27 @@ _LIBCPP_BEGIN_NAMESPACE_STD
#if _LIBCPP_STD_VER > 17
-namespace __format_spec {
-
template <__formatter::__char_type _CharT>
-class _LIBCPP_TEMPLATE_VIS __formatter_pointer : public __parser_pointer<_CharT> {
+struct _LIBCPP_TEMPLATE_VIS __formatter_pointer {
public:
- _LIBCPP_HIDE_FROM_ABI auto format(const void* __ptr, auto& __ctx) -> decltype(__ctx.out()) {
- _LIBCPP_ASSERT(this->__alignment != _Flags::_Alignment::__default,
- "The call to parse should have updated the alignment");
- if (this->__width_needs_substitution())
- this->__substitute_width_arg_id(__ctx.arg(this->__width));
-
- // This code looks a lot like the code to format a hexadecimal integral,
- // but that code isn't public. Making that code public requires some
- // refactoring.
- // TODO FMT Remove code duplication.
- char __buffer[2 + 2 * sizeof(uintptr_t)];
- __buffer[0] = '0';
- __buffer[1] = 'x';
- char* __last = __formatter::__to_buffer(__buffer + 2, _VSTD::end(__buffer), reinterpret_cast<uintptr_t>(__ptr), 16);
+ constexpr __formatter_pointer() { __parser_.__alignment_ = __format_spec::__alignment::__right; }
- unsigned __size = __last - __buffer;
- if (__size >= this->__width)
- return _VSTD::copy(__buffer, __last, __ctx.out());
+ _LIBCPP_HIDE_FROM_ABI constexpr auto
+ parse(basic_format_parse_context<_CharT>& __parse_ctx) -> decltype(__parse_ctx.begin()) {
+ auto __result = __parser_.__parse(__parse_ctx, __format_spec::__fields_pointer);
+ __format_spec::__process_display_type_pointer(__parser_.__type_);
+ return __result;
+ }
- return __formatter::__write(__ctx.out(), __buffer, __last, __size, this->__width, this->__fill, this->__alignment);
+ _LIBCPP_HIDE_FROM_ABI auto format(const void* __ptr, auto& __ctx) const -> decltype(__ctx.out()) {
+ __format_spec::__parsed_specifications<_CharT> __specs = __parser_.__get_parsed_std_specifications(__ctx);
+ __specs.__std_.__alternate_form_ = true;
+ __specs.__std_.__type_ = __format_spec::__type::__hexadecimal_lower_case;
+ return __formatter::__format_integer(reinterpret_cast<uintptr_t>(__ptr), __ctx, __specs);
}
-};
-} // namespace __format_spec
+ __format_spec::__parser<_CharT> __parser_;
+};
// [format.formatter.spec]/2.4
// For each charT, the pointer type specializations template<>
@@ -68,13 +58,13 @@ class _LIBCPP_TEMPLATE_VIS __formatter_pointer : public __parser_pointer<_CharT>
// - template<> struct formatter<const void*, charT>;
template <__formatter::__char_type _CharT>
struct _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FORMAT formatter<nullptr_t, _CharT>
- : public __format_spec::__formatter_pointer<_CharT> {};
+ : public __formatter_pointer<_CharT> {};
template <__formatter::__char_type _CharT>
-struct _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FORMAT formatter<void*, _CharT>
- : public __format_spec::__formatter_pointer<_CharT> {};
+struct _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FORMAT formatter<void*, _CharT> : public __formatter_pointer<_CharT> {
+};
template <__formatter::__char_type _CharT>
struct _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FORMAT formatter<const void*, _CharT>
- : public __format_spec::__formatter_pointer<_CharT> {};
+ : public __formatter_pointer<_CharT> {};
#endif //_LIBCPP_STD_VER > 17
diff --git a/libcxx/include/__format/parser_std_format_spec.h b/libcxx/include/__format/parser_std_format_spec.h
index 36998bb61167a..739bdf457e409 100644
--- a/libcxx/include/__format/parser_std_format_spec.h
+++ b/libcxx/include/__format/parser_std_format_spec.h
@@ -1407,6 +1407,7 @@ inline constexpr __fields __fields_integral{
.__locale_specific_form_ = true,
.__type_ = true};
inline constexpr __fields __fields_string{.__precision_ = true, .__type_ = true};
+inline constexpr __fields __fields_pointer{.__type_ = true};
enum class _LIBCPP_ENUM_VIS __alignment : uint8_t {
/// No alignment is set in the format string.
@@ -1948,6 +1949,17 @@ _LIBCPP_HIDE_FROM_ABI constexpr void __process_parsed_integer(__parser<_CharT>&
}
}
+_LIBCPP_HIDE_FROM_ABI constexpr void __process_display_type_pointer(__format_spec::__type __type) {
+ switch (__type) {
+ case __format_spec::__type::__default:
+ case __format_spec::__type::__pointer:
+ break;
+
+ default:
+ std::__throw_format_error("The format-spec type has a type not supported for a pointer argument");
+ }
+}
+
} // namespace __format_spec
#endif //_LIBCPP_STD_VER > 17
diff --git a/libcxx/test/libcxx/utilities/format/format.string/format.string.std/std_format_spec_pointer.pass.cpp b/libcxx/test/libcxx/utilities/format/format.string/format.string.std/std_format_spec_pointer.pass.cpp
deleted file mode 100644
index 021a7b3dd5e18..0000000000000
--- a/libcxx/test/libcxx/utilities/format/format.string/format.string.std/std_format_spec_pointer.pass.cpp
+++ /dev/null
@@ -1,253 +0,0 @@
-//===----------------------------------------------------------------------===//
-// 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
-// UNSUPPORTED: libcpp-has-no-incomplete-format
-
-// <format>
-
-// Tests the parsing of the format string as specified in [format.string.std].
-// It validates whether the std-format-spec is valid for a pointer type.
-
-#include <format>
-#include <cassert>
-#ifndef _LIBCPP_HAS_NO_LOCALIZATION
-# include <iostream>
-#endif
-
-#include "concepts_precision.h"
-#include "test_macros.h"
-#include "make_string.h"
-#include "test_exception.h"
-
-#define CSTR(S) MAKE_CSTRING(CharT, S)
-
-using namespace std::__format_spec;
-
-template <class CharT>
-using Parser = __parser_pointer<CharT>;
-
-template <class CharT>
-struct Expected {
- CharT fill = CharT(' ');
- _Flags::_Alignment alignment = _Flags::_Alignment::__right;
- _Flags::_Sign sign = _Flags::_Sign::__default;
- bool alternate_form = false;
- bool zero_padding = false;
- uint32_t width = 0;
- bool width_as_arg = false;
- bool locale_specific_form = false;
- _Flags::_Type type = _Flags::_Type::__pointer;
-};
-
-template <class CharT>
-constexpr void test(Expected<CharT> expected, size_t size, std::basic_string_view<CharT> fmt) {
- // Initialize parser with sufficient arguments to avoid the parsing to fail
- // due to insufficient arguments.
- std::basic_format_parse_context<CharT> parse_ctx(fmt, std::__format::__number_max);
- auto begin = parse_ctx.begin();
- auto end = parse_ctx.end();
- Parser<CharT> parser;
- auto it = parser.parse(parse_ctx);
-
- assert(begin == parse_ctx.begin());
- assert(end == parse_ctx.end());
-
- assert(begin + size == it);
- assert(parser.__fill == expected.fill);
- assert(parser.__alignment == expected.alignment);
- assert(parser.__sign == expected.sign);
- assert(parser.__alternate_form == expected.alternate_form);
- assert(parser.__zero_padding == expected.zero_padding);
- assert(parser.__width == expected.width);
- assert(parser.__width_as_arg == expected.width_as_arg);
- assert(parser.__locale_specific_form == expected.locale_specific_form);
- assert(parser.__type == expected.type);
-}
-
-template <class CharT>
-constexpr void test(Expected<CharT> expected, size_t size, const CharT* f) {
- // The format-spec is valid if completely consumed or terminates at a '}'.
- // The valid inputs all end with a '}'. The test is executed twice:
- // - first with the terminating '}',
- // - second consuming the entire input.
- std::basic_string_view<CharT> fmt{f};
- assert(fmt.back() == CharT('}') && "Pre-condition failure");
-
- test(expected, size, fmt);
- fmt.remove_suffix(1);
- test(expected, size, fmt);
-}
-
-template <class CharT>
-constexpr void test() {
- Parser<CharT> parser;
-
- assert(parser.__fill == CharT(' '));
- assert(parser.__alignment == _Flags::_Alignment::__right);
- assert(parser.__sign == _Flags::_Sign::__default);
- assert(parser.__alternate_form == false);
- assert(parser.__zero_padding == false);
- assert(parser.__width == 0);
- assert(parser.__width_as_arg == false);
- assert(parser.__locale_specific_form == false);
- assert(parser.__type == _Flags::_Type::__default);
-
- test({}, 0, CSTR("}"));
-
- // *** Align-fill ***
- test({.alignment = _Flags::_Alignment::__left}, 1, CSTR("<}"));
- test({.alignment = _Flags::_Alignment::__center}, 1, "^}");
- test({.alignment = _Flags::_Alignment::__right}, 1, ">}");
-
- test({.fill = CharT('L'), .alignment = _Flags::_Alignment::__left}, 2, CSTR("L<}"));
- test({.fill = CharT('#'), .alignment = _Flags::_Alignment::__center}, 2, CSTR("#^}"));
- test({.fill = CharT('0'), .alignment = _Flags::_Alignment::__right}, 2, CSTR("0>}"));
-
- test_exception<Parser<CharT>>("The format-spec fill field contains an invalid character", CSTR("{<"));
- test_exception<Parser<CharT>>("The format-spec fill field contains an invalid character", CSTR("}<"));
-
- // *** Sign ***
- test_exception<Parser<CharT>>("The format-spec should consume the input or end with a '}'", CSTR("+"));
- test_exception<Parser<CharT>>("The format-spec should consume the input or end with a '}'", CSTR("-"));
- test_exception<Parser<CharT>>("The format-spec should consume the input or end with a '}'", CSTR(" "));
-
- // *** Alternate form ***
- test_exception<Parser<CharT>>("The format-spec should consume the input or end with a '}'", CSTR("#"));
-
- // *** Zero padding ***
- test_exception<Parser<CharT>>("A format-spec width field shouldn't have a leading zero", CSTR("0"));
-
- // *** Width ***
- test({.width = 0, .width_as_arg = false}, 0, CSTR("}"));
- test({.width = 1, .width_as_arg = false}, 1, CSTR("1}"));
- test({.width = 10, .width_as_arg = false}, 2, CSTR("10}"));
- test({.width = 1000, .width_as_arg = false}, 4, CSTR("1000}"));
- test({.width = 1000000, .width_as_arg = false}, 7, CSTR("1000000}"));
-
- test({.width = 0, .width_as_arg = true}, 2, CSTR("{}}"));
- test({.width = 0, .width_as_arg = true}, 3, CSTR("{0}}"));
- test({.width = 1, .width_as_arg = true}, 3, CSTR("{1}}"));
-
- test_exception<Parser<CharT>>("A format-spec width field shouldn't have a leading zero", CSTR("00"));
-
- static_assert(std::__format::__number_max == 2'147'483'647, "Update the assert and the test.");
- test({.width = 2'147'483'647, .width_as_arg = false}, 10, CSTR("2147483647}"));
- test_exception<Parser<CharT>>("The numeric value of the format-spec is too large", CSTR("2147483648"));
- test_exception<Parser<CharT>>("The numeric value of the format-spec is too large", CSTR("5000000000"));
- test_exception<Parser<CharT>>("The numeric value of the format-spec is too large", CSTR("10000000000"));
-
- test_exception<Parser<CharT>>("End of input while parsing format-spec arg-id", CSTR("{"));
- test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{0"));
- test_exception<Parser<CharT>>("The arg-id of the format-spec starts with an invalid character", CSTR("{a"));
- test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{1"));
- test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{9"));
- test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{9:"));
- test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{9a"));
- static_assert(std::__format::__number_max == 2'147'483'647, "Update the assert and the test.");
- // Note the static_assert tests whether the arg-id is valid.
- // Therefore the following should be true arg-id < __format::__number_max.
- test({.width = 2'147'483'646, .width_as_arg = true}, 12, CSTR("{2147483646}}"));
- test_exception<Parser<CharT>>("The numeric value of the format-spec is too large", CSTR("{2147483648}"));
- test_exception<Parser<CharT>>("The numeric value of the format-spec is too large", CSTR("{5000000000}"));
- test_exception<Parser<CharT>>("The numeric value of the format-spec is too large", CSTR("{10000000000}"));
-
- // *** Precision ***
- test_exception<Parser<CharT>>("The format-spec should consume the input or end with a '}'", CSTR("."));
- test_exception<Parser<CharT>>("The format-spec should consume the input or end with a '}'", CSTR(".1"));
-
- // *** Locale-specific form ***
- test_exception<Parser<CharT>>("The format-spec should consume the input or end with a '}'", CSTR("L"));
-
- // *** Type ***
- {
- const char* unsuported_type = "The format-spec type has a type not supported for a pointer argument";
- const char* not_a_type = "The format-spec should consume the input or end with a '}'";
-
- test_exception<Parser<CharT>>(unsuported_type, CSTR("A}"));
- test_exception<Parser<CharT>>(unsuported_type, CSTR("B}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("C}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("D}"));
- test_exception<Parser<CharT>>(unsuported_type, CSTR("E}"));
- test_exception<Parser<CharT>>(unsuported_type, CSTR("F}"));
- test_exception<Parser<CharT>>(unsuported_type, CSTR("G}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("H}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("I}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("J}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("K}"));
- test_exception<Parser<CharT>>("The format-spec should consume the input or end with a '}'", CSTR("L"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("M}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("N}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("O}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("P}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("Q}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("R}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("S}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("T}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("U}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("V}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("W}"));
- test_exception<Parser<CharT>>(unsuported_type, CSTR("X}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("Y}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("Z}"));
-
- test_exception<Parser<CharT>>(unsuported_type, CSTR("a}"));
- test_exception<Parser<CharT>>(unsuported_type, CSTR("b}"));
- test_exception<Parser<CharT>>(unsuported_type, CSTR("c}"));
- test_exception<Parser<CharT>>(unsuported_type, CSTR("d}"));
- test_exception<Parser<CharT>>(unsuported_type, CSTR("e}"));
- test_exception<Parser<CharT>>(unsuported_type, CSTR("f}"));
- test_exception<Parser<CharT>>(unsuported_type, CSTR("g}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("h}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("i}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("j}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("k}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("l}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("m}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("n}"));
- test_exception<Parser<CharT>>(unsuported_type, CSTR("o}"));
- test({.type = _Flags::_Type::__pointer}, 1, CSTR("p}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("q}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("r}"));
- test_exception<Parser<CharT>>(unsuported_type, CSTR("s}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("t}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("u}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("v}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("w}"));
- test_exception<Parser<CharT>>(unsuported_type, CSTR("x}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("y}"));
- test_exception<Parser<CharT>>(not_a_type, CSTR("z}"));
- }
-
- // **** General ***
- test_exception<Parser<CharT>>("The format-spec should consume the input or end with a '}'", CSTR("ss"));
-}
-
-constexpr bool test() {
- test<char>();
-#ifndef TEST_HAS_NO_WIDE_CHARACTERS
- test<wchar_t>();
-#endif
-
- return true;
-}
-
-int main(int, char**) {
-#if !defined(_WIN32) && !defined(_AIX)
- // Make sure the parsers match the expectations. The layout of the
- // subobjects is chosen to minimize the size required.
- static_assert(sizeof(Parser<char>) == 2 * sizeof(uint32_t));
-# ifndef TEST_HAS_NO_WIDE_CHARACTERS
- static_assert(sizeof(Parser<wchar_t>) == (sizeof(wchar_t) <= 2 ? 2 * sizeof(uint32_t) : 3 * sizeof(uint32_t)));
-# endif
-#endif
-
- test();
- static_assert(test());
-
- return 0;
-}
More information about the libcxx-commits
mailing list