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

Normative: Clarification on Date.prototype.to(UTC)String formatting of negative years. #1035

Open
dilijev opened this Issue Nov 21, 2017 · 6 comments

Comments

Projects
None yet
4 participants
@dilijev
Contributor

dilijev commented Nov 21, 2017

@ljharb, @bterlson, @littledan
/cc @xiaoyinl

The engines disagree with each other over how to format negative years in Date.prototype.toString()

See test case:

## Source
let year1BC = new Date("0001-10-13T05:16:33Z");
let year11BC = new Date("0001-10-13T05:16:33Z");
year1BC.setFullYear(-1, 10, 13);
year11BC.setFullYear(-11, 10, 13);

print(year1BC.toString());
print(year11BC.toString());
print(year1BC.toUTCString());
print(year11BC.toUTCString());


┌─────┬────────────────────────────────────────────────────────────┐
│ d8  │ Sat Nov 13 -001 22:16:33 GMT-0800 (Pacific Standard Time)  │
│ jsc │ Mon Nov 13 -011 22:16:33 GMT-0800 (Pacific Standard Time)  │
│     │ Sun, 14 Nov -001 06:16:33 GMT                              │
│     │ Tue, 14 Nov -011 06:16:33 GMT                              │
├─────┼────────────────────────────────────────────────────────────┤
│ sm  │ Sat Nov 13 -0001 22:16:33 GMT-0800 (Pacific Standard Time) │
│     │ Mon Nov 13 -0011 22:16:33 GMT-0800 (Pacific Standard Time) │
│     │ Sun, 14 Nov -0001 06:16:33 GMT                             │
│     │ Tue, 14 Nov -0011 06:16:33 GMT                             │
├─────┼────────────────────────────────────────────────────────────┤
│ ch  │ Sat Nov 13 -1 22:16:33 GMT-0800 (Pacific Standard Time)    │
│     │ Mon Nov 13 -11 22:16:33 GMT-0800 (Pacific Standard Time)   │
│     │ Sun, 14 Nov 2 B.C. 06:16:33 GMT                            │
│     │ Tue, 14 Nov 12 B.C. 06:16:33 GMT                           │
└─────┴────────────────────────────────────────────────────────────┘

In Microsoft/ChakraCore#4067 we are addressing the fact that ChakraCore does not pad the year with leading 0's and uses B.C. instead of a negative year. However the spec versus current plurality implementation seems to be unclear.

In our opinion (ChakraCore), SpiderMonkey's output is more true to the spec text.

@xiaoyinl at Microsoft/ChakraCore#4067 (comment) writes:

Section 20.3.4.43 specifies the exact format of toUTCString(). It says:

  1. Let year be the String representation of YearFromTime(tv), formatted as a number of at least four digits, padded to the left with zeroes if necessary.

Because the spec reads that the year should be (emphasis mine):

formatted as a number of at least four digits, padded to the left with zeroes if necessary.

Thus the spec seems to imply the digits should be treated separately (by padding the absolute value of the year out to 4 digits) and then if the year was negative, inserting a - before it.

As I wrote in Microsoft/ChakraCore#4067 (comment):

