Revert of Remove Scope::scope_uses_this_ flag (patchset #1 id:1 of...

Revert of Remove Scope::scope_uses_this_ flag (patchset #1 id:1 of https://codereview.chromium.org/1129823002/)

Reason for revert:
The reason for reverting is: [Sheriff] Need to fix compilation after this revert:
https://chromium.googlesource.com/v8/v8/+/5cab6be83a44567a3ee5747d728a399025d38411

Original issue's description:
> Remove Scope::scope_uses_this_ flag
>
> Use of the "this" variable is now tracked using scopes, like any other variable.
>
> R=arv@chromium.org
> LOG=N
> BUG=
>
> Committed: https://crrev.com/afba55965118d9ba57e53c729f52be2340e626e0
> Cr-Commit-Position: refs/heads/master@{#28268}

TBR=arv@chromium.org,wingo@igalia.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

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

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

src/compiler.cc

@@ -215,7 +215,8 @@ bool CompilationInfo::is_simple_parameter_list() {
 
 
 bool CompilationInfo::MayUseThis() const {
-  return scope()->has_this_declaration() && scope()->receiver()->is_used();
+  return scope()->uses_this() || scope()->inner_uses_this() ||
+         scope()->calls_sloppy_eval();
 }
 
 

src/compiler/ast-graph-builder.cc

@@ -2639,7 +2639,9 @@ Node* AstGraphBuilder::BuildPatchReceiverToGlobalProxy(Node* receiver) {
   // object). Otherwise there is nothing left to do here.
   if (is_strict(language_mode()) || info()->is_native()) return receiver;
 
-  // There is no need to perform patching if the receiver will never be used.
+  // There is no need to perform patching if the receiver is never used. Note
+  // that scope predicates are purely syntactical, a call to eval might still
+  // inspect the receiver value.
   if (!info()->MayUseThis()) return receiver;
 
   IfBuilder receiver_check(this);

src/preparser.h

@@ -2220,6 +2220,7 @@ ParserBase<Traits>::ParsePrimaryExpression(ExpressionClassifier* classifier,
           break;
         }
       }
+      scope_->RecordThisUsage();
       result = this->ThisExpression(scope_, factory(), beg_pos);
       break;
     }
@@ -3245,6 +3246,7 @@ ParserBase<Traits>::ParseStrongInitializationExpression(
   Consume(Token::THIS);
   int pos = position();
   function_state_->set_this_location(scanner()->location());
+  scope_->RecordThisUsage();
   ExpressionT this_expr = this->ThisExpression(scope_, factory(), pos);
 
   ExpressionT left = this->EmptyExpression();

src/scopes.cc

@@ -165,6 +165,7 @@ void Scope::SetDefaults(ScopeType scope_type, Scope* outer_scope,
   scope_calls_eval_ = false;
   scope_uses_arguments_ = false;
   scope_uses_super_property_ = false;
+  scope_uses_this_ = false;
   asm_module_ = false;
   asm_function_ = outer_scope != NULL && outer_scope->asm_module_;
   // Inherit the language mode from the parent scope.
@@ -172,6 +173,7 @@ void Scope::SetDefaults(ScopeType scope_type, Scope* outer_scope,
   outer_scope_calls_sloppy_eval_ = false;
   inner_scope_calls_eval_ = false;
   inner_scope_uses_arguments_ = false;
+  inner_scope_uses_this_ = false;
   inner_scope_uses_super_property_ = false;
   force_eager_compilation_ = false;
   force_context_allocation_ = (outer_scope != NULL && !is_function_scope())
@@ -378,6 +380,7 @@ Scope* Scope::FinalizeBlockScope() {
   // Propagate usage flags to outer scope.
   if (uses_arguments()) outer_scope_->RecordArgumentsUsage();
   if (uses_super_property()) outer_scope_->RecordSuperPropertyUsage();
+  if (uses_this()) outer_scope_->RecordThisUsage();
   if (scope_calls_eval_) outer_scope_->RecordEvalCall();
 
   return NULL;
@@ -927,11 +930,13 @@ void Scope::Print(int n) {
   if (scope_uses_arguments_) Indent(n1, "// scope uses 'arguments'\n");
   if (scope_uses_super_property_)
     Indent(n1, "// scope uses 'super' property\n");
+  if (scope_uses_this_) Indent(n1, "// scope uses 'this'\n");
   if (inner_scope_uses_arguments_) {
     Indent(n1, "// inner scope uses 'arguments'\n");
   }
   if (inner_scope_uses_super_property_)
     Indent(n1, "// inner scope uses 'super' property\n");
+  if (inner_scope_uses_this_) Indent(n1, "// inner scope uses 'this'\n");
   if (outer_scope_calls_sloppy_eval_) {
     Indent(n1, "// outer scope calls 'eval' in sloppy context\n");
   }
@@ -1287,6 +1292,9 @@ void Scope::PropagateScopeInfo(bool outer_scope_calls_sloppy_eval ) {
           inner->inner_scope_uses_super_property_) {
         inner_scope_uses_super_property_ = true;
       }
+      if (inner->scope_uses_this_ || inner->inner_scope_uses_this_) {
+        inner_scope_uses_this_ = true;
+      }
     }
     if (inner->force_eager_compilation_) {
       force_eager_compilation_ = true;

src/scopes.h

@@ -217,6 +217,9 @@ class Scope: public ZoneObject {
   // Inform the scope that the corresponding code uses "super".
   void RecordSuperPropertyUsage() { scope_uses_super_property_ = true; }
 
+  // Inform the scope that the corresponding code uses "this".
+  void RecordThisUsage() { scope_uses_this_ = true; }
+
   // Set the language mode flag (unless disabled by a global flag).
   void SetLanguageMode(LanguageMode language_mode) {
     language_mode_ = language_mode;
@@ -320,6 +323,10 @@ class Scope: public ZoneObject {
   bool inner_uses_super_property() const {
     return inner_scope_uses_super_property_;
   }
+  // Does this scope access "this".
+  bool uses_this() const { return scope_uses_this_; }
+  // Does any inner scope access "this".
+  bool inner_uses_this() const { return inner_scope_uses_this_; }
 
   const Scope* NearestOuterEvalScope() const {
     if (is_eval_scope()) return this;
@@ -570,6 +577,8 @@ class Scope: public ZoneObject {
   bool scope_uses_arguments_;
   // This scope uses "super" property ('super.foo').
   bool scope_uses_super_property_;
+  // This scope uses "this".
+  bool scope_uses_this_;
   // This scope contains an "use asm" annotation.
   bool asm_module_;
   // This scope's outer context is an asm module.
@@ -585,6 +594,7 @@ class Scope: public ZoneObject {
   bool inner_scope_calls_eval_;
   bool inner_scope_uses_arguments_;
   bool inner_scope_uses_super_property_;
+  bool inner_scope_uses_this_;
   bool force_eager_compilation_;
   bool force_context_allocation_;
 

test/cctest/test-parsing.cc

@@ -962,6 +962,7 @@ TEST(ScopeUsesArgumentsSuperThis) {
     THIS = 1 << 2,
     INNER_ARGUMENTS = 1 << 3,
     INNER_SUPER_PROPERTY = 1 << 4,
+    INNER_THIS = 1 << 5
   };
 
   static const struct {
@@ -976,7 +977,7 @@ TEST(ScopeUsesArgumentsSuperThis) {
     {"return this + arguments[0]", ARGUMENTS | THIS},
     {"return this + arguments[0] + super.x",
      ARGUMENTS | SUPER_PROPERTY | THIS},
-    {"return x => this + x", THIS},
+    {"return x => this + x", INNER_THIS},
     {"return x => super.f() + x", INNER_SUPER_PROPERTY},
     {"this.foo = 42;", THIS},
     {"this.foo();", THIS},
@@ -990,7 +991,9 @@ TEST(ScopeUsesArgumentsSuperThis) {
     {"while (true) { while (true) { while (true) return this } }", THIS},
     {"while (true) { while (true) { while (true) return super.f() } }",
      SUPER_PROPERTY},
-    {"if (1) { return () => { while (true) new this() } }", THIS},
+    {"if (1) { return () => { while (true) new this() } }", INNER_THIS},
+    // Note that propagation of the inner_uses_this() value does not
+    // cross boundaries of normal functions onto parent scopes.
     {"return function (x) { return this + x }", NONE},
     {"return { m(x) { return super.m() + x } }", NONE},
     {"var x = function () { this.foo = 42 };", NONE},
@@ -1001,10 +1004,10 @@ TEST(ScopeUsesArgumentsSuperThis) {
     {"return { m(x) { return () => super.m() } }", NONE},
     // Flags must be correctly set when using block scoping.
     {"\"use strict\"; while (true) { let x; this, arguments; }",
-     INNER_ARGUMENTS | THIS},
+     INNER_ARGUMENTS | INNER_THIS},
     {"\"use strict\"; while (true) { let x; this, super.f(), arguments; }",
-     INNER_ARGUMENTS | INNER_SUPER_PROPERTY | THIS},
-    {"\"use strict\"; if (foo()) { let x; this.f() }", THIS},
+     INNER_ARGUMENTS | INNER_SUPER_PROPERTY | INNER_THIS},
+    {"\"use strict\"; if (foo()) { let x; this.f() }", INNER_THIS},
     {"\"use strict\"; if (foo()) { let x; super.f() }",
      INNER_SUPER_PROPERTY},
     {"\"use strict\"; if (1) {"
@@ -1068,15 +1071,13 @@ TEST(ScopeUsesArgumentsSuperThis) {
                scope->uses_arguments());
       CHECK_EQ((source_data[i].expected & SUPER_PROPERTY) != 0,
                scope->uses_super_property());
-      if ((source_data[i].expected & THIS) != 0) {
-        // Currently the is_used() flag is conservative; all variables in a
-        // script scope are marked as used.
-        CHECK(scope->LookupThis()->is_used());
-      }
+      CHECK_EQ((source_data[i].expected & THIS) != 0, scope->uses_this());
       CHECK_EQ((source_data[i].expected & INNER_ARGUMENTS) != 0,
                scope->inner_uses_arguments());
       CHECK_EQ((source_data[i].expected & INNER_SUPER_PROPERTY) != 0,
                scope->inner_uses_super_property());
+      CHECK_EQ((source_data[i].expected & INNER_THIS) != 0,
+               scope->inner_uses_this());
     }
   }
 }