Title: [242360] trunk
Revision
242360
Author
da...@apple.com
Date
2019-03-04 08:58:33 -0800 (Mon, 04 Mar 2019)

Log Message

Prepare to improve handling of conversion of float to strings
https://bugs.webkit.org/show_bug.cgi?id=195262

Reviewed by Daniel Bates.

Source/WTF:

* wtf/dtoa.cpp:
(WTF::truncateTrailingZeros): Renamed from
formatStringTruncatingTrailingZerosIfNeeded and removed the calls
to double_conversion::StringBuilder::Finalizer, since the caller
already does that.
(WTF::numberToFixedPrecisionString): Added an overload for float
and updated to use the new truncateTrailingZeros.
(WTF::numberToFixedWidthString): Added an overload for float.

* wtf/text/AtomicString.cpp:
(WTF::AtomicString::number): Added float overload. This is a
behavior change, but in all cases for the better. The old behavior
was to convert to double first and then do "shortest form"
conversion, and it's always better to just do that as float.
* wtf/text/AtomicString.h: Added float overload of AtomicString::number.

* wtf/text/StringBuilder.cpp:
(WTF::StringBuilder::appendFixedPrecisionNumber): Added float
overload.
(WTF::StringBuilder::appendShortestFormNumber): Renamed from
appendECMAScriptNumber and did the above.
(WTF::StringBuilder::appendFixedWidthNumber): Ditto.
* wtf/text/StringBuilder.h: Added overloads for float and
appendShortestFormNumber. The appendNumber and appendECMAScriptNumber
functions are now inlines in the header, since they are expressed
entirely in terms of the other functions.

* wtf/text/WTFString.cpp:
(WTF::String::numberToStringFixedPrecision): Added float overload.
Removed unnecessary explicit conversion to String.
(WTF::String::numberToStringShortest): Renamed from
numberToStringECMAScript and did the above.
(WTF::String::numberToStringFixedWidth): Ditto.

* wtf/text/WTFString.h: Added overloads for float and
numberToStringShortest. The number and numberToStringECMAScript
functions are now inlines in the header, since they are expressed
entirely in terms of the other functions.

LayoutTests:

* svg/dom/SVGAngle-expected.txt:
* svg/dom/SVGAngle.html:
* svg/dom/SVGLength-px-expected.txt:
* svg/dom/SVGLength-px-with-context-expected.txt:
* svg/dom/SVGLength-px-with-context.html:
* svg/dom/SVGLength-px.html:
Remove checks that depend on the precision and number of serialization.
The current tests depend on behavior that we'd like to change in the future and
some of them had already been updated since the behavior is different in different
web browsers. Tricky issue because of the mix of single and double precision in
the SVG engine and the _javascript_ language, and straightforward to keep the tests
useful without this unnecessary dependency. Generally we check the units of the
result of valueAsString, and not the numeric result. Another idea would be to
check the numeric part of the result by converting it back to a number and
comparing it as a number rather than as a string.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (242359 => 242360)


--- trunk/LayoutTests/ChangeLog	2019-03-04 16:57:38 UTC (rev 242359)
+++ trunk/LayoutTests/ChangeLog	2019-03-04 16:58:33 UTC (rev 242360)
@@ -1,3 +1,26 @@
+2019-03-03  Darin Adler  <da...@apple.com>
+
+        Prepare to improve handling of conversion of float to strings
+        https://bugs.webkit.org/show_bug.cgi?id=195262
+
+        Reviewed by Daniel Bates.
+
+        * svg/dom/SVGAngle-expected.txt:
+        * svg/dom/SVGAngle.html:
+        * svg/dom/SVGLength-px-expected.txt:
+        * svg/dom/SVGLength-px-with-context-expected.txt:
+        * svg/dom/SVGLength-px-with-context.html:
+        * svg/dom/SVGLength-px.html:
+        Remove checks that depend on the precision and number of serialization.
+        The current tests depend on behavior that we'd like to change in the future and
+        some of them had already been updated since the behavior is different in different
+        web browsers. Tricky issue because of the mix of single and double precision in
+        the SVG engine and the _javascript_ language, and straightforward to keep the tests
+        useful without this unnecessary dependency. Generally we check the units of the
+        result of valueAsString, and not the numeric result. Another idea would be to
+        check the numeric part of the result by converting it back to a number and
+        comparing it as a number rather than as a string.
+
 2019-03-04  Truitt Savell  <tsav...@apple.com>
 
         webkit.org/b/195210 resolve by r242308, unskipping tests.

Modified: trunk/LayoutTests/svg/dom/SVGAngle-expected.txt (242359 => 242360)


--- trunk/LayoutTests/svg/dom/SVGAngle-expected.txt	2019-03-04 16:57:38 UTC (rev 242359)
+++ trunk/LayoutTests/svg/dom/SVGAngle-expected.txt	2019-03-04 16:58:33 UTC (rev 242360)
@@ -66,7 +66,7 @@
 
 Check valid arguments for 'newValueSpecifiedUnits', that should only modify the 'valueAsString'
 PASS angle.newValueSpecifiedUnits(SVGAngle.SVG_ANGLETYPE_RAD, parseFloat(Math.PI.toFixed(5))) is undefined.
