891974 - Test ecma_3/Date/15.9.5.5-02.js fails

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Norbert Lindenberg]

The test ecma_3/Date/15.9.5.5-02.js fails when the ECMAScript Internationalization API is enabled. Error message:

4/19/271822, 4:07:02 PM
 FAILED! [reported from test()] Date.prototype.toLocaleString should not clamp year: check toLocaleString : Expected value '-271821', Actual value '271822' 

The expected value depends on whether the API is enabled, and apparently the test for whether it is doesn't work anymore:
  if (this.hasOwnProperty("Intl")) {
Replacing this with
  if (true) {
makes the test pass with the API enabled.

Comment 1

[Norbert Lindenberg]

If I add a line
  print("this: " + this + "; this.Intl: " + this.Intl);
before
  if (this.hasOwnProperty("Intl")) {
two things happen:
- I get output:
  this: [object global]; this.Intl: [object Object]
- the test passes.

So apparently hasOwnProperty() no longer triggers the initialization of the property that it's checking.

Comment 2

[Norbert Lindenberg]

hg bisect blames this change set:
https://hg.mozilla.org/mozilla-central/rev/39c322f034cc

Comment 3

[Jeff Walden [:Waldo]]

That's seriously whacked, if that bisect is correct.

I stepped through the code a bit to see if I could figure this out, and it looks like somehow the Intl class is getting initialized, but it's not installing the Intl property.  Or something.  My mind has just about turned to mush today, so I'll look into this more tomorrow.

Comment 4

[Norbert Lindenberg]

The changes in the change set that trigger the failure are:

-DateTimeFormat(JSContext *cx, CallArgs args, bool construct)
+DateTimeFormat(JSContext *cx, CallArgs args)
 {
     RootedObject obj(cx);
+    bool construct = args.isConstructing();

@@

 JSBool
 js::intl_DateTimeFormat(JSContext *cx, unsigned argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     JS_ASSERT(args.length() == 2);
-    return DateTimeFormat(cx, args, true);
+    return DateTimeFormat(cx, args);
 }

intl_DateTimeFormat is not a normal constructor; it's an intrinsic for self-hosted JavaScript code. Such intrinsics cannot be called with "new", so args.isConstructing() cannot detect their constructorness. The change means that calls from intl_DateTimeFormat now go into the !construct branch in DateTimeFormat, which includes a call cx->global()->getOrCreateIntlObject(cx), which causes the semi-initialization of Intl. The call still ends up constructing a valid DateTimeFormat object because that's what the Intl constructors do when called with undefined as the this value.

The test case happens to call Date.prototype.toLocaleString before this.hasOwnProperty("Intl"). Date.prototype.toLocaleString is the one function that actually uses intl_DateTimeFormat.

Comment 5

[Norbert Lindenberg]

Attachment: Partial rollback of the change set that broke the test

Comment 6

[Steve Fink [:sfink] [:s:]]

Yeah, that's the part that looked fishy to me when I reread the patch. But I'll let Waldo confirm.

Comment 7

[Jeff Walden [:Waldo]]

Comment on attachment 776918 [details] [diff] [review]
Partial rollback of the change set that broke the test

Review of attachment 776918 [details] [diff] [review]:
-----------------------------------------------------------------

Ugh, right.  I should have seen this.  :-\

Comment 8

[Jeff Walden [:Waldo]]

Comment on attachment 776918 [details] [diff] [review]
Partial rollback of the change set that broke the test

https://hg.mozilla.org/integration/mozilla-inbound/rev/552d924e65fc

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/552d924e65fc