PreParsing inner functions: Fix declaration-only variables.

If an inner function only declares a variable but doesn't use it, Parser
and PreParser produced different unresolved variables, and that confused
the pessimistic context allocation.

BUG=chromium:650969

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

src/ast/scopes.cc

@@ -997,6 +997,25 @@ bool Scope::RemoveUnresolved(VariableProxy* var) {
   return false;
 }
 
+bool Scope::RemoveUnresolved(const AstRawString* name) {
+  if (unresolved_->raw_name() == name) {
+    VariableProxy* removed = unresolved_;
+    unresolved_ = unresolved_->next_unresolved();
+    removed->set_next_unresolved(nullptr);
+    return true;
+  }
+  VariableProxy* current = unresolved_;
+  while (current != nullptr) {
+    VariableProxy* next = current->next_unresolved();
+    if (next->raw_name() == name) {
+      current->set_next_unresolved(next->next_unresolved());
+      next->set_next_unresolved(nullptr);
+      return true;
+    }
+    current = next;
+  }
+  return false;
+}
 
 Variable* Scope::NewTemporary(const AstRawString* name) {
   DeclarationScope* scope = GetClosureScope();

src/ast/scopes.h

@@ -172,6 +172,7 @@ class Scope: public ZoneObject {
   // allocated globally as a "ghost" variable. RemoveUnresolved removes
   // such a variable again if it was added; otherwise this is a no-op.
   bool RemoveUnresolved(VariableProxy* var);
+  bool RemoveUnresolved(const AstRawString* name);
 
   // Creates a new temporary variable in this scope's TemporaryScope.  The
   // name is only used for printing and cannot be used to find the variable.

src/parsing/preparser.cc

@@ -231,6 +231,29 @@ PreParserExpression PreParser::ExpressionFromIdentifier(
   return PreParserExpression::FromIdentifier(name);
 }
 
+void PreParser::DeclareAndInitializeVariables(
+    PreParserStatement block,
+    const DeclarationDescriptor* declaration_descriptor,
+    const DeclarationParsingResult::Declaration* declaration,
+    ZoneList<const AstRawString*>* names, bool* ok) {
+  if (declaration->pattern.string_) {
+    /* Mimic what Parser does when declaring variables (see
+       Parser::PatternRewriter::VisitVariableProxy).
+
+       var + no initializer -> RemoveUnresolved
+       let + no initializer -> RemoveUnresolved
+       var + initializer -> RemoveUnresolved followed by NewUnresolved
+       let + initializer -> RemoveUnresolved
+    */
+
+    if (declaration->initializer.IsEmpty() ||
+        declaration_descriptor->mode == VariableMode::LET) {
+      declaration_descriptor->scope->RemoveUnresolved(
+          declaration->pattern.string_);
+    }
+  }
+}
+
 #undef CHECK_OK
 #undef CHECK_OK_CUSTOM
 

src/parsing/preparser.h

@@ -137,7 +137,8 @@ class PreParserExpression {
 
   static PreParserExpression FromIdentifier(PreParserIdentifier id) {
     return PreParserExpression(TypeField::encode(kIdentifierExpression) |
-                               IdentifierTypeField::encode(id.type_));
+                                   IdentifierTypeField::encode(id.type_),
+                               id.string_);
   }
 
   static PreParserExpression BinaryOperation(PreParserExpression left,
@@ -343,8 +344,9 @@ class PreParserExpression {
     kAssignment
   };
 
-  explicit PreParserExpression(uint32_t expression_code)
-      : code_(expression_code) {}
+  explicit PreParserExpression(uint32_t expression_code,
+                               const AstRawString* string = nullptr)
+      : code_(expression_code), string_(string) {}
 
   // The first three bits are for the Type.
   typedef BitField<Type, 0, 3> TypeField;
@@ -366,6 +368,10 @@ class PreParserExpression {
   typedef BitField<bool, TypeField::kNext, 1> HasCoverInitializedNameField;
 
   uint32_t code_;
+  // Non-nullptr if the expression is one identifier.
+  const AstRawString* string_;
+
+  friend class PreParser;
 };
 
 
@@ -922,11 +928,12 @@ class PreParser : public ParserBase<PreParser> {
   }
   V8_INLINE void RewriteNonPattern(bool* ok) { ValidateExpression(ok); }
 
-  V8_INLINE void DeclareAndInitializeVariables(
+  void DeclareAndInitializeVariables(
       PreParserStatement block,
       const DeclarationDescriptor* declaration_descriptor,
       const DeclarationParsingResult::Declaration* declaration,
-      ZoneList<const AstRawString*>* names, bool* ok) {}
+      ZoneList<const AstRawString*>* names, bool* ok);
+
   V8_INLINE ZoneList<const AstRawString*>* DeclareLabel(
       ZoneList<const AstRawString*>* labels, PreParserExpression expr,
       bool* ok) {

test/mjsunit/fixed-context-shapes-when-recompiling.js

@@ -251,3 +251,112 @@
     assertEquals(3, c);
   }
 })();
+
+// A cluster of similar tests where the inner function only declares a variable
+// whose name clashes with an outer function variable name, but doesn't use it.
+(function TestRegress650969_1() {
+  for (var i = 0; i < 3; ++i) {
+    if (i == 1) {
+      %OptimizeOsr();
+    }
+    var a;
+    function inner() {
+      var a;
+    }
+  }
+})();
+
+(function TestRegress650969_2() {
+  for (var i = 0; i < 3; ++i) {
+    if (i == 1) {
+      %OptimizeOsr();
+    }
+    var a;
+    function inner() {
+      var a = 6;
+    }
+  }
+})();
+
+(function TestRegress650969_3() {
+  for (var i = 0; i < 3; ++i) {
+    if (i == 1) {
+      %OptimizeOsr();
+    }
+    var a;
+    function inner() {
+      var a, b;
+    }
+  }
+})();
+
+(function TestRegress650969_4() {
+  for (var i = 0; i < 3; ++i) {
+    if (i == 1) {
+      %OptimizeOsr();
+    }
+    var a;
+    function inner() {
+      var a = 6, b;
+    }
+  }
+})();
+
+(function TestRegress650969_5() {
+  for (var i = 0; i < 3; ++i) {
+    if (i == 1) {
+      %OptimizeOsr();
+    }
+    var a;
+    function inner() {
+      let a;
+    }
+  }
+})();
+
+(function TestRegress650969_6() {
+  for (var i = 0; i < 3; ++i) {
+    if (i == 1) {
+      %OptimizeOsr();
+    }
+    var a;
+    function inner() {
+      let a = 6;
+    }
+  }
+})();
+
+(function TestRegress650969_7() {
+  for (var i = 0; i < 3; ++i) {
+    if (i == 1) {
+      %OptimizeOsr();
+    }
+    var a;
+    function inner() {
+      let a, b;
+    }
+  }
+})();
+
+(function TestRegress650969_8() {
+  for (var i = 0; i < 3; ++i) {
+    if (i == 1) {
+      %OptimizeOsr();
+    }
+    var a;
+    function inner() {
+      let a = 6, b;
+    }
+  }
+})();
+
+(function TestRegress650969_9() {
+  for (var i = 0; i < 3; ++i) {
+    if (i == 1) {
+      %OptimizeOsr();
+    }
+    var a;
+    function inner(a) {
+    }
+  }
+})();