Merge remote-tracking branch 'upstream/master' into invalidcolons

# Conflicts:
#	test/chrono-test.cc

include/fmt/chrono.h

@@ -585,18 +585,20 @@ struct formatter<std::chrono::duration<Rep, Period>, Char> {
     }
   };
 
- public:
-  formatter() : spec(), precision(-1) {}
+  typedef typename basic_parse_context<Char>::iterator iterator;
+  struct parse_range {
+    iterator begin;
+    iterator end;
+  };
 
-  FMT_CONSTEXPR auto parse(basic_parse_context<Char>& ctx)
-      -> decltype(ctx.begin()) {
+  FMT_CONSTEXPR parse_range do_parse(basic_parse_context<Char>& ctx) {
     auto begin = ctx.begin(), end = ctx.end();
-    if (begin == end) return begin;
+    if (begin == end) return {begin, end};
     spec_handler handler{*this, ctx, format_str};
     begin = internal::parse_align(begin, end, handler);
-    if (begin == end) return begin;
+    if (begin == end) return {begin, end};
     begin = internal::parse_width(begin, end, handler);
-    if (begin == end) return begin;
+    if (begin == end) return {begin, end};
     if (*begin == '.') {
       if (std::is_floating_point<Rep>::value)
         begin = internal::parse_precision(begin, end, handler);
@@ -604,9 +606,18 @@ struct formatter<std::chrono::duration<Rep, Period>, Char> {
         handler.on_error("precision not allowed for this argument type");
     }
     end = parse_chrono_format(begin, end, internal::chrono_format_checker());
-    format_str =
-        basic_string_view<Char>(&*begin, internal::to_unsigned(end - begin));
-    return end;
+    return {begin, end};
+  }
+
+ public:
+  formatter() : spec(), precision(-1) {}
+
+  FMT_CONSTEXPR auto parse(basic_parse_context<Char>& ctx)
+      -> decltype(ctx.begin()) {
+    auto range = do_parse(ctx);
+    format_str = basic_string_view<Char>(
+        &*range.begin, internal::to_unsigned(range.end - range.begin));
+    return range.end;
   }
 
   template <typename FormatContext>

include/fmt/format-inl.h

@@ -494,9 +494,11 @@ enum result {
 };
 }
 
-// Generates output using Grisu2 digit-gen algorithm.
+// Generates output using the Grisu digit-gen algorithm.
+// error: the size of the region (lower, upper) outside of which numbers
+// definitely do not round to value (Delta in Grisu3).
 template <typename Handler>
-digits::result grisu2_gen_digits(fp value, uint64_t error, int& exp,
+digits::result grisu_gen_digits(fp value, uint64_t error, int& exp,
                                 Handler& handler) {
   fp one(1ull << -value.e, value.e);
   // The integral part of scaled value (p1 in Grisu) = value / one. It cannot be
@@ -635,34 +637,57 @@ struct fixed_handler {
 };
 
 // The shortest representation digit handler.
-struct shortest_handler {
+template <int GRISU_VERSION> struct grisu_shortest_handler {
   char* buf;
   int size;
-  fp diff;  // wp_w in Grisu.
+  // Distance between scaled value and upper bound (wp_W in Grisu3).
+  uint64_t diff;
 
   digits::result on_start(uint64_t, uint64_t, uint64_t, int&) {
     return digits::more;
   }
+
+  // This implements Grisu3's round_weed.
   digits::result on_digit(char digit, uint64_t divisor, uint64_t remainder,
                           uint64_t error, int exp, bool integral) {
     buf[size++] = digit;
-    if (remainder > error) return digits::more;
-    uint64_t d = integral ? diff.f : diff.f * data::POWERS_OF_10_64[-exp];
-    while (
-        remainder < d && error - remainder >= divisor &&
-        (remainder + divisor < d || d - remainder > remainder + divisor - d)) {
+    if (remainder >= error) return digits::more;
+    if (GRISU_VERSION != 3) {
+      uint64_t d = integral ? diff : diff * data::POWERS_OF_10_64[-exp];
+      while (remainder < d && error - remainder >= divisor &&
+             (remainder + divisor < d ||
+              d - remainder > remainder + divisor - d)) {
         --buf[size - 1];
         remainder += divisor;
       }
       return digits::done;
     }
+    uint64_t unit = integral ? 1 : data::POWERS_OF_10_64[-exp];
+    uint64_t up = diff + unit;  // wp_Wup
+    while (remainder < up && error - remainder >= divisor &&
+           (remainder + divisor < up ||
+            up - remainder > remainder + divisor - up)) {
+      --buf[size - 1];
+      remainder += divisor;
+    }
+    uint64_t down = diff - unit;  // wp_Wdown
+    if (remainder < down && error - remainder >= divisor &&
+        (remainder + divisor < down ||
+         down - remainder > remainder + divisor - down)) {
+      return digits::error;
+    }
+    return 2 * unit <= remainder && remainder <= error - 4 * unit
+               ? digits::done
+               : digits::error;
+  }
 };
 
 template <typename Double, typename std::enable_if<
                                sizeof(Double) == sizeof(uint64_t), int>::type>
-FMT_FUNC bool grisu2_format(Double value, buffer<char>& buf, int precision,
-                            bool fixed, int& exp) {
+FMT_FUNC bool grisu_format(Double value, buffer<char>& buf, int precision,
+                           unsigned options, int& exp) {
   FMT_ASSERT(value >= 0, "value is negative");
+  bool fixed = (options & grisu_options::fixed) != 0;
   if (value <= 0) {  // <= instead of == to silence a warning.
     if (precision < 0 || !fixed) {
       exp = 0;
@@ -685,7 +710,7 @@ FMT_FUNC bool grisu2_format(Double value, buffer<char>& buf, int precision,
         min_exp - (fp_value.e + fp::significand_size), cached_exp10);
     fp_value = fp_value * cached_pow;
     fixed_handler handler{buf.data(), 0, precision, -cached_exp10, fixed};
-    if (grisu2_gen_digits(fp_value, 1, exp, handler) == digits::error)
+    if (grisu_gen_digits(fp_value, 1, exp, handler) == digits::error)
       return false;
     buf.resize(to_unsigned(handler.size));
   } else {
@@ -695,17 +720,29 @@ FMT_FUNC bool grisu2_format(Double value, buffer<char>& buf, int precision,
     // the exponent in the range [min_exp, -32].
     auto cached_pow = get_cached_power(  // \tilde{c}_{-k} in Grisu.
         min_exp - (upper.e + fp::significand_size), cached_exp10);
-    upper = upper * cached_pow;  // \tilde{M}^+ in Grisu.
-    --upper.f;                   // \tilde{M}^+ - 1 ulp -> M^+_{\downarrow}.
-    assert(min_exp <= upper.e && upper.e <= -32);
     fp_value.normalize();
     fp_value = fp_value * cached_pow;
     lower = lower * cached_pow;  // \tilde{M}^- in Grisu.
+    upper = upper * cached_pow;  // \tilde{M}^+ in Grisu.
+    assert(min_exp <= upper.e && upper.e <= -32);
+    auto result = digits::result();
+    int size = 0;
+    if ((options & grisu_options::grisu3) != 0) {
+      --lower.f;  // \tilde{M}^- - 1 ulp -> M^-_{\downarrow}.
+      ++upper.f;  // \tilde{M}^+ + 1 ulp -> M^+_{\uparrow}.
+      // Numbers outside of (lower, upper) definitely do not round to value.
+      grisu_shortest_handler<3> handler{buf.data(), 0, (upper - fp_value).f};
+      result = grisu_gen_digits(upper, upper.f - lower.f, exp, handler);
+      size = handler.size;
+    } else {
       ++lower.f;  // \tilde{M}^- + 1 ulp -> M^-_{\uparrow}.
-    shortest_handler handler{buf.data(), 0, upper - fp_value};
-    auto result = grisu2_gen_digits(upper, upper.f - lower.f, exp, handler);
+      --upper.f;  // \tilde{M}^+ - 1 ulp -> M^+_{\downarrow}.
+      grisu_shortest_handler<2> handler{buf.data(), 0, (upper - fp_value).f};
+      result = grisu_gen_digits(upper, upper.f - lower.f, exp, handler);
+      size = handler.size;
+    }
     if (result == digits::error) return false;
-    buf.resize(to_unsigned(handler.size));
+    buf.resize(to_unsigned(size));
   }
   exp -= cached_exp10;
   return true;

include/fmt/format.h

@@ -1124,13 +1124,19 @@ FMT_CONSTEXPR unsigned basic_parse_context<Char, ErrorHandler>::next_arg_id() {
 
 namespace internal {
 
-// Formats value using Grisu2 algorithm:
+namespace grisu_options {
+enum {
+  fixed = 1,
+  grisu3 = 2
+};
+}
+
+// Formats value using the Grisu algorithm:
 // https://www.cs.tufts.edu/~nr/cs257/archive/florian-loitsch/printf.pdf
 template <typename Double, FMT_ENABLE_IF(sizeof(Double) == sizeof(uint64_t))>
-FMT_API bool grisu2_format(Double value, buffer<char>& buf, int precision,
-                           bool fixed, int& exp);
+FMT_API bool grisu_format(Double, buffer<char>&, int, unsigned, int&);
 template <typename Double, FMT_ENABLE_IF(sizeof(Double) != sizeof(uint64_t))>
-inline bool grisu2_format(Double, buffer<char>&, int, bool, int&) {
+inline bool grisu_format(Double, buffer<char>&, int, unsigned, int&) {
   return false;
 }
 
@@ -1166,7 +1172,7 @@ template <typename Char, typename It> It write_exponent(int exp, It it) {
 
 // The number is given as v = digits * pow(10, exp).
 template <typename Char, typename It>
-It grisu2_prettify(const char* digits, int size, int exp, It it,
+It grisu_prettify(const char* digits, int size, int exp, It it,
                   gen_digits_params params) {
   // pow(10, full_exp - 1) <= v <= pow(10, full_exp).
   int full_exp = size + exp;
@@ -1213,6 +1219,8 @@ It grisu2_prettify(const char* digits, int size, int exp, It it,
     if (params.num_digits >= 0 && params.num_digits < num_zeros)
       num_zeros = params.num_digits;
     it = std::fill_n(it, num_zeros, static_cast<Char>('0'));
+    if (!params.trailing_zeros)
+      while (size > 0 && digits[size - 1] == '0') --size;
     it = copy_str<Char>(digits, digits + size, it);
   }
   return it;
@@ -2661,7 +2669,7 @@ template <typename Range> class basic_writer {
       int full_exp = num_digits + exp - 1;
       int precision = params.num_digits > 0 ? params.num_digits : 11;
       params_.fixed |= full_exp >= -4 && full_exp < precision;
-      auto it = internal::grisu2_prettify<char>(
+      auto it = internal::grisu_prettify<char>(
           digits.data(), num_digits, exp, internal::counting_iterator<char>(),
           params_);
       size_ = it.count();
@@ -2673,8 +2681,8 @@ template <typename Range> class basic_writer {
     template <typename It> void operator()(It&& it) {
       if (sign_) *it++ = static_cast<char_type>(sign_);
       int num_digits = static_cast<int>(digits_.size());
-      it = internal::grisu2_prettify<char_type>(digits_.data(), num_digits,
-                                                exp_, it, params_);
+      it = internal::grisu_prettify<char_type>(digits_.data(), num_digits, exp_,
+                                               it, params_);
     }
   };
 
@@ -2874,11 +2882,12 @@ void basic_writer<Range>::write_double(T value, const format_specs& spec) {
   memory_buffer buffer;
   int exp = 0;
   int precision = spec.has_precision() || !spec.type ? spec.precision : 6;
+  unsigned options = handler.fixed ? internal::grisu_options::fixed : 0;
   bool use_grisu = fmt::internal::use_grisu<T>() &&
                    (spec.type != 'a' && spec.type != 'A' && spec.type != 'e' &&
                     spec.type != 'E') &&
-                   internal::grisu2_format(static_cast<double>(value), buffer,
-                                           precision, handler.fixed, exp);
+                   internal::grisu_format(static_cast<double>(value), buffer,
+                                           precision, options, exp);
   if (!use_grisu) internal::sprintf_format(value, buffer, spec);
 
   if (handler.as_percentage) {

src/format.cc

@@ -10,9 +10,9 @@
 FMT_BEGIN_NAMESPACE
 template struct internal::basic_data<void>;
 
-// Workaround a bug in MSVC2013 that prevents instantiation of grisu2_format.
-bool (*instantiate_grisu2_format)(double, internal::buffer<char>&, int, bool,
-                                  int&) = internal::grisu2_format;
+// Workaround a bug in MSVC2013 that prevents instantiation of grisu_format.
+bool (*instantiate_grisu_format)(double, internal::buffer<char>&, int, unsigned,
+                                 int&) = internal::grisu_format;
 
 #ifndef FMT_STATIC_THOUSANDS_SEPARATOR
 template FMT_API internal::locale_ref::locale_ref(const std::locale& loc);

test/chrono-test.cc

@@ -305,4 +305,5 @@ TEST(ChronoTest, InvalidColons) {
   EXPECT_THROW(fmt::format("{0}=:{0::", std::chrono::seconds(0)),
                fmt::format_error);
 }
+
 #endif  // FMT_STATIC_THOUSANDS_SEPARATOR

test/format-impl-test.cc

@@ -137,10 +137,10 @@ TEST(FPTest, FixedHandler) {
             digits::error);
 }
 
-TEST(FPTest, Grisu2FormatCompilesWithNonIEEEDouble) {
+TEST(FPTest, GrisuFormatCompilesWithNonIEEEDouble) {
   fmt::memory_buffer buf;
   int exp = 0;
-  grisu2_format(4.2f, buf, -1, false, exp);
+  grisu_format(4.2f, buf, -1, false, exp);
 }
 
 template <typename T> struct ValueExtractor : fmt::internal::function<T> {

test/format-test.cc

@@ -1488,6 +1488,7 @@ TEST(FormatterTest, PrecisionRounding) {
   EXPECT_EQ("0.002", format("{:.3f}", 0.0015));
   EXPECT_EQ("1.000", format("{:.3f}", 0.9999));
   EXPECT_EQ("0.00123", format("{:.3}", 0.00123));
+  EXPECT_EQ("0.1", format("{:.16g}", 0.1));
   // Trigger rounding error in Grisu by a carefully chosen number.
   auto n = 3788512123356.985352;
   char buffer[64];
@@ -2451,7 +2452,7 @@ TEST(FormatTest, FormatStringErrors) {
   EXPECT_ERROR("{:d}", "invalid type specifier", std::string);
   EXPECT_ERROR("{:s}", "invalid type specifier", void*);
 #  else
-  fmt::print("warning: constexpr is broken in this versio of MSVC\n");
+  fmt::print("warning: constexpr is broken in this version of MSVC\n");
 #  endif
   EXPECT_ERROR("{foo", "compile-time checks don't support named arguments",
                int);