Revert "Fix "this" value in lazily-parsed module functions."

This reverts commit c3bd741e.

Reason for revert: Breaks layout tests:
https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/builds/20384

Original change's description:
> Fix "this" value in lazily-parsed module functions.
> 
> When preparsing top-level functions in a module, we didn't track
> unresolved variables. Consequently, "this" ended up referencing
> the global "this", which has the wrong value (in a module "this"
> is supposed to be the undefined value).
> 
> This patch fixes that. This also lets us stop forcing context
> allocation of all variables in module scopes, which the patch
> takes care of as well.
> 
> Bug: chromium:791334
> Change-Id: Ifac1f1adc033f3facfb3d29dd4bca32ee27bffcf
> Reviewed-on: https://chromium-review.googlesource.com/808938
> Reviewed-by: Marja Hölttä <marja@chromium.org>
> Reviewed-by: Adam Klein <adamk@chromium.org>
> Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
> Commit-Queue: Georg Neis <neis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#50025}

TBR=adamk@chromium.org,marja@chromium.org,neis@chromium.org,kozyatinskiy@chromium.org

Change-Id: I81f69334ed2ce104c00e6205d50001e4bdf07d15
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:791334
Reviewed-on: https://chromium-review.googlesource.com/822258Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50036}

src/ast/scopes.cc

@@ -147,6 +147,8 @@ Scope::Scope(Zone* zone, Scope* outer_scope, ScopeType scope_type)
   DCHECK_NE(SCRIPT_SCOPE, scope_type);
   SetDefaults();
   set_language_mode(outer_scope->language_mode());
+  force_context_allocation_ =
+      !is_function_scope() && outer_scope->has_forced_context_allocation();
   outer_scope_->AddInnerScope(this);
 }
 
@@ -1368,8 +1370,12 @@ bool Scope::AllowsLazyParsingWithoutUnresolvedVariables(
     if (s->is_catch_scope()) continue;
     // With scopes do not introduce variables that need allocation.
     if (s->is_with_scope()) continue;
-    DCHECK(s->is_module_scope() || s->is_block_scope() ||
-           s->is_function_scope());
+    // Module scopes context-allocate all variables, and have no
+    // {this} or {arguments} variables whose existence depends on
+    // references to them.
+    if (s->is_module_scope()) continue;
+    // Only block scopes and function scopes should disallow preparsing.
+    DCHECK(s->is_block_scope() || s->is_function_scope());
     return false;
   }
   return true;
@@ -1728,6 +1734,9 @@ void Scope::Print(int n) {
     if (scope->was_lazily_parsed()) Indent(n1, "// lazily parsed\n");
     if (scope->ShouldEagerCompile()) Indent(n1, "// will be compiled\n");
   }
