From 928a07bb045c77d293c4a12372ea1b4aaf993cf9 Mon Sep 17 00:00:00 2001
From: Victor Zverovich <viz@meta.com>
Date: Sun, 4 Aug 2024 09:09:01 -0700
Subject: [PATCH] Simplify handling of dynamic specs

---
 include/fmt/format.h | 28 +++++++++-------------------
 test/format-test.cc  | 20 ++++++++++----------
 2 files changed, 19 insertions(+), 29 deletions(-)

diff --git a/include/fmt/format.h b/include/fmt/format.h
index cb35ab2d..13cac3d9 100644
--- a/include/fmt/format.h
+++ b/include/fmt/format.h
@@ -3680,8 +3680,7 @@ template <typename Char> struct arg_formatter {
 struct dynamic_spec_getter {
   template <typename T, FMT_ENABLE_IF(is_integer<T>::value)>
   FMT_CONSTEXPR auto operator()(T value) -> unsigned long long {
-    if (is_negative(value)) report_error("negative width/precision");
-    return static_cast<unsigned long long>(value);
+    return is_negative(value) ? ~0ull : static_cast<unsigned long long>(value);
   }
 
   template <typename T, FMT_ENABLE_IF(!is_integer<T>::value)>
@@ -3691,13 +3690,6 @@ struct dynamic_spec_getter {
   }
 };
 
-template <typename Context>
-FMT_CONSTEXPR auto get_dynamic_spec(basic_format_arg<Context> arg) -> int {
-  unsigned long long value = arg.visit(dynamic_spec_getter());
-  if (value > to_unsigned(max_value<int>())) report_error("number is too big");
-  return static_cast<int>(value);
-}
-
 template <typename Context, typename ID>
 FMT_CONSTEXPR auto get_arg(Context& ctx, ID id) -> basic_format_arg<Context> {
   auto arg = ctx.arg(id);
@@ -3709,16 +3701,14 @@ template <typename Context>
 FMT_CONSTEXPR void handle_dynamic_spec(int& value,
                                        arg_ref<typename Context::char_type> ref,
                                        Context& ctx) {
-  switch (ref.kind) {
-  case arg_id_kind::none:
-    break;
-  case arg_id_kind::index:
-    value = get_dynamic_spec(get_arg(ctx, ref.val.index));
-    break;
-  case arg_id_kind::name:
-    value = get_dynamic_spec(get_arg(ctx, ref.val.name));
-    break;
-  }
+  if (ref.kind == arg_id_kind::none) return;
+  auto arg = ref.kind == arg_id_kind::index ? ctx.arg(ref.val.index)
+                                            : ctx.arg(ref.val.name);
+  if (!arg) report_error("argument not found");
+  unsigned long long arg_value = arg.visit(dynamic_spec_getter());
+  if (arg_value > to_unsigned(max_value<int>()))
+    report_error("width/precision is out of range");
+  value = static_cast<int>(arg_value);
 }
 
 #if FMT_USE_USER_DEFINED_LITERALS
diff --git a/test/format-test.cc b/test/format-test.cc
index 32f03ec5..077109c6 100644
--- a/test/format-test.cc
+++ b/test/format-test.cc
@@ -897,18 +897,18 @@ TEST(format_test, runtime_width) {
                    "invalid format string");
 
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:{1}}"), 0, -1), format_error,
-                   "negative width/precision");
+                   "width/precision is out of range");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:{1}}"), 0, (INT_MAX + 1u)),
-                   format_error, "number is too big");
+                   format_error, "width/precision is out of range");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:{1}}"), 0, -1l), format_error,
-                   "negative width/precision");
+                   "width/precision is out of range");
   if (fmt::detail::const_check(sizeof(long) > sizeof(int))) {
     long value = INT_MAX;
     EXPECT_THROW_MSG((void)fmt::format(runtime("{0:{1}}"), 0, (value + 1)),
-                     format_error, "number is too big");
+                     format_error, "width/precision is out of range");
   }
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:{1}}"), 0, (INT_MAX + 1ul)),
-                   format_error, "number is too big");
+                   format_error, "width/precision is out of range");
 
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:{1}}"), 0, '0'), format_error,
                    "width/precision is not integer");
@@ -1118,18 +1118,18 @@ TEST(format_test, runtime_precision) {
                    "invalid format string");
 
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0.0, -1),
-                   format_error, "negative width/precision");
+                   format_error, "width/precision is out of range");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0.0, (INT_MAX + 1u)),
-                   format_error, "number is too big");
+                   format_error, "width/precision is out of range");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0.0, -1l),
-                   format_error, "negative width/precision");
+                   format_error, "width/precision is out of range");
   if (fmt::detail::const_check(sizeof(long) > sizeof(int))) {
     long value = INT_MAX;
     EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0.0, (value + 1)),
-                     format_error, "number is too big");
+                     format_error, "width/precision is out of range");
   }
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0.0, (INT_MAX + 1ul)),
-                   format_error, "number is too big");
+                   format_error, "width/precision is out of range");
 
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0.0, '0'),
                    format_error, "width/precision is not integer");