From d03191beb1888b272e51912ebc45c4e8794f2060 Mon Sep 17 00:00:00 2001 From: littledan Date: Tue, 11 Aug 2015 14:30:26 -0700 Subject: [PATCH] Use a new lexical context for sloppy-mode eval In ES6, direct eval() in sloppy mode uses the enclosing function-level ("var") scope for var-style bindings and a new lexical scope for lexical bindings like let and class. This patch implements that feature by making lexical bindings that are directly within an EVAL_SCOPE be on the local scope rather than the enclosing one. BUG=v8:4288 LOG=Y R=adamk Review URL: https://codereview.chromium.org/1274193004 Cr-Commit-Position: refs/heads/master@{#30120} --- src/parser.cc | 39 +++++++++++++++----------- src/scopes.h | 3 -- test/mjsunit/harmony/block-conflicts-sloppy.js | 17 ++++------- test/webkit/class-syntax-name-expected.txt | 4 +-- test/webkit/class-syntax-name.js | 2 +- 5 files changed, 30 insertions(+), 35 deletions(-) diff --git a/src/parser.cc b/src/parser.cc index 8299e29..11acc93 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -2008,15 +2008,17 @@ Variable* Parser::Declare(Declaration* declaration, // variable and also set its mode. In any case, a Declaration node // will be added to the scope so that the declaration can be added // to the corresponding activation frame at runtime if necessary. - // For instance declarations inside an eval scope need to be added - // to the calling function context. - // Similarly, strict mode eval scope does not leak variable declarations to - // the caller's scope so we declare all locals, too. + // For instance, var declarations inside a sloppy eval scope need + // to be added to the calling function context. Similarly, strict + // mode eval scope and lexical eval bindings do not leak variable + // declarations to the caller's scope so we declare all locals, too. if (declaration_scope->is_function_scope() || - declaration_scope->is_strict_eval_scope() || declaration_scope->is_block_scope() || declaration_scope->is_module_scope() || - declaration_scope->is_script_scope()) { + declaration_scope->is_script_scope() || + (declaration_scope->is_eval_scope() && + (is_strict(declaration_scope->language_mode()) || + IsLexicalVariableMode(mode)))) { // Declare the variable in the declaration scope. var = declaration_scope->LookupLocal(name); if (var == NULL) { @@ -2069,8 +2071,22 @@ Variable* Parser::Declare(Declaration* declaration, } else if (mode == VAR) { var->set_maybe_assigned(); } + } else if (declaration_scope->is_eval_scope() && + is_sloppy(declaration_scope->language_mode()) && + !IsLexicalVariableMode(mode)) { + // In a var binding in a sloppy direct eval, pollute the enclosing scope + // with this new binding by doing the following: + // The proxy is bound to a lookup variable to force a dynamic declaration + // using the DeclareLookupSlot runtime function. + Variable::Kind kind = Variable::NORMAL; + // TODO(sigurds) figure out if kNotAssigned is OK here + var = new (zone()) Variable(declaration_scope, name, mode, kind, + declaration->initialization(), kNotAssigned); + var->AllocateTo(VariableLocation::LOOKUP, -1); + resolve = true; } + // We add a declaration node for every declaration. The compiler // will only generate code if necessary. In particular, declarations // for inner local variables that do not represent functions won't @@ -2095,17 +2111,6 @@ Variable* Parser::Declare(Declaration* declaration, Variable::Kind kind = Variable::NORMAL; var = new (zone()) Variable(declaration_scope, name, mode, kind, kNeedsInitialization, kNotAssigned); - } else if (declaration_scope->is_eval_scope() && - is_sloppy(declaration_scope->language_mode())) { - // For variable declarations in a sloppy eval scope the proxy is bound - // to a lookup variable to force a dynamic declaration using the - // DeclareLookupSlot runtime function. - Variable::Kind kind = Variable::NORMAL; - // TODO(sigurds) figure out if kNotAssigned is OK here - var = new (zone()) Variable(declaration_scope, name, mode, kind, - declaration->initialization(), kNotAssigned); - var->AllocateTo(VariableLocation::LOOKUP, -1); - resolve = true; } // If requested and we have a local variable, bind the proxy to the variable diff --git a/src/scopes.h b/src/scopes.h index acfad4c..2f600b9 100644 --- a/src/scopes.h +++ b/src/scopes.h @@ -279,9 +279,6 @@ class Scope: public ZoneObject { bool is_with_scope() const { return scope_type_ == WITH_SCOPE; } bool is_arrow_scope() const { return scope_type_ == ARROW_SCOPE; } bool is_declaration_scope() const { return is_declaration_scope_; } - bool is_strict_eval_scope() const { - return is_eval_scope() && is_strict(language_mode_); - } void set_is_declaration_scope() { is_declaration_scope_ = true; } diff --git a/test/mjsunit/harmony/block-conflicts-sloppy.js b/test/mjsunit/harmony/block-conflicts-sloppy.js index 14dc242..9569c24 100644 --- a/test/mjsunit/harmony/block-conflicts-sloppy.js +++ b/test/mjsunit/harmony/block-conflicts-sloppy.js @@ -44,12 +44,10 @@ function TestAll(expected,s,opt_e) { var e = ""; var msg = s; if (opt_e) { e = opt_e; msg += opt_e; } - // TODO(littledan): https://code.google.com/p/v8/issues/detail?id=4288 - // It is also not clear whether these tests makes sense in sloppy mode. // TODO(littledan): Add tests using Realm.eval to ensure that global eval // works as expected. - // assertEquals(expected === 'LocalConflict' ? 'NoConflict' : expected, - // TestGlobal(s,e), "global:'" + msg + "'"); + assertEquals(expected === 'LocalConflict' ? 'NoConflict' : expected, + TestGlobal(s,e), "global:'" + msg + "'"); assertEquals(expected === 'LocalConflict' ? 'NoConflict' : expected, TestFunction(s,e), "function:'" + msg + "'"); assertEquals(expected === 'LocalConflict' ? 'Conflict' : expected, @@ -59,22 +57,17 @@ function TestAll(expected,s,opt_e) { function TestConflict(s) { TestAll('Conflict', s); - // TODO(littledan): https://code.google.com/p/v8/issues/detail?id=4288 - // It is also not clear whether these tests makes sense in sloppy mode. - // TestAll('Conflict', 'eval("' + s + '");'); + TestAll('Conflict', 'eval("' + s + '");'); } function TestNoConflict(s) { TestAll('NoConflict', s, "'NoConflict'"); - // TODO(littledan): https://code.google.com/p/v8/issues/detail?id=4288 - // TestAll('NoConflict', 'eval("' + s + '");', "'NoConflict'"); + TestAll('NoConflict', 'eval("' + s + '");', "'NoConflict'"); } function TestLocalConflict(s) { TestAll('LocalConflict', s, "'NoConflict'"); - // TODO(littledan): https://code.google.com/p/v8/issues/detail?id=4288 - // It is also not clear whether these tests makes sense in sloppy mode. - // TestAll('NoConflict', 'eval("' + s + '");', "'NoConflict'"); + TestAll('NoConflict', 'eval("' + s + '");', "'NoConflict'"); } var letbinds = [ "let x;", diff --git a/test/webkit/class-syntax-name-expected.txt b/test/webkit/class-syntax-name-expected.txt index 10f38ff..ed49be3 100644 --- a/test/webkit/class-syntax-name-expected.txt +++ b/test/webkit/class-syntax-name-expected.txt @@ -108,7 +108,7 @@ PASS 'use strict'; var VarA = class A { constructor() {} }; var VarB = class B e Class statement binding in other circumstances PASS var result = A; result threw exception ReferenceError: A is not defined. PASS 'use strict'; var result = A; result threw exception ReferenceError: A is not defined. -FAIL var result = A; class A {}; result should throw an exception. Was undefined. +PASS var result = A; class A {}; result threw exception ReferenceError: A is not defined. PASS 'use strict'; var result = A; class A {}; result threw exception ReferenceError: A is not defined. PASS class A { constructor() { A = 1; } }; new A threw exception TypeError: Assignment to constant variable.. PASS 'use strict'; class A { constructor() { A = 1; } }; new A threw exception TypeError: Assignment to constant variable.. @@ -118,7 +118,7 @@ PASS class A {}; var result = A; result did not throw exception. PASS 'use strict'; class A {}; var result = A; result did not throw exception. PASS eval('var Foo = 10'); Foo is 10 PASS 'use strict'; eval('var Foo = 10'); Foo threw exception ReferenceError: Foo is not defined. -PASS eval('class Bar { constructor() {} }'); Bar.toString() is 'class Bar { constructor() {} }' +PASS eval('class Bar { constructor() {} }; Bar.toString()') is 'class Bar { constructor() {} }' PASS 'use strict'; eval('class Bar { constructor() {} }'); Bar.toString() threw exception ReferenceError: Bar is not defined. PASS successfullyParsed is true diff --git a/test/webkit/class-syntax-name.js b/test/webkit/class-syntax-name.js index 09faa3a..1604565 100644 --- a/test/webkit/class-syntax-name.js +++ b/test/webkit/class-syntax-name.js @@ -111,5 +111,5 @@ runTestShouldBe("class A { constructor() { } }; A = 1; A", "1"); runTestShouldNotThrow("class A {}; var result = A; result"); shouldBe("eval('var Foo = 10'); Foo", "10"); shouldThrow("'use strict'; eval('var Foo = 10'); Foo"); -shouldBe("eval('class Bar { constructor() {} }'); Bar.toString()", "'class Bar { constructor() {} }'"); +shouldBe("eval('class Bar { constructor() {} }; Bar.toString()')", "'class Bar { constructor() {} }'"); shouldThrow("'use strict'; eval('class Bar { constructor() {} }'); Bar.toString()"); -- 2.7.4