Skip to content

Commit 9a3e71d

Browse files
committed
Do not allow presentation types for generic format parameters
Originally, either "{}" or "{:s}" were permitted due to a misreading of the std::format specification. The string presentation type should not be alowed. Eventually, a mechanism similar to std::formatter should be supported, allowing callers to fully specify the format string for generic types.
1 parent b1b2093 commit 9a3e71d

File tree

5 files changed

+105
-96
lines changed

5 files changed

+105
-96
lines changed

fly/types/string/detail/string_formatter.hpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -332,14 +332,16 @@ template <
332332
std::is_arithmetic<T>,
333333
is_default_formatted_enum<StringType, T>>>
334334
inline void BasicStringFormatter<StringType, ParameterTypes...>::format_value(
335-
FormatSpecifier &&specifier,
335+
FormatSpecifier &&,
336336
const T &value)
337337
{
338338
static thread_local ostringstream_type s_stream;
339339

340340
s_stream << value;
341-
format_value(std::move(specifier), s_stream.str());
341+
const auto formatted = s_stream.str();
342342
s_stream.str({});
343+
344+
append_string(formatted, formatted.size());
343345
}
344346

345347
//==================================================================================================

fly/types/string/detail/string_formatter_types.hpp

+24-38
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ namespace fly::detail {
104104
* Strong enumeration types - If an overload of operator<< is defined, valid presentations
105105
* are: none, "s". Else, valid presentations are: none, "c", b", "B", "o", "d", "x", "X".
106106
*
107-
* Other (general) types - Valid presentations: none, "s". An overload of operator<< must be
107+
* Other (generic) types - Valid presentations: none. An overload of operator<< must be
108108
* defined for generic types.
109109
*
110110
* For details on each presentation type, see the above links.
@@ -390,37 +390,14 @@ class BasicFormatString
390390
* Helper trait to determine if a type is either streamable or string-like.
391391
*/
392392
template <typename T>
393-
using is_formatable_type = std::disjunction<
393+
using is_formattable_type = std::disjunction<
394394
typename traits::OstreamTraits::template is_declared<T>,
395395
detail::is_like_supported_string<T>,
396396
detail::is_supported_character<T>,
397397
std::is_enum<std::remove_cvref_t<T>>>;
398398

399399
template <typename T>
400-
static inline constexpr bool is_formatable_type_v = is_formatable_type<T>::value;
401-
402-
/**
403-
* Helper trait to classify an enumeration type as default-formatted (i.e. the user has not
404-
* defined a custom operator<< for this type).
405-
*/
406-
template <typename T>
407-
using is_default_formatted_enum = std::conjunction<
408-
std::is_enum<T>,
409-
std::negation<typename traits::OstreamTraits::template is_declared<T>>>;
410-
411-
template <typename T>
412-
static inline constexpr bool is_default_formatted_enum_v = is_default_formatted_enum<T>::value;
413-
414-
/**
415-
* Helper trait to classify an enumeration type as user-formatted (i.e. the user has defined a
416-
* custom operator<< for this type).
417-
*/
418-
template <typename T>
419-
using is_user_formatted_enum =
420-
std::conjunction<std::is_enum<T>, typename traits::OstreamTraits::template is_declared<T>>;
421-
422-
template <typename T>
423-
static inline constexpr bool is_user_formatted_enum_v = is_user_formatted_enum<T>::value;
400+
static inline constexpr bool is_formattable_type_v = is_formattable_type<T>::value;
424401

425402
/**
426403
* Helper trait to classify a type as an integer, excluding character and boolean types.
@@ -434,6 +411,18 @@ class BasicFormatString
434411
template <typename T>
435412
static inline constexpr bool is_integer_v = is_integer<T>::value;
436413

414+
/**
415+
* Helper trait to classify an enumeration type as default-formatted (i.e. the user has not
416+
* defined a custom operator<< for this type).
417+
*/
418+
template <typename T>
419+
using is_default_formatted_enum = std::conjunction<
420+
std::is_enum<T>,
421+
std::negation<typename traits::OstreamTraits::template is_declared<T>>>;
422+
423+
template <typename T>
424+
static inline constexpr bool is_default_formatted_enum_v = is_default_formatted_enum<T>::value;
425+
437426
/**
438427
* Upon parsing an un-escaped opening brace, parse a single replacement field in the format
439428
* string. If valid, the lexer will be advanced to the character after the closing brace.
@@ -563,12 +552,12 @@ class BasicFormatString
563552
constexpr void validate_type(ParameterType type, const FormatSpecifier &specifier);
564553

565554
/**
566-
* Determine the type of a format parameter. Returns ParameterType::Generic if the given index
567-
* was not found, or if the type of the format parameter is unknown.
555+
* Determine the type of a format parameter. Returns ParameterType::Generic if the type of the
556+
* format parameter is unknown.
568557
*
569558
* @param index The index of the format parameter to inspect.
570559
*
571-
* @return The type of the format parameter.
560+
* @return If found, the type of the format parameter. Otherwise, an uninitialized value.
572561
*/
573562
template <size_t N = 0>
574563
constexpr std::optional<ParameterType> parameter_type(size_t index);
@@ -791,7 +780,7 @@ FLY_CONSTEVAL BasicFormatString<StringType, ParameterTypes...>::BasicFormatStrin
791780
{
792781
std::optional<char_type> ch;
793782

794-
if constexpr (!(is_formatable_type_v<ParameterTypes> && ...))
783+
if constexpr (!(is_formattable_type_v<ParameterTypes> && ...))
795784
{
796785
on_error("An overloaded operator<< must be defined for all format parameters");
797786
}
@@ -1204,10 +1193,9 @@ constexpr void BasicFormatString<StringType, ParameterTypes...>::validate_type(
12041193
switch (type)
12051194
{
12061195
case ParameterType::Generic:
1207-
if ((specifier.m_type != FormatSpecifier::Type::None) &&
1208-
(specifier.m_type != FormatSpecifier::Type::String))
1196+
if (specifier.m_type != FormatSpecifier::Type::None)
12091197
{
1210-
on_error("Generic types must be formatted with {} or {:s}");
1198+
on_error("Generic types must be formatted with {}");
12111199
}
12121200
break;
12131201

@@ -1227,7 +1215,7 @@ constexpr void BasicFormatString<StringType, ParameterTypes...>::validate_type(
12271215
if ((specifier.m_type != FormatSpecifier::Type::None) &&
12281216
(specifier.m_type != FormatSpecifier::Type::String))
12291217
{
1230-
on_error("String types and user-formatted enums must be formatted with {} or {:s}");
1218+
on_error("String types must be formatted with {} or {:s}");
12311219
}
12321220
break;
12331221

@@ -1247,9 +1235,7 @@ constexpr void BasicFormatString<StringType, ParameterTypes...>::validate_type(
12471235
(specifier.m_type != FormatSpecifier::Type::Decimal) &&
12481236
(specifier.m_type != FormatSpecifier::Type::Hex))
12491237
{
1250-
on_error(
1251-
"Integral types and default-formatted enums must be formatted with {} or one "
1252-
"of {:cbBodxX}");
1238+
on_error("Integral types must be formatted with {} or one of {:cbBodxX}");
12531239
}
12541240
break;
12551241

@@ -1298,7 +1284,7 @@ constexpr auto BasicFormatString<StringType, ParameterTypes...>::parameter_type(
12981284
{
12991285
return ParameterType::Character;
13001286
}
1301-
else if constexpr (is_like_supported_string_v<T> || is_user_formatted_enum_v<T>)
1287+
else if constexpr (is_like_supported_string_v<T>)
13021288
{
13031289
return ParameterType::String;
13041290
}

fly/types/string/string.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ class BasicString
414414
* the type of the format string.
415415
*
416416
* 3. Any generic type for which an operator<< overload is defined will be converted to a
417-
* string using that overload. Formatting options will then be applied to that string.
417+
* string using that overload.
418418
*
419419
* 4. Formatting of strong enumeration types defaults to the format of the enumeration's
420420
* underlying type. However, if an overload of operator<< is defined, the type is treated
@@ -474,7 +474,7 @@ class BasicString
474474
* the type of the format string.
475475
*
476476
* 3. Any generic type for which an operator<< overload is defined will be converted to a
477-
* string using that overload. Formatting options will then be applied to that string.
477+
* string using that overload.
478478
*
479479
* 4. Formatting of strong enumeration types defaults to the format of the enumeration's
480480
* underlying type. However, if an overload of operator<< is defined, the type is treated

test/types/string/string_formatter.cpp

+48-27
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,46 @@ namespace {
2121

2222
#define FMT(format) FLY_ARR(char_type, format)
2323

24+
struct GenericType
25+
{
26+
};
27+
28+
[[maybe_unused]] std::ostream &operator<<(std::ostream &stream, const GenericType &)
29+
{
30+
stream << "GenericType";
31+
return stream;
32+
}
33+
34+
[[maybe_unused]] std::wostream &operator<<(std::wostream &stream, const GenericType &)
35+
{
36+
stream << L"GenericType";
37+
return stream;
38+
}
39+
40+
enum class DefaultFormattedEnum
41+
{
42+
One = 1,
43+
Two = 2,
44+
};
45+
46+
enum class UserFormattedEnum
47+
{
48+
One = 1,
49+
Two = 2,
50+
};
51+
52+
[[maybe_unused]] std::ostream &operator<<(std::ostream &stream, const UserFormattedEnum &u)
53+
{
54+
stream << (u == UserFormattedEnum::One ? "One" : "Two");
55+
return stream;
56+
}
57+
58+
[[maybe_unused]] std::wostream &operator<<(std::wostream &stream, const UserFormattedEnum &u)
59+
{
60+
stream << (u == UserFormattedEnum::One ? L"One" : L"Two");
61+
return stream;
62+
}
63+
2464
template <typename StringType, typename... ParameterTypes>
2565
using FormatString = fly::detail::BasicFormatString<
2666
fly::detail::is_like_supported_string_t<StringType>,
@@ -62,30 +102,6 @@ StringType reserved_codepoint()
62102
return result;
63103
}
64104

65-
enum class DefaultFormattedEnum
66-
{
67-
One = 1,
68-
Two = 2,
69-
};
70-
71-
enum class UserFormattedEnum
72-
{
73-
One = 1,
74-
Two = 2,
75-
};
76-
77-
[[maybe_unused]] std::ostream &operator<<(std::ostream &stream, const UserFormattedEnum &u)
78-
{
79-
stream << (u == UserFormattedEnum::One ? "One" : "Two");
80-
return stream;
81-
}
82-
83-
[[maybe_unused]] std::wostream &operator<<(std::wostream &stream, const UserFormattedEnum &u)
84-
{
85-
stream << (u == UserFormattedEnum::One ? "One" : "Two");
86-
return stream;
87-
}
88-
89105
} // namespace
90106

91107
CATCH_TEMPLATE_TEST_CASE(
@@ -460,6 +476,14 @@ CATCH_TEMPLATE_TEST_CASE(
460476
test_format(FMT("{1:.{0}s}"), FMT("abcdef"), -3, FLY_STR(char_type, "abcdef"));
461477
}
462478

479+
CATCH_SECTION("Generic types may be formatted without presentation type")
480+
{
481+
GenericType gt {};
482+
test_format(FMT("{}"), FMT("GenericType"), gt);
483+
test_format(FMT("{}"), FMT("One"), UserFormattedEnum::One);
484+
test_format(FMT("{}"), FMT("Two"), UserFormattedEnum::Two);
485+
}
486+
463487
CATCH_SECTION("Presentation type may be set (character)")
464488
{
465489
test_format(FMT("{:c}"), FMT("a"), 'a');
@@ -515,9 +539,6 @@ CATCH_TEMPLATE_TEST_CASE(
515539

516540
test_format(FMT("{:s}"), FMT("true"), true);
517541
test_format(FMT("{:s}"), FMT("false"), false);
518-
519-
test_format(FMT("{:s}"), FMT("One"), UserFormattedEnum::One);
520-
test_format(FMT("{:s}"), FMT("Two"), UserFormattedEnum::Two);
521542
}
522543

523544
CATCH_SECTION("Presentation type may be set (pointer)")

0 commit comments

Comments
 (0)