Editorial: Early return & throw in Number.prototype.to* methods

[shvaikalesh]

This change makes handling of non-finite this values more concise (by using Number::toString) and moves it earlier in steps, as well as argument range checks. This makes observable steps grouped and order of observable steps easier to read (especially if a runtime uses something like std::isfinite as JSC does).

[shvaikalesh]

With this change, it is easier to spot a discrepancy in handling of non-finite this values:

0..toExponential(-1) // RangeError
0..toFixed(-1) // RangeError
0..toPrecision(-1) // RangeError

NaN.toExponential(-1) // => "NaN"
NaN.toFixed(-1) // RangeError
NaN.toPrecision(-1) // => "NaN"

Would you consider a normative PR to align it?

We can either a) move range check after finiteness check in toFixed or b) move range checks before finiteness checks in both toExponential and toPrecision.

a) is most certainly web-compatible, and it makes sense to perform all checks on this value before arguments.

[ljharb]

This looks observably equivalent.

[ljharb]

@shvaikalesh regarding #1938 (comment), it seems like you're suggesting either a) making NaN.toFixed(-1) return NaN instead of throwing a RangeError, or b) making NaN.toExponential(-1) and NaN.toPrecision(-1) both throw a RangeError.

The latter seems unlikely to be web compatible, and the former does not seem desirable. Can you elaborate?

[bakkot]

I like handling special cases as early as possible and this does appear to be equivalent.

[shvaikalesh]

The latter seems unlikely to be web compatible, and the former does not seem desirable. Can you elaborate?

My only argument for the normative change is consistency, but it doesn't justify adding undesirable behavior. While all built-in methods check this value before arguments, they never return early for a subset of values.