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

@@ -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

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; }
 

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;",

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
 

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()");