-PASS angle.valueAsString is "3.14159rad"
+PASS angle.valueAsString.slice(-3) is "rad"
 PASS angle.value.toFixed(1) is "180.0"
 PASS angle.valueInSpecifiedUnits.toFixed(5) is Math.PI.toFixed(5)
 PASS angle.unitType is SVGAngle.SVG_ANGLETYPE_RAD
@@ -170,7 +170,7 @@
 PASS angle.convertToSpecifiedUnits(SVGAngle.SVG_ANGLETYPE_RAD) is undefined.
 PASS angle.value.toFixed(1) is "180.0"
 PASS angle.valueInSpecifiedUnits.toFixed(5) is "3.14159"
-PASS angle.valueAsString is "3.14159rad"
+PASS angle.valueAsString.slice(-3) is "rad"
 PASS angle.unitType is SVGAngle.SVG_ANGLETYPE_RAD
 
 Now try converting the RAD value into an unknown value, that should fail and throw
@@ -177,7 +177,7 @@
 PASS angle.convertToSpecifiedUnits(SVGAngle.SVG_ANGLETYPE_UNKNOWN) threw exception NotSupportedError: The operation is not supported..
 PASS angle.value.toFixed(1) is "180.0"
 PASS angle.valueInSpecifiedUnits.toFixed(5) is "3.14159"
-PASS angle.valueAsString is "3.14159rad"
+PASS angle.valueAsString.slice(-3) is "rad"
 PASS angle.unitType is SVGAngle.SVG_ANGLETYPE_RAD
 
 Now convert the RAD value into a DEG value, and assure that all properties have been synchronized

Modified: trunk/LayoutTests/svg/dom/SVGAngle.html (242359 => 242360)


--- trunk/LayoutTests/svg/dom/SVGAngle.html	2019-03-04 16:57:38 UTC (rev 242359)
+++ trunk/LayoutTests/svg/dom/SVGAngle.html	2019-03-04 16:58:33 UTC (rev 242360)
@@ -89,7 +89,8 @@
 debug("");
 debug("Check valid arguments for 'newValueSpecifiedUnits', that should only modify the 'valueAsString'");
 shouldBeUndefined("angle.newValueSpecifiedUnits(SVGAngle.SVG_ANGLETYPE_RAD, parseFloat(Math.PI.toFixed(5)))");
-shouldBeEqualToString("angle.valueAsString", Math.PI.toFixed(5) + "rad");
+// Exact result of valueAsString depends on precision, no trivial way to correctly test it, especially across engines.
+shouldBeEqualToString("angle.valueAsString.slice(-3)", "rad");
 shouldBeEqualToString("angle.value.toFixed(1)", "180.0");
 shouldBe("angle.valueInSpecifiedUnits.toFixed(5)", "Math.PI.toFixed(5)");
 shouldBe("angle.unitType", "SVGAngle.SVG_ANGLETYPE_RAD");
@@ -213,7 +214,8 @@
 shouldBeUndefined("angle.convertToSpecifiedUnits(SVGAngle.SVG_ANGLETYPE_RAD)");
 shouldBeEqualToString("angle.value.toFixed(1)", "180.0");
 shouldBeEqualToString("angle.valueInSpecifiedUnits.toFixed(5)", Math.PI.toFixed(5));
-shouldBeEqualToString("angle.valueAsString", Math.PI.toFixed(5) + "rad");
+// Exact result of valueAsString depends on precision, no trivial way to correctly test it, especially across engines.
+shouldBeEqualToString("angle.valueAsString.slice(-3)", "rad");
 shouldBe("angle.unitType", "SVGAngle.SVG_ANGLETYPE_RAD");
 
 debug("");
@@ -221,7 +223,8 @@
 shouldThrow("angle.convertToSpecifiedUnits(SVGAngle.SVG_ANGLETYPE_UNKNOWN)");
 shouldBeEqualToString("angle.value.toFixed(1)", "180.0");
 shouldBeEqualToString("angle.valueInSpecifiedUnits.toFixed(5)", Math.PI.toFixed(5));
-shouldBeEqualToString("angle.valueAsString", Math.PI.toFixed(5) + "rad");
+// Exact result of valueAsString depends on precision, no trivial way to correctly test it, especially across engines.
+shouldBeEqualToString("angle.valueAsString.slice(-3)", "rad");
 shouldBe("angle.unitType", "SVGAngle.SVG_ANGLETYPE_RAD");
 
 debug("");

Modified: trunk/LayoutTests/svg/dom/SVGLength-px-expected.txt (242359 => 242360)


--- trunk/LayoutTests/svg/dom/SVGLength-px-expected.txt	2019-03-04 16:57:38 UTC (rev 242359)
+++ trunk/LayoutTests/svg/dom/SVGLength-px-expected.txt	2019-03-04 16:58:33 UTC (rev 242360)
@@ -48,7 +48,7 @@
 
 Convert from px to cm
 PASS length.convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_CM) is undefined.
-PASS length.valueAsString is "0.0529167cm"
+PASS length.valueAsString.slice(-2) is "cm"
 PASS length.valueInSpecifiedUnits.toFixed(7) is "0.0529167"
 PASS length.value.toFixed(1) is "2.0"
 PASS length.unitType is SVGLength.SVG_LENGTHTYPE_CM
@@ -57,7 +57,7 @@
 
 Convert from px to mm
 PASS length.convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_MM) is undefined.
