Skip to content

Commit a15640c

Browse files
committed
Split BasicStringFormatter::format_value based on type
This method was just pretty huge. Create SFINAE overloads of a helper method to format values based on their types. This also removes the need for several constant if statements that were used to prevent the compiler from errantly generating type-specific code for generic types.
1 parent 0efb9d8 commit a15640c

File tree

2 files changed

+195
-49
lines changed

2 files changed

+195
-49
lines changed

fly/types/string/detail/string_formatter.hpp

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

3+
#include "fly/traits/traits.hpp"
34
#include "fly/types/string/detail/string_formatter_types.hpp"
45
#include "fly/types/string/detail/string_streamer.hpp"
56
#include "fly/types/string/detail/string_traits.hpp"
@@ -11,6 +12,13 @@
1112

1213
namespace fly::detail {
1314

15+
/**
16+
* Helper trait to classify a type as an integer, excluding boolean types.
17+
*/
18+
template <typename T>
19+
using is_format_integral =
20+
std::conjunction<std::is_integral<T>, std::negation<std::is_same<T, bool>>>;
21+
1422
/**
1523
* Helper class to format and stream generic values into a std::basic_string's output stream type.
1624
*
@@ -22,6 +30,7 @@ class BasicStringFormatter
2230
{
2331
using traits = BasicStringTraits<StringType>;
2432
using streamer = BasicStringStreamer<StringType>;
33+
using stream_modifiers = BasicStreamModifiers<StringType>;
2534
using size_type = typename traits::size_type;
2635
using char_type = typename traits::char_type;
2736
using view_type = typename traits::view_type;
@@ -113,6 +122,74 @@ class BasicStringFormatter
113122
template <typename T>
114123
static void format_value(ostream_type &stream, FormatSpecifier &&specifier, const T &value);
115124

125+
/**
126+
* Format a single replacement field with the provided boolean value.
127+
*
128+
* @tparam T The type of the value to format.
129+
*
130+
* @param stream The stream to insert the formatted value into.
131+
* @param modifiers The active stream manipulator container.
132+
* @param specifier The replacement field to format.
133+
* @param value The value to format.
134+
*/
135+
template <typename T, fly::enable_if<std::is_same<T, bool>> = 0>
136+
static void format_value_for_type(
137+
ostream_type &stream,
138+
stream_modifiers &&modifiers,
139+
FormatSpecifier &&specifier,
140+
const T &value);
141+
142+
/**
143+
* Format a single replacement field with the provided non-boolean integral value.
144+
*
145+
* @tparam T The type of the value to format.
146+
*
147+
* @param stream The stream to insert the formatted value into.
148+
* @param modifiers The active stream manipulator container.
149+
* @param specifier The replacement field to format.
150+
* @param value The value to format.
151+
*/
152+
template <typename T, fly::enable_if<is_format_integral<T>> = 0>
153+
static void format_value_for_type(
154+
ostream_type &stream,
155+
stream_modifiers &&modifiers,
156+
FormatSpecifier &&specifier,
157+
const T &value);
158+
159+
/**
160+
* Format a single replacement field with the provided floating point value.
161+
*
162+
* @tparam T The type of the value to format.
163+
*
164+
* @param stream The stream to insert the formatted value into.
165+
* @param modifiers The active stream manipulator container.
166+
* @param specifier The replacement field to format.
167+
* @param value The value to format.
168+
*/
169+
template <typename T, fly::enable_if<std::is_floating_point<T>> = 0>
170+
static void format_value_for_type(
171+
ostream_type &stream,
172+
stream_modifiers &&modifiers,
173+
FormatSpecifier &&specifier,
174+
const T &value);
175+
176+
/**
177+
* Format a single replacement field with the provided generic value.
178+
*
179+
* @tparam T The type of the value to format.
180+
*
181+
* @param stream The stream to insert the formatted value into.
182+
* @param modifiers The active stream manipulator container.
183+
* @param specifier The replacement field to format.
184+
* @param value The value to format.
185+
*/
186+
template <typename T, fly::enable_if_none<std::is_integral<T>, std::is_floating_point<T>> = 0>
187+
static void format_value_for_type(
188+
ostream_type &stream,
189+
stream_modifiers &&modifiers,
190+
FormatSpecifier &&specifier,
191+
const T &value);
192+
116193
static constexpr const auto s_left_brace = FLY_CHR(char_type, '{');
117194
static constexpr const auto s_right_brace = FLY_CHR(char_type, '}');
118195
static constexpr const auto s_zero = FLY_CHR(streamed_char_type, '0');
@@ -199,8 +276,7 @@ void BasicStringFormatter<StringType>::format_value(
199276
{
200277
using U = std::remove_cvref_t<T>;
201278

202-
BasicStreamModifiers<StringType> modifiers(stream);
203-
std::ios_base::fmtflags flags {};
279+
stream_modifiers modifiers(stream);
204280

205281
if (specifier.m_fill)
206282
{
@@ -210,40 +286,43 @@ void BasicStringFormatter<StringType>::format_value(
210286
switch (specifier.m_alignment)
211287
{
212288
case FormatSpecifier::Alignment::Left:
213-
flags |= std::ios_base::left;
289+
stream.setf(std::ios_base::left);
214290
break;
291+
215292
case FormatSpecifier::Alignment::Right:
216-
flags |= std::ios_base::right;
293+
stream.setf(std::ios_base::right);
217294
break;
295+
218296
case FormatSpecifier::Alignment::Center: // TODO: Implement center-alignment.
219297
case FormatSpecifier::Alignment::Default:
220-
flags |= specifier.is_numeric() ? std::ios_base::right : std::ios_base::left;
298+
stream.setf(specifier.is_numeric() ? std::ios_base::right : std::ios_base::left);
221299
break;
222300
}
223301

224302
switch (specifier.m_sign)
225303
{
226304
case FormatSpecifier::Sign::Always:
227-
flags |= std::ios_base::showpos;
305+
stream.setf(std::ios_base::showpos);
228306
break;
307+
229308
case FormatSpecifier::Sign::NegativeOnlyWithPositivePadding:
230309
modifiers.template locale<PositivePaddingFacet<streamed_char_type>>();
231-
flags |= std::ios_base::showpos;
310+
stream.setf(std::ios_base::showpos);
232311
break;
312+
233313
default:
234314
break;
235315
}
236316

237317
if (specifier.m_alternate_form)
238318
{
239-
flags |= std::ios_base::showbase;
240-
flags |= std::ios_base::showpoint;
319+
stream.setf(std::ios_base::showbase);
320+
stream.setf(std::ios_base::showpoint);
241321
}
242322

243323
if (specifier.m_zero_padding)
244324
{
245-
flags &= ~std::ios_base::adjustfield;
246-
flags |= std::ios_base::internal;
325+
stream.setf(std::ios_base::internal, std::ios_base::adjustfield);
247326
stream.fill(s_zero);
248327
}
249328

@@ -262,7 +341,6 @@ void BasicStringFormatter<StringType>::format_value(
262341
// string that are written to the stream. Instead, stream a substring view if needed.
263342
if (typename traits_type::view_type view(value); *specifier.m_precision < view.size())
264343
{
265-
stream.flags(flags);
266344
streamer::stream_value(stream, view.substr(0, *specifier.m_precision));
267345
return;
268346
}
@@ -275,71 +353,139 @@ void BasicStringFormatter<StringType>::format_value(
275353

276354
if (specifier.m_case == FormatSpecifier::Case::Upper)
277355
{
278-
flags |= std::ios_base::uppercase;
356+
stream.setf(std::ios_base::uppercase);
279357
}
280358

359+
format_value_for_type(stream, std::move(modifiers), std::move(specifier), value);
360+
}
361+
362+
//==================================================================================================
363+
template <typename StringType>
364+
template <typename T, fly::enable_if<std::is_same<T, bool>>>
365+
void BasicStringFormatter<StringType>::format_value_for_type(
366+
ostream_type &stream,
367+
stream_modifiers &&modifiers,
368+
FormatSpecifier &&specifier,
369+
const T &value)
370+
{
281371
switch (specifier.m_type)
282372
{
283373
case FormatSpecifier::Type::String:
284-
if constexpr (std::is_same_v<U, bool>)
285-
{
286-
flags |= std::ios_base::boolalpha;
287-
}
374+
stream.setf(std::ios_base::boolalpha);
288375
break;
376+
289377
case FormatSpecifier::Type::Binary:
290378
modifiers.template locale<BinaryFacet<streamed_char_type>>();
291379
break;
380+
292381
case FormatSpecifier::Type::Octal:
293-
flags |= std::ios_base::oct;
382+
stream.setf(std::ios_base::oct);
294383
break;
384+
295385
case FormatSpecifier::Type::Hex:
296-
flags |= std::ios_base::hex;
386+
stream.setf(std::ios_base::hex);
297387
break;
298-
case FormatSpecifier::Type::HexFloat:
299-
flags |= std::ios_base::fixed | std::ios_base::scientific;
388+
389+
default:
300390
break;
301-
case FormatSpecifier::Type::Scientific:
302-
flags |= std::ios_base::scientific;
391+
}
392+
393+
if (specifier.m_type == FormatSpecifier::Type::Character)
394+
{
395+
// TODO: Validate the value fits into streamed_char_type / convert Unicode encoding.
396+
streamer::template stream_value<decltype(value), streamed_char_type>(stream, value);
397+
}
398+
else
399+
{
400+
streamer::stream_value(stream, value);
401+
}
402+
}
403+
404+
//==================================================================================================
405+
template <typename StringType>
406+
template <typename T, fly::enable_if<is_format_integral<T>>>
407+
void BasicStringFormatter<StringType>::format_value_for_type(
408+
ostream_type &stream,
409+
stream_modifiers &&modifiers,
410+
FormatSpecifier &&specifier,
411+
const T &value)
412+
{
413+
switch (specifier.m_type)
414+
{
415+
case FormatSpecifier::Type::Binary:
416+
modifiers.template locale<BinaryFacet<streamed_char_type>>();
303417
break;
304-
case FormatSpecifier::Type::Fixed:
305-
// Note: this branch will only ever be entered with floating point types, but the
306-
// compiler will generate the below code for other types, so it must be protected.
307-
if constexpr (std::is_floating_point_v<U>)
308-
{
309-
// Only Apple's Clang seems to respect std::uppercase with std::fixed values. To
310-
// ensure consistency, format these values as general types.
311-
if (!std::isnan(value) && !std::isinf(value))
312-
{
313-
flags |= std::ios_base::fixed;
314-
}
315-
}
418+
419+
case FormatSpecifier::Type::Octal:
420+
stream.setf(std::ios_base::oct);
421+
break;
422+
423+
case FormatSpecifier::Type::Hex:
424+
stream.setf(std::ios_base::hex);
316425
break;
426+
317427
default:
318428
break;
319429
}
320430

321-
stream.flags(flags);
322-
323431
if (specifier.m_type == FormatSpecifier::Type::Character)
324432
{
325433
// TODO: Validate the value fits into streamed_char_type / convert Unicode encoding.
326434
streamer::template stream_value<decltype(value), streamed_char_type>(stream, value);
327435
}
328-
else if (specifier.is_integral())
436+
else
329437
{
330-
// Note: this branch will only ever be entered with integral types, but the compiler will
331-
// generate the below code for other types, so it must be protected.
332-
if constexpr (std::is_integral_v<U>)
333-
{
334-
using integral_type = std::
335-
conditional_t<std::numeric_limits<U>::is_signed, std::intmax_t, std::uintmax_t>;
336-
streamer::template stream_value<decltype(value), integral_type>(stream, value);
337-
}
438+
using integral_type =
439+
std::conditional_t<std::numeric_limits<T>::is_signed, std::intmax_t, std::uintmax_t>;
440+
streamer::template stream_value<decltype(value), integral_type>(stream, value);
338441
}
339-
else
442+
}
443+
444+
//==================================================================================================
445+
template <typename StringType>
446+
template <typename T, fly::enable_if<std::is_floating_point<T>>>
447+
void BasicStringFormatter<StringType>::format_value_for_type(
448+
ostream_type &stream,
449+
stream_modifiers &&,
450+
FormatSpecifier &&specifier,
451+
const T &value)
452+
{
453+
switch (specifier.m_type)
340454
{
341-
streamer::stream_value(stream, value);
455+
case FormatSpecifier::Type::HexFloat:
456+
stream.setf(std::ios_base::fixed | std::ios_base::scientific);
457+
break;
458+
459+
case FormatSpecifier::Type::Scientific:
460+
stream.setf(std::ios_base::scientific);
461+
break;
462+
463+
case FormatSpecifier::Type::Fixed:
464+
// Only Apple's Clang seems to respect std::uppercase with std::fixed values. To ensure
465+
// consistency, format these values as general types.
466+
if (!std::isnan(value) && !std::isinf(value))
467+
{
468+
stream.setf(std::ios_base::fixed);
469+
}
470+
break;
471+
472+
default:
473+
break;
342474
}
475+
476+
streamer::stream_value(stream, value);
477+
}
478+
479+
//==================================================================================================
480+
template <typename StringType>
481+
template <typename T, fly::enable_if_none<std::is_integral<T>, std::is_floating_point<T>>>
482+
inline void BasicStringFormatter<StringType>::format_value_for_type(
483+
ostream_type &stream,
484+
stream_modifiers &&,
485+
FormatSpecifier &&,
486+
const T &value)
487+
{
488+
streamer::stream_value(stream, value);
343489
}
344490

345491
} // namespace fly::detail

fly/types/string/detail/string_formatter_types.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ class BasicFormatString
329329
using is_integer = std::conjunction<
330330
std::is_integral<T>,
331331
std::negation<is_supported_character<T>>,
332-
std::negation<std::is_same<bool, T>>>;
332+
std::negation<std::is_same<T, bool>>>;
333333

334334
template <typename T>
335335
static inline constexpr bool is_integer_v = is_integer<T>::value;

0 commit comments

Comments
 (0)