Skip to content

Commit aae9b19

Browse files
committed
Validate replacement fields for user-defined types at compile-time
While the result cannot be stored at compile-time, replacement fields of user-defined types can be parsed for validation. A couple of things were moved around to accomplish this. Parsing up to and including the colon occurs before parsing standard or user-defined replacement fields. Parsing the closing brace of standard fields now occurs in FormatSpecifier to align with |parse| requirements.
1 parent 18a5822 commit aae9b19

File tree

6 files changed

+161
-127
lines changed

6 files changed

+161
-127
lines changed

fly/types/string/detail/format_parameters.hpp

+2-14
Original file line numberDiff line numberDiff line change
@@ -309,27 +309,15 @@ inline void format_user_defined_value(
309309
BasicFormatSpecifier<typename FormatContext::char_type> &&specifier)
310310
{
311311
using Formatter = typename FormatContext::template formatter_type<T>;
312-
313312
Formatter formatter;
314-
parse_context.lexer().set_position(specifier.m_parse_index);
315313

316314
if constexpr (fly::FormattableWithParsing<decltype(parse_context), Formatter>)
317315
{
316+
parse_context.lexer().set_position(specifier.m_parse_index);
318317
formatter.parse(parse_context);
319318
}
320-
else
321-
{
322-
if (!parse_context.lexer().consume_if(FLY_CHR(typename FormatContext::char_type, '}')))
323-
{
324-
parse_context.on_error(
325-
"User-defined formatter without a parse method may not have formatting options");
326-
}
327-
}
328319

329-
if (!parse_context.has_error())
330-
{
331-
formatter.format(*static_cast<const T *>(value), context);
332-
}
320+
formatter.format(*static_cast<const T *>(value), context);
333321
}
334322

335323
//==================================================================================================

fly/types/string/detail/format_specifier.hpp

+5
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,11 @@ constexpr void BasicFormatSpecifier<CharType>::parse(FormatParseContext &context
486486
parse_type(context);
487487

488488
validate(context);
489+
490+
if (!context.has_error() && !context.lexer().consume_if(s_right_brace))
491+
{
492+
context.on_error("Detected unclosed replacement field - must end with }");
493+
}
489494
}
490495

491496
//==================================================================================================

fly/types/string/detail/format_string.hpp

+38-58
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,14 @@
66
#include "fly/types/string/detail/format_parse_context.hpp"
77
#include "fly/types/string/detail/format_specifier.hpp"
88
#include "fly/types/string/detail/traits.hpp"
9+
#include "fly/types/string/formatters.hpp"
910
#include "fly/types/string/lexer.hpp"
1011
#include "fly/types/string/literals.hpp"
1112

1213
#include <array>
1314
#include <cstdint>
1415
#include <optional>
16+
#include <tuple>
1517
#include <type_traits>
1618