-PASS length.valueAsString is "0.529167mm"
+PASS length.valueAsString.slice(-2) is "mm"
 PASS length.valueInSpecifiedUnits.toFixed(6) is "0.529167"
 PASS length.value.toFixed(1) is "2.0"
 PASS length.unitType is SVGLength.SVG_LENGTHTYPE_MM
@@ -66,7 +66,7 @@
 
 Convert from px to in
 PASS length.convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_IN) is undefined.
-PASS length.valueAsString is "0.0208333in"
+PASS length.valueAsString.slice(-2) is "in"
 PASS length.valueInSpecifiedUnits.toFixed(7) is "0.0208333"
 PASS length.value.toFixed(1) is "2.0"
 PASS length.unitType is SVGLength.SVG_LENGTHTYPE_IN

Modified: trunk/LayoutTests/svg/dom/SVGLength-px-with-context-expected.txt (242359 => 242360)


--- trunk/LayoutTests/svg/dom/SVGLength-px-with-context-expected.txt	2019-03-04 16:57:38 UTC (rev 242359)
+++ trunk/LayoutTests/svg/dom/SVGLength-px-with-context-expected.txt	2019-03-04 16:58:33 UTC (rev 242360)
@@ -21,7 +21,7 @@
 
 Convert from px to percentage
 PASS length.convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_PERCENTAGE) is undefined.
-PASS length.valueAsString is "1.33333%"
+PASS length.valueAsString.slice(-1) is "%"
 PASS length.valueInSpecifiedUnits.toFixed(5) is "1.33333"
 PASS length.value.toFixed(1) is "2.0"
 PASS length.unitType is SVGLength.SVG_LENGTHTYPE_PERCENTAGE
@@ -30,7 +30,7 @@
 
 Convert from px to ems
 PASS length.convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_EMS) is undefined.
-PASS length.valueAsString is "0.166667em"
+PASS length.valueAsString.slice(-2) is "em"
 PASS length.valueInSpecifiedUnits.toFixed(6) is "0.166667"
 PASS length.value.toFixed(1) is "2.0"
 PASS length.unitType is SVGLength.SVG_LENGTHTYPE_EMS
@@ -39,6 +39,7 @@
 
 Convert from px to exs
 PASS length.convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_EXS) is undefined.
+PASS length.valueAsString.slice(-2) is "ex"
 PASS length.valueInSpecifiedUnits.toFixed(1) is "0.2"
 PASS length.value.toFixed(1) is "2.0"
 PASS length.unitType is SVGLength.SVG_LENGTHTYPE_EXS
@@ -47,7 +48,7 @@
 
 Convert from px to cm
 PASS length.convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_CM) is undefined.
-PASS length.valueAsString is "0.0529167cm"
+PASS length.valueAsString.slice(-2) is "cm"
 PASS length.valueInSpecifiedUnits.toFixed(7) is "0.0529167"
 PASS length.value.toFixed(1) is "2.0"
 PASS length.unitType is SVGLength.SVG_LENGTHTYPE_CM
@@ -56,7 +57,7 @@
 
 Convert from px to mm
 PASS length.convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_MM) is undefined.
-PASS length.valueAsString is "0.529167mm"
+PASS length.valueAsString.slice(-2) is "mm"
 PASS length.valueInSpecifiedUnits.toFixed(6) is "0.529167"
 PASS length.value.toFixed(1) is "2.0"
 PASS length.unitType is SVGLength.SVG_LENGTHTYPE_MM
@@ -65,7 +66,7 @@
 
 Convert from px to in
 PASS length.convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_IN) is undefined.
-PASS length.valueAsString is "0.0208333in"
+PASS length.valueAsString.slice(-2) is "in"
 PASS length.valueInSpecifiedUnits.toFixed(7) is "0.0208333"
 PASS length.value.toFixed(1) is "2.0"
 PASS length.unitType is SVGLength.SVG_LENGTHTYPE_IN
@@ -83,6 +84,7 @@
 
 Convert from px to pc
 PASS length.convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_PC) is undefined.
+PASS length.valueAsString.slice(-2) is "pc"
 PASS length.valueInSpecifiedUnits.toFixed(3) is "0.125"
 PASS length.value.toFixed(1) is "2.0"
 PASS length.unitType is SVGLength.SVG_LENGTHTYPE_PC

Modified: trunk/LayoutTests/svg/dom/SVGLength-px-with-context.html (242359 => 242360)


--- trunk/LayoutTests/svg/dom/SVGLength-px-with-context.html	2019-03-04 16:57:38 UTC (rev 242359)
+++ trunk/LayoutTests/svg/dom/SVGLength-px-with-context.html	2019-03-04 16:58:33 UTC (rev 242360)
@@ -80,7 +80,8 @@
 debug("Convert from px to percentage");
 shouldBeUndefined("length.convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_PERCENTAGE)");
 referenceValue = Number(2 / svgWidth * 100).toFixed(5);
-shouldBeEqualToString("length.valueAsString", referenceValue + "%");
+// Exact result of valueAsString depends on precision, no trivial way to correctly test it, especially across engines.
+shouldBeEqualToString("length.valueAsString.slice(-1)", "%");
 shouldBeEqualToString("length.valueInSpecifiedUnits.toFixed(5)", referenceValue);
 shouldBeEqualToString("length.value.toFixed(1)", "2.0");
 shouldBe("length.unitType", "SVGLength.SVG_LENGTHTYPE_PERCENTAGE");
