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

Specify Date.UTC when called with fewer than two arguments #642

Merged
merged 2 commits into from
Feb 27, 2017

Conversation

claudepache
Copy link
Contributor

No description provided.

@claudepache
Copy link
Contributor Author

Followup of whatwg/javascript#31

@bterlson
Copy link
Member

@bterlson bterlson added the needs consensus This needs committee consensus before it can be eligible to be merged. label Jul 22, 2016
@claudepache
Copy link
Contributor Author

claudepache commented Jul 22, 2016

Note that, according to the proposed semantics, when Date.UTC is called with 1 argument, step 1 may have side-effects, although the final result will always be either NaN or an abrupt completion:

Date.UTC({ valueOf: function() { throw "hey!" } })

(Firefox, Chrome and Safari throw "hey!" in that testcase.)

@dilijev
Copy link
Contributor

dilijev commented Jul 27, 2016

@claudepache Thanks for pointing that test case out. Edge also throws "hey!" in that case.

@bterlson
Copy link
Member

Consensus is Date.UTC() returns NaN and Date.UTC(2017) === Date.UTC(2017, 0). This PR needs a slight update for this.

@bterlson bterlson removed the needs consensus This needs committee consensus before it can be eligible to be merged. label Sep 29, 2016
@@ -25643,7 +25643,7 @@
<!-- es6num="20.3.3.4" -->
<emu-clause id="sec-date.utc">
<h1>Date.UTC ( _year_, _month_ [ , _date_ [ , _hours_ [ , _minutes_ [ , _seconds_ [ , _ms_ ] ] ] ] ] )</h1>
Copy link
Contributor

Choose a reason for hiding this comment

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

_month_ is optional now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

corrected

@claudepache
Copy link
Contributor Author

I've updated the PR.

<emu-alg>
1. Let _y_ be ? ToNumber(_year_).
1. Let _m_ be ? ToNumber(_month_).
1. If _month_ is supplied, let _m_ be ? ToNumber(_month_); else let _m_ be 0.
Copy link
Member

Choose a reason for hiding this comment

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

@bterlson I just want to confirm this for clarification: we'll have a different behavior for supplied but undefined month. Like the following expected values:

Date.UTC(2016) // 1451606400000
Date.UTC(2016, 0) // 1451606400000
Date.UTC(2016, undefined) // NaN

This will be consistent with the other parameters, but Date.UTC will never be perfect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is also consistent with the Date constructor:

new Date(2016, 0) // Fri Jan 01 2016 00:00:00 GMT+0100 (CET)
new Date(2016, undefined) // Invalid Date

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, this is just how it is :-P

@littledan
Copy link
Member

Is this patch ready to land? Should it go in ES2017? Seems all good to me.

kisg pushed a commit to paul99/v8mips that referenced this pull request Feb 14, 2017
After tc39/ecma262#642, Date.UTC no longer requires
the month argument to be specified. The spec provides 0 as its default value.

This CL updates the builtins-date.cc code to reflect that and drops the test
suppression for test262/built-ins/Date/UTC/return-value.

BUG=v8:5534

Review-Url: https://codereview.chromium.org/2689173003
Cr-Commit-Position: refs/heads/master@{#43193}
@evilpie
Copy link
Contributor

evilpie commented Feb 25, 2017

@claudepache @bterlson can we land this?

@bterlson
Copy link
Member

@evilpie yep seems good to me. @littledan unless you object I was planning on bringing this to the es2017 branch since we have consensus on the semantics from a long while back.

@bterlson bterlson merged commit d28ebe6 into tc39:master Feb 27, 2017
@bterlson
Copy link
Member

Also merged into es2017.

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.

7 participants