Block-scoped functions in evals are now only conditionally hoisted out.

Annex B.3.3 of the spec requires that sloppy-mode block-scoped functions
declared by "eval" are hoisted unless doing so would cause an early
error (which is to say, conflict with a lexical declaration). This patch
amends the check for conflicting declarations to include those outside
of the eval itself.

BUG=v8:4468, v8:4479

Review-Url: https://codereview.chromium.org/2112163002
Cr-Commit-Position: refs/heads/master@{#37783}

src/parsing/parser.cc

@@ -5233,7 +5233,11 @@ void Parser::InsertSloppyBlockFunctionVarBindings(Scope* scope,
     auto delegates = static_cast<SloppyBlockFunctionMap::Vector*>(p->value);
     for (SloppyBlockFunctionStatement* delegate : *delegates) {
       // Check if there's a conflict with a lexical declaration
-      Scope* outer_scope = scope->outer_scope();
+      Scope* decl_scope = scope;
+      while (decl_scope->is_eval_scope()) {
+        decl_scope = decl_scope->outer_scope()->DeclarationScope();
+      }
+      Scope* outer_scope = decl_scope->outer_scope();
       Scope* query_scope = delegate->scope()->outer_scope();
       Variable* var = nullptr;
       bool should_hoist = true;

test/mjsunit/es6/block-eval-var-over-let.js

@@ -124,8 +124,6 @@ try {
 }
 assertTrue(caught);
 
-// TODO(littledan): Hoisting x out of the block should be
-// prevented in this case BUG(v8:4479)
 caught = false
 try {
   (function() {
@@ -137,5 +135,4 @@ try {
 } catch (e) {
   caught = true;
 }
-// TODO(littledan): switch to assertTrue when bug is fixed
+assertFalse(caught);
-assertTrue(caught);

test/mjsunit/es6/block-sloppy-function.js

@@ -461,7 +461,7 @@
 
   try {
     throw 0;
-  } catch(f) {
+  } catch (f) {
     {
       assertEquals(4, f());
 
@@ -471,6 +471,8 @@
 
       assertEquals(4, f());
     }
+
+    assertEquals(0, f);
   }
 
   assertEquals(4, f());
@@ -479,7 +481,7 @@
 (function noHoistingThroughComplexCatch() {
   try {
     throw 0;
-  } catch({f}) {
+  } catch ({f}) {
     {
       assertEquals(4, f());
 
@@ -494,6 +496,26 @@
   assertThrows(()=>f, ReferenceError);
 })();
 
+(function hoistingThroughWith() {
+  with ({f: 0}) {
+    assertEquals(0, f);
+
+    {
+      assertEquals(4, f());
+
+      function f() {
+        return 4;
+      }
+
+      assertEquals(4, f());
+    }
+
+    assertEquals(0, f);
+  }
+
+  assertEquals(4, f());
+})();
+
 // Test that hoisting from blocks does happen in global scope
 function globalHoisted() { return 0; }
 {
@@ -572,30 +594,63 @@ eval(`
   `);
 }();
 
+// This test is incorrect BUG(v8:5168). The commented assertions are correct.
+(function evalHoistingThroughSimpleCatch() {
+  try {
+    throw 0;
+  } catch (f) {
+    eval(`{ function f() {
+      return 4;
+    } }`);
+
+    // assertEquals(0, f);
+    assertEquals(4, f());
+  }
+
+  // assertEquals(4, f());
+  assertEquals(undefined, f);
+})();
+
+// This test is incorrect BUG(v8:5168). The commented assertions are correct.
+(function evalHoistingThroughWith() {
+  with ({f: 0}) {
+    eval(`{ function f() {
+      return 4;
+    } }`);
+
+    // assertEquals(0, f);
+    assertEquals(4, f());
+  }
+
+  // assertEquals(4, f());
+  assertEquals(undefined, f);
+})();
+
 let dontHoistGlobal;
 { function dontHoistGlobal() {} }
 assertEquals(undefined, dontHoistGlobal);
 
 let dontHoistEval;
-// BUG(v8:) This shouldn't hoist and shouldn't throw
 var throws = false;
 try {
   eval("{ function dontHoistEval() {} }");
 } catch (e) {
   throws = true;
 }
-assertTrue(throws);
+assertFalse(throws);
 
 // When the global object is frozen, silently don't hoist
 // Currently this actually throws BUG(v8:4452)
 Object.freeze(this);
-throws = false;
+{
-try {
+  let throws = false;
-  eval('{ function hoistWhenFrozen() {} }');
+  try {
-} catch (e) {
+    eval('{ function hoistWhenFrozen() {} }');
-  throws = true;
+  } catch (e) {
+    throws = true;
+  }
+  assertFalse(this.hasOwnProperty("hoistWhenFrozen"));
+  assertThrows(() => hoistWhenFrozen, ReferenceError);
+  // Should be assertFalse BUG(v8:4452)
+  assertTrue(throws);
 }
-assertFalse(this.hasOwnProperty("hoistWhenFrozen"));
-assertThrows(() => hoistWhenFrozen, ReferenceError);
-// Should be assertFalse BUG(v8:4452)
-assertTrue(throws);