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>
This commit is contained in:
Lasse R.H. Nielsen 2023-03-30 14:02:38 +00:00 committed by Commit Queue
parent ea3000f1d4
commit d274ef0a4a
4 changed files with 59 additions and 14 deletions

View file

@ -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 @@ class DateTime {
@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) =>

View file

@ -161,8 +161,9 @@ class Duration implements Comparable<Duration> {
/// 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 @@ class Duration implements Comparable<Duration> {
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 @@ class Duration implements Comparable<Duration> {
/// 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 @@ class Duration implements Comparable<Duration> {
/// ```
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 @@ class Duration implements Comparable<Duration> {
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.

View file

@ -315,4 +315,19 @@ main() {
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());
}

View file

@ -317,4 +317,19 @@ main() {
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());
}