Fix toString/.value saturation for Duration and DataSize values too large for a long (#1935)

This commit is contained in:
Aashish Mahato authored and GitHub committed 2026-10-29 08:23:50 -07:00
1 parent ea68f7b258
commit e0d727be60
10 files changed
+86 -8

No files matched your search

@@ -253,6 +253,8 @@ public final class DataSize implements Value {
@Override
public String toString() {
return MathUtils.isMathematicalInteger(value) ? (long) value + "." + unit : value + "." + unit;
return MathUtils.isMathematicalIntegerFittingLong(value)
? (long) value + "." + unit
: value + "." + unit;
}
}
@@ -157,7 +157,9 @@ public final class VmDataSize extends VmValue implements Comparable<VmDataSize>
@Override
public String toPklString() {
return MathUtils.isMathematicalInteger(value) ? (long) value + "." + unit : value + "." + unit;
return MathUtils.isMathematicalIntegerFittingLong(value)
? (long) value + "." + unit
: value + "." + unit;
}
private double convertValueTo(DataSizeUnit other) {
@@ -29,12 +29,12 @@ public final class DataSizeNodes {
@ImportStatic(MathUtils.class)
@PklName("value")
public abstract static class valueProperty extends ExternalPropertyNode {
@Specialization(guards = "isMathematicalInteger(self.getValue())")
@Specialization(guards = "isMathematicalIntegerFittingLong(self.getValue())")
protected long evalInt(VmDataSize self) {
return (long) self.getValue();
}
@Specialization(guards = "!isMathematicalInteger(self.getValue())")
@Specialization(guards = "!isMathematicalIntegerFittingLong(self.getValue())")
protected double evalFloat(VmDataSize self) {
return self.getValue();
}
@@ -31,12 +31,12 @@ public final class DurationNodes {
@ImportStatic(MathUtils.class)
@PklName("value")
public abstract static class valueProperty extends ExternalPropertyNode {
@Specialization(guards = "isMathematicalInteger(self.getValue())")
@Specialization(guards = "isMathematicalIntegerFittingLong(self.getValue())")
protected long evalInt(VmDuration self) {
return (long) self.getValue();
}
@Specialization(guards = "!isMathematicalInteger(self.getValue())")
@Specialization(guards = "!isMathematicalIntegerFittingLong(self.getValue())")
protected double evalFloat(VmDuration self) {
return self.getValue();
}
@@ -1,5 +1,5 @@
/*
* Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved.
* Copyright © 2024-2026 Apple Inc. and the Pkl project authors. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -21,7 +21,9 @@ public final class DurationUtils {
private DurationUtils() {}
public static String toPklString(double value, DurationUnit unit) {
return MathUtils.isMathematicalInteger(value) ? (long) value + "." + unit : value + "." + unit;
return MathUtils.isMathematicalIntegerFittingLong(value)
? (long) value + "." + unit
: value + "." + unit;
}
// see: https://standards.calconnect.org/csd/cc-18011.html#toc32
@@ -55,6 +55,15 @@ public final class MathUtils {
<= StrictMath.getExponent(x));
}
/**
* Returns {@code true} if {@code x} is a mathematical integer that can be represented exactly as
* a {@code long}.
*/
@TruffleBoundary
public static boolean isMathematicalIntegerFittingLong(double x) {
return isMathematicalInteger(x) && MIN_LONG_AS_DOUBLE <= x && x < MAX_LONG_AS_DOUBLE_PLUS_ONE;
}
/**
* Returns {@code true} if {@code x} represents a power of two.
*
@@ -0,0 +1,19 @@
amends "../snippetTest.pkl"
facts {
["value"] {
1e300.d.value == 1e300
1e300.b.value == 1e300
1.0E18.d.value == 1000000000000000000
7.d.value == 7
2.2.h.value == 2.2
}
}
examples {
["display"] {
1e300.d
1e300.b
7.d
}
}
@@ -0,0 +1,16 @@
facts {
["value"] {
true
true
true
true
true
}
}
examples {
["display"] {
1.0E300.d
1.0E300.b
7.d
}
}
@@ -127,4 +127,18 @@ class DataSizeTest {
assertThat(DataSize(0.0, BYTES).hashCode()).isEqualTo(DataSize(-0.0, BYTES).hashCode())
assertThat(DataSize(0.0, BYTES).hashCode()).isEqualTo(DataSize(-0.0, PEBIBYTES).hashCode())
}
@Test
fun `toString()`() {
assertThat(DataSize(3.0, BYTES).toString()).isEqualTo("3.b")
assertThat(DataSize(1.5, BYTES).toString()).isEqualTo("1.5.b")
assertThat(DataSize(1e18, BYTES).toString()).isEqualTo("1000000000000000000.b")
// -2^63 still fits into a `long`
assertThat(DataSize(-9.223372036854776E18, BYTES).toString())
.isEqualTo("-9223372036854775808.b")
// values too large to fit into a `long` are printed in their `double` form,
// instead of saturating to 9223372036854775807
assertThat(DataSize(9.223372036854776E18, BYTES).toString()).isEqualTo("9.223372036854776E18.b")
assertThat(DataSize(1e300, PETABYTES).toString()).isEqualTo("1.0E300.pb")
}
}
@@ -291,4 +291,18 @@ class DurationTest {
assertThat(Duration(0.0, MILLIS).hashCode()).isEqualTo(Duration(-0.0, MILLIS).hashCode())
assertThat(Duration(0.0, NANOS).hashCode()).isEqualTo(Duration(-0.0, DAYS).hashCode())
}
@Test
fun `toString()`() {
assertThat(Duration(3.0, DAYS).toString()).isEqualTo("3.d")
assertThat(Duration(1.5, DAYS).toString()).isEqualTo("1.5.d")
assertThat(Duration(1e18, DAYS).toString()).isEqualTo("1000000000000000000.d")
// -2^63 still fits into a `long`
assertThat(Duration(-9.223372036854776E18, DAYS).toString()).isEqualTo("-9223372036854775808.d")
// values too large to fit into a `long` are printed in their `double` form,
// instead of saturating to 9223372036854775807
assertThat(Duration(9.223372036854776E18, DAYS).toString()).isEqualTo("9.223372036854776E18.d")
assertThat(Duration(1e300, DAYS).toString()).isEqualTo("1.0E300.d")
assertThat(Duration(-1e300, DAYS).toString()).isEqualTo("-1.0E300.d")
}
}