jsc and d8 behave as printf formatting a negative number with printf("%04d", negativeNumber);, which for int negativeNumber = -3; would print -003. (Java's String.format also agrees with that interpretation of %04d with a negative argument.)

This is what d8 and jsc have done in terms of output, and I'd imagine that the implemented the formatting behavior with something akin to sprintf instead of rolling their own as we've done here.

See discussion here:

Microsoft/ChakraCore#4067 (comment)
Microsoft/ChakraCore#4067 (comment)

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Nov 21, 2017

Member

Before the recent specification, there was disagreement for positive year padding. I changed to the 2/4 semantics, disagreeing with Edge and V8 semantics. IIRC I implemented and shipped this change in V8; I haven't seen any compat bug reports, but maybe @natorion has. If there are no compat issues for that change, I bet you're safe for choosing the SM option here too.

Member

littledan commented Nov 21, 2017

Before the recent specification, there was disagreement for positive year padding. I changed to the 2/4 semantics, disagreeing with Edge and V8 semantics. IIRC I implemented and shipped this change in V8; I haven't seen any compat bug reports, but maybe @natorion has. If there are no compat issues for that change, I bet you're safe for choosing the SM option here too.

@dilijev

This comment has been minimized.

Show comment
Hide comment
@dilijev

dilijev Nov 21, 2017

Contributor

Okay, good to know about safety. However, still wondering if we can clarify the spec text so that there is a normative "correct" format, or explicitly allow certain differences in implementation like this (which seems less good than a single correct format).

Contributor

dilijev commented Nov 21, 2017

Okay, good to know about safety. However, still wondering if we can clarify the spec text so that there is a normative "correct" format, or explicitly allow certain differences in implementation like this (which seems less good than a single correct format).

@dilijev

This comment has been minimized.

Show comment
Hide comment
@dilijev

dilijev Nov 22, 2017

Contributor

For completeness of this discussion, here's current state of formatting positive years:

## Source
let year1 = new Date("0001-10-13T05:16:33Z");
let year11 = new Date("0011-10-13T05:16:33Z");

print(year1.toString());
print(year11.toString());
print(year1.toUTCString());
print(year11.toUTCString());


┌─────┬───────────────────────────────────────────────────────────┐
│ d8  │ Fri Oct 12 0001 22:16:33 GMT-0700 (Pacific Daylight Time) │
│ jsc │ Wed Oct 12 0011 22:16:33 GMT-0700 (Pacific Daylight Time) │
│ sm  │ Sat, 13 Oct 0001 05:16:33 GMT                             │
│     │ Thu, 13 Oct 0011 05:16:33 GMT                             │
├─────┼───────────────────────────────────────────────────────────┤
│ ch  │ Fri Oct 12 1 22:16:33 GMT-0700 (Pacific Daylight Time)    │
│     │ Wed Oct 12 11 22:16:33 GMT-0700 (Pacific Daylight Time)   │
│     │ Sat, 13 Oct 1 05:16:33 GMT                                │
│     │ Thu, 13 Oct 11 05:16:33 GMT                               │
└─────┴───────────────────────────────────────────────────────────┘

Microsoft/ChakraCore#4067 will bring us in line with the current web reality for formatting positive years so that we have 4/4 agreement.

Contributor

dilijev commented Nov 22, 2017

For completeness of this discussion, here's current state of formatting positive years:

## Source
let year1 = new Date("0001-10-13T05:16:33Z");
let year11 = new Date("0011-10-13T05:16:33Z");

print(year1.toString());
print(year11.toString());
print(year1.toUTCString());
print(year11.toUTCString());


┌─────┬───────────────────────────────────────────────────────────┐
│ d8  │ Fri Oct 12 0001 22:16:33 GMT-0700 (Pacific Daylight Time) │
│ jsc │ Wed Oct 12 0011 22:16:33 GMT-0700 (Pacific Daylight Time) │
│ sm  │ Sat, 13 Oct 0001 05:16:33 GMT                             │
│     │ Thu, 13 Oct 0011 05:16:33 GMT                             │
├─────┼───────────────────────────────────────────────────────────┤
│ ch  │ Fri Oct 12 1 22:16:33 GMT-0700 (Pacific Daylight Time)    │
│     │ Wed Oct 12 11 22:16:33 GMT-0700 (Pacific Daylight Time)   │
│     │ Sat, 13 Oct 1 05:16:33 GMT                                │
│     │ Thu, 13 Oct 11 05:16:33 GMT                               │
└─────┴───────────────────────────────────────────────────────────┘

Microsoft/ChakraCore#4067 will bring us in line with the current web reality for formatting positive years so that we have 4/4 agreement.

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Nov 23, 2017

Member

I think you should go for it and specify one of the options normatively! No strong opinion on which one you choose, but it's be great if the spec change/clarification came with test262 tests and an implementation in some engine that didn't have those semantics before. Apologies for my omission of this case previously.

Member

littledan commented Nov 23, 2017

I think you should go for it and specify one of the options normatively! No strong opinion on which one you choose, but it's be great if the spec change/clarification came with test262 tests and an implementation in some engine that didn't have those semantics before. Apologies for my omission of this case previously.

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Nov 23, 2017

Member

The spec. should also be explicit about the handling of the minus sign for negative years.

I think the thing that makes most sense is 0 padding to 4 digits for year absolute values < 1000 and for negative years to explicit prepend a "-" to the (possibly padded) year.

Member

allenwb commented Nov 23, 2017

The spec. should also be explicit about the handling of the minus sign for negative years.

I think the thing that makes most sense is 0 padding to 4 digits for year absolute values < 1000 and for negative years to explicit prepend a "-" to the (possibly padded) year.

@dilijev

This comment has been minimized.

Show comment
Hide comment
@dilijev

dilijev Nov 27, 2017

Contributor

@allenwb That is our opinion as well.

Contributor

dilijev commented Nov 27, 2017

@allenwb That is our opinion as well.

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