1719
namespace fly::detail {
@@ -68,19 +70,15 @@ class BasicFormatString
6870
constexpr FormatSpecifier parse_specifier();
6971

7072
/**
71-
* Parse a replacement field for a user-defined type.
73+
* Parse a replacement field for a user-defined type. If the formatter for the user-defined type
74+
* defines a |parse| method, it is invoked to parse any formatting options. Otherwise, it is an
75+
* error if any formatting options are specified.
7276
*
7377
* @param specifier The replacement field being parsed.
7478
*/
79+
template <std::size_t N = 0>
7580
constexpr void parse_user_defined_specifier(FormatSpecifier &specifier);
7681

77-
/**
78-
* Parse a replacement field for a standard type.
79-
*
80-
* @param specifier The replacement field being parsed.
81-
*/
82-
constexpr void parse_standard_specifier(FormatSpecifier &specifier);
83-
8482
static constexpr const auto s_left_brace = FLY_CHR(CharType, '{');
8583
static constexpr const auto s_right_brace = FLY_CHR(CharType, '}');
8684
static constexpr const auto s_colon = FLY_CHR(CharType, ':');
@@ -164,13 +162,22 @@ constexpr auto BasicFormatString<CharType, ParameterTypes...>::parse_specifier()
164162
FormatSpecifier specifier(m_context);
165163
specifier.m_parse_index = m_context.lexer().position();
166164

167-
if (m_context.parameter_type(specifier.m_position) == ParameterType::UserDefined)
165+
if (m_context.lexer().consume_if(s_colon))
168166
{
169-
parse_user_defined_specifier(specifier);
167+
specifier.m_parse_index = m_context.lexer().position();
168+
169+
if (m_context.parameter_type(specifier.m_position) == ParameterType::UserDefined)
170+
{
171+
parse_user_defined_specifier(specifier);
172+
}
173+
else
174+
{
175+
specifier.parse(m_context);
176+
}
170177
}
171-
else
178+
else if (!m_context.lexer().consume_if(s_right_brace))
172179
{
173-
parse_standard_specifier(specifier);
180+
m_context.on_error("Detected unclosed replacement field - must end with }");
174181
}
175182

176183
specifier.m_size = m_context.lexer().position() - starting_position;
@@ -179,62 +186,35 @@ constexpr auto BasicFormatString<CharType, ParameterTypes...>::parse_specifier()
179186

180187
//==================================================================================================
181188
template <fly::StandardCharacter CharType, typename... ParameterTypes>
189+
template <std::size_t N>
182190
constexpr void BasicFormatString<CharType, ParameterTypes...>::parse_user_defined_specifier(
183191
FormatSpecifier &specifier)
184192
{
185-
// Replacement fields for user-defined types are parsed at runtime.
186-
//
187-
// TODO: Formatters that inherit from standard formatters might be parsed at compile time.
188-
// TODO: Allow nested format specifiers.
189-
std::size_t expected_close_brace_count = 1;
190-
std::size_t nested_specifier_count = 0;
191-
192-
while (expected_close_brace_count != 0)
193+
if constexpr (N < sizeof...(ParameterTypes))
193194
{
194-
if (auto ch = m_context.lexer().consume(); ch)
195+
if (N != specifier.m_position)
195196
{
196-
if (ch == s_right_brace)
197-
{
198-
--expected_close_brace_count;
199-
}
200-
else if (ch == s_left_brace)
201-
{
202-
++expected_close_brace_count;
203-
++nested_specifier_count;
204-
}
205-
else if (ch == s_colon)
206-
{
207-
specifier.m_parse_index = m_context.lexer().position();
208-
}
197+
parse_user_defined_specifier<N + 1>(specifier);
198+
return;
199+
}
200+
201+
using T = std::tuple_element_t<N, std::tuple<std::remove_cvref_t<ParameterTypes>...>>;
202+
using Formatter = fly::Formatter<T, CharType>;
203+
204+
if constexpr (fly::FormattableWithParsing<FormatParseContext, Formatter>)
205+
{
206+
Formatter formatter;
207+
formatter.parse(m_context);
209208
}
210209
else
211210
{
212-
m_context.on_error("Detected unclosed replacement field - must end with }");
213-
expected_close_brace_count = 0;
211+
if (!m_context.lexer().consume_if(s_right_brace))
212+
{
213+
m_context.on_error(
214+
"User-defined formatter without a parser may not have formatting options");
215+
}
214216
}
215217
}
216-
217-
if (nested_specifier_count != 0)
218-
{
219-
m_context.on_error("Nested replacement fields are not allowed in user-defined formatters");
220-
}
221-
}
222-
223-
//==================================================================================================
224-
template <fly::StandardCharacter CharType, typename... ParameterTypes>
225-
constexpr void
226-
BasicFormatString<CharType, ParameterTypes...>::parse_standard_specifier(FormatSpecifier &specifier)
227-
{
228-
if (m_context.lexer().consume_if(s_colon))
229-
{
230-
specifier.m_parse_index = m_context.lexer().position();
231-
specifier.parse(m_context);
232-
}
233-
234-
if (!m_context.has_error() && !m_context.lexer().consume_if(s_right_brace))
235-
{
236-
m_context.on_error("Detected unclosed replacement field - must end with }");
237-
}
238218
}
239219

240220
} // namespace fly::detail

fly/types/string/string.hpp

