Auto merge of #50364 - LukasKalbertodt:improve-duration-debug-impl, r=KodrAus

Improve `Debug` impl of `time::Duration`

Hi there!

For a long time now, I was getting annoyed by the derived `Debug` impl of `Duration`. Usually, I use `Duration` to either do quick'n'dirty benchmarking or measuring the time of some operation in general. The output of the derived Debug impl is hard to parse for humans: is { secs: 0, nanos: 968360102 } or { secs: 0, nanos 98507324 } longer?

So after running into the annoyance several times (sometimes building my own function to print the Duration properly), I decided to tackle this. Now the output looks like this:

```
Duration::new(1, 0)                 => 1s
Duration::new(1, 1)                 => 1.000000001s
Duration::new(1, 300)               => 1.0000003s
Duration::new(1, 4000)              => 1.000004s
Duration::new(1, 600000)            => 1.0006s
Duration::new(1, 7000000)           => 1.007s
Duration::new(0, 0)                 => 0ns
Duration::new(0, 1)                 => 1ns
Duration::new(0, 300)               => 300ns
Duration::new(0, 4001)              => 4.001µs
Duration::new(0, 600300)            => 600.3µs
Duration::new(0, 7000000)           => 7ms
```

Note that I implemented the formatting manually and didn't use floats. No information is "lost" when printing. So `Duration::new(123_456_789_000, 900_000_001)` prints as `123456789000.900000001s`.

~~This is not yet finished~~, but I wanted to open the PR now already in order to get some feedback (maybe everyone likes the derived impl).

### Still ToDo:

- [x] Respect precision ~~and width~~ parameter of the formatter (see [this comment](https://github.com/rust-lang/rust/pull/50364#issuecomment-386107107))

### Alternatives/Decisions

- Should large durations displayed in minutes, hours, days, ...? For now, I decided not to because the current formatting is close the how a `Duration` is stored. From this new `Debug` output, you can still easily see what the values of `secs` and `nanos` are. A formatting like `3h 27m 12s 9ms` might be more appropriate for a `Display` impl?
- Should this rather be a `Display` impl and should `Debug` be derived? Maybe this formatting is too fancy for `Debug`? In my opinion it's not and, as already mentioned, from the current format one can still very easily determine the values for `secs` and `nanos`.
- Whitespace between the number and the unit?

### Notes for reviewers

- ~~The combined diff sucks. Rather review both commits individually.~~
- ~~In the unit test, I am building my own type implementing `fmt::Write` to test the output. Maybe there is already something like that which I can use?~~
- My `Debug` impl block is marked as `#[stable(...)]`... but that's fine since the derived Debug impl was stable already, right?

---

~~Apart from the main change, I moved all `time` unit tests into the `tests` directory. All other `libcore` tests are there, so I guess it was simply an oversight. Prior to this change, the `time` tests weren't run, so I guess this is kind of a bug fix. If my `Debug` impl is rejected, I can of course just send the fix as PR.~~ (this was already merged in #50466)

2 files changed, 246 insertions, 1 deletions

diff --git a/src/libcore/tests/time.rs b/src/libcore/tests/time.rs
index 0f3b95236f0..df139965753 100644
--- a/src/libcore/tests/time.rs
+++ b/src/libcore/tests/time.rs
@@ -160,3 +160,131 @@ fn checked_div() {
     assert_eq!(Duration::new(1, 0).checked_div(2), Some(Duration::new(0, 500_000_000)));
     assert_eq!(Duration::new(2, 0).checked_div(0), None);
 }
+
+#[test]
+fn debug_formatting_extreme_values() {
+    assert_eq!(
+        format!("{:?}", Duration::new(18_446_744_073_709_551_615, 123_456_789)),
+        "18446744073709551615.123456789s"
+    );
+}
+
+#[test]
+fn debug_formatting_secs() {
+    assert_eq!(format!("{:?}", Duration::new(7, 000_000_000)), "7s");
+    assert_eq!(format!("{:?}", Duration::new(7, 100_000_000)), "7.1s");
+    assert_eq!(format!("{:?}", Duration::new(7, 000_010_000)), "7.00001s");
+    assert_eq!(format!("{:?}", Duration::new(7, 000_000_001)), "7.000000001s");
+    assert_eq!(format!("{:?}", Duration::new(7, 123_456_789)), "7.123456789s");
+
+    assert_eq!(format!("{:?}", Duration::new(88, 000_000_000)), "88s");
+    assert_eq!(format!("{:?}", Duration::new(88, 100_000_000)), "88.1s");
+    assert_eq!(format!("{:?}", Duration::new(88, 000_010_000)), "88.00001s");
+    assert_eq!(format!("{:?}", Duration::new(88, 000_000_001)), "88.000000001s");
+    assert_eq!(format!("{:?}", Duration::new(88, 123_456_789)), "88.123456789s");
+
+    assert_eq!(format!("{:?}", Duration::new(999, 000_000_000)), "999s");
+    assert_eq!(format!("{:?}", Duration::new(999, 100_000_000)), "999.1s");
+    assert_eq!(format!("{:?}", Duration::new(999, 000_010_000)), "999.00001s");
+    assert_eq!(format!("{:?}", Duration::new(999, 000_000_001)), "999.000000001s");
+    assert_eq!(format!("{:?}", Duration::new(999, 123_456_789)), "999.123456789s");
+}
+
+#[test]
+fn debug_formatting_millis() {
+    assert_eq!(format!("{:?}", Duration::new(0, 7_000_000)), "7ms");
+    assert_eq!(format!("{:?}", Duration::new(0, 7_100_000)), "7.1ms");
+    assert_eq!(format!("{:?}", Duration::new(0, 7_000_001)), "7.000001ms");
+    assert_eq!(format!("{:?}", Duration::new(0, 7_123_456)), "7.123456ms");
+
+    assert_eq!(format!("{:?}", Duration::new(0, 88_000_000)), "88ms");
+    assert_eq!(format!("{:?}", Duration::new(0, 88_100_000)), "88.1ms");
+    assert_eq!(format!("{:?}", Duration::new(0, 88_000_001)), "88.000001ms");
+    assert_eq!(format!("{:?}", Duration::new(0, 88_123_456)), "88.123456ms");
+
+    assert_eq!(format!("{:?}", Duration::new(0, 999_000_000)), "999ms");
+    assert_eq!(format!("{:?}", Duration::new(0, 999_100_000)), "999.1ms");
+    assert_eq!(format!("{:?}", Duration::new(0, 999_000_001)), "999.000001ms");
+    assert_eq!(format!("{:?}", Duration::new(0, 999_123_456)), "999.123456ms");
+}
+
+#[test]
+fn debug_formatting_micros() {
+    assert_eq!(format!("{:?}", Duration::new(0, 7_000)), "7µs");
+    assert_eq!(format!("{:?}", Duration::new(0, 7_100)), "7.1µs");
+    assert_eq!(format!("{:?}", Duration::new(0, 7_001)), "7.001µs");
+    assert_eq!(format!("{:?}", Duration::new(0, 7_123)), "7.123µs");
+
+    assert_eq!(format!("{:?}", Duration::new(0, 88_000)), "88µs");
+    assert_eq!(format!("{:?}", Duration::new(0, 88_100)), "88.1µs");
+    assert_eq!(format!("{:?}", Duration::new(0, 88_001)), "88.001µs");
+    assert_eq!(format!("{:?}", Duration::new(0, 88_123)), "88.123µs");
+
+    assert_eq!(format!("{:?}", Duration::new(0, 999_000)), "999µs");
+    assert_eq!(format!("{:?}", Duration::new(0, 999_100)), "999.1µs");
+    assert_eq!(format!("{:?}", Duration::new(0, 999_001)), "999.001µs");
+    assert_eq!(format!("{:?}", Duration::new(0, 999_123)), "999.123µs");
+}
+
+#[test]
+fn debug_formatting_nanos() {
+    assert_eq!(format!("{:?}", Duration::new(0, 0)), "0ns");
+    assert_eq!(format!("{:?}", Duration::new(0, 1)), "1ns");
+    assert_eq!(format!("{:?}", Duration::new(0, 88)), "88ns");
+    assert_eq!(format!("{:?}", Duration::new(0, 999)), "999ns");
+}
+
+#[test]
+fn debug_formatting_precision_zero() {
+    assert_eq!(format!("{:.0?}", Duration::new(0, 0)), "0ns");
+    assert_eq!(format!("{:.0?}", Duration::new(0, 123)), "123ns");
+
+    assert_eq!(format!("{:.0?}", Duration::new(0, 1_001)), "1µs");
+    assert_eq!(format!("{:.0?}", Duration::new(0, 1_499)), "1µs");
+    assert_eq!(format!("{:.0?}", Duration::new(0, 1_500)), "2µs");
+    assert_eq!(format!("{:.0?}", Duration::new(0, 1_999)), "2µs");
+
+    assert_eq!(format!("{:.0?}", Duration::new(0, 1_000_001)), "1ms");
+    assert_eq!(format!("{:.0?}", Duration::new(0, 1_499_999)), "1ms");
+    assert_eq!(format!("{:.0?}", Duration::new(0, 1_500_000)), "2ms");
+    assert_eq!(format!("{:.0?}", Duration::new(0, 1_999_999)), "2ms");
+
+    assert_eq!(format!("{:.0?}", Duration::new(1, 000_000_001)), "1s");
+    assert_eq!(format!("{:.0?}", Duration::new(1, 499_999_999)), "1s");
+    assert_eq!(format!("{:.0?}", Duration::new(1, 500_000_000)), "2s");
+    assert_eq!(format!("{:.0?}", Duration::new(1, 999_999_999)), "2s");
+}
+
+#[test]
+fn debug_formatting_precision_two() {
+    assert_eq!(format!("{:.2?}", Duration::new(0, 0)), "0.00ns");
+    assert_eq!(format!("{:.2?}", Duration::new(0, 123)), "123.00ns");
+
+    assert_eq!(format!("{:.2?}", Duration::new(0, 1_000)), "1.00µs");
+    assert_eq!(format!("{:.2?}", Duration::new(0, 7_001)), "7.00µs");
+    assert_eq!(format!("{:.2?}", Duration::new(0, 7_100)), "7.10µs");
+    assert_eq!(format!("{:.2?}", Duration::new(0, 7_109)), "7.11µs");
+    assert_eq!(format!("{:.2?}", Duration::new(0, 7_199)), "7.20µs");
+    assert_eq!(format!("{:.2?}", Duration::new(0, 1_999)), "2.00µs");
+
+    assert_eq!(format!("{:.2?}", Duration::new(0, 1_000_000)), "1.00ms");
+    assert_eq!(format!("{:.2?}", Duration::new(0, 3_001_000)), "3.00ms");
+    assert_eq!(format!("{:.2?}", Duration::new(0, 3_100_000)), "3.10ms");
+    assert_eq!(format!("{:.2?}", Duration::new(0, 1_999_999)), "2.00ms");
+
+    assert_eq!(format!("{:.2?}", Duration::new(1, 000_000_000)), "1.00s");
+    assert_eq!(format!("{:.2?}", Duration::new(4, 001_000_000)), "4.00s");
+    assert_eq!(format!("{:.2?}", Duration::new(2, 100_000_000)), "2.10s");
+    assert_eq!(format!("{:.2?}", Duration::new(2, 104_990_000)), "2.10s");
+    assert_eq!(format!("{:.2?}", Duration::new(2, 105_000_000)), "2.11s");
+    assert_eq!(format!("{:.2?}", Duration::new(8, 999_999_999)), "9.00s");
+}
+
+#[test]
+fn debug_formatting_precision_high() {
+    assert_eq!(format!("{:.5?}",  Duration::new(0, 23_678)), "23.67800µs");
+
+    assert_eq!(format!("{:.9?}",  Duration::new(1, 000_000_000)), "1.000000000s");
+    assert_eq!(format!("{:.10?}", Duration::new(4, 001_000_000)), "4.0010000000s");
+    assert_eq!(format!("{:.20?}", Duration::new(4, 001_000_000)), "4.00100000000000000000s");
+}
diff --git a/src/libcore/time.rs b/src/libcore/time.rs
index c0b2b2a0bc6..a1815b5f5ef 100644
--- a/src/libcore/time.rs
+++ b/src/libcore/time.rs
@@ -21,6 +21,7 @@
 //! assert_eq!(Duration::new(5, 0), Duration::from_secs(5));
 //! ```
 
+use fmt;
 use iter::Sum;
 use ops::{Add, Sub, Mul, Div, AddAssign, SubAssign, MulAssign, DivAssign};
 
@@ -59,7 +60,7 @@ const MICROS_PER_SEC: u64 = 1_000_000;
 /// let ten_millis = Duration::from_millis(10);
 /// ```
 #[stable(feature = "duration", since = "1.3.0")]
-#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Hash, Default)]
+#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Default)]
 pub struct Duration {
     secs: u64,
     nanos: u32, // Always 0 <= nanos < NANOS_PER_SEC
@@ -485,3 +486,119 @@ impl<'a> Sum<&'a Duration> for Duration {
         iter.fold(Duration::new(0, 0), |a, b| a + *b)
     }
 }
+
+#[stable(feature = "duration_debug_impl", since = "1.27.0")]
+impl fmt::Debug for Duration {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        /// Formats a floating point number in decimal notation.
+        ///
+        /// The number is given as the `integer_part` and a fractional part.
+        /// The value of the fractional part is `fractional_part / divisor`. So
+        /// `integer_part` = 3, `fractional_part` = 12 and `divisor` = 100
+        /// represents the number `3.012`. Trailing zeros are omitted.
+        ///
+        /// `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`!
+        fn fmt_decimal(
+            f: &mut fmt::Formatter,
+            mut integer_part: u64,
+            mut fractional_part: u32,
+            mut divisor: u32,
+        ) -> fmt::Result {
+            // Encode the fractional part into a temporary buffer. The buffer
+            // only need to hold 9 elements, because `fractional_part` has to
+            // be smaller than 10^9. The buffer is prefilled with '0' digits
+            // to simplify the code below.
+            let mut buf = [b'0'; 9];
+
+            // The next digit is written at this position
+            let mut pos = 0;
+
+            // We keep writing digits into the buffer while there are non-zero
+            // digits left and we haven't written enough digits yet.
+            while fractional_part > 0 && pos < f.precision().unwrap_or(9) {
+                // Write new digit into the buffer
+                buf[pos] = b'0' + (fractional_part / divisor) as u8;
+
+                fractional_part %= divisor;
+                divisor /= 10;
+                pos += 1;
+            }
+
+            // If a precision < 9 was specified, there may be some non-zero
+            // digits left that weren't written into the buffer. In that case we
+            // need to perform rounding to match the semantics of printing
+            // normal floating point numbers. However, we only need to do work
+            // when rounding up. This happens if the first digit of the
+            // remaining ones is >= 5.
+            if fractional_part > 0 && fractional_part >= divisor * 5 {
+                // Round up the number contained in the buffer. We go through
+                // the buffer backwards and keep track of the carry.
+                let mut rev_pos = pos;
+                let mut carry = true;
+                while carry && rev_pos > 0 {
+                    rev_pos -= 1;
+
+                    // If the digit in the buffer is not '9', we just need to
+                    // increment it and can stop then (since we don't have a
+                    // carry anymore). Otherwise, we set it to '0' (overflow)
+                    // and continue.
+                    if buf[rev_pos] < b'9' {
+                        buf[rev_pos] += 1;
+                        carry = false;
+                    } else {
+                        buf[rev_pos] = b'0';
+                    }
+                }
+
+                // If we still have the carry bit set, that means that we set
+                // the whole buffer to '0's and need to increment the integer
+                // part.
+                if carry {
+                    integer_part += 1;
+                }
+            }
+
+            // Determine the end of the buffer: if precision is set, we just
+            // use as many digits from the buffer (capped to 9). If it isn't
+            // set, we only use all digits up to the last non-zero one.
+            let end = f.precision().map(|p| ::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 {
+                // We are only writing ASCII digits into the buffer and it was
+                // initialized with '0's, so it contains valid UTF8.
+                let s = unsafe {
+                    ::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$}", integer_part, s, width = w)
+            }
+        }
+
+        // Print leading '+' sign if requested
+        if f.sign_plus() {
+            write!(f, "+")?;
+        }
+
+        if self.secs > 0 {
+            fmt_decimal(f, self.secs, self.nanos, 100_000_000)?;
+            f.write_str("s")
+        } else if self.nanos >= 1_000_000 {
+            fmt_decimal(f, self.nanos as u64 / 1_000_000, self.nanos % 1_000_000, 100_000)?;
+            f.write_str("ms")
+        } else if self.nanos >= 1_000 {
+            fmt_decimal(f, self.nanos as u64 / 1_000, self.nanos % 1_000, 100)?;
+            f.write_str("µs")
+        } else {
+            fmt_decimal(f, self.nanos as u64, 0, 1)?;
+            f.write_str("ns")
+        }
+    }
+}