@@ -93,7 +94,8 @@
 debug("Convert from px to ems");
 shouldBeUndefined("length.convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_EMS)");
 referenceValue = Number(2 / fontSize).toFixed(6);
-shouldBeEqualToString("length.valueAsString", referenceValue + "em");
+// Exact result of valueAsString depends on precision, no trivial way to correctly test it, especially across engines.
+shouldBeEqualToString("length.valueAsString.slice(-2)", "em");
 shouldBeEqualToString("length.valueInSpecifiedUnits.toFixed(6)", referenceValue);
 shouldBeEqualToString("length.value.toFixed(1)", "2.0");
 shouldBe("length.unitType", "SVGLength.SVG_LENGTHTYPE_EMS");
@@ -106,7 +108,8 @@
 debug("Convert from px to exs");
 shouldBeUndefined("length.convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_EXS)");
 referenceValue = Number(2 / calculateXHeight()).toFixed(1);
-// Don't check valueAsString here, it's unreliable across browsers.
+// Exact result of valueAsString depends on precision, no trivial way to correctly test it, especially across engines.
+shouldBeEqualToString("length.valueAsString.slice(-2)", "ex");
 shouldBeEqualToString("length.valueInSpecifiedUnits.toFixed(1)", referenceValue);
 shouldBeEqualToString("length.value.toFixed(1)", "2.0");
 shouldBe("length.unitType", "SVGLength.SVG_LENGTHTYPE_EXS");
@@ -119,7 +122,8 @@
 debug("Convert from px to cm");
 shouldBeUndefined("length.convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_CM)");
 referenceValue = Number(2 * 2.54 / cssPixelsPerInch).toFixed(7);
-shouldBeEqualToString("length.valueAsString", referenceValue + "cm");
+// Exact result of valueAsString depends on precision, no trivial way to correctly test it, especially across engines.
+shouldBeEqualToString("length.valueAsString.slice(-2)", "cm");
 shouldBeEqualToString("length.valueInSpecifiedUnits.toFixed(7)", referenceValue);
 shouldBeEqualToString("length.value.toFixed(1)", "2.0");
 shouldBe("length.unitType", "SVGLength.SVG_LENGTHTYPE_CM");
@@ -132,7 +136,8 @@
 debug("Convert from px to mm");
 shouldBeUndefined("length.convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_MM)");
 referenceValue = Number(2 * 25.4 / cssPixelsPerInch).toFixed(6);
-shouldBeEqualToString("length.valueAsString", referenceValue + "mm");
+// Exact result of valueAsString depends on precision, no trivial way to correctly test it, especially across engines.
+shouldBeEqualToString("length.valueAsString.slice(-2)", "mm");
 shouldBeEqualToString("length.valueInSpecifiedUnits.toFixed(6)", referenceValue);
 shouldBeEqualToString("length.value.toFixed(1)", "2.0");
 shouldBe("length.unitType", "SVGLength.SVG_LENGTHTYPE_MM");
@@ -145,7 +150,8 @@
 debug("Convert from px to in");
 shouldBeUndefined("length.convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_IN)");
 referenceValue = Number(2 / cssPixelsPerInch).toFixed(7);
-shouldBeEqualToString("length.valueAsString", referenceValue + "in");
+// Exact result of valueAsString depends on precision, no trivial way to correctly test it, especially across engines.
+shouldBeEqualToString("length.valueAsString.slice(-2)", "in");
 shouldBeEqualToString("length.valueInSpecifiedUnits.toFixed(7)", referenceValue);
 shouldBeEqualToString("length.value.toFixed(1)", "2.0");
 shouldBe("length.unitType", "SVGLength.SVG_LENGTHTYPE_IN");
@@ -171,7 +177,8 @@
 debug("Convert from px to pc");
 shouldBeUndefined("length.convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_PC)");
 referenceValue = Number(2 / cssPixelsPerInch * 6).toFixed(3);
-// Don't check valueAsString here, it's unreliable across browsers.
+// Exact result of valueAsString depends on precision, no trivial way to correctly test it, especially across engines.
+shouldBeEqualToString("length.valueAsString.slice(-2)", "pc");
 shouldBeEqualToString("length.valueInSpecifiedUnits.toFixed(3)", referenceValue);
 shouldBeEqualToString("length.value.toFixed(1)", "2.0");
 shouldBe("length.unitType", "SVGLength.SVG_LENGTHTYPE_PC");

Modified: trunk/LayoutTests/svg/dom/SVGLength-px.html (242359 => 242360)


--- trunk/LayoutTests/svg/dom/SVGLength-px.html	2019-03-04 16:57:38 UTC (rev 242359)
+++ trunk/LayoutTests/svg/dom/SVGLength-px.html	2019-03-04 16:58:33 UTC (rev 242360)
@@ -93,7 +93,8 @@
 debug("Convert from px to cm");
 shouldBeUndefined("length.convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_CM)");
 referenceValue = Number(2 * 2.54 / cssPixelsPerInch).toFixed(7);
-shouldBeEqualToString("length.valueAsString", referenceValue + "cm");
+// Exact result of valueAsString depends on precision, no trivial way to correctly test it, especially across engines.
+shouldBeEqualToString("length.valueAsString.slice(-2)", "cm");
 shouldBeEqualToString("length.valueInSpecifiedUnits.toFixed(7)", referenceValue);
 shouldBeEqualToString("length.value.toFixed(1)", "2.0");
 shouldBe("length.unitType", "SVGLength.SVG_LENGTHTYPE_CM");
