瀏覽代碼

LibJS: Limit scope of 'for' loop variables

This required 2 changes:
1. In the parser, create a new variable scope, so the variable is
   declared in it instead of the scope in which the 'for' is found.
2. On execute, push the variable into the newly created block. Existing
   code created an empty block (no variables, no arguments) which
   allows Interpreter::enter_scope() to skip the creation of a new
   environment, therefore when the variable initializer is executed, it
   sets the variable to the outer scope. By attaching the variable to
   the new block, the block gets a new environment.

This is only needed for 'let' / 'const' declarations, since 'var'
declarations are expected to leak.

Fixes: #2103
Yonatan Goldschmidt 5 年之前
父節點
當前提交
b184f12aaf
共有 3 個文件被更改,包括 35 次插入0 次删除
  1. 3 0
      Libraries/LibJS/AST.cpp
  2. 10 0
      Libraries/LibJS/Parser.cpp
  3. 22 0
      Libraries/LibJS/Tests/for-scopes.js

+ 3 - 0
Libraries/LibJS/AST.cpp

@@ -251,6 +251,9 @@ Value ForStatement::execute(Interpreter& interpreter) const
 
 
     if (m_init && m_init->is_variable_declaration() && static_cast<const VariableDeclaration*>(m_init.ptr())->declaration_kind() != DeclarationKind::Var) {
     if (m_init && m_init->is_variable_declaration() && static_cast<const VariableDeclaration*>(m_init.ptr())->declaration_kind() != DeclarationKind::Var) {
         wrapper = create_ast_node<BlockStatement>();
         wrapper = create_ast_node<BlockStatement>();
+        NonnullRefPtrVector<VariableDeclaration> decls;
+        decls.append(*static_cast<const VariableDeclaration*>(m_init.ptr()));
+        wrapper->add_variables(decls);
         interpreter.enter_scope(*wrapper, {}, ScopeType::Block);
         interpreter.enter_scope(*wrapper, {}, ScopeType::Block);
     }
     }
 
 

+ 10 - 0
Libraries/LibJS/Parser.cpp

@@ -1133,6 +1133,7 @@ NonnullRefPtr<ForStatement> Parser::parse_for_statement()
     consume(TokenType::ParenOpen);
     consume(TokenType::ParenOpen);
 
 
     bool first_semicolon_consumed = false;
     bool first_semicolon_consumed = false;
+    bool in_scope = false;
     RefPtr<ASTNode> init;
     RefPtr<ASTNode> init;
     switch (m_parser_state.m_current_token.type()) {
     switch (m_parser_state.m_current_token.type()) {
     case TokenType::Semicolon:
     case TokenType::Semicolon:
@@ -1141,6 +1142,11 @@ NonnullRefPtr<ForStatement> Parser::parse_for_statement()
         if (match_expression()) {
         if (match_expression()) {
             init = parse_expression(0);
             init = parse_expression(0);
         } else if (match_variable_declaration()) {
         } else if (match_variable_declaration()) {
+            if (m_parser_state.m_current_token.type() != TokenType::Var) {
+                m_parser_state.m_let_scopes.append(NonnullRefPtrVector<VariableDeclaration>());
+                in_scope = true;
+            }
+
             init = parse_variable_declaration();
             init = parse_variable_declaration();
             first_semicolon_consumed = true;
             first_semicolon_consumed = true;
         } else {
         } else {
@@ -1176,6 +1182,10 @@ NonnullRefPtr<ForStatement> Parser::parse_for_statement()
 
 
     auto body = parse_statement();
     auto body = parse_statement();
 
 
+    if (in_scope) {
+        m_parser_state.m_let_scopes.take_last();
+    }
+
     return create_ast_node<ForStatement>(move(init), move(test), move(update), move(body));
     return create_ast_node<ForStatement>(move(init), move(test), move(update), move(body));
 }
 }
 
 

+ 22 - 0
Libraries/LibJS/Tests/for-scopes.js

@@ -0,0 +1,22 @@
+load("test-common.js");
+
+try {
+    for (var v = 5; false; );
+    assert(v == 5);
+
+    const options = {error: ReferenceError};
+
+    assertThrowsError(() => {
+        for (let l = 5; false; );
+        l;
+    }, options);
+
+    assertThrowsError(() => {
+        for (const c = 5; false; );
+        c;
+    }, options)
+
+    console.log("PASS");
+} catch (e) {
+    console.log("FAIL: " + e);
+}