Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always use FMT_STRING internally where possible [Issue #2002] #2006

Merged
merged 13 commits into from
Nov 15, 2020
Merged

Always use FMT_STRING internally where possible [Issue #2002] #2006

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
9501da5
Always use FMT_STRING internally where possible
Nov 10, 2020
854bf9a
fixed chrono.h changes
Nov 11, 2020
67a4447
fixup type errors
Nov 11, 2020
969dcce
revert removal of const
Nov 12, 2020
45c7c29
use constexpr
Nov 13, 2020
eefa1e5
use FMT_CONSTEXPR_DECL
Nov 13, 2020
93f1e22
use FMT_ENABLE_IF for compile-time if branching
Nov 13, 2020
03c29de
ignore unusable param in non-float overload
Nov 14, 2020
2c271ac
workaround MSVC bug, add test
Nov 14, 2020
45b9f96
#ifdef test broken in MSVC
Nov 14, 2020
98a3557
improve test, fix conditional
Nov 14, 2020
fc3832f
attempt simplification to work around MSVC problem
Nov 14, 2020
d5e3e34
clang-format
Nov 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions include/fmt/chrono.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -764,13 +764,20 @@ inline std::chrono::duration<Rep, std::milli> get_milliseconds(

template <typename Char, typename Rep, typename OutputIt>
OutputIt format_duration_value(OutputIt out, Rep val, int precision) {
using context = FMT_BUFFER_CONTEXT(type_identity_t<Char>);

const Char pr_f[] = {'{', ':', '.', '{', '}', 'f', '}', 0};
if (precision >= 0) return format_to(out, pr_f, val, precision);
if (precision >= 0) {
return vformat_to(out, to_string_view(pr_f),
make_format_args<context>(val, precision));
}
Copy link
Contributor

@vitaut vitaut Nov 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make pr_f static constexpr and keep format_to (don't change it to vformat_to). static constexpr strings should work with FMT_STRING:

static constexpr const Char pr_f[] = {'{', ':', '.', '{', '}', 'f', '}', 0};
  if (precision >= 0) return format_to(out, FMT_STRING(pr_f), val, precision);

Same below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, good to know! Won't that break on platforms where FMT_CONSTEXPR_DECL evaluates to noop though?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On that platform FMT_STRING should also evaluate to noop (at least in theory).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This apparently breaks on windows, even after correcting it to use SFINAE to avoid compiling the {:.{}f} format string with an integral type. I'm adding a test for using FMT_STRING with a static FMT_CONSTEXPR_DECL, and I'll see if I can get a workaround

const Char fp_f[] = {'{', ':', 'g', '}', 0};
const Char format[] = {'{', '}', 0};
return format_to(out, std::is_floating_point<Rep>::value ? fp_f : format,
val);
return vformat_to(
out, to_string_view(std::is_floating_point<Rep>::value ? fp_f : format),
make_format_args<context>(val));
}

template <typename Char, typename OutputIt>
OutputIt copy_unit(string_view unit, OutputIt out, Char) {
return std::copy(unit.begin(), unit.end(), out);
Expand All @@ -786,12 +793,19 @@ OutputIt copy_unit(string_view unit, OutputIt out, wchar_t) {

template <typename Char, typename Period, typename OutputIt>
OutputIt format_duration_unit(OutputIt out) {
if (const char* unit = get_units<Period>())
using context = FMT_BUFFER_CONTEXT(type_identity_t<Char>);

if (const char* unit = get_units<Period>()) {
return copy_unit(string_view(unit), out, Char());
}
const Char num_f[] = {'[', '{', '}', ']', 's', 0};
if (const_check(Period::den == 1)) return format_to(out, num_f, Period::num);
if (const_check(Period::den == 1)) {
return vformat_to(out, to_string_view(num_f),
make_format_args<context>(Period::num));
}
const Char num_def_f[] = {'[', '{', '}', '/', '{', '}', ']', 's', 0};
return format_to(out, num_def_f, Period::num, Period::den);
return vformat_to(out, to_string_view(num_def_f),
make_format_args<context>(Period::num, Period::den));
}

template <typename FormatContext, typename OutputIt, typename Rep,
Expand Down
15 changes: 8 additions & 7 deletions include/fmt/format-inl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ FMT_FUNC void format_error_code(detail::buffer<char>& out, int error_code,
error_code_size += detail::to_unsigned(detail::count_digits(abs_value));
auto it = buffer_appender<char>(out);
if (message.size() <= inline_buffer_size - error_code_size)
format_to(it, "{}{}", message, SEP);
format_to(it, "{}{}", ERROR_STR, error_code);
format_to(it, FMT_STRING("{}{}"), message, SEP);
format_to(it, FMT_STRING("{}{}"), ERROR_STR, error_code);
assert(out.size() <= inline_buffer_size);
}

Expand Down Expand Up @@ -2662,14 +2662,15 @@ template <> struct formatter<detail::bigint> {
for (auto i = n.bigits_.size(); i > 0; --i) {
auto value = n.bigits_[i - 1u];
if (first) {
out = format_to(out, "{:x}", value);
out = format_to(out, FMT_STRING("{:x}"), value);
first = false;
continue;
}
out = format_to(out, "{:08x}", value);
out = format_to(out, FMT_STRING("{:08x}"), value);
}
if (n.exp_ > 0)
out = format_to(out, "p{}", n.exp_ * detail::bigint::bigit_bits);
out = format_to(out, FMT_STRING("p{}"),
n.exp_ * detail::bigint::bigit_bits);
return out;
}
};
Expand Down Expand Up @@ -2715,8 +2716,8 @@ FMT_FUNC void format_system_error(detail::buffer<char>& out, int error_code,
int result =
detail::safe_strerror(error_code, system_message, buf.size());
if (result == 0) {
format_to(detail::buffer_appender<char>(out), "{}: {}", message,
system_message);
format_to(detail::buffer_appender<char>(out), FMT_STRING("{}: {}"),
message, system_message);
return;
}
if (result != ERANGE)
Expand Down