+13-12
Original file line numberDiff line numberDiff line change
@@ -477,12 +477,19 @@ class BasicString
477477
* };
478478
*
479479
* The |parse| method is optional. If defined, it is provided a BasicFormatParseContext which
480-
* contains a lexer that may be used to parse the format string. The lexer is positioned such
481-
* that it is pointed at the first character after the ":" in the replacement field (if there is
482-
* one), or after the opening "{" character. The |parse| method is expected to consume up to and
483-
* including the closing "}" character. It may indicate any parsing errors through the parsing
484-
* context; if an error occurs, the error is written to the formatted string, and formatting
485-
* will halt.
480+
* contains a lexer that may be used to parse the format string. The position of the lexer
481+
* will be one of the following within the replacement field:
482+
*
483+
* 1. The position immediately after the colon, if there is one.
484+
* 2. Otherwise, the position immediately after the format parameter index, if there is one.
485+
* 3. Otherwise, the position immeidately after the opening brace.
486+
*
487+
* The |parse| method is expected to consume up to and including the closing "}" character. It
488+
* must be declared constexpr, as it will be invoked at compile time to validate the replacement
489+
* field. The parser may indicate any parsing errors through the parsing context; if an error
490+
* occurs, the error is handled the same as any standard replacement field (see above). Even
491+
* though the parser is invoked at compile time, the result of user-defined parsing cannot be
492+
* stored generically. Thus, parsing is invoked again at runtime immediately before |format|.
486493
*
487494
* @tparam ParameterTypes Variadic format parameter types.
488495
*
@@ -969,12 +976,6 @@ void BasicString<CharType>::format_to(
969976

970977
const auto parameter = context.arg(specifier.m_position);
971978
parameter.format(parse_context, context, std::move(specifier));
972-
973-
if (parse_context.has_error())
974-
{
975-
format_to(output, FLY_ARR(char_type, "{}"), parse_context.error());
976-
return;
977-
}
978979
}
979980
break;
980981

test/types/string/format.cpp

+12-7
Original file line numberDiff line numberDiff line change
@@ -781,7 +781,7 @@ CATCH_TEMPLATE_TEST_CASE(
781781
using char_type = typename BasicString::char_type;
782782

783783
UserDefinedType u {};
784-
UserDefinedTypeWithParser p {};
784+
UserDefinedTypeWithParser up {};
785785

786786
CATCH_SECTION("User-defined types inherit parent's parse method")
787787
{
@@ -792,13 +792,16 @@ CATCH_TEMPLATE_TEST_CASE(
792792

793793
CATCH_SECTION("User-defined types may define a parse method")
794794
{
795-
test_format(FMT("{}"), FMT("false"), p);
796-
test_format(FMT("{:o}"), FMT("true"), p);
795+
test_format(FMT("{}"), FMT("false"), up);
796+
test_format(FMT("{:o}"), FMT("true"), up);
797797
}
798798

799799
CATCH_SECTION("User-defined types with a parse method may report errors")
800800
{
801-
test_format(FMT("{:x}"), FMT("UserDefinedTypeWithParser error!"), p);
801+
test_format(
802+
FMT("{:x}"),
803+
FMT("Ignored invalid formatter: UserDefinedTypeWithParser error!"),
804+
up);
802805
}
803806

804807
CATCH_SECTION("User-defined types do not need to define a parse method")
@@ -808,15 +811,17 @@ CATCH_TEMPLATE_TEST_CASE(
808811
test_format(FMT("{:}"), FMT("UserDefinedType"), u);
809812
}
810813

811-
CATCH_SECTION("User-defined formatter without a parse method may not have formatting options")
814+
CATCH_SECTION("User-defined formatter without a parser may not have formatting options")
812815
{
813816
test_format(
814817
FMT("{:s}"),
815-
FMT("User-defined formatter without a parse method may not have formatting options"),
818+
FMT("Ignored invalid formatter: User-defined formatter without a parser may not have "
819+
"formatting options"),
816820
u);
817821
test_format(
818822
FMT("{:.3}"),
819-
FMT("User-defined formatter without a parse method may not have formatting options"),
823+
FMT("Ignored invalid formatter: User-defined formatter without a parser may not have "
824+
"formatting options"),
820825
u);
821826
}
822827
}

0 commit comments

Comments
 (0)