Extend Annex B 3.3 sloppy-mode block-scoped hoisting to scripts, eval

The ES2015 spec is missing an extension of sloppy-mode block-scoped function
behavior to the global scope in scripts, as well as to eval. This patch
brings that hoisting to those two areas. The behavior is not perfectly
spec-compliant since properties created on the global scope should be
set as enumerable even if they are non-enumerable previously, but the
attributes will not be modified if the property already exists under
this patch.

BUG=v8:4441
LOG=Y
R=adamk
TEST=reddit comment functionality seems to be fixed

Review URL: https://codereview.chromium.org/1376623002

Cr-Commit-Position: refs/heads/master@{#31037}

2 files changed, 113 insertions, 7 deletions

diff --git a/src/parser.cc b/src/parser.cc
index 534a13828..17d64f2fe 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -1066,6 +1066,13 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
     if (ok && is_strict(language_mode())) {
       CheckStrictOctalLiteral(beg_pos, scanner()->location().end_pos, &ok);
     }
+    if (ok && is_sloppy(language_mode()) && allow_harmony_sloppy_function()) {
+      // TODO(littledan): Function bindings on the global object that modify
+      // pre-existing bindings should be made writable, enumerable and
+      // nonconfigurable if possible, whereas this code will leave attributes
+      // unchanged if the property already exists.
+      InsertSloppyBlockFunctionVarBindings(scope, &ok);
+    }
     if (ok && (is_strict(language_mode()) || allow_harmony_sloppy())) {
       CheckConflictingVarDeclarations(scope_, &ok);
     }
diff --git a/test/mjsunit/harmony/block-sloppy-function.js b/test/mjsunit/harmony/block-sloppy-function.js
index a17a4c079..ff895d5b8 100644
--- a/test/mjsunit/harmony/block-sloppy-function.js
+++ b/test/mjsunit/harmony/block-sloppy-function.js
@@ -146,13 +146,6 @@
   assertEquals(2, f());
 })();
 
-// Test that hoisting from blocks doesn't happen in global scope
-function globalUnhoisted() { return 0; }
-{
-  function globalUnhoisted() { return 1; }
-}
-assertEquals(0, globalUnhoisted());
-
 // Test that shadowing arguments is fine
 (function shadowArguments(x) {
   assertArrayEquals([1], arguments);
@@ -201,3 +194,109 @@ assertThrows(function notInDefaultScope(x = y) {
   assertEquals('function', typeof y);
   assertEquals(x, undefined);
 }, ReferenceError);
+
+// Test that hoisting from blocks does happen in global scope
+function globalHoisted() { return 0; }
+{
+  function globalHoisted() { return 1; }
+}
+assertEquals(1, globalHoisted());
+
+// Also happens when not previously defined
+assertEquals(undefined, globalUndefinedHoisted);
+{
+  function globalUndefinedHoisted() { return 1; }
+}
+assertEquals(1, globalUndefinedHoisted());
+var globalUndefinedHoistedDescriptor =
+    Object.getOwnPropertyDescriptor(this, "globalUndefinedHoisted");
+assertFalse(globalUndefinedHoistedDescriptor.configurable);
+assertTrue(globalUndefinedHoistedDescriptor.writable);
+assertTrue(globalUndefinedHoistedDescriptor.enumerable);
+assertEquals(1, globalUndefinedHoistedDescriptor.value());
+
+// When a function property is hoisted, it should be
+// made enumerable.
+// BUG(v8:4451)
+Object.defineProperty(this, "globalNonEnumerable", {
+  value: false,
+  configurable: true,
+  writable: true,
+  enumerable: false
+});
+eval("{function globalNonEnumerable() { return 1; }}");
+var globalNonEnumerableDescriptor
+    = Object.getOwnPropertyDescriptor(this, "globalNonEnumerable");
+// BUG(v8:4451): Should be made non-configurable
+assertTrue(globalNonEnumerableDescriptor.configurable);
+assertTrue(globalNonEnumerableDescriptor.writable);
+// BUG(v8:4451): Should be made enumerable
+assertFalse(globalNonEnumerableDescriptor.enumerable);
+assertEquals(1, globalNonEnumerableDescriptor.value());
+
+// When a function property is hoisted, it should be overwritten and
+// made writable and overwritten, even if the property was non-writable.
+Object.defineProperty(this, "globalNonWritable", {
+  value: false,
+  configurable: true,
+  writable: false,
+  enumerable: true
+});
+eval("{function globalNonWritable() { return 1; }}");
+var globalNonWritableDescriptor
+    = Object.getOwnPropertyDescriptor(this, "globalNonWritable");
+// BUG(v8:4451): Should be made non-configurable
+assertTrue(globalNonWritableDescriptor.configurable);
+// BUG(v8:4451): Should be made writable
+assertFalse(globalNonWritableDescriptor.writable);
+assertFalse(globalNonEnumerableDescriptor.enumerable);
+// BUG(v8:4451): Should be overwritten
+assertEquals(false, globalNonWritableDescriptor.value);
+
+// Test that hoisting from blocks does happen in an eval
+eval(`
+  function evalHoisted() { return 0; }
+  {
+    function evalHoisted() { return 1; }
+  }
+  assertEquals(1, evalHoisted());
+`);
+
+// Test that hoisting from blocks happens from eval in a function
+!function() {
+  eval(`
+    function evalInFunctionHoisted() { return 0; }
+    {
+      function evalInFunctionHoisted() { return 1; }
+    }
+    assertEquals(1, evalInFunctionHoisted());
+  `);
+}();
+
+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);
+
+// When the global object is frozen, silently don't hoist
+// Currently this actually throws BUG(v8:4452)
+Object.freeze(this);
+throws = false;
+try {
+  eval('{ function hoistWhenFrozen() {} }');
+} catch (e) {
+  throws = true;
+}
+assertFalse(this.hasOwnProperty("hoistWhenFrozen"));
+assertThrows(() => hoistWhenFrozen, ReferenceError);
+// Should be assertFalse BUG(v8:4452)
+assertTrue(throws);