+  if (has_forced_context_allocation()) {
+    Indent(n1, "// forces context allocation\n");
+  }
   if (num_stack_slots_ > 0) {
     Indent(n1, "// ");
     PrintF("%d stack slots\n", num_stack_slots_);
@@ -2102,8 +2111,11 @@ bool Scope::MustAllocateInContext(Variable* var) {
   // an eval() call or a runtime with lookup), it must be allocated in the
   // context.
   //
-  // Temporary variables are always stack-allocated.  Catch-bound variables are
+  // Exceptions: If the scope as a whole has forced context allocation, all
+  // variables will have context allocation, even temporaries.  Otherwise
+  // temporary variables are always stack-allocated.  Catch-bound variables are
   // always context-allocated.
+  if (has_forced_context_allocation()) return true;
   if (var->mode() == TEMPORARY) return false;
   if (is_catch_scope()) return true;
   if ((is_script_scope() || is_eval_scope()) &&

src/ast/scopes.h

@@ -334,6 +334,14 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   bool is_hidden() const { return is_hidden_; }
   void set_is_hidden() { is_hidden_ = true; }
 
+  // In some cases we want to force context allocation for a whole scope.
+  void ForceContextAllocation() {
+    DCHECK(!already_resolved_);
+    force_context_allocation_ = true;
+  }
+  bool has_forced_context_allocation() const {
+    return force_context_allocation_;
+  }
   void ForceContextAllocationForParameters() {
     DCHECK(!already_resolved_);
     force_context_allocation_for_parameters_ = true;

src/parsing/parser.cc

@@ -690,6 +690,7 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
       var->AllocateTo(VariableLocation::PARAMETER, 0);
 
       PrepareGeneratorVariables();
+      scope->ForceContextAllocation();
       Expression* initial_yield =
           BuildInitialYield(kNoSourcePosition, kGeneratorFunction);
       body->Add(

test/cctest/test-parsing.cc

@@ -6224,9 +6224,9 @@ TEST(ModuleParsingInternals) {
 
   CHECK(declarations->AtForTest(8)->proxy()->raw_name()->IsOneByteEqualTo(
       "nonexport"));
-  CHECK(!declarations->AtForTest(8)->proxy()->var()->binding_needs_init());
-  CHECK(declarations->AtForTest(8)->proxy()->var()->location() ==
-        i::VariableLocation::LOCAL);
+  CHECK(declarations->AtForTest(8)->proxy()->var()->binding_needs_init());
+  CHECK(declarations->AtForTest(8)->proxy()->var()->location() !=
+        i::VariableLocation::MODULE);
 
   CHECK(
       declarations->AtForTest(9)->proxy()->raw_name()->IsOneByteEqualTo("mm"));

test/debugger/debug/debug-modules-set-variable-value.js

@@ -271,7 +271,7 @@ let salad = 12;
   function listener(event, exec_state) {
     if (event == Debug.DebugEvent.Break) {
       let scope_count = exec_state.frame().scopeCount();
-      let module_scope = exec_state.frame().scope(1);
+      let module_scope = exec_state.frame().scope(2);
       assertEquals(debug.ScopeType.Module, module_scope.scopeType());
       module_scope.setVariableValue('salad', 42);
     }
@@ -311,7 +311,7 @@ export let ham = 1;
   function listener(event, exec_state) {
     if (event == Debug.DebugEvent.Break) {
       let scope_count = exec_state.frame().scopeCount();
-      let module_scope = exec_state.frame().scope(1);
+      let module_scope = exec_state.frame().scope(2);
       assertEquals(debug.ScopeType.Module, module_scope.scopeType());
       module_scope.setVariableValue('ham', 2);
     }

test/debugger/debug/harmony/modules-debug-scopes2.js

@@ -139,10 +139,10 @@ listener_delegate = function(exec_state) {
                    debug.ScopeType.Script,
                    debug.ScopeType.Global], exec_state);
   CheckScopeContent(
-      {exported_var: undefined, imported_var: undefined},
+      {local_var: undefined, exported_var: undefined, imported_var: undefined},
       0, exec_state);
   CheckScopeDoesNotHave(
-      ["local_var", "doesntexist", "local_let", "exported_let", "imported_let"],
+      ["doesnotexist", "local_let", "exported_let", "imported_let"],
       0, exec_state);
 };
 debugger;
@@ -161,9 +161,8 @@ listener_delegate = function(exec_state) {
                    debug.ScopeType.Script,
                    debug.ScopeType.Global], exec_state);
   CheckScopeContent(
-      {exported_let: 3, exported_var: 4,
+      {local_let: 1, local_var: 2, exported_let: 3, exported_var: 4,
        imported_let: 3, imported_var: 4}, 0, exec_state);
-  CheckScopeDoesNotHave(["local_var", "local_let"], 0, exec_state);
 };
 debugger;
 EndTest();
@@ -179,9 +178,8 @@ listener_delegate = function(exec_state) {
                    debug.ScopeType.Script,
                    debug.ScopeType.Global], exec_state);
   CheckScopeContent(
-      {exported_let: 13, exported_var: 14,
+      {local_let: 11, local_var: 12, exported_let: 13, exported_var: 14,
        imported_let: 13, imported_var: 14}, 0, exec_state);
-  CheckScopeDoesNotHave(["local_var", "local_let"], 0, exec_state);
 };
 debugger;
 EndTest();

test/inspector/debugger/evaluate-on-call-frame-in-module-expected.txt

@@ -12,8 +12,9 @@ local:foo1
 module
 [
     [0] : a1 = 10
-    [1] : b1 = 11
-    [2] : foo1 = function foo1() {   let c1 = 12;   let g1 = 2;   debugger;   return a1 + b1 + c1 + g1; }
+    [1] : g1 = 1
+    [2] : b1 = 11
+    [3] : foo1 = function foo1() {   let c1 = 12;   let g1 = 2;   debugger;   return a1 + b1 + c1 + g1; }
 ]
 global
 [
@@ -179,8 +180,9 @@ foo2 =
 }
 module
 [
-    [0] : foo2 = function foo2() {   let c2 = 22;   return foo1() + a2 + b2 + c2; }
-    [1] : b3 = 31
+    [0] : a3 = 30
+    [1] : foo2 = function foo2() {   let c2 = 22;   return foo1() + a2 + b2 + c2; }
+    [2] : b3 = 31
 ]
 global
 [
@@ -198,6 +200,20 @@ Array =
     objectId : <objectId>
     type : function
 }
+a3 = 
+{
+    description : 30
+    type : number
+    value : 30
+}
+Evaluating: ++a3
+updated a3 = 
+{
+    description : 31
+    type : number
+    value : 31
+}
+Evaluating: --a3
 foo2 = 
 {
     className : Function
@@ -231,6 +247,12 @@ closure:bar
 [
     [0] : a = 0
 ]
+module
+[
+    [0] : a = 1
+    [1] : b = 2
+    [2] : bar = function bar() {   let a = 0;   (() => {a; debugger;})(); }
+]
 global
 [
   ...
@@ -261,10 +283,37 @@ updated a =
     value : 1
 }
 Evaluating: --a
+b = 
+{
+    description : 2
+    type : number
+    value : 2
+}
+Evaluating: ++b
+updated b = 
+{
+    description : 3
+    type : number
+    value : 3
+}
+Evaluating: --b
+bar = 
+{
+    className : Function
+    description : function bar() {   let a = 0;   (() => {a; debugger;})(); }
+    objectId : <objectId>
+    type : function
+}
 local:bar
 [
     [0] : a = 0
 ]
+module
+[
+    [0] : a = 1
+    [1] : b = 2
+    [2] : bar = function bar() {   let a = 0;   (() => {a; debugger;})(); }
+]
 global
 [
   ...
@@ -295,6 +344,33 @@ updated a =
     value : 1
 }
 Evaluating: --a
+b = 
+{
+    description : 2
+    type : number
+    value : 2
+}
+Evaluating: ++b
+updated b = 
+{
+    description : 3
+    type : number
+    value : 3
+}
+Evaluating: --b
+bar = 
+{
+    className : Function
+    description : function bar() {   let a = 0;   (() => {a; debugger;})(); }
+    objectId : <objectId>
+    type : function
+}
+module
+[
+    [0] : a = 1
+    [1] : b = 2
+    [2] : bar = function bar() {   let a = 0;   (() => {a; debugger;})(); }
+]
 global
 [
   ...
@@ -310,6 +386,41 @@ Array =
     objectId : <objectId>
     type : function
 }
+a = 
+{
+    description : 1
+    type : number
+    value : 1
+}
+Evaluating: ++a
+updated a = 
+{
+    description : 2
+    type : number
+    value : 2
+}
+Evaluating: --a
+b = 
+{
+    description : 2
+    type : number
+    value : 2
+}
+Evaluating: ++b
+updated b = 
+{
+    description : 3
+    type : number
+    value : 3
+}
+Evaluating: --b
+bar = 
+{
+    className : Function
+    description : function bar() {   let a = 0;   (() => {a; debugger;})(); }
+    objectId : <objectId>
+    type : function
+}
 
 Running test: testDifferentModuleVariables
 (anonymous) (module5:5:0)
@@ -392,112 +503,3 @@ updated c =
     value : 1
 }
 Evaluating: --c
-
-Running test: testCapturedLocalVariable
-(anonymous) (module6:2:25)
-(anonymous) (module6:2:37)
-local
-[
-    [0] : y = 5
-]
-module
-[
-    [0] : x = 5
-]
-global
-[
-  ...
-]
-Check variables in frame#0
-let x = 5;
-(function() { let y = x; #debugger; })()
-
-
-Array = 
-{
-    className : Function
-    description : function Array() { [native code] }
-    objectId : <objectId>
-    type : function
-}
-y = 
-{
-    description : 5
-    type : number
-    value : 5
-}
-Evaluating: ++y
-updated y = 
-{
-    description : 6
-    type : number
-    value : 6
-}
-Evaluating: --y
-x = 
-{
-    description : 5
-    type : number
-    value : 5
-}
-Evaluating: ++x
-updated x = 
-{
-    description : 6
-    type : number
-    value : 6
-}
-Evaluating: --x
-module
-[
-    [0] : x = 5
-]
-global
-[
-  ...
-]
-Check variables in frame#1
-let x = 5;
-(function() { let y = x; debugger; })#()
-
-
-Array = 
-{
-    className : Function
-    description : function Array() { [native code] }
-    objectId : <objectId>
-    type : function
-}
-x = 
-{
-    description : 5
-    type : number
-    value : 5
-}
-Evaluating: ++x
-updated x = 
-{
-    description : 6
-    type : number
-    value : 6
-}
-Evaluating: --x
-
-Running test: testLocalVariableToplevel
-(anonymous) (module7:2:0)
-global
-[
-  ...
-]
-Check variables in frame#0
-let x = 5;
-#debugger;
-
-
-Array = 
-{
-    className : Function
-    description : function Array() { [native code] }
-    objectId : <objectId>
-    type : function
-}

test/inspector/debugger/evaluate-on-call-frame-in-module.js

@@ -49,16 +49,6 @@ export var c = 0;
 debugger;
 `;
 
-var module6 = `
-let x = 5;
-(function() { let y = x; debugger; })()
-`;
-
-var module7 = `
-let x = 5;
-debugger;
-`;
-
 InspectorTest.runAsyncTestSuite([
   async function testTotal() {
     session.setupScriptMap();
@@ -92,26 +82,6 @@ InspectorTest.runAsyncTestSuite([
       await checkFrame(callFrames[i], i);
     }
     await Protocol.Debugger.resume();
-  },
-
-  async function testCapturedLocalVariable() {
-    contextGroup.addModule(module6, 'module6');
-    let {params:{callFrames}} = (await Protocol.Debugger.oncePaused());
-    session.logCallFrames(callFrames);
-    for (let i = 0; i < callFrames.length; ++i) {
-      await checkFrame(callFrames[i], i);
-    }
-    await Protocol.Debugger.resume();
-  },
-
-  async function testLocalVariableToplevel() {
-    contextGroup.addModule(module7, 'module7');
-    let {params:{callFrames}} = (await Protocol.Debugger.oncePaused());
-    session.logCallFrames(callFrames);
-    for (let i = 0; i < callFrames.length; ++i) {
-      await checkFrame(callFrames[i], i);
-    }
-    await Protocol.Debugger.resume();
   }
 ]);
 

test/mjsunit/regress/regress-791334.js

-// Copyright 2017 the V8 project authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-// MODULE
-
-let foo = () => { return this };
-assertEquals(undefined, foo());