@@ -106,7 +107,8 @@
 debug("Convert from px to mm");
 shouldBeUndefined("length.convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_MM)");
 referenceValue = Number(2 * 25.4 / cssPixelsPerInch).toFixed(6);
-shouldBeEqualToString("length.valueAsString", referenceValue + "mm");
+// Exact result of valueAsString depends on precision, no trivial way to correctly test it, especially across engines.
+shouldBeEqualToString("length.valueAsString.slice(-2)", "mm");
 shouldBeEqualToString("length.valueInSpecifiedUnits.toFixed(6)", referenceValue);
 shouldBeEqualToString("length.value.toFixed(1)", "2.0");
 shouldBe("length.unitType", "SVGLength.SVG_LENGTHTYPE_MM");
@@ -119,7 +121,8 @@
 debug("Convert from px to in");
 shouldBeUndefined("length.convertToSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_IN)");
 referenceValue = Number(2 / cssPixelsPerInch).toFixed(7);
-shouldBeEqualToString("length.valueAsString", referenceValue + "in");
+// Exact result of valueAsString depends on precision, no trivial way to correctly test it, especially across engines.
+shouldBeEqualToString("length.valueAsString.slice(-2)", "in");
 shouldBeEqualToString("length.valueInSpecifiedUnits.toFixed(7)", referenceValue);
 shouldBeEqualToString("length.value.toFixed(1)", "2.0");
 shouldBe("length.unitType", "SVGLength.SVG_LENGTHTYPE_IN");

Modified: trunk/Source/WTF/ChangeLog (242359 => 242360)


--- trunk/Source/WTF/ChangeLog	2019-03-04 16:57:38 UTC (rev 242359)
+++ trunk/Source/WTF/ChangeLog	2019-03-04 16:58:33 UTC (rev 242360)
@@ -1,3 +1,49 @@
+2019-03-03  Darin Adler  <da...@apple.com>
+
+        Prepare to improve handling of conversion of float to strings
+        https://bugs.webkit.org/show_bug.cgi?id=195262
+
+        Reviewed by Daniel Bates.
+
+        * wtf/dtoa.cpp:
+        (WTF::truncateTrailingZeros): Renamed from
+        formatStringTruncatingTrailingZerosIfNeeded and removed the calls
+        to double_conversion::StringBuilder::Finalizer, since the caller
+        already does that.
+        (WTF::numberToFixedPrecisionString): Added an overload for float
+        and updated to use the new truncateTrailingZeros.
+        (WTF::numberToFixedWidthString): Added an overload for float.
+
+        * wtf/text/AtomicString.cpp:
+        (WTF::AtomicString::number): Added float overload. This is a
+        behavior change, but in all cases for the better. The old behavior
+        was to convert to double first and then do "shortest form"
+        conversion, and it's always better to just do that as float.
+        * wtf/text/AtomicString.h: Added float overload of AtomicString::number.
+
+        * wtf/text/StringBuilder.cpp:
+        (WTF::StringBuilder::appendFixedPrecisionNumber): Added float
+        overload.
+        (WTF::StringBuilder::appendShortestFormNumber): Renamed from
+        appendECMAScriptNumber and did the above.
+        (WTF::StringBuilder::appendFixedWidthNumber): Ditto.
+        * wtf/text/StringBuilder.h: Added overloads for float and
+        appendShortestFormNumber. The appendNumber and appendECMAScriptNumber
+        functions are now inlines in the header, since they are expressed
+        entirely in terms of the other functions.
+
+        * wtf/text/WTFString.cpp:
+        (WTF::String::numberToStringFixedPrecision): Added float overload.
+        Removed unnecessary explicit conversion to String.
+        (WTF::String::numberToStringShortest): Renamed from
+        numberToStringECMAScript and did the above.
+        (WTF::String::numberToStringFixedWidth): Ditto.
+
+        * wtf/text/WTFString.h: Added overloads for float and
+        numberToStringShortest. The number and numberToStringECMAScript
+        functions are now inlines in the header, since they are expressed
+        entirely in terms of the other functions.
+
 2019-03-04  Andy Estes  <aes...@apple.com>
 
         [Apple Pay] Move WebPaymentCoordinatorProxy from Source/WebKit/UIProcess to Source/WebKit/Shared

Modified: trunk/Source/WTF/wtf/dtoa.cpp (242359 => 242360)


--- trunk/Source/WTF/wtf/dtoa.cpp	2019-03-04 16:57:38 UTC (rev 242359)
+++ trunk/Source/WTF/wtf/dtoa.cpp	2019-03-04 16:58:33 UTC (rev 242360)
@@ -38,7 +38,7 @@
     return builder.Finalize();
 }
 
