343765 - Function defined in a let statement/expression doesn't work correctly outside the let scope

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[nanto_vi (TOYAMA Nao)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; ja; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060706 Minefield/3.0a1

If a function contains variables boud by a let statement/expression, the variables' values change after leaving the let scope.


Reproducible: Always

Steps to Reproduce:
js> let (a = 2, b = 3) { function f() { return [a, b]; } f(); }
2,3
js> f();
Actual Results:  
function () {
    return [a, b];
},[object global]

Expected Results:  
2,3

This bug blocks such a operation:

var divs = document.getElementById("div");
for (let i = 0; i < divs.length; i++)
  divs[i].onclick = 
    let (div = divs[i]) function () { alert(div.id + " is clicked."); };

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Ugh, what does 'at top level' mean anyway?

Comment 2

[nanto_vi (TOYAMA Nao)]

Sorry, I made typos.

> If a function contains variables boud by a let statement/expression,
variables bound by

> var divs = document.getElementById("div");
var divs = document.getElementsByTagName("div");

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix, v1

We needed to clear the block objects' private data's and update their slots with the current values when popping blocks (both in the normal JSOP_LEAVEBLOCK case and in JSOP_SETSP for exceptions).

Brendan and I also noticed that the condition for breaking out of the scopechain popping loop was incorrect, the block depth needed to be strictly less than the new stack depth to stay on the scope chain.

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 228328 [details] [diff] [review]
Fix, v1

Kind of a joint patch, so shaver's sr= is more meaningful here.  And Mike wrote JSOP_SETSP, long ago (and we jointly updated it to handle with scope popping belatedly ;-).

/be

Comment 5

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 228328 [details] [diff] [review]
Fix, v1

sr=shaver

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix, v2

The previous patch missed the case where returning from the midst of a block (let expression, etc.) would fail to clean up the block objects on the scope chain.

Comment 7

[Brendan Eich [:brendan]]

Comment on attachment 228343 [details] [diff] [review]
Fix, v2

r=me again!

/be

Comment 8

[Brendan Eich [:brendan]]

The v2 patch emerged when I pointed out the return environment-capturing function from block case, and mrbkap had been nicely writing up testcase in a single file. We need those attached here so bc can harness them.

/be

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk.

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: testcases

Comment 11

[nanto_vi (TOYAMA Nao)]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060706 Minefield/3.0a1 Build ID: 2006070614
The result of URL-testcase is still incorrect.

Comment 12

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Oops, missed that one

Sorry, I didn't see the testcase in the URL field. It's an easy fix, though: we can't store the result in the stack before we call js_PutBlockObject.

Comment 13

[Brendan Eich [:brendan]]

Comment on attachment 228351 [details] [diff] [review]
Oops, missed that one

I was looking on, again.  Should have seen this last time.  Shaver, send your tachyons!

/be

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

Oops, this was checked in at 2006-07-06 15:46.

Comment 15

[Bob Clary [:bc] (inactive)]

(In reply to comment #14)
> Oops, this was checked in at 2006-07-06 15:46.

Blake, it looks like this was fixed on 1.8 due to the js1.7 landing. If that is not correct, please remove the fixed1.8.1 kw. Thanks.

Checking in regress-343765.js;
/cvsroot/mozilla/js/tests/js1_7/block/regress-343765.js,v  <--  regress-343765.js
initial revision: 1.1

Comment 16

[Bob Clary [:bc] (inactive)]

verified fixed 1.8.1, trunk 20060723 win/linux/mac(ppc|tel)