71107 - lexical closure scoping regression in js breaks ldap datasource

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Dan Mosedale (:dmosedale, :dmose)]

From IRC:

<peterv> dmose: seems the inner objects don't see the variables in the
CreateDelegate function anymore
<peterv> dmose: JavaScript Error: "connection is not defined" {file: 
"nsLDAPDataSource.js" line: 831}

Looking at the code, it seems possible that the scope chain of the callback
function is wrong (ie not lexically closed).

Comment 1

[Mike Shaver (:shaver emeritus)]

Yeah, we lose.  We don't propagate heavyweightness back up the function-nesting
chain when we set it in js_EmitTree/TOK_FUNCTION.

Simple test case:

function outer () {
  var outer_var = 5;
  function inner() {
    function way_inner() {
      return outer_var;
    }
    return way_inner;
  }
  return inner;
}

js> outer()()()
/tmp/nest.js:5: ReferenceError: outer_var is not defined

Expected:

js> outer()()()
5

Patch coming right up.

Comment 2

[Mike Shaver (:shaver emeritus)]

Attachment: Fix: propagate emit-time heavyweighting up the nesting chain

Comment 3

[Mike Shaver (:shaver emeritus)]

I crave brendan's r and someone's sr.  Phil, can you add this baby to our test
suite?

Comment 4

[Brendan Eich [:brendan]]

r/sr=[e-mail].

D'oh!

/be

Comment 5

[Phil Schwartau]

Yes, I will add this test now. Thanks, Mike - 

Comment 6

[Brendan Eich [:brendan]]

Need fast r= for JS1.5 RC3.

/be

Comment 7

[Phil Schwartau]

Mike's test added to JS test suite: 

            js/tests/js1_5/Regress/regress-71107.js

Comment 8

[Phil Schwartau]

Just ran the entire JS test suite on WinNT and Linux with Mike's patch applied.
Suite now includes the regression test for this bug, above. 

I got 0 errors - 

Comment 9

[John Bandhauer]

r=jband

Comment 10

[Mike Shaver (:shaver emeritus)]

All better.  Sorry about that, dmose.

Comment 11

[Phil Schwartau]

Also ran the new regression test on the Mac - passed. 
Marking VERIFIED -