1780025 - Tidy up NumberToCString API, part deux

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

[André Bargull [:anba]]

Bug 1771874 enabled some further clean-ups and improvements for NumberToCString.

Comment 1

[André Bargull [:anba]]

Attachment: Bug 1780025 - Part 1: Prefer member initialiser instead of a default constructor for ToCStringBuf. r=jandem!

And add an assertion to ensure double_conversion and dtoa use the same limits
for the largest string conversion.

Comment 2

[André Bargull [:anba]]

Attachment: Bug 1780025 - Part 2: Replace NumberToCStringWithBase with NumberToHexCString. r=jandem!

NumberToCStringWithBase was only ever used with base = 16. Adding a dedicated
function for hex-strings enables us to perform more clean-ups in the next patches.

Also change Int32ToCStringWithBase to accept signed and unsigned integers. That
way we don't have to convert unsigned integers larger than UINT32_MAX to doubles.

Depends on D152066

Comment 3

[André Bargull [:anba]]

Attachment: Bug 1780025 - Part 3: Add NumberToCString overloads for integers. r=jandem!

32-bit integers (signed and unsigned) need less space than provided by
ToCStringBuf and don't need any dynamically allocated memory, which avoids
calling ToCStringBuf's out-of-line destructor.

Depends on D152067

Comment 4

[André Bargull [:anba]]

Attachment: Bug 1780025 - Part 4: Split the integer and double cases in NumberToStringWithBase. r=jandem!

Split both cases in preparation for the next patches.

Drive-by change:
base = 10 doesn't need to execute the unsigned(i) < unsigned(base) and
unsigned(i) < unsigned(base * base) cases, because StaticStrings::hasInt(i)
covers this case already.

Depends on D152068

Comment 5

[André Bargull [:anba]]

Attachment: Bug 1780025 - Part 5: Inline FracNumberToCStringWithBase into its single caller. r=jandem!

This will allow to remove ToCStringBuf::dbuf in the next part.

Depends on D152069

Comment 6

[André Bargull [:anba]]

Attachment: Bug 1780025 - Part 6: Remove unused ToCStringBuf::dbuf member. r=jandem!

ToCStringBuf::dbuf is no longer used after the last part.

Depends on D152070

Comment 7

[André Bargull [:anba]]

Attachment: Bug 1780025 - Part 7: Change ToCStringBuf buffer size to only support base 10. r=jandem!

No external caller is using ToCStringBuf for a different base than base-10.
This allows to change the buffer size to JS::MaximumNumberToStringLength, which
is a multiple of 4 resp. 8, which should be more efficiently to stack allocate
than 34 bytes.

Depends on D152071

Comment 8

[André Bargull [:anba]]

Attachment: Bug 1780025 - Part 8: Avoid calling strlen after FracNumberToCString. r=jandem!

This avoids another call to strlen.

Depends on D152072

Comment 9

[André Bargull [:anba]]

Attachment: Bug 1780025 - Part 9: Inline NumberToCString into NumberValueToStringBuffer. r=jandem!

This avoids another call to strlen.

Depends on D152073

Comment 10

[André Bargull [:anba]]

Attachment: Bug 1780025 - Part 10: Prefer NumberToCString over JS::NumberToString. r=jandem!

This was the only caller to JS::NumberToString within SpiderMonkey (ignoring test code).

Depends on D152074

Comment 11

[André Bargull [:anba]]

Attachment: Bug 1780025 - Part 11: Update a comment for NumberToString. r=jandem!

NumberToString doesn't accept a base argument.

Depends on D152075

Comment 12

[André Bargull [:anba]]

Attachment: Bug 1780025 - Part 12: Remove unused jsnum functions. r=jandem!

Depends on D152076

Comment 13

[André Bargull [:anba]]

Attachment: Bug 1780025 - Part 13: Add an optional "length" out-param. r=jandem!

Depends on D152077

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2dc7b7595fd1
Part 1: Prefer member initialiser instead of a default constructor for ToCStringBuf. r=jandem
https://hg.mozilla.org/integration/autoland/rev/a203428fface
Part 2: Replace NumberToCStringWithBase with NumberToHexCString. r=jandem
https://hg.mozilla.org/integration/autoland/rev/b35e532a5c02
Part 3: Add NumberToCString overloads for integers. r=jandem
https://hg.mozilla.org/integration/autoland/rev/65558ee2c725
Part 4: Split the integer and double cases in NumberToStringWithBase. r=jandem
https://hg.mozilla.org/integration/autoland/rev/39c3be46d8fc
Part 5: Inline FracNumberToCStringWithBase into its single caller. r=jandem
https://hg.mozilla.org/integration/autoland/rev/5f69be822f72
Part 6: Remove unused ToCStringBuf::dbuf member. r=jandem
https://hg.mozilla.org/integration/autoland/rev/fd161c0434e5
Part 7: Change ToCStringBuf buffer size to only support base 10. r=jandem
https://hg.mozilla.org/integration/autoland/rev/60243a95e18d
Part 8: Avoid calling strlen after FracNumberToCString. r=jandem
https://hg.mozilla.org/integration/autoland/rev/f387bc1fcdfc
Part 9: Inline NumberToCString into NumberValueToStringBuffer. r=jandem
https://hg.mozilla.org/integration/autoland/rev/c2ebd8c58db2
Part 10: Prefer NumberToCString over JS::NumberToString. r=jandem
https://hg.mozilla.org/integration/autoland/rev/8d22a331a662
Part 11: Update a comment for NumberToString. r=jandem
https://hg.mozilla.org/integration/autoland/rev/3a3d26fc4d69
Part 12: Remove unused jsnum functions. r=jandem
https://hg.mozilla.org/integration/autoland/rev/4c221ac039f7
Part 13: Add an optional "length" out-param. r=jandem

Comment 15

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/2dc7b7595fd1
https://hg.mozilla.org/mozilla-central/rev/a203428fface
https://hg.mozilla.org/mozilla-central/rev/b35e532a5c02
https://hg.mozilla.org/mozilla-central/rev/65558ee2c725
https://hg.mozilla.org/mozilla-central/rev/39c3be46d8fc
https://hg.mozilla.org/mozilla-central/rev/5f69be822f72
https://hg.mozilla.org/mozilla-central/rev/fd161c0434e5
https://hg.mozilla.org/mozilla-central/rev/60243a95e18d
https://hg.mozilla.org/mozilla-central/rev/f387bc1fcdfc
https://hg.mozilla.org/mozilla-central/rev/c2ebd8c58db2
https://hg.mozilla.org/mozilla-central/rev/8d22a331a662
https://hg.mozilla.org/mozilla-central/rev/3a3d26fc4d69
https://hg.mozilla.org/mozilla-central/rev/4c221ac039f7