Reviewers: adamk,

Description:
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

Please review this at https://codereview.chromium.org/1274193004/

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+12, -17 lines):
  M src/parser.cc
  M test/mjsunit/harmony/block-conflicts-sloppy.js
  M test/webkit/class-syntax-name.js
  M test/webkit/class-syntax-name-expected.txt


Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index 45202cb3a82b922c7d487061cfff03578cc3a3c9..52936198f0f37647596db79f52c3b94b8074b0af 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -2019,7 +2019,8 @@ Variable* Parser::Declare(Declaration* declaration,
       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() && IsLexicalVariableMode(mode))) {
     // Declare the variable in the declaration scope.
     var = declaration_scope->LookupLocal(name);
     if (var == NULL) {
@@ -2099,7 +2100,8 @@ Variable* Parser::Declare(Declaration* declaration,
     var = new (zone()) Variable(declaration_scope, name, mode, kind,
                                 kNeedsInitialization, kNotAssigned);
   } else if (declaration_scope->is_eval_scope() &&
-             is_sloppy(declaration_scope->language_mode())) {
+             is_sloppy(declaration_scope->language_mode()) &&
+             !IsLexicalVariableMode(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.
Index: test/mjsunit/harmony/block-conflicts-sloppy.js
diff --git a/test/mjsunit/harmony/block-conflicts-sloppy.js b/test/mjsunit/harmony/block-conflicts-sloppy.js index 14dc24219ac85868710ac01ebe73f363aa7851de..9569c24ad4d60092f69ed73c2d9d18aede7eb196 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;",
Index: test/webkit/class-syntax-name-expected.txt
diff --git a/test/webkit/class-syntax-name-expected.txt b/test/webkit/class-syntax-name-expected.txt index 10f38ff2c2e077eee69070f4bb37e6ff02a74dc0..ed49be3309e8ce73d7ecdabcfdfe5281dc6ddbed 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

Index: test/webkit/class-syntax-name.js
diff --git a/test/webkit/class-syntax-name.js b/test/webkit/class-syntax-name.js index 09faa3a54aed5fe6d09c08e51506409afea3c0e6..16045651ef35adc5869ec73246e6cf658a4e0cea 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()");


--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to