395868 - eval'ed local function should replace existing var with deletable binding

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect, P3)

Description

[mozilla]

In the following program the alert should print "global" and not "0".
===
var x = "global";
function f() {
  var x = 0;
  eval("function x() { return false; }");
  delete x;
  alert(x);
}
f();
===
Firefox makes the new function x (added by the eval) correctly 'deletable', but still seems to leave the old 'x' somewhere. (btw Firefox was the only browser which actually made the evaled 'x' deletable).

Relevant sections of the spec are: 10.1.3 where it states: "... If the
variable object already has a property with this name, replace its value
and attributes",
and 10.2.2 with "Variable instantiation is performed using the calling
context's variable object and using empty property attributes".

Comment 1

[AKS]

There is another problem with it.

===
function f() {
    var x = 0;
    
    eval('function x() {}'); 
    
    function check() {
        return typeof x;
    };
    return [check, check()];
    
};
alert([f()[0](), f()[1]]); // -> number, function (Gecko) vs function, function (others) 

===

Thus x becomes function, but this function is accessible only in a code of outer function (not directly, but through a wrapper).

===

(function () {
    var x = 0;
    
    eval('function x() { alert("X!"); }'); 
    
    function check() {
        return x();
    };
    // x(); -> error 
    
    check(); // calls function x and returns its result
    
})();

===

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Why'd two different people file the same bug in the same day? :)

Comment 3

[AKS]