-static inline const char* formatStringTruncatingTrailingZerosIfNeeded(NumberToStringBuffer& buffer, double_conversion::StringBuilder& builder)
+static inline void truncateTrailingZeros(NumberToStringBuffer& buffer, double_conversion::StringBuilder& builder)
 {
     size_t length = builder.position();
     size_t decimalPointPosition = 0;
@@ -49,7 +49,7 @@
 
     // No decimal separator found, early exit.
     if (decimalPointPosition == length)
-        return builder.Finalize();
+        return;
 
     size_t pastMantissa = decimalPointPosition + 1;
     for (; pastMantissa < length; ++pastMantissa) {
@@ -65,7 +65,7 @@
 
     // No trailing zeros found to strip.
     if (truncatedLength == pastMantissa)
-        return builder.Finalize();
+        return;
 
     // If we removed all trailing zeros, remove the decimal point as well.
     if (truncatedLength == decimalPointPosition + 1)
@@ -73,12 +73,18 @@
 
     // Truncate the mantissa, and return the final result.
     builder.RemoveCharacters(truncatedLength, pastMantissa);
-    return builder.Finalize();
 }
 
-const char* numberToFixedPrecisionString(double d, unsigned significantFigures, NumberToStringBuffer& buffer, bool truncateTrailingZeros)
+const char* numberToFixedPrecisionString(float number, unsigned significantFigures, NumberToStringBuffer& buffer, bool shouldTruncateTrailingZeros)
 {
-    // Mimic sprintf("%.[precision]g", ...), but use dtoas rounding facilities.
+    // For now, just call the double precision version.
+    // Do that here instead of at callers to pave the way to add a more efficient code path later.
+    return numberToFixedPrecisionString(static_cast<double>(number), significantFigures, buffer, shouldTruncateTrailingZeros);
+}
+
+const char* numberToFixedPrecisionString(double d, unsigned significantFigures, NumberToStringBuffer& buffer, bool shouldTruncateTrailingZeros)
+{
+    // Mimic sprintf("%.[precision]g", ...).
     // "g": Signed value printed in f or e format, whichever is more compact for the given value and precision.
     // The e format is used only when the exponent of the value is less than –4 or greater than or equal to the
     // precision argument. Trailing zeros are truncated, and the decimal point appears only if one or more digits follow it.
@@ -86,14 +92,21 @@
     double_conversion::StringBuilder builder(&buffer[0], sizeof(buffer));
     auto& converter = double_conversion::DoubleToStringConverter::EcmaScriptConverter();
     converter.ToPrecision(d, significantFigures, &builder);
-    if (!truncateTrailingZeros)
-        return builder.Finalize();
-    return formatStringTruncatingTrailingZerosIfNeeded(buffer, builder);
+    if (shouldTruncateTrailingZeros)
+        truncateTrailingZeros(buffer, builder);
+    return builder.Finalize();
 }
 
+const char* numberToFixedWidthString(float number, unsigned decimalPlaces, NumberToStringBuffer& buffer)
+{
+    // For now, just call the double precision version.
+    // Do that here instead of at callers to pave the way to add a more efficient code path later.
+    return numberToFixedWidthString(static_cast<double>(number), decimalPlaces, buffer);
+}
+
 const char* numberToFixedWidthString(double d, unsigned decimalPlaces, NumberToStringBuffer& buffer)
 {
-    // Mimic sprintf("%.[precision]f", ...), but use dtoas rounding facilities.
+    // Mimic sprintf("%.[precision]f", ...).
     // "f": Signed value having the form [ – ]dddd.dddd, where dddd is one or more decimal digits.
     // The number of digits before the decimal point depends on the magnitude of the number, and
     // the number of digits after the decimal point depends on the requested precision.

Modified: trunk/Source/WTF/wtf/text/AtomicString.cpp (242359 => 242360)


--- trunk/Source/WTF/wtf/text/AtomicString.cpp	2019-03-04 16:57:38 UTC (rev 242359)
+++ trunk/Source/WTF/wtf/text/AtomicString.cpp	2019-03-04 16:58:33 UTC (rev 242360)
@@ -101,6 +101,12 @@
     return numberToStringUnsigned<AtomicString>(number);
 }
 
+AtomicString AtomicString::number(float number)
+{
+    NumberToStringBuffer buffer;
+    return numberToString(number, buffer);
+}
+
 AtomicString AtomicString::number(double number)
 {
     NumberToStringBuffer buffer;

Modified: trunk/Source/WTF/wtf/text/AtomicString.h (242359 => 242360)


--- trunk/Source/WTF/wtf/text/AtomicString.h	2019-03-04 16:57:38 UTC (rev 242359)
+++ trunk/Source/WTF/wtf/text/AtomicString.h	2019-03-04 16:58:33 UTC (rev 242360)
@@ -107,6 +107,7 @@
     WTF_EXPORT_PRIVATE static AtomicString number(unsigned);
     WTF_EXPORT_PRIVATE static AtomicString number(unsigned long);
     WTF_EXPORT_PRIVATE static AtomicString number(unsigned long long);
+    WTF_EXPORT_PRIVATE static AtomicString number(float);
     WTF_EXPORT_PRIVATE static AtomicString number(double);
     // If we need more overloads of the number function, we can add all the others that String has, but these seem to do for now.
 

Modified: trunk/Source/WTF/wtf/text/StringBuilder.cpp (242359 => 242360)


--- trunk/Source/WTF/wtf/text/StringBuilder.cpp	2019-03-04 16:57:38 UTC (rev 242359)
+++ trunk/Source/WTF/wtf/text/StringBuilder.cpp	2019-03-04 16:58:33 UTC (rev 242360)
@@ -378,7 +378,7 @@
     numberToStringSigned<StringBuilder>(number, this);
 }
 
-void StringBuilder::appendNumber(unsigned int number)
+void StringBuilder::appendNumber(unsigned number)
 {
     numberToStringUnsigned<StringBuilder>(number, this);
 }
@@ -403,18 +403,36 @@
     numberToStringUnsigned<StringBuilder>(number, this);
 }
 
