325951 - Simplify jsconfig.h

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Brendan Eich [:brendan]]

This file goes back more than 8 years, and we don't need the complexity it brings. Proposals:

* Get rid of all the JS_BUG_* ancient history macros.
* Drop support for JS1.4 and earlier.
* Eliminate JS_HAS_CALL_OBJECT and other must-haves.

Comments?

/be

Comment 1

[Brendan Eich [:brendan]]

I'll want to fix this before landing iterators and generators, to avoid impossible configurability for things such as JS_HAS_EXCEPTIONS (which will be required by the for-in loop code, period full stop).

/be

Comment 2

[Brendan Eich [:brendan]]

Attachment: proposed fix

Lots of systematic changing, by hand alas (Dave Yost's unifdef can't handle #if, and my old SGI unifdef which could is apparently lost behind SGI's firewall). Get out your rubber stamp! ;-)

/be

Comment 3

[Brendan Eich [:brendan]]

Attachment: diff -w version of last patch

This is easier to read in places.

/be

Comment 4

[Bob Clary [:bc] (inactive)]

Attachment: spidermonkey-n.tests.patch

obsolete the js1_1,...js1_4 tests.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 219820 [details] [diff] [review]
proposed fix

In all likely hood, I'm missing something, but this looks fine.

Comment 6

[Brendan Eich [:brendan]]

Bob, do we have split tests for the ECMA-262 Edition 3 behavior elsewhere, and so on?  Also, the *proto* tests under js1_3 -- are they __proto__ and therefore something we want to test still?  In general I'm concerned that tests were put under a js1_* directory that, in light of the final standard, should be under an ecma3 directory.

/be

Comment 7

[Brendan Eich [:brendan]]

Attachment: version that I'm checking in

I turned off -pu8 for my diff options in .cvsrc, generated the flat diff, deleted all lines matching /^< #if JS_/ and /^< #endif/, went through the residuum by hand and found it all good.

/be

Comment 8

[Bob Clary [:bc] (inactive)]

(In reply to comment #6)
> Bob, do we have split tests for the ECMA-262 Edition 3 behavior elsewhere, and
> so on?  

"split tests" ? as in String.prototype.split? Most of those are in ecma/String and ecma_2/String.

> Also, the *proto* tests under js1_3 -- are they __proto__ and therefore
> something we want to test still? 

Yes, several _proto_ related tests are in js1_3/inherit/

>  In general I'm concerned that tests were put
> under a js1_* directory that, in light of the final standard, should be under
> an ecma3 directory.

There are 121 test files in js1_*. 85 of those are in js1_2 which I doubt we care about many of them. 12 of the 24 tests in js1_3 relate to inheritance with the others scattered around.

I could run the tests without setting the version and only remove those that fail when not run with the appropriate version (e.g javascript1.2 for js1_2). That would eliminate the version dependent results while retaining the tests for daily testing. Ok?

Comment 9

[Brendan Eich [:brendan]]

(In reply to comment #8)
> I could run the tests without setting the version and only remove those that
> fail when not run with the appropriate version (e.g javascript1.2 for js1_2).
> That would eliminate the version dependent results while retaining the tests
> for daily testing. Ok?

That sounds like a great idea.

/be

Comment 10

[Bob Clary [:bc] (inactive)]

(In reply to comment #8)
> There are 121 test files in js1_*.
That should have read 121 tests files in js1_1 - js1_4.

(In reply to comment #8)
Will do.

Comment 11

[Brendan Eich [:brendan]]

Fix checked in.

/be

Comment 12

[Brendan Eich [:brendan]]

Attachment: missing jskeyword.tbl patch + jsparse.c fix

#if math is hard, let's go shopping.

Should be all better in a tinder-cycle.

/be

Comment 13

[Igor Bukanov]

Comment on attachment 219939 [details] [diff] [review]
missing jskeyword.tbl patch + jsparse.c fix

>--- jskeyword.tbl	26 Jan 2006 08:47:50 -0000	3.3
>-#if JS_HAS_INSTANCEOF
> JS_KEYWORD(instanceof,  TOK_INSTANCEOF, JSOP_INSTANCEOF,JSVERSION_1_4)
>-#else
>-JS_KEYWORD(instanceof,  TOK_RESERVED,   JSOP_NOP,       JSVERSION_1_4)
>-#endif
> 

Should JSVERSION_1_4 be changed to JSVERSION_DEFAULT since JSVERSION_1_4 is no longer supported and similarly for export, enum and debugger? And since AFAIK the keyword set is not going to be extended with JS2.0, perhaps it makes sence to remove the version argument and the checks in the scanner alltogether?

Comment 14

[Brendan Eich [:brendan]]

We're currently thinking about "unreserving" keywords in property identifier contexts (in object initialisers, and after the . and .. operators).  But we may need to reserve some operators for Edition 4, so I think we'll want the keyword version facility.

Igor, could you patch away the low-numbered versions if you are so inclined, or at least file a bug for later?  I've got a full plate and am trying to move on (after I torched the tree twice today -- what a vandal!).  Thanks for any help.

/be

Comment 15

[Bob Clary [:bc] (inactive)]

Attachment: spidermonkey-n.tests.patch

Checking in spidermonkey-n.tests;
/cvsroot/mozilla/js/tests/spidermonkey-n.tests,v  <--  spidermonkey-n.tests
new revision: 1.9; previous revision: 1.8
done

Comment 16

[Igor Bukanov]

Attachment: JSVERSION_DEFAULT for all keywords

Comment 17

[Igor Bukanov]

Attachment: Removal of JS_HAS_SPARSE_ARRAYS

Even before config changes JS_HAS_SPARSE_ARRAYS was always defined to 0 and was not used anywhere outside jsconfig.c. This patch removes this. 

There is another define, JS_HAS_FUN_EXPR_STMT, that is not used outside jsconfig.h. But at least it is defined to be different depending on the version even after the config changes. Is it a bug that jsparse.c does not check for it?



The config cleanups made JS_HAS_SPARSE_ARRAYS to be defined always as 0.  Since it is no longer used,

Comment 18

[Brendan Eich [:brendan]]

The patch to jsarray.c regressed splice, see bug 326466 comment 35.

I promise not to inflict runtime versionitis on the engine, ever again.

/be

Comment 19

[Bill Gianopoulos [:WG9s]]

FYI. This appears to have broken IE View.  ANnattempt to launch IE via IE View now results in a pop-up saying "TypeError: arguments.push is not a function".

I filed IE View bug number 14065 on this issue.

Comment 20

[Blake Kaplan (:mrbkap) (inactive)]

That sounds like bug 336100.

Comment 21

[Brendan Eich [:brendan]]

(In reply to comment #20)
> That sounds like bug 336100.

Which was not due to this bug, but to bug 304376, right?

/be

Comment 22

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to comment #21)
> Which was not due to this bug, but to bug 304376, right?

A combination: this patch made js_CallClass use JSCLASS_IS_ANONYMOUS (introduced in that bug).

Comment 23

[Brendan Eich [:brendan]]

(In reply to comment #22)
> (In reply to comment #21)
> > Which was not due to this bug, but to bug 304376, right?
> 
> A combination: this patch made js_CallClass use JSCLASS_IS_ANONYMOUS
> (introduced in that bug).

Why don't we back that one line change out, for now?  r=me, let's help unregress for the moment.  We should track this flappage with a separate bug.

/be

Comment 24

[Bill Gianopoulos [:WG9s]]

(In reply to comment #23)
> (In reply to comment #22)
> > (In reply to comment #21)
> > > Which was not due to this bug, but to bug 304376, right?
> > 
> > A combination: this patch made js_CallClass use JSCLASS_IS_ANONYMOUS
> > (introduced in that bug).
> 
> Why don't we back that one line change out, for now?  r=me, let's help
> unregress for the moment.  We should track this flappage with a separate bug.
> 
I verified with my own build that backing out that one line does fix the IE View issue.
> /be
>