(In reply to comment #2)
> Why'd two different people file the same bug in the same day? :)
> 

I have tried to show a problem completely. I have shown on an example, that function x is accessible in a context of the nested functions, but not is itself. Is there there something similar (in bug 395907)?
I have spent some time to describe a problem as needed. Is it funny?

Comment 4

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #3)
> I have spent some time to describe a problem as needed. Is it funny?

No, nothing funny. I just found it strange that two people filed nearly the same bug on the same day, out of the blue. I'm guessing the problem was probably discussed in the newsgroup, or something. Either way, I've marked yours as a duplicate of bug 395907 because it describes the same issue, and that bug was already in the correct component.

Comment 5

[mozilla]

This bug is not a duplicate of Bug 395907. The test-case still does not yield the correct result. (At least if I read the spec correctly).

Comment 6

[Brendan Eich [:brendan]]

Florian: right you are. Patch anon.

/be

Comment 7

[Brendan Eich [:brendan]]

Note bug was filed in wrong product.

/be

Comment 8

[Brendan Eich [:brendan]]

(In reply to comment #0)
> Firefox makes the new function x (added by the eval) correctly 'deletable', 

No, delete x is just compiled as false (the false value).

> but
> still seems to leave the old 'x' somewhere. (btw Firefox was the only browser
> which actually made the evaled 'x' deletable).

How did you detect that the eval'ed function x deleteable?

/be

Comment 9

[Brendan Eich [:brendan]]

I'm wondering if this bug should be WONTFIXed. ECMA-262 was not intended to force deoptimization of local variables, such that you could delete the function created by eval and thereafter not find the var by its name (optimized to a stack slot).

/be

Comment 10

[mozilla]

(In reply to comment #8)
> (In reply to comment #0)
> > Firefox makes the new function x (added by the eval) correctly 'deletable', 
> 
> No, delete x is just compiled as false (the false value).
> 
> > but
> > still seems to leave the old 'x' somewhere. (btw Firefox was the only browser
> > which actually made the evaled 'x' deletable).
> 
> How did you detect that the eval'ed function x deleteable?
> 
> /be
> 
I was wrong at that time. I just noticed, that the value was not "function x...", and assumed, that the 'function...' had been deleted.

Comment 11

[mozilla]

(In reply to comment #9)
> I'm wondering if this bug should be WONTFIXed. ECMA-262 was not intended to
> force deoptimization of local variables, such that you could delete the
> function created by eval and thereafter not find the var by its name (optimized
> to a stack slot).
> 
> /be
> 
It is really annoying, but not _that_ bad, as one can restrict the usage of 'eval' (as in 15.1.2.1). Generally this poses problems with some websites, though. Also one could allow to remove local variables, if the eval, is explicitely used (15.1.2.1), but not, if the eval function was used indirectly.

Comment 12

[mozilla]

(In reply to comment #7)
> Note bug was filed in wrong product.
> 
> /be
> 
which explains why I didn't find the JS component. mea culpa.

Comment 13

[Brendan Eich [:brendan]]

(In reply to comment #10)
> I was wrong at that time. I just noticed, that the value was not "function
> x...", and assumed, that the 'function...' had been deleted.

How many browsers pass your test (fixed to assert that alert(x) => "global")?

/be

Comment 14

[mozilla]

(In reply to comment #13)
> (In reply to comment #10)
> > I was wrong at that time. I just noticed, that the value was not "function
> > x...", and assumed, that the 'function...' had been deleted.
> 
> How many browsers pass your test (fixed to assert that alert(x) => "global")?
ATM none. But I have submitted bug-reports to all the others (except for Safari) as well. And I have no access to IE. So I really don't have any idea what's happening there...
// florian

Comment 15

[Brendan Eich [:brendan]]

Throwing back to the pool. This is low priority, if not WONTFIX. As no other browser does it, and in view of the risk of deoptimizing current Mozilla chrome JS that (a) calls eval (but does not replace a local var with a function), and (b) uses local vars heavily, I have no desire to try to fix it any time soon.

The ES4 spec may change to reflect reality, in which case this bug would be INVALID.

/be

Comment 16

[Hallvord R. M. Steen]

Is this really a violation of ES3? Quoting my own comment on Florian's corresponding Opera bug report:

This doesn't seem to be about eval. All browsers I tested agree that "delete x" returns false and has no effect. This is true even if we remove the eval and leave just the "var x=0" statement. 

"delete" will *only* return false if you try to delete something that has dontDelete attribute set. (Per spec. I don't know how to test that we implement it correctly :-p.) So are local variables inside a function dontDelete? See ECMAscript spec chapter 12.2 which ambiguously states:

-----8<--------------
If the variable statement occurs inside a FunctionDeclaration, the variables are defined with function-local scope in that function, as described in s10.1.3. Otherwise, they are defined with global scope (that is, they are created as members of the global object, as described in 10.1.3) using property attributes { DontDelete }. 
-----8<--------------

..so it says clearly that global, user-defined variables are dontDelete, but what about local ones? As all UAs I tested (O,IE,FF) agree on not deleting them I guess that's the interpretation we've collectively selected.

Comment 17

[mozilla]

first let me state, that I'm perfectly happy with the "wontfix" solution.

(In reply to comment #16)
> Is this really a violation of ES3?
I think the spec is broken here. yes.

> Quoting my own comment on Florian's
> corresponding Opera bug report:
> 
> This doesn't seem to be about eval. All browsers I tested agree that "delete x"
> returns false and has no effect. This is true even if we remove the eval and
> leave just the "var x=0" statement. 
it's about eval. The problem is, that according to the spec function declarations should replace any existing entry in the scope.
Furthermore 'eval'ed variables do _not_ have attribute 'dontdelete'.
As example see:
(function() { eval("var x = 3"); print(x); delete(x); print(x); })();

a function declared within an 'eval' therefore replaces any existing variable with a variable _not_ having attribute 'dontdelete'.
Thus it can then be deleted.

> "delete" will *only* return false if you try to delete something that has
> dontDelete attribute set. (Per spec. I don't know how to test that we implement
> it correctly :-p.) So are local variables inside a function dontDelete?
Yes they are. (It's clearly defined). But the problem is, that the original variable is replaced.

[SKIP]
> ..so it says clearly that global, user-defined variables are dontDelete, but
> what about local ones? As all UAs I tested (O,IE,FF) agree on not deleting them
It's not just local variables. The same happens with global variables. A function declared inside an eval (executed at top-level) can replace any variable.

> I guess that's the interpretation we've collectively selected.
deleting local variables is ugly, and not following the spec is in this case probable better.