342359 - overriding ReferenceError doesn't stick

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Joey Minta]

As reported by shaver/mrbkap:
js> ReferenceError = 5
5
js> ReferenceError    
5
js> foo.blitz
typein:4: ReferenceError: foo is not defined
js> ReferenceError
function ReferenceError() {
     [native code]
}

Also visible on JS 1.7 alpha branch.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix

I blame Mccabe for using 1 class for all of the exception classes. If he didn't, we'd be able to use JS_InitClass directly, instead of hand-unrolling it and forgetting to actually set the constructor value in the global object. This patch makes sure we do that.

Comment 2

[Brendan Eich [:brendan]]

Comment on attachment 226562 [details] [diff] [review]
Fix

Looks good -- thanks for fixing.

/be

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Fixed.

Comment 4

[Bob Clary [:bc] (inactive)]

I still see this in the browser 20060622 trunk build when evaluating the test in Jesse's jsshell.

Comment 5

[Bob Clary [:bc] (inactive)]

If you do it twice, it appears to stick...

ReferenceError = 5
5
foo.blitz
ReferenceError on line 1: foo is not defined
ReferenceError
function ReferenceError() { [native code] }
ReferenceError = 5
5
foo.blitz
ReferenceError on line 1: foo is not defined
ReferenceError
5

Comment 6

[Bob Clary [:bc] (inactive)]

Attachment: js1_5/Exceptions/regress-342359.js

But trunk appears to pass this test. If the failure is just a problem with using the jsshell, we can mark this fixed again.

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Make that work too

Resolve implementations have to make sure they initialize standard classes even when assigning so that given the above testcase, we initialize the exception classes before we assign to the name ReferenceError. Otherwise when the class is defined, it'll overwrite the names.

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 226701 [details] [diff] [review]
Make that work too

Sure.  Anything else to comment or bracket with not-assigning guards in nsDOMClassInfo.cpp?

/be

Comment 9

[Johnny Stenback (:jst)]

Comment on attachment 226701 [details] [diff] [review]
Make that work too

sr=jst

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

Really fixed this time.

Comment 11

[Bob Clary [:bc] (inactive)]

verified fixed 1.9a1 20060630 win/mac(ppc|tel)/linux

Comment 12

[Bob Clary [:bc] (inactive)]

fixed by Bug 336373 on the 1.8.1 branch. 
verified fixed 1.8.1 with windows/macppc/linux 20060707

Comment 13

[Bob Clary [:bc] (inactive)]

forgot to check this in.

Checking in regress-342359.js;
/cvsroot/mozilla/js/tests/js1_5/Exceptions/regress-342359.js,v  <--  regress-342359.js
initial revision: 1.1

Comment 14

[Bob Clary [:bc] (inactive)]

(In reply to comment #13)

> /cvsroot/mozilla/js/tests/js1_5/Exceptions/regress-342359.js,v  <-- 

this now fails on trunk due to bug 376957.

Comment 15

[Bob Clary [:bc] (inactive)]

Checking in regress-342359.js;
/cvsroot/mozilla/js/tests/js1_5/Exceptions/regress-342359.js,v  <--  regress-342359.js
new revision: 1.4; previous revision: 1.3
done