Fix bug in Duration.toString().

The `toString` called `(-this).toString()` recursively
for negative durations.
If instantiated with the minimal integer on native,
that call would recurse infinitely.

The code has been wrong since we removed arbitrary precision integers.

TEST= Add test for regression case, otherwise covered by existing tests.

Change-Id: Ifb58bbc5aee38ef760f9352aede1694b79bae931
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/212300
Commit-Queue: Lasse R.H. Nielsen <lrn@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>

pkg/vm/testcases/transformations/type_flow/transformer/enum_from_lib_used_as_type.dart.expect

@@ -17,12 +17,12 @@ import self as self;
 import "dart:core" as core;
 
 abstract class Enum extends core::Object implements core::Enum {
-[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasNonThisUses:false,hasTearOffUses:false,getterSelectorId:3267]  abstract get /*isLegacy*/ index() → core::int;
+[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasNonThisUses:false,hasTearOffUses:false,getterSelectorId:3265]  abstract get /*isLegacy*/ index() → core::int;
 }
 class Class extends core::Object {
   synthetic constructor •() → self::Class
     : super core::Object::•()
     ;
-[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasTearOffUses:false,methodOrSetterSelectorId:3268,getterSelectorId:3269]  method method([@vm.inferred-type.metadata=dart.core::Null? (value: null)] self::Enum e) → core::int
+[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasTearOffUses:false,methodOrSetterSelectorId:3266,getterSelectorId:3267]  method method([@vm.inferred-type.metadata=dart.core::Null? (value: null)] self::Enum e) → core::int
     return [@vm.inferred-type.metadata=!] e.{self::Enum::index}{core::int};
 }

pkg/vm/testcases/transformations/type_flow/transformer/tree_shake_enum_from_lib.dart.expect

@@ -48,12 +48,12 @@ import self as self;
 import "dart:core" as core;
 
 abstract class ConstEnum extends core::Object implements core::Enum {
-[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasNonThisUses:false,hasTearOffUses:false,getterSelectorId:3273]  abstract get /*isLegacy*/ index() → core::int;
+[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasNonThisUses:false,hasTearOffUses:false,getterSelectorId:3271]  abstract get /*isLegacy*/ index() → core::int;
 }
 class ConstClass extends core::Object {
   synthetic constructor •() → self::ConstClass
     : super core::Object::•()
     ;
-[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasTearOffUses:false,methodOrSetterSelectorId:3274,getterSelectorId:3275]  method method([@vm.inferred-type.metadata=dart.core::Null? (value: null)] self::ConstEnum e) → core::int
+[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasTearOffUses:false,methodOrSetterSelectorId:3272,getterSelectorId:3273]  method method([@vm.inferred-type.metadata=dart.core::Null? (value: null)] self::ConstEnum e) → core::int
     return [@vm.inferred-type.metadata=!] e.{self::ConstEnum::index}{core::int};
 }

sdk/lib/core/duration.dart

