Editor review: type confusion around number types

[syg]

Since the arithmetic fix in ecma262, the editorial convention is that:

• All arithmetic and comparisons are done on the Mathematical Values.
• Internal fields should store MVs.
• When MVs need to be passed back to user code, they are converted to Number values.
• MVs no longer have any subscripts (no subscript ℝ), while Number values have a subscript 𝔽.

For more details see 5.2.5.

This spec draft has type confusion for operating on numbers in several places. I haven't read through everything to make an exhaustive list. Some places I noticed:

• https://tc39.es/proposal-temporal/#sec-temporal-validateinstant compares BigInt values against MVs.
• Step 1 of https://tc39.es/proposal-temporal/#sec-temporal-systemutcepochnanoseconds should probably explicitly say it's the MV of the approximated ns since epoch.
• ToIntegerOrInfinity returns an MV. Several getters like https://tc39.es/proposal-temporal/#sec-get-temporal.plaindate.prototype.day return the result directly to user code. This should go through 𝔽().
• ToIntegerOrInfinity can return infinity. Several places aren't correctly checking for this. For example, https://tc39.es/proposal-temporal/#sec-temporal.plaindate passes d, m, and y to https://tc39.es/proposal-temporal/#sec-temporal-createtemporaldate, which then asserts that they are integers. They might be infinity.

---

• Change internal slots of Temporal.Duration to store mathematical values #1604
• Is there a better way to do string parsing than ToIntegerOrInfinity? #1566
• Verify that modulo and remainder are only called on mathematical values #1887
• Reconsider Sign abstract operation #1593
• Handling of infinities in ToTemporalRoundingIncrement
• Handling of infinities in ToTemporalTimeRecord
• PreparePartialTemporalFields passes MVs to CreateDataPropertyOrThrow (sometimes)
• GetISOPartsFromEpoch: 262 operations use Numbers

[ptomato]

Thanks, I was afraid there might be a few places where this is still wrong.

[ptomato]

Should note that while doing this issue, comma thousands separators are currently not used in the spec text.

[ptomato]

Note, make sure that all operations make clear what the number types of the inputs are, by assertions.

[ptomato]

• Internal fields should store MVs.

This won't be a problem for most of Temporal, but @anba raised in #1534 (comment) that this requirement is a problem for Temporal.Duration's internal slots, because they are unbounded. This could make any future additional properties on Temporal.Duration unpolyfillable. @syg do you have any opinion on this?

[syg]

I don't quite understand the concern @anba raised. Why can't the polyfills use BigInts and convert them to Numbers at the boundary?

[ptomato]

If I'm understanding @anba correctly, suppose a new method was added to Duration in the future, which used the bigint values from the internal slots in its calculations. A polyfill wouldn't be able to correctly do this.

[ljharb]

Why wouldn't it? The polyfill would still store the bigint value internally, and its version of the new method would have access to those, no?

(Separately, if the values are being stored as bigints/mathematical integers internally, why are they exposed as Numbers in the API?)

[Ms2ger]

Why wouldn't it? The polyfill would still store the bigint value internally, and its version of the new method would have access to those, no?

Depends on whether the polyfill would replace the entire API or just add the new function, presumably.

[ljharb]

That’s a fair point; it would likely force a new method being polyfilled to also polyfill all of that Temporal type.

[anba]

Yes, I was concerned about the case when a possible Temporal v2 extends Temporal.Duration and a polyfill wants to implement these new methods by extending the (browser) built-in Temporal.Duration object. It seems unlikely that a polyfill will want to replace the whole built-in Temporal.Duration object for that case. (And to be properly spec-compliant, the polyfill will actually need to replace everything from Temporal, because other Temporal objects are also creating Temporal.Duration objects.)

[anba]

The patch from #1534 was only redefining that the internal slots are now stored as mathematical values instead of Number values. It didn't update any existing call sites to use ℝ resp. 𝔽 to convert to/from mathematical values. I don't have any issues when the spec defines that the internal slots are storing mathematical values as long as all callers are updated to use 𝔽 when reading these slots.

[syg]

Thanks for the explanation. The "do math on MVs, convert to Numbers at the boundary" is pretty pervasive in the spec. I don't feel like partial polyfilling is a use case that we ought to go out of our way to enable, especially for Temporal, one of whose main value adds is that we can start shipping this and the related data in browsers to cut down on payload size. So I'd rather not do anything different here, which could result in subtle arithmetic bugs and gotchas down the road.

[ptomato]

Understood on the editorial preference. @Ms2ger and I will investigate whether this change is possible without a normative change to the approved spec text. In #1534 @anba suggests that it is not.

[Ms2ger]

I think I agree that we should do all internal maths on MVs. My apologies for not realizing earlier that this would be a normative change in the Duration case. Any changes to the other classes should be purely editorial, thanks to the existing range checks, so we should be able to continue with those pending a plenary decision.

[ptomato]

I'd like to split off #1604 for the normative change to Duration, so that Ms2ger can close this when all of the other editorial issues are resolved.

[ptomato]

Verify things on checklist.

[ptomato]

• Handling of infinities in ToTemporalRoundingIncrement: these are now handled.
• PreparePartialTemporalFields passes MVs to CreateDataPropertyOrThrow (sometimes): PreparePartialTemporalFields was folded into PrepareTemporalFields and the cases where a MV was passed to CreateDataPropertyOrThrow are fixed.
• Comma thousands separators: The only place where these remain is in prose, which is consistent with the prose in ECMA-262.
• Numeric type assertions: There were still a few AOs operating on numeric types that didn't have these type assertions, either explicitly or via structured headers, and one (RoundTemporalInstant) should've been returning a BigInt. This is fixed in Editorial: Convert operations with numeric arguments to structured headers #2455.