348685 - Let-scoped variables in for loop can be referenced outside the loop

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[nanto_vi (TOYAMA Nao)]

Using let-scoped for loop variables outside the loop doesn't throw a ReferenceError exception under some conditions:

- In a function
- Repeat the loop more than one time
- Use let declarations/statements/expressions in the loop body
- Define functions or use function expressions in the loop body

And the variable is evalueated to:

- Last assigned value in the for loop
- A function that uses the variable as a argument
- Undefined if the variable is used as a argument and returned

js> function f() {
  for (let i = 0; i < 2; i++) { let j = 42; function g() {} }
  var a = i;
  print(a);
  print(i);
  return i;
}
js> print(f());
2
function print() {
    [native code]
}
undefined

Comment 1

[Brendan Eich [:brendan]]

Argh -- once we clone block objects and clear fp->blockChain, we must never again set it.  Otherwise, we end up with duplicate clones on fp->scopeChain after the second iteration!

/be

Comment 2

[Brendan Eich [:brendan]]

Attachment: fix

Comment says it all.

/be

Comment 3

[Brendan Eich [:brendan]]

If we kept count of cloned blocks on the scope chain, or counted remaining ones on every LEAVEBLOCK, we could clear JSFRAME_POP_BLOCKS and do better for cases that open new, unrelated blocks later, after a block contains a closure or other scope capturing construct.  Not sure this is worth optimizing.  We could optimize nested functions harder to avoid capturing run-time scope, too.

/be

Comment 4

[Brendan Eich [:brendan]]

Attachment: with correct assertion

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 233719 [details] [diff] [review]
with correct assertion

Seems reasonable.

Comment 6

[Brendan Eich [:brendan]]

Fixed on trunk, already.

/be

Comment 7

[Bob Clary [:bc] (inactive)]

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

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 233719 [details] [diff] [review]
with correct assertion

This is ready.

/be

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 233719 [details] [diff] [review]
with correct assertion

a=beltzner, on behalf of drivers for the 18branch

Comment 10

[Brendan Eich [:brendan]]

Fixed on the 1.8 branch.

/be

Comment 11

[Bob Clary [:bc] (inactive)]

verified fixed 1.8 20060818 windows/mac*/linux
not verified trunk crashed opt/dbg all platforms. I'll deal when I have fresh builds.

Comment 12

[Bob Clary [:bc] (inactive)]

verified fixed 1.9 20060824 windows/mac*/linux