@@ -124,6 +124,10 @@ class Duration implements Comparable<Duration> {
   /// Likewise, values can be negative, in which case they
   /// underflow and subtract from the next larger unit.
   ///
+  /// If the total number of microseconds cannot be represented
+  /// as an integer value, the number of microseconds might be truncated
+  /// and it might lose precision.
+  ///
   /// All arguments are 0 by default.
   const Duration(
       {int days = 0,
@@ -132,12 +136,12 @@ class Duration implements Comparable<Duration> {
       int seconds = 0,
       int milliseconds = 0,
       int microseconds = 0})
-      : this._microseconds(microsecondsPerDay * days +
-            microsecondsPerHour * hours +
-            microsecondsPerMinute * minutes +
-            microsecondsPerSecond * seconds +
+      : this._microseconds(microseconds +
             microsecondsPerMillisecond * milliseconds +
-            microseconds);
+            microsecondsPerSecond * seconds +
+            microsecondsPerMinute * minutes +
+            microsecondsPerHour * hours +
+            microsecondsPerDay * days);
 
   // Fast path internal direct constructor to avoids the optional arguments and
   // [_microseconds] recomputation.
@@ -257,30 +261,35 @@ class Duration implements Comparable<Duration> {
   /// d.toString();  // "1:10:00.000500"
   /// ```
   String toString() {
-    String sixDigits(int n) {
-      if (n >= 100000) return "$n";
-      if (n >= 10000) return "0$n";
-      if (n >= 1000) return "00$n";
-      if (n >= 100) return "000$n";
-      if (n >= 10) return "0000$n";
-      return "00000$n";
-    }
+    var buffer = List<String>.filled(9, "");
+    var microseconds = inMicroseconds;
 
-    String twoDigits(int n) {
-      if (n >= 10) return "$n";
-      return "0$n";
-    }
+    var hours = microseconds ~/ microsecondsPerHour;
+    microseconds = microseconds.remainder(microsecondsPerHour);
 
-    if (inMicroseconds < 0) {
-      return "-${-this}";
-    }
-    String twoDigitMinutes =
-        twoDigits(inMinutes.remainder(minutesPerHour) as int);
-    String twoDigitSeconds =
-        twoDigits(inSeconds.remainder(secondsPerMinute) as int);
-    String sixDigitUs =
-        sixDigits(inMicroseconds.remainder(microsecondsPerSecond) as int);
-    return "$inHours:$twoDigitMinutes:$twoDigitSeconds.$sixDigitUs";
+    if (microseconds < 0) microseconds = -microseconds;
+    var hoursString = hours.toString();
+    buffer
+      ..[0] = hoursString
+      ..[1] = ":";
+
+    var minutes = microseconds ~/ microsecondsPerMinute;
+    microseconds = microseconds.remainder(microsecondsPerMinute);
+
+    if (minutes < 10) buffer[2] = "0";
+    buffer
+      ..[3] = minutes.toString()
+      ..[4] = ":";
+
+    var seconds = microseconds ~/ microsecondsPerSecond;
+    microseconds = microseconds.remainder(microsecondsPerSecond);
+
+    if (seconds < 10) buffer[5] = "0";
+    buffer
+      ..[6] = seconds.toString()
+      ..[7] = "."
+      ..[8] = microseconds.toString().padLeft(6, "0");
+    return buffer.join("");
   }
 
   /// Whether this [Duration] is negative.
@@ -293,13 +302,13 @@ class Duration implements Comparable<Duration> {
   /// [Duration].
   ///
   /// The returned [Duration] has the same length as this one, but is always
-  /// positive.
+  /// positive where possible.
   Duration abs() => Duration._microseconds(_duration.abs());
 
   /// Creates a new [Duration] with the opposite direction of this [Duration].
   ///
   /// The returned [Duration] has the same length as this one, but will have the
-  /// opposite sign (as reported by [isNegative]) as this one.
+  /// opposite sign (as reported by [isNegative]) as this one where possible.
   // Using subtraction helps dart2js avoid negative zeros.
   Duration operator -() => Duration._microseconds(0 - _duration);
 }

tests/corelib/duration_test.dart

@@ -281,6 +281,14 @@ main() {
   d = const Duration(microseconds: 1000000);
   Expect.equals("0:00:01.000000", d.toString());
 
+  // Regression test: Infinite loop prior to fix.
+  d = const Duration(microseconds: -0x8000000000000000);
+  Expect.equals("-2562047788:00:54.775808", d.toString());
+
+  d = const Duration(
+      hours: -2562047788, minutes: 0, seconds: -54, microseconds: -775808);
+  Expect.equals(-0x8000000000000000, d.inMicroseconds);
+
   d1 = const Duration(hours: 1);
   d2 = const Duration(hours: -1);
   Expect.isFalse(d1.isNegative);

tests/corelib_2/duration_test.dart

@@ -283,6 +283,14 @@ main() {
   d = const Duration(microseconds: 1000000);
   Expect.equals("0:00:01.000000", d.toString());
 
+  // Regression test: Infinite loop prior to fix.
+  d = const Duration(microseconds: -0x8000000000000000);
+  Expect.equals("-2562047788:00:54.775808", d.toString());
+
+  d = const Duration(
+      hours: -2562047788, minutes: 0, seconds: -54, microseconds: -775808);
+  Expect.equals(-0x8000000000000000, d.inMicroseconds);
+
   d1 = const Duration(hours: 1);
   d2 = const Duration(hours: -1);
   Expect.isFalse(d1.isNegative);