Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

let variable in loop scope retains previous iteration value #1386

Open
szegedi opened this issue Sep 7, 2023 · 2 comments
Open

let variable in loop scope retains previous iteration value #1386

szegedi opened this issue Sep 7, 2023 · 2 comments
Labels
Ecma Incompatibility Issues about Rhino being incompatible with the EcmaScript spec

Comments

@szegedi
Copy link
Contributor

szegedi commented Sep 7, 2023

(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
Copy link
Collaborator

p-bakker commented Sep 7, 2023

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
Copy link
Contributor Author

szegedi commented Sep 7, 2023

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!

@p-bakker p-bakker added the Ecma Incompatibility Issues about Rhino being incompatible with the EcmaScript spec label Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ecma Incompatibility Issues about Rhino being incompatible with the EcmaScript spec
Projects
None yet
Development

No branches or pull requests

2 participants