Skip to content

Commit df633f1

Browse files
committed
Re-use compile-time parsed replacement fields for user-defined types
For user-defined types which: 1. Are derived from a standard formatter. 2. Do not define a custom parsing method. The formatter does not need to reparse the replacement field at runtime. Instead, the data that was parsed at compile time can be copied into the derived formatter. This has the added benefit of allowing nested replacement fields to work with these kinds of user-defined types. Previously, reparsing the field would cause BasicFormatParseContext::next_position() to roll past the end of the format parameters.
1 parent 42c0845 commit df633f1

File tree

4 files changed

+60
-2
lines changed

4 files changed

+60
-2
lines changed

fly/types/string/detail/format_parameters.hpp

+13-2
Original file line numberDiff line numberDiff line change
@@ -313,8 +313,19 @@ inline void format_user_defined_value(
313313

314314
if constexpr (fly::FormattableWithParsing<decltype(parse_context), Formatter>)
315315
{
316-
parse_context.lexer().set_position(specifier.m_parse_index);
317-
formatter.parse(parse_context);
316+
bool formatter_requires_parsing = true;
317+
318+
if constexpr (fly::DerivedFrom<Formatter, decltype(specifier)>)
319+
{
320+
formatter_requires_parsing = !specifier.m_was_parsed_as_standard_formatter;
321+
specifier.copy_formatting_options_into(formatter);
322+
}
323+
324+
if (formatter_requires_parsing)
325+
{
326+
parse_context.lexer().set_position(specifier.m_parse_index);
327+
formatter.parse(parse_context);
328+
}
318329
}
319330

320331
formatter.format(*static_cast<const T *>(value), context);

fly/types/string/detail/format_specifier.hpp

+37
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,39 @@ struct BasicFormatSpecifier
213213
template <typename FormatContext>
214214
std::size_t precision(FormatContext &context, std::size_t fallback) const;
215215

216+
/**
217+
* Copy formatting options from this replacement field into another replacement field (which may
218+
* be an instance of or derived from this class).
219+
*
220+
* @param formatter The replacement field to copy formatting options into.
221+
*/
222+
template <fly::DerivedFrom<BasicFormatSpecifier> FormatterType>
223+
constexpr void copy_formatting_options_into(FormatterType &formatter) const
224+
{
225+
// Note: This is defined inline due to: https://bugs.llvm.org/show_bug.cgi?id=48020
226+
formatter.m_position = m_position;
227+
228+
formatter.m_fill = m_fill;
229+
formatter.m_alignment = m_alignment;
230+
231+
formatter.m_sign = m_sign;
232+
formatter.m_alternate_form = m_alternate_form;
233+
formatter.m_zero_padding = m_zero_padding;
234+
235+
formatter.m_width = m_width;
236+
formatter.m_width_position = m_width_position;
237+
238+
formatter.m_precision = m_precision;
239+
formatter.m_precision_position = m_precision_position;
240+
241+
formatter.m_locale_specific_form = m_locale_specific_form;
242+
243+
formatter.m_type = m_type;
244+
formatter.m_case = m_case;
245+
246+
formatter.m_was_parsed_as_standard_formatter = m_was_parsed_as_standard_formatter;
247+
}
248+
216249
/**
217250
* Compare two replacement field instances for equality.
218251
*
@@ -248,6 +281,8 @@ struct BasicFormatSpecifier
248281
std::size_t m_parse_index {0};
249282
std::size_t m_size {0};
250283

284+
bool m_was_parsed_as_standard_formatter {false};
285+
251286
private:
252287
/**
253288
* Parse the optional fill and alignment arguments of the replacement field.
@@ -474,6 +509,8 @@ constexpr BasicFormatSpecifier<CharType>::BasicFormatSpecifier(FormatParseContex
474509
template <fly::StandardCharacter CharType>
475510
constexpr void BasicFormatSpecifier<CharType>::parse(FormatParseContext &context)
476511
{
512+
m_was_parsed_as_standard_formatter = true;
513+
477514
parse_fill_and_alignment(context);
478515
parse_sign(context);
479516
parse_alternate_form_and_zero_padding(context);

fly/types/string/detail/format_string.hpp

+6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#pragma once
22

3+
#include "fly/concepts/concepts.hpp"
34
#include "fly/fly.hpp"
45
#include "fly/types/string/concepts.hpp"
56
#include "fly/types/string/detail/format_parameter_type.hpp"
@@ -205,6 +206,11 @@ constexpr void BasicFormatString<CharType, ParameterTypes...>::parse_user_define
205206
{
206207
Formatter formatter;
207208
formatter.parse(m_context);
209+
210+
if constexpr (fly::DerivedFrom<Formatter, FormatSpecifier>)
211+
{
212+
formatter.copy_formatting_options_into(specifier);
213+
}
208214
}
209215
else
210216
{

test/types/string/format.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -788,6 +788,10 @@ CATCH_TEMPLATE_TEST_CASE(
788788
test_format(FMT("{:.1s}"), FMT("O"), UserFormattedEnum::One);
789789
test_format(FMT("{:.2s}"), FMT("On"), UserFormattedEnum::One);
790790
test_format(FMT("{:.3s}"), FMT("One"), UserFormattedEnum::One);
791+
792+
test_format(FMT("{:.{}s}"), FMT("T"), UserFormattedEnum::Two, 1);
793+
test_format(FMT("{:.{}s}"), FMT("Tw"), UserFormattedEnum::Two, 2);
794+
test_format(FMT("{:.{}s}"), FMT("Two"), UserFormattedEnum::Two, 3);
791795
}
792796

793797
CATCH_SECTION("User-defined types may define a parse method")

0 commit comments

Comments
 (0)