-void StringBuilder::appendNumber(double number, unsigned precision, TrailingZerosTruncatingPolicy trailingZerosTruncatingPolicy)
+void StringBuilder::appendFixedPrecisionNumber(float number, unsigned precision, TrailingZerosTruncatingPolicy policy)
 {
     NumberToStringBuffer buffer;
-    append(numberToFixedPrecisionString(number, precision, buffer, trailingZerosTruncatingPolicy == TruncateTrailingZeros));
+    append(numberToFixedPrecisionString(number, precision, buffer, policy == TruncateTrailingZeros));
 }
 
-void StringBuilder::appendECMAScriptNumber(double number)
+void StringBuilder::appendFixedPrecisionNumber(double number, unsigned precision, TrailingZerosTruncatingPolicy policy)
 {
     NumberToStringBuffer buffer;
+    append(numberToFixedPrecisionString(number, precision, buffer, policy == TruncateTrailingZeros));
+}
+
+void StringBuilder::appendShortestFormNumber(float number)
+{
+    NumberToStringBuffer buffer;
     append(numberToString(number, buffer));
 }
 
+void StringBuilder::appendShortestFormNumber(double number)
+{
+    NumberToStringBuffer buffer;
+    append(numberToString(number, buffer));
+}
+
+void StringBuilder::appendFixedWidthNumber(float number, unsigned decimalPlaces)
+{
+    NumberToStringBuffer buffer;
+    append(numberToFixedWidthString(number, decimalPlaces, buffer));
+}
+
 void StringBuilder::appendFixedWidthNumber(double number, unsigned decimalPlaces)
 {
     NumberToStringBuffer buffer;

Modified: trunk/Source/WTF/wtf/text/StringBuilder.h (242359 => 242360)


--- trunk/Source/WTF/wtf/text/StringBuilder.h	2019-03-04 16:57:38 UTC (rev 242359)
+++ trunk/Source/WTF/wtf/text/StringBuilder.h	2019-03-04 16:58:33 UTC (rev 242360)
@@ -216,15 +216,26 @@
     ALWAYS_INLINE void appendLiteral(const char (&characters)[characterCount]) { append(characters, characterCount - 1); }
 
     WTF_EXPORT_PRIVATE void appendNumber(int);
-    WTF_EXPORT_PRIVATE void appendNumber(unsigned int);
+    WTF_EXPORT_PRIVATE void appendNumber(unsigned);
     WTF_EXPORT_PRIVATE void appendNumber(long);
     WTF_EXPORT_PRIVATE void appendNumber(unsigned long);
     WTF_EXPORT_PRIVATE void appendNumber(long long);
     WTF_EXPORT_PRIVATE void appendNumber(unsigned long long);
-    WTF_EXPORT_PRIVATE void appendNumber(double, unsigned precision = 6, TrailingZerosTruncatingPolicy = TruncateTrailingZeros);
-    WTF_EXPORT_PRIVATE void appendECMAScriptNumber(double);
+    // FIXME: Change appendNumber to be appendShortestFormNumber instead of appendFixedPrecisionNumber.
+    void appendNumber(float);
+    void appendNumber(double, unsigned precision = 6, TrailingZerosTruncatingPolicy = TruncateTrailingZeros);
+
+    WTF_EXPORT_PRIVATE void appendShortestFormNumber(float);
+    WTF_EXPORT_PRIVATE void appendShortestFormNumber(double);
+    WTF_EXPORT_PRIVATE void appendFixedPrecisionNumber(float, unsigned precision = 6, TrailingZerosTruncatingPolicy = TruncateTrailingZeros);
+    WTF_EXPORT_PRIVATE void appendFixedPrecisionNumber(double, unsigned precision = 6, TrailingZerosTruncatingPolicy = TruncateTrailingZeros);
+    WTF_EXPORT_PRIVATE void appendFixedWidthNumber(float, unsigned decimalPlaces);
     WTF_EXPORT_PRIVATE void appendFixedWidthNumber(double, unsigned decimalPlaces);
 
+    // FIXME: Delete in favor of the name appendShortestFormNumber or just appendNumber.
+    void appendECMAScriptNumber(float);
+    void appendECMAScriptNumber(double);
+
     String toString()
     {
         if (!m_string.isNull()) {
@@ -382,6 +393,28 @@
     return m_bufferCharacters16;
 }
 
+inline void StringBuilder::appendNumber(float number)
+{
+    appendFixedPrecisionNumber(number);
+}
+
+inline void StringBuilder::appendNumber(double number, unsigned precision, TrailingZerosTruncatingPolicy policy)
+{
+    appendFixedPrecisionNumber(number, precision, policy);
+}
+
+inline void StringBuilder::appendECMAScriptNumber(float number)
+{
+    // FIXME: This preserves existing behavior but is not what we want.
+    // In the future, this should either be a compilation error or call appendShortestFormNumber without converting to double.
+    appendShortestFormNumber(static_cast<double>(number));
+}
+
+inline void StringBuilder::appendECMAScriptNumber(double number)
+{
+    appendShortestFormNumber(number);
+}
+
 template <typename CharType>
 bool equal(const StringBuilder& s, const CharType* buffer, unsigned length)
 {

Modified: trunk/Source/WTF/wtf/text/WTFString.cpp (242359 => 242360)


--- trunk/Source/WTF/wtf/text/WTFString.cpp	2019-03-04 16:57:38 UTC (rev 242359)
+++ trunk/Source/WTF/wtf/text/WTFString.cpp	2019-03-04 16:58:33 UTC (rev 242360)
@@ -454,7 +454,7 @@
     return numberToStringSigned<String>(number);
 }
 
-String String::number(unsigned int number)
+String String::number(unsigned number)
 {
     return numberToStringUnsigned<String>(number);
 }
@@ -479,22 +479,34 @@
     return numberToStringUnsigned<String>(number);
 }
 
-String String::number(double number, unsigned precision, TrailingZerosTruncatingPolicy trailingZerosTruncatingPolicy)
+String String::numberToStringFixedPrecision(float number, unsigned precision, TrailingZerosTruncatingPolicy trailingZerosTruncatingPolicy)
 {
     NumberToStringBuffer buffer;
-    return String(numberToFixedPrecisionString(number, precision, buffer, trailingZerosTruncatingPolicy == TruncateTrailingZeros));
+    return numberToFixedPrecisionString(number, precision, buffer, trailingZerosTruncatingPolicy == TruncateTrailingZeros);
 }
 
-String String::numberToStringECMAScript(double number)
+String String::numberToStringFixedPrecision(double number, unsigned precision, TrailingZerosTruncatingPolicy trailingZerosTruncatingPolicy)
 {
     NumberToStringBuffer buffer;
-    return String(numberToString(number, buffer));
+    return numberToFixedPrecisionString(number, precision, buffer, trailingZerosTruncatingPolicy == TruncateTrailingZeros);
 }
 
+String String::numberToStringShortest(float number)
+{
+    NumberToStringBuffer buffer;
+    return numberToString(number, buffer);
+}
+
+String String::numberToStringShortest(double number)
+{
+    NumberToStringBuffer buffer;
+    return numberToString(number, buffer);
+}
+
 String String::numberToStringFixedWidth(double number, unsigned decimalPlaces)
 {
     NumberToStringBuffer buffer;
-    return String(numberToFixedWidthString(number, decimalPlaces, buffer));
+    return numberToFixedWidthString(number, decimalPlaces, buffer);
 }
 
 int String::toIntStrict(bool* ok, int base) const

Modified: trunk/Source/WTF/wtf/text/WTFString.h (242359 => 242360)


--- trunk/Source/WTF/wtf/text/WTFString.h	2019-03-04 16:57:38 UTC (rev 242359)
+++ trunk/Source/WTF/wtf/text/WTFString.h	2019-03-04 16:58:33 UTC (rev 242360)
@@ -175,13 +175,21 @@
     WTF_EXPORT_PRIVATE static String number(unsigned long);
     WTF_EXPORT_PRIVATE static String number(long long);
     WTF_EXPORT_PRIVATE static String number(unsigned long long);
+    // FIXME: Change number to be numberToStringShortest instead of numberToStringFixedPrecision.
+    static String number(float);
+    static String number(double, unsigned precision = 6, TrailingZerosTruncatingPolicy = TruncateTrailingZeros);
 
-    WTF_EXPORT_PRIVATE static String number(double, unsigned precision = 6, TrailingZerosTruncatingPolicy = TruncateTrailingZeros);
-
-    // Number to String conversion following the ECMAScript definition.
-    WTF_EXPORT_PRIVATE static String numberToStringECMAScript(double);
+    WTF_EXPORT_PRIVATE static String numberToStringShortest(float);
+    WTF_EXPORT_PRIVATE static String numberToStringShortest(double);
+    WTF_EXPORT_PRIVATE static String numberToStringFixedPrecision(float, unsigned precision = 6, TrailingZerosTruncatingPolicy = TruncateTrailingZeros);
+    WTF_EXPORT_PRIVATE static String numberToStringFixedPrecision(double, unsigned precision = 6, TrailingZerosTruncatingPolicy = TruncateTrailingZeros);
+    WTF_EXPORT_PRIVATE static String numberToStringFixedWidth(float, unsigned decimalPlaces);
     WTF_EXPORT_PRIVATE static String numberToStringFixedWidth(double, unsigned decimalPlaces);
 
+    // FIXME: Delete in favor of the name numberToStringShortest or just number.
+    static String numberToStringECMAScript(float);
+    static String numberToStringECMAScript(double);
+
     // Find a single character or string, also with match function & latin1 forms.
     size_t find(UChar character, unsigned start = 0) const { return m_impl ? m_impl->find(character, start) : notFound; }
 
@@ -630,6 +638,28 @@
     return startsWithLettersIgnoringASCIICase(string.impl(), lowercaseLetters);
 }
 
+inline String String::number(float number)
+{
+    return numberToStringFixedPrecision(number);
+}
+
+inline String String::number(double number, unsigned precision, TrailingZerosTruncatingPolicy policy)
+{
+    return numberToStringFixedPrecision(number, precision, policy);
+}
+
+inline String String::numberToStringECMAScript(float number)
+{
+    // FIXME: This preserves existing behavior but is not what we want.
+    // In the future, this should either be a compilation error or call numberToStringShortest without converting to double.
+    return numberToStringShortest(static_cast<double>(number));
+}
+
+inline String String::numberToStringECMAScript(double number)
+{
+    return numberToStringShortest(number);
+}
+
 inline namespace StringLiterals {
 
 inline String operator"" _str(const char* characters, size_t)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to