NaN, undefined, and Infinity in ToTemporalDurationRecord

[FrankYFTang]

ToTemporalDurationRecord
https://tc39.es/proposal-temporal/#sec-temporal-totemporaldurationrecord

Let's say we have the following code

let d2 = Temporal.Duration.from({months: undefined, seconds:5})
let d3 = Temporal.Duration.from({weeks: NaN, seconds:5})
let d4 = Temporal.Duration.from({days: Infinity, seconds:5})

What should happen?
Temporal.Duration.from will pass the object to ToTemporalDuration(item).
https://tc39.es/proposal-temporal/#sec-temporal-totemporalduration

b. Let result be ? ToTemporalDurationRecord(item).
will call ToTemporalDurationRecord
https://tc39.es/proposal-temporal/#sec-temporal-totemporaldurationrecord
and step 5 will be run
b. Let val be ? Get(temporalDurationLike, prop).
will be called
and now val could be undefined, NaN or Infinity
then
d. Let val be ? ToNumber(val).
https://tc39.es/ecma262/#sec-tonumber
will be called and turn them into NaN, NaN, Infinity

right?
then the next line

e. If floor(val) ≠ val, then
i. Throw a RangeError exception.

now.... should we throw RangeError for them or not? I do not know what is the definintion of floor on NaN and Infinity.

If we throw RangeError for them, then any undefined will be thrown so {year:1} will also throw RangeError because the Get('month') will get undefined back.
Should we check the val is NaN or some special case first and skip that if check first?

@justingrant @ptomato @littledan @ljharb @Ms2ger @ryzokuken

[linusg]

This is also mentioned in #1502 (long list of issues):

7.5.2 ToTemporalDurationRecord ( temporalDurationLike )

• ToNumber() returns a Number value, but floor expects a mathematical value.
• If val is undefined, ToNumber(undefined) returns NaN, which means floor(NaN) ≠ NaN will evaluate to true when assuming Math.floor() semantics should be applied. So an absent value will always throw a RangeError.

Regarding the second point you mention:

If we throw RangeError for them, then any undefined will be thrown so {year:1} will also throw RangeError because the Get('month') will get undefined back.

I also stumbled across this when implementing this part of Temporal in SerenityOS's LibJS, and chose to mirror the polyfill in the meantime: only doing the steps d, e, and f if the value from step b is not undefined.

• proposal-temporal/polyfill/lib/ecmascript.mjs

  Lines 556 to 607 in 4b4dbd4

  	ToTemporalDurationRecord: (item) => {
  	if (ES.IsTemporalDuration(item)) {
  	return {
  	years: GetSlot(item, YEARS),
  	months: GetSlot(item, MONTHS),
  	weeks: GetSlot(item, WEEKS),
  	days: GetSlot(item, DAYS),
  	hours: GetSlot(item, HOURS),
  	minutes: GetSlot(item, MINUTES),
  	seconds: GetSlot(item, SECONDS),
  	milliseconds: GetSlot(item, MILLISECONDS),
  	microseconds: GetSlot(item, MICROSECONDS),
  	nanoseconds: GetSlot(item, NANOSECONDS)
  	};
  	}
  	const props = ES.ToPartialRecord(
  	item,
  	[
  	'days',
  	'hours',
  	'microseconds',
  	'milliseconds',
  	'minutes',
  	'months',
  	'nanoseconds',
  	'seconds',
  	'weeks',
  	'years'
  	],
  	(v) => {
  	v = ES.ToNumber(v);
  	if (MathFloor(v) !== v) {
  	throw new RangeError(`unsupported fractional value ${v}`);
  	}
  	return v;
  	}
  	);
  	if (!props) throw new TypeError('invalid duration-like');
  	let {
  	years = 0,
  	months = 0,
  	weeks = 0,
  	days = 0,
  	hours = 0,
  	minutes = 0,
  	seconds = 0,
  	milliseconds = 0,
  	microseconds = 0,
  	nanoseconds = 0
  	} = props;
  	return { years, months, weeks, days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds };
  	},

• proposal-temporal/polyfill/lib/ecmascript.mjs

  Lines 876 to 893 in 4b4dbd4

  	ToPartialRecord: (bag, fields, callerCast) => {
  	if (ES.Type(bag) !== 'Object') return false;
  	let any;
  	for (const property of fields) {
  	const value = bag[property];
  	if (value !== undefined) {
  	any = any || {};
  	if (callerCast === undefined && BUILTIN_CASTS.has(property)) {
  	any[property] = BUILTIN_CASTS.get(property)(value);
  	} else if (callerCast !== undefined) {
  	any[property] = callerCast(value);
  	} else {
  	any[property] = value;
  	}
  	}
  	}
  	return any ? any : false;
  	},

i.e. in this case the polyfill's observable behavior differs from the spec as it does not throw a RangeError for undefined properties of the duration-like object.

[FrankYFTang]

The reason I reaslize this issue is I implement my v8 prototype strickly follow the current spec text and it turn out it will always throw RangeError if I call Temporal.Duration.from({years: 1}) because while go down the table to call Get("monhts") it will get undefined as val and turn become NaN and floor(NaN) != NaN will cause the throw RangeError. Therefore, if we do not change the spec, we will throw on Temporal.Duration.from({}) unless all 10 fields are specified. I do not believe that was the intention of the Temporal spec but that could be an option we just not change Temporal since it is already in Stage 3 and always throw RangeError unless all 10 fileds are presented.

[FrankYFTang]

ok, I have coded the test for the current spec text in tc39/test262#3068 . If the champion decide to take my PR in #1659 I could of course chagne 3068 to reflect that but otherwise, it could go in with the current spec text which mandate all fields are presented and are not undefined.

[ptomato]

This was obviously not the intention. This should go into the next batch of normative changes that we ask for consensus on.