Auto merge of #88999 - Migi:master, r=oli-obk

bors · bors · commit f06f9bbd3a2b · 2021-09-24T15:22:26.000Z
Make `Duration` respect `width` when formatting using `Debug` When printing or writing a `std::time::Duration` using `Debug` formatting, it previously completely ignored any specified `width`. This is unlike types like integers and floats, which do pad to `width`, for both `Display` and `Debug`, though not all types consider `width` in their `Debug` output (see e.g. #30164). Curiously, `Duration`'s `Debug` formatting *did* consider `precision`. This PR makes `Duration` pad to `width` just like integers and floats, so that ```rust format!("|{:8?}|", Duration::from_millis(1234)) ``` returns ``` |1.234s | ``` Before you ask "who formats `Debug` output?", note that `Duration` doesn't actually implement `Display`, so `Debug` is currently the only way to format `Duration`s. I think that's wrong, and `Duration` should get a `Display` implementation, but in the meantime there's no harm in making the `Debug` formatting respect `width` rather than ignore it. I chose the default alignment to be left-aligned. The general rule Rust uses is: numeric types are right-aligned by default, non-numeric types left-aligned. It wasn't clear to me whether `Duration` is a numeric type or not. The fact that a formatted `Duration` can end with suffixes of variable length (`"s"`, `"ms"`, `"µs"`, etc.) made me lean towards left-alignment, but it would be trivial to change it. Fixes issue #88059.
diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs
@@ -1224,7 +1224,7 @@ unsafe fn getcount(args: &[ArgumentV1<'_>], cnt: &rt::v1::Count) -> Option<usize
 
 /// Padding after the end of something. Returned by `Formatter::padding`.
 #[must_use = "don't forget to write the post padding"]
-struct PostPadding {
+pub(crate) struct PostPadding {
     fill: char,
     padding: usize,
 }
@@ -1235,9 +1235,9 @@ impl PostPadding {
     }
 
     /// Write this post padding.
-    fn write(self, buf: &mut dyn Write) -> Result {
+    pub(crate) fn write(self, f: &mut Formatter<'_>) -> Result {
         for _ in 0..self.padding {
-            buf.write_char(self.fill)?;
+            f.buf.write_char(self.fill)?;
         }
         Ok(())
     }
@@ -1360,7 +1360,7 @@ impl<'a> Formatter<'a> {
                 write_prefix(self, sign, prefix)?;
                 let post_padding = self.padding(min - width, rt::v1::Alignment::Right)?;
                 self.buf.write_str(buf)?;
-                post_padding.write(self.buf)?;
+                post_padding.write(self)?;
                 self.fill = old_fill;
                 self.align = old_align;
                 Ok(())
@@ -1370,7 +1370,7 @@ impl<'a> Formatter<'a> {
                 let post_padding = self.padding(min - width, rt::v1::Alignment::Right)?;
                 write_prefix(self, sign, prefix)?;
                 self.buf.write_str(buf)?;
-                post_padding.write(self.buf)
+                post_padding.write(self)
             }
         }
     }
@@ -1445,7 +1445,7 @@ impl<'a> Formatter<'a> {
                     let align = rt::v1::Alignment::Left;
                     let post_padding = self.padding(width - chars_count, align)?;
                     self.buf.write_str(s)?;
-                    post_padding.write(self.buf)
+                    post_padding.write(self)
                 }
             }
         }
@@ -1454,7 +1454,7 @@ impl<'a> Formatter<'a> {
     /// Write the pre-padding and return the unwritten post-padding. Callers are
     /// responsible for ensuring post-padding is written after the thing that is
     /// being padded.
-    fn padding(
+    pub(crate) fn padding(
         &mut self,
         padding: usize,
         default: rt::v1::Alignment,
@@ -1509,7 +1509,7 @@ impl<'a> Formatter<'a> {
             } else {
                 let post_padding = self.padding(width - len, align)?;
                 self.write_formatted_parts(&formatted)?;
-                post_padding.write(self.buf)
+                post_padding.write(self)
             };
             self.fill = old_fill;
             self.align = old_align;
diff --git a/library/core/src/time.rs b/library/core/src/time.rs
@@ -1049,11 +1049,16 @@ impl fmt::Debug for Duration {
         /// `divisor` must not be above 100_000_000. It also should be a power
         /// of 10, everything else doesn't make sense. `fractional_part` has
         /// to be less than `10 * divisor`!
+        ///
+        /// A prefix and postfix may be added. The whole thing is padded
+        /// to the formatter's `width`, if specified.
         fn fmt_decimal(
             f: &mut fmt::Formatter<'_>,
             mut integer_part: u64,
             mut fractional_part: u32,
             mut divisor: u32,
+            prefix: &str,
+            postfix: &str,
         ) -> fmt::Result {
             // Encode the fractional part into a temporary buffer. The buffer
             // only need to hold 9 elements, because `fractional_part` has to
@@ -1114,48 +1119,91 @@ impl fmt::Debug for Duration {
             // set, we only use all digits up to the last non-zero one.
             let end = f.precision().map(|p| crate::cmp::min(p, 9)).unwrap_or(pos);
 
-            // If we haven't emitted a single fractional digit and the precision
-            // wasn't set to a non-zero value, we don't print the decimal point.
-            if end == 0 {
-                write!(f, "{}", integer_part)
-            } else {
-                // SAFETY: We are only writing ASCII digits into the buffer and it was
-                // initialized with '0's, so it contains valid UTF8.
-                let s = unsafe { crate::str::from_utf8_unchecked(&buf[..end]) };
+            // This closure emits the formatted duration without emitting any
+            // padding (padding is calculated below).
+            let emit_without_padding = |f: &mut fmt::Formatter<'_>| {
+                write!(f, "{}{}", prefix, integer_part)?;
+
+                // Write the decimal point and the fractional part (if any).
+                if end > 0 {
+                    // SAFETY: We are only writing ASCII digits into the buffer and
+                    // it was initialized with '0's, so it contains valid UTF8.
+                    let s = unsafe { crate::str::from_utf8_unchecked(&buf[..end]) };
+
+                    // If the user request a precision > 9, we pad '0's at the end.
+                    let w = f.precision().unwrap_or(pos);
+                    write!(f, ".{:0<width$}", s, width = w)?;
+                }
 
-                // If the user request a precision > 9, we pad '0's at the end.
-                let w = f.precision().unwrap_or(pos);
-                write!(f, "{}.{:0<width$}", integer_part, s, width = w)
+                write!(f, "{}", postfix)
+            };
+
+            match f.width() {
+                None => {
+                    // No `width` specified. There's no need to calculate the
+                    // length of the output in this case, just emit it.
+                    emit_without_padding(f)
+                }
+                Some(requested_w) => {
+                    // A `width` was specified. Calculate the actual width of
+                    // the output in order to calculate the required padding.
+                    // It consists of 4 parts:
+                    // 1. The prefix: is either "+" or "", so we can just use len().
+                    // 2. The postfix: can be "µs" so we have to count UTF8 characters.
+                    let mut actual_w = prefix.len() + postfix.chars().count();
+                    // 3. The integer part:
+                    if let Some(log) = integer_part.checked_log10() {
+                        // integer_part is > 0, so has length log10(x)+1
+                        actual_w += 1 + log as usize;
+                    } else {
+                        // integer_part is 0, so has length 1.
+                        actual_w += 1;
+                    }
+                    // 4. The fractional part (if any):
+                    if end > 0 {
+                        let frac_part_w = f.precision().unwrap_or(pos);
+                        actual_w += 1 + frac_part_w;
+                    }
+
+                    if requested_w <= actual_w {
+                        // Output is already longer than `width`, so don't pad.
+                        emit_without_padding(f)
+                    } else {
+                        // We need to add padding. Use the `Formatter::padding` helper function.
+                        let default_align = crate::fmt::rt::v1::Alignment::Left;
+                        let post_padding = f.padding(requested_w - actual_w, default_align)?;
+                        emit_without_padding(f)?;
+                        post_padding.write(f)
+                    }
+                }
             }
         }
 
         // Print leading '+' sign if requested
-        if f.sign_plus() {
-            write!(f, "+")?;
-        }
+        let prefix = if f.sign_plus() { "+" } else { "" };
 
         if self.secs > 0 {
-            fmt_decimal(f, self.secs, self.nanos, NANOS_PER_SEC / 10)?;
-            f.write_str("s")
+            fmt_decimal(f, self.secs, self.nanos, NANOS_PER_SEC / 10, prefix, "s")
         } else if self.nanos >= NANOS_PER_MILLI {
             fmt_decimal(
                 f,
                 (self.nanos / NANOS_PER_MILLI) as u64,
                 self.nanos % NANOS_PER_MILLI,
                 NANOS_PER_MILLI / 10,
-            )?;
-            f.write_str("ms")
+                prefix,
+                "ms",
+            )
         } else if self.nanos >= NANOS_PER_MICRO {
             fmt_decimal(
                 f,
                 (self.nanos / NANOS_PER_MICRO) as u64,
                 self.nanos % NANOS_PER_MICRO,
                 NANOS_PER_MICRO / 10,
-            )?;
-            f.write_str("µs")
+                prefix,
+                "µs",
+            )
         } else {
-            fmt_decimal(f, self.nanos as u64, 0, 1)?;
-            f.write_str("ns")
+            fmt_decimal(f, self.nanos as u64, 0, 1, prefix, "ns")
         }
     }
 }
diff --git a/library/core/tests/time.rs b/library/core/tests/time.rs
@@ -313,6 +313,34 @@ fn debug_formatting_precision_two() {
     assert_eq!(format!("{:.2?}", Duration::new(8, 999_999_999)), "9.00s");
 }
 
+#[test]
+fn debug_formatting_padding() {
+    assert_eq!("0ns      ", format!("{:<9?}", Duration::new(0, 0)));
+    assert_eq!("      0ns", format!("{:>9?}", Duration::new(0, 0)));
+    assert_eq!("   0ns   ", format!("{:^9?}", Duration::new(0, 0)));
+    assert_eq!("123ns    ", format!("{:<9.0?}", Duration::new(0, 123)));
+    assert_eq!("    123ns", format!("{:>9.0?}", Duration::new(0, 123)));
+    assert_eq!("  123ns  ", format!("{:^9.0?}", Duration::new(0, 123)));
+    assert_eq!("123.0ns  ", format!("{:<9.1?}", Duration::new(0, 123)));
+    assert_eq!("  123.0ns", format!("{:>9.1?}", Duration::new(0, 123)));
+    assert_eq!(" 123.0ns ", format!("{:^9.1?}", Duration::new(0, 123)));
+    assert_eq!("7.1µs    ", format!("{:<9?}", Duration::new(0, 7_100)));
+    assert_eq!("    7.1µs", format!("{:>9?}", Duration::new(0, 7_100)));
+    assert_eq!("  7.1µs  ", format!("{:^9?}", Duration::new(0, 7_100)));
+    assert_eq!("999.123456ms", format!("{:<9?}", Duration::new(0, 999_123_456)));
+    assert_eq!("999.123456ms", format!("{:>9?}", Duration::new(0, 999_123_456)));
+    assert_eq!("999.123456ms", format!("{:^9?}", Duration::new(0, 999_123_456)));
+    assert_eq!("5s       ", format!("{:<9?}", Duration::new(5, 0)));
+    assert_eq!("       5s", format!("{:>9?}", Duration::new(5, 0)));
+    assert_eq!("   5s    ", format!("{:^9?}", Duration::new(5, 0)));
+    assert_eq!("5.000000000000s", format!("{:<9.12?}", Duration::new(5, 0)));
+    assert_eq!("5.000000000000s", format!("{:>9.12?}", Duration::new(5, 0)));
+    assert_eq!("5.000000000000s", format!("{:^9.12?}", Duration::new(5, 0)));
+
+    // default alignment is left:
+    assert_eq!("5s       ", format!("{:9?}", Duration::new(5, 0)));
+}
+
 #[test]
 fn debug_formatting_precision_high() {
     assert_eq!(format!("{:.5?}", Duration::new(0, 23_678)), "23.67800µs");
