Fix bug in `Duration.toString`.
On the web, some inputs to `Duration` would make -0.0
occur in positions where the code assumed an integer
with a reasonable `toString`.
Adds `+ 0` and using `0 - x` instead of `-x` to
avoid ever ending up with a `-0.0`.
Fixes #51584
CoreLibraryReviewExempt: Bugfix, no API changes.
Change-Id: Idecfaf049f2ae5677792387ce24e95add99596cb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/291280
Commit-Queue: Nate Bosch <nbosch@google.com>
Auto-Submit: Lasse Nielsen <lrn@google.com>
Reviewed-by: Nate Bosch <nbosch@google.com>
diff --git a/sdk/lib/_internal/vm_shared/lib/date_patch.dart b/sdk/lib/_internal/vm_shared/lib/date_patch.dart
index 4aaf62a..8daa59b 100644
--- a/sdk/lib/_internal/vm_shared/lib/date_patch.dart
+++ b/sdk/lib/_internal/vm_shared/lib/date_patch.dart
@@ -2,7 +2,7 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
-import "dart:_internal" show patch, unsafeCast;
+import "dart:_internal" show checkNotNullable, patch, unsafeCast;
// VM implementation of DateTime.
@patch
@@ -47,12 +47,11 @@
@patch
DateTime._internal(int year, int month, int day, int hour, int minute,
int second, int millisecond, int microsecond, bool isUtc)
- : this.isUtc = isUtc,
+ : this.isUtc = checkNotNullable(isUtc, "isUtc"),
this._value = _brokenDownDateToValue(year, month, day, hour, minute,
second, millisecond, microsecond, isUtc) ??
-1 {
if (_value == -1) throw new ArgumentError();
- if (isUtc == null) throw new ArgumentError();
}
static int _validateMilliseconds(int millisecondsSinceEpoch) =>
diff --git a/sdk/lib/core/duration.dart b/sdk/lib/core/duration.dart
index bd73e56..3f1542b 100644
--- a/sdk/lib/core/duration.dart
+++ b/sdk/lib/core/duration.dart
@@ -161,8 +161,9 @@
/// 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.
+ /// as an integer value, the number of microseconds might overflow
+ /// and be truncated to a smaller number of bits,
+ /// or it might lose precision.
///
/// All arguments are 0 by default.
/// ```dart
@@ -184,9 +185,10 @@
microsecondsPerHour * hours +
microsecondsPerDay * days);
- // Fast path internal direct constructor to avoids the optional arguments and
- // [_microseconds] recomputation.
- const Duration._microseconds(this._duration);
+ // Fast path internal direct constructor to avoids the optional arguments
+ // and [_microseconds] recomputation.
+ // The `+ 0` prevents -0.0 on the web, if the incoming duration happens to be -0.0.
+ const Duration._microseconds(int duration) : _duration = duration + 0;
/// Adds this Duration and [other] and
/// returns the sum as a new Duration object.
@@ -212,7 +214,7 @@
/// Divides this Duration by the given [quotient] and returns the truncated
/// result as a new Duration object.
///
- /// Throws an [IntegerDivisionByZeroException] if [quotient] is `0`.
+ /// The [quotient] must not be `0`.
Duration operator ~/(int quotient) {
// By doing the check here instead of relying on "~/" below we get the
// exception even with dart2js.
@@ -331,12 +333,19 @@
/// ```
String toString() {
var microseconds = inMicroseconds;
- var sign = (microseconds < 0) ? "-" : "";
+ var sign = "";
+ var negative = microseconds < 0;
var hours = microseconds ~/ microsecondsPerHour;
microseconds = microseconds.remainder(microsecondsPerHour);
- if (microseconds < 0) microseconds = -microseconds;
+ // Correcting for being negative after first division, instead of before,
+ // to avoid negating min-int, -(2^31-1), of a native int64.
+ if (negative) {
+ hours = 0 - hours; // Not using `-hours` to avoid creating -0.0 on web.
+ microseconds = 0 - microseconds;
+ sign = "-";
+ }
var minutes = microseconds ~/ microsecondsPerMinute;
microseconds = microseconds.remainder(microsecondsPerMinute);
@@ -348,10 +357,17 @@
var secondsPadding = seconds < 10 ? "0" : "";
- var paddedMicroseconds = microseconds.toString().padLeft(6, "0");
- return "$sign${hours.abs()}:"
+ var microsecondsText = microseconds.toString();
+
+ // Padding up to six digits for microseconds.
+ const zeroPadding = ["00000", "0000", "000", "00", "0", ""];
+ assert(microsecondsText.length >= 1 && microsecondsText.length <= 6);
+ var microsecondsPadding = zeroPadding[microsecondsText.length - 1];
+
+ return "$sign$hours:"
"$minutesPadding$minutes:"
- "$secondsPadding$seconds.$paddedMicroseconds";
+ "$secondsPadding$seconds."
+ "$microsecondsPadding$microsecondsText";
}
/// Whether this [Duration] is negative.
diff --git a/tests/corelib/duration_test.dart b/tests/corelib/duration_test.dart
index e60efb8..3d7327b 100644
--- a/tests/corelib/duration_test.dart
+++ b/tests/corelib/duration_test.dart
@@ -315,4 +315,19 @@
Expect.isFalse(d.toString().startsWith("--"));
d = const Duration(minutes: -0); // is -0.0 on web.
Expect.equals("0:00:00.000000", d.toString());
+
+ // Regression test for https://github.com/dart-lang/sdk/issues/51584
+ // (Didn't fix it all in 48841.)
+ d = const Duration(hours: -1);
+ Expect.equals("-1:00:00.000000", d.toString());
+
+ // Adding -0.0's gives -0.0 again.
+ d = const Duration(
+ days: -0,
+ hours: -0,
+ minutes: -0,
+ seconds: -0,
+ milliseconds: -0,
+ microseconds: -0);
+ Expect.equals("0:00:00.000000", d.toString());
}
diff --git a/tests/corelib_2/duration_test.dart b/tests/corelib_2/duration_test.dart
index 33ebf1f..9cab8e6 100644
--- a/tests/corelib_2/duration_test.dart
+++ b/tests/corelib_2/duration_test.dart
@@ -317,4 +317,19 @@
Expect.isFalse(d.toString().startsWith("--"));
d = const Duration(minutes: -0); // is -0.0 on web.
Expect.equals("0:00:00.000000", d.toString());
+
+ // Regression test for https://github.com/dart-lang/sdk/issues/51584
+ // (Didn't fix it all in 48841.)
+ d = const Duration(hours: -1);
+ Expect.equals("-1:00:00.000000", d.toString());
+
+ // Adding -0.0's gives -0.0 again.
+ d = const Duration(
+ days: -0,
+ hours: -0,
+ minutes: -0,
+ seconds: -0,
+ milliseconds: -0,
+ microseconds: -0);
+ Expect.equals("0:00:00.000000", d.toString());
}
