Bug 96132 – Avoid strlen() when converting DTOA result to String

• Status: NEW
• Component: Web Template Framework

[Patrick R. Gansterer]

Avoid strlen() when converting DTOA result to String

[Patrick R. Gansterer]

Created attachment 162814 [details]
Patch

[Benjamin Poulain]

This is very interesting

Since it touches JSC's runtime. Could you please check SunSpider to ensure there are no regressions? Better safe than getting yelled at ;)

[Build Bot]

Comment on attachment 162814 [details]
Patch

Attachment 162814 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13777684

[Build Bot]

Comment on attachment 162814 [details]
Patch

Attachment 162814 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13776782

[Early Warning System Bot]

Comment on attachment 162814 [details]
Patch

Attachment 162814 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13774835

[Early Warning System Bot]

Comment on attachment 162814 [details]
Patch

Attachment 162814 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13772829

[Gyuyoung Kim]

Comment on attachment 162814 [details]
Patch

Attachment 162814 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/13786551

[Peter Beverloo (cr-android ews)]

Comment on attachment 162814 [details]
Patch

Attachment 162814 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/13777711

[WebKit Review Bot]

Comment on attachment 162814 [details]
Patch

Attachment 162814 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13788308

[Darin Adler]

Comment on attachment 162814 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=162814&action=review

review- because patch doesn’t build and so needs some additional work; probably should review a version that does build

> Source/JavaScriptCore/runtime/NumberPrototype.cpp:388
> +    WTF::NumberToStringBuffer buffer;

The WTF:: prefix should not be needed here.

> Source/WTF/wtf/dtoa.h:47
> -WTF_EXPORT_PRIVATE const char* numberToString(double, NumberToStringBuffer);
> -WTF_EXPORT_PRIVATE const char* numberToFixedPrecisionString(double, unsigned significantFigures, NumberToStringBuffer, bool truncateTrailingZeros = false);
> -WTF_EXPORT_PRIVATE const char* numberToFixedWidthString(double, unsigned decimalPlaces, NumberToStringBuffer);
> +WTF_EXPORT_PRIVATE int numberToString(double, NumberToStringBuffer);
> +WTF_EXPORT_PRIVATE int numberToFixedPrecisionString(double, unsigned significantFigures, NumberToStringBuffer, bool truncateTrailingZeros = false);
> +WTF_EXPORT_PRIVATE int numberToFixedWidthString(double, unsigned decimalPlaces, NumberToStringBuffer);

This return value is a lot less self-explanatory than the old one. We probably need a comment or some other form of explanation. Perhaps this should even be an out argument instead of a return value for clarity.