mozilla / rhino

Rhino is an open-source implementation of JavaScript written entirely in Java
https://rhino.github.io
Other
4.18k stars 850 forks source link

let variable in loop scope retains previous iteration value #1386

Open szegedi opened 1 year ago

szegedi commented 1 year ago
(function() { for(let i = 0; i < 2; ++i) { let x; console.log(x); x=1;} })()

prints undefined then 1. Same code in V8 prints undefined twice as expected. For a quick shell repro:

./gradlew assemble
echo "(function() { for(let i = 0; i < 2; ++i) { let x; console.log(x); x=1;} })()" | java -jar ./buildGradle/libs/`ls ./buildGradle/libs | grep -v -e runtime -e engine -e sources` 

Investigating the issue, the JS bytecode created is:

 [0] LINE : 6
 [3] ZERO
 [4] SETVAR1 0
 [6] POP
 [7] GOTO 27
 [10] REG_STR_C0
 [11] NAME
 [12] REG_STR_C1
 [13] PROP_AND_THIS
 [14] GETVAR1 1
 [16] REG_IND_C1
 [17] CALL 1
 [18] POP
 [19] ONE
 [20] SETVAR1 1
 [22] POP
 [23] REG_IND_C0
 [24] VAR_INC_DEC 0
 [26] POP
 [27] GETVAR1 0
 [29] SHORTNUMBER 2
 [32] LT
 [33] IFEQ 10
 [36] RETUNDEF

We see there's a ONE; SETVAR1 1 to set the value to 1 after the console.log call, but the original let x; resulted in no code.

The problem most likely lies in NodeTransformer that'll entirely transform away an initializer-less (LET (NAME x)) AST node instead of initializing the variable to undefined.

p-bakker commented 1 year ago

Am working on #939 and attempting to make the const impl. use the existing let implementation as in EcmaScript they are exactly the same scope-wise, and the only main difference being consts having to get initialized on declaration and being readOnly from that point forward

In the process I might already fix this issue as well, although so far I've just been fixing it in interpreted mode, while I think you're trying it compiled?

szegedi commented 1 year ago

Hi Paul, good to hear you're working on this!

I tried it both in interpreted and compiled. I think the AST transformations in question take place in the common path for both interpreted and compiled code generation. This came to my attention through some folks that use Rhino for their commercial product, and they're using it in interpreted mode 'cause they use continuations, so if you first fix it in interpreted, that's great!