Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Editorial: More tweaks to low-level Date AOs #3193

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

jmdyck
Copy link
Collaborator

@jmdyck jmdyck commented Oct 12, 2023

This is a follow-up to PR #3092.

The first two commits:

  • inline DaysInYear into InLeapYear, and
  • rename msFromTime as MillisecFromTime

as suggested at the bottom of this comment.


The remaining commits try to shift things in this area from integral Numbers to mathematical integers, as suggested by michaelficarra and syg.

Note that this doesn't have much effect on the space in which arithmetic is done. Mostly it just avoids unnecessary conversions between the two. Roughly, the idea is to delay 𝔽 calls as much as reasonable.

(I've submitted this part as one commit per AO, in case that helps with review, but I expect I'll squash them down to one commit for merging.)


Effects on downstream specs:
None for WebIDL and HTML.

In Ecma 402, Table 8 uses WeekDay() and the various FooFromTime() AOs to set the fields of the record returned by ToLocalTime(). Since this PR changes the return value of these AOs from an integral Number to a mathematical integer, we might expect that 402 would need to change. However, the uses of ToLocalTime (in FormatDateTimePattern and PartitionDateTimeRangePattern) actually seem happier with mathematical values than Numbers in these fields.

As for the Temporal proposal, I'll let others decide whether this PR makes things better or worse for it.

@anba
Copy link
Contributor

anba commented Oct 12, 2023

In Ecma 402, Table 8 uses WeekDay() and the various FooFromTime() AOs to set the fields of the record returned by ToLocalTime(). Since this PR changes the return value of these AOs from an integral Number to a mathematical integer,
we might expect that 402 would need to change. However, the uses of ToLocalTime (in FormatDateTimePattern and PartitionDateTimeRangePattern)
actually seem happier with mathematical values than Numbers in these fields.

I can confirm this. tc39/ecma402#822 will change ECMA-402 to convert the current 𝔽 outputs to ℝ. This won't be necessary anymore when ECMA-262 already returns ℝ values.

@jmdyck
Copy link
Collaborator Author

jmdyck commented Jan 25, 2024

(force-pushed to resolve a merge conflict)

spec.html Outdated Show resolved Hide resolved
@michaelficarra
Copy link
Member

michaelficarra commented Feb 15, 2024

There's a call to MakeDay in a note in the UTC AO that passes mathematical values. If you rebase, you should get a version of ecmarkup that would catch this.

That said, a lot of calls to MakeTime/MakeDay/MakeDate convert mathematical values to Numbers before getting converted right back. What would it look like to make them take/produce maths values instead?

Copy link
Member

@michaelficarra michaelficarra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM otherwise, but we should explore changing the types of MakeTime/MakeDay/MakeDate before landing (or as a follow-up).

</h1>
<dl class="header">
<dt>description</dt>
<dd>It returns a Number identifying the month in which _t_ falls. A month value of *+0*<sub>𝔽</sub> specifies January; *1*<sub>𝔽</sub> specifies February; *2*<sub>𝔽</sub> specifies March; *3*<sub>𝔽</sub> specifies April; *4*<sub>𝔽</sub> specifies May; *5*<sub>𝔽</sub> specifies June; *6*<sub>𝔽</sub> specifies July; *7*<sub>𝔽</sub> specifies August; *8*<sub>𝔽</sub> specifies September; *9*<sub>𝔽</sub> specifies October; *10*<sub>𝔽</sub> specifies November; and *11*<sub>𝔽</sub> specifies December. Note that <emu-eqn>MonthFromTime(*+0*<sub>𝔽</sub>) = *+0*<sub>𝔽</sub></emu-eqn>, corresponding to Thursday, 1 January 1970.</dd>
<dd>It returns a Number identifying the month in which _t_ falls. A month value of 0 specifies January; 1 specifies February; 2 specifies March; 3 specifies April; 4 specifies May; 5 specifies June; 6 specifies July; 7 specifies August; 8 specifies September; 9 specifies October; 10 specifies November; and 11 specifies December. Note that <emu-eqn>MonthFromTime(*+0*<sub>𝔽</sub>) = 0</emu-eqn>, corresponding to Thursday, 1 January 1970.</dd>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<dd>It returns a Number identifying the month in which _t_ falls. A month value of 0 specifies January; 1 specifies February; 2 specifies March; 3 specifies April; 4 specifies May; 5 specifies June; 6 specifies July; 7 specifies August; 8 specifies September; 9 specifies October; 10 specifies November; and 11 specifies December. Note that <emu-eqn>MonthFromTime(*+0*<sub>𝔽</sub>) = 0</emu-eqn>, corresponding to Thursday, 1 January 1970.</dd>
<dd>It returns an integer identifying the month in which _t_ falls. A month value of 0 specifies January; 1 specifies February; 2 specifies March; 3 specifies April; 4 specifies May; 5 specifies June; 6 specifies July; 7 specifies August; 8 specifies September; 9 specifies October; 10 specifies November; and 11 specifies December. Note that <emu-eqn>MonthFromTime(*+0*<sub>𝔽</sub>) = 0</emu-eqn>, corresponding to Thursday, 1 January 1970.</dd>

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

spec.html Outdated
</h1>
<dl class="header">
<dt>description</dt>
<dd>It returns a Number identifying the day of the week in which _t_ falls. A weekday value of *+0*<sub>𝔽</sub> specifies Sunday; *1*<sub>𝔽</sub> specifies Monday; *2*<sub>𝔽</sub> specifies Tuesday; *3*<sub>𝔽</sub> specifies Wednesday; *4*<sub>𝔽</sub> specifies Thursday; *5*<sub>𝔽</sub> specifies Friday; and *6*<sub>𝔽</sub> specifies Saturday. Note that <emu-eqn>WeekDay(*+0*<sub>𝔽</sub>) = *4*<sub>𝔽</sub></emu-eqn>, corresponding to Thursday, 1 January 1970.</dd>
<dd>It returns a Number identifying the day of the week in which _t_ falls. A weekday value of 0 specifies Sunday; 1 specifies Monday; 2 specifies Tuesday; 3 specifies Wednesday; 4 specifies Thursday; 5 specifies Friday; and 6 specifies Saturday. Note that <emu-eqn>WeekDay(*+0*<sub>𝔽</sub>) = 4</emu-eqn>, corresponding to Thursday, 1 January 1970.</dd>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<dd>It returns a Number identifying the day of the week in which _t_ falls. A weekday value of 0 specifies Sunday; 1 specifies Monday; 2 specifies Tuesday; 3 specifies Wednesday; 4 specifies Thursday; 5 specifies Friday; and 6 specifies Saturday. Note that <emu-eqn>WeekDay(*+0*<sub>𝔽</sub>) = 4</emu-eqn>, corresponding to Thursday, 1 January 1970.</dd>
<dd>It returns an integer identifying the day of the week in which _t_ falls. A weekday value of 0 specifies Sunday; 1 specifies Monday; 2 specifies Tuesday; 3 specifies Wednesday; 4 specifies Thursday; 5 specifies Friday; and 6 specifies Saturday. Note that <emu-eqn>WeekDay(*+0*<sub>𝔽</sub>) = 4</emu-eqn>, corresponding to Thursday, 1 January 1970.</dd>

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@bakkot
Copy link
Contributor

bakkot commented Feb 21, 2024

This will need to be paired with an update to the Temporal spec, which @michaelficarra has volunteered to do.

@jmdyck
Copy link
Collaborator Author

jmdyck commented Feb 27, 2024

There's a call to MakeDay in a note in the UTC AO that passes mathematical values.

Yup. Since it's a Note, I thought it could be looser. But I can insert subscripts if you like.

If you rebase, you should get a version of ecmarkup that would catch this.

I pushed up a rebase, but all checks passed.

That said, a lot of calls to MakeTime/MakeDay/MakeDate convert mathematical values to Numbers before getting converted right back. What would it look like to make them take/produce maths values instead?

If every argument in every call to MakeTime & MakeDay were F(_m_) where _m_ was some finite and not-too-big mathematical value, then yes we could do some simplification and end up directly passing _m_. But that's not case: in almost every call, some args are arbitrary Number values, which have to be caught by the 'not finite' test in step 1 of MakeTime and MakeDay. You could hoist that test out to the call-sites, but I don't think that would be an improvement.

Moreover, MakeDay has two other steps that can return *NaN*: what should it do there if it can only produce math values?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants