54684 - JS strict option should complain about var/function redecl across compilation units

Status: VERIFIED WONTFIX

(Core :: JavaScript Engine, defect, P3)

Description

[Brendan Eich [:brendan]]

var x=42; function x(){} and vice versa should get a strict warning.  This is a 
simple patch that alters behavior only if the javascript.options.strict pref, or 
the window.options.strict property, is set.  It helps JS authors find dubious 
practice in their scripts.  I'm going to mark js1.5 and rtm.

/be

Comment 1

[Brendan Eich [:brendan]]

Attachment: proposed fix

Comment 2

[Brendan Eich [:brendan]]

Urk, duplicated warning in the case of redeclaration within the same compilation 
unit, because the compiler checks this strict warning condition properly already 
(and gives a pointer to the offending token, a better diagnostic).  But for the 
case where the var and/or function declaring the same name occur in different 
compilation units (eval takes a string as "compilation unit"), the patch should 
take effect.  I'll try to come up with a patch that avoids duplicated warnings.

/be

Comment 3

[Brendan Eich [:brendan]]

Changing summary, removing premature rtm.

/be

Comment 4

[Brendan Eich [:brendan]]

"Didn't know my own strenth -- watch me pull a rabbit out of my hat!"

Ok, so I covered the same-compilation-unit case well when I implemented strict 
and const.  The two-compilation-units case is hard: ideally we have some notion 
of "compilation unit identifier" that can be associated cheaply with a property, 
and we can compare cuids to decide to whine at runtime.  Of course, properties 
have no such cuid.

Now there is this tempting uint8 spare; field in JSScopeProperty, and it could 
count compilation units, although it's a bit small(!), and it exists only for 
native objects (most JS objects in Mozilla use js_ObjectOps.newObjectMap, OTOH, 
so almost all objects have JSScopeProperty-flavored properties in them).

A better idea: in js_CheckRedeclaration, run through the current script's prolog 
and if the atom being declared/defined is not found before the current pc, then 
do the run-time strict warning.  I may hack up a patch to do this.  It'll make 
strict mode slower with some O(n^2)-growth-rate cost (scanning bytecodes and 
getting atom immediates).

I'm about to WONTFIX this.  Thought I had something, and it turned out mostly to 
work already, with the remaining case being "hard to fix cheaply".

/be

Comment 5

[Brendan Eich [:brendan]]

And (of course) the reason I didn't notice var x=42; function x(){} getting a 
strict warning is that you have to use -w to tell the shell to report warnings 
in the first place.  But there's a bug on that, already (right?).

/be

Comment 6

[Mike McCabe]

There is - 41872.  Think I'll look at it now.

Comment 7

[Brendan Eich [:brendan]]

Uncle.

/be

Comment 8

[Phil Schwartau]

Marking Verified -

Comment 9

[Brendan Eich [:brendan]]

*** Bug 68477 has been marked as a duplicate of this bug. ***