Merged: Squashed multiple commits.

Merged: [parser] Fix crash when lazy arrow func params contain destructuring assignments. Revision: bc39a51 Merged: [parser] don't rewrite destructuring assignments in params for lazy top level arrow functions Revision: 5f782db BUG=chromium:704811,chromium:706234,chromium:706761,v8:6182 LOG=N NOTRY=true NOPRESUBMIT=true NOTREECHECKS=true Change-Id: If5c04c3b9f6ac9c6879052b6a34446f895624200 Reviewed-on: https://chromium-review.googlesource.com/474746 Reviewed-by: Michael Achenbach <[email protected]> Cr-Commit-Position: refs/branch-heads/5.8@{crosswalk-project#58} Cr-Branched-From: eda659c-refs/heads/5.8.283@{crosswalk-project#1} Cr-Branched-From: 4310cd0-refs/heads/master@{#43429}
asifhisam · Apr 11, 2017 · 01d19f7 · 01d19f7
1 parent 296298d
commit 01d19f7
Show file tree

Hide file tree

Showing 6 changed files with 178 additions and 5 deletions.
diff --git a/src/ast/scopes.cc b/src/ast/scopes.cc
@@ -1430,7 +1430,11 @@ void DeclarationScope::ResetAfterPreparsing(AstValueFactory* ast_value_factory,
   DCHECK(is_function_scope());
 
   // Reset all non-trivial members.
-  params_.Clear();
+  if (!aborted || !IsArrowFunction(function_kind_)) {
+    // Do not remove parameters when lazy parsing an Arrow Function has failed,
+    // as the formal parameters are not re-parsed.
+    params_.Clear();
+  }
   decls_.Clear();
   locals_.Clear();
   inner_scope_ = nullptr;

diff --git a/src/parsing/parser-base.h b/src/parsing/parser-base.h
@@ -395,6 +395,17 @@ class ParserBase {
       return scope()->promise_var();
     }
 
+    void RewindDestructuringAssignments(int pos) {
+      destructuring_assignments_to_rewrite_.Rewind(pos);
+    }
+
+    void SetDestructuringAssignmentsScope(int pos, Scope* scope) {
+      for (int i = pos; i < destructuring_assignments_to_rewrite_.length();
+           ++i) {
+        destructuring_assignments_to_rewrite_[i].scope = scope;
+      }
+    }
+
     const ZoneList<DestructuringAssignment>&
         destructuring_assignments_to_rewrite() const {
       return destructuring_assignments_to_rewrite_;
@@ -1138,9 +1149,14 @@ class ParserBase {
   ExpressionT ParseMemberExpression(bool* is_async, bool* ok);
   ExpressionT ParseMemberExpressionContinuation(ExpressionT expression,
                                                 bool* is_async, bool* ok);
+
+  // `rewritable_length`: length of the destructuring_assignments_to_rewrite()
+  // queue in the parent function state, prior to parsing of formal parameters.
+  // If the arrow function is lazy, any items added during formal parameter
+  // parsing are removed from the queue.
   ExpressionT ParseArrowFunctionLiteral(bool accept_IN,
                                         const FormalParametersT& parameters,
-                                        bool* ok);
+                                        int rewritable_length, bool* ok);
   void ParseAsyncFunctionBody(Scope* scope, StatementListT body,
                               FunctionKind kind, FunctionBodyType type,
                               bool accept_IN, int pos, bool* ok);
@@ -2679,6 +2695,8 @@ ParserBase<Impl>::ParseAssignmentExpression(bool accept_IN, bool* ok) {
       this, classifier()->duplicate_finder());
 
   Scope::Snapshot scope_snapshot(scope());
+  int rewritable_length =
+      function_state_->destructuring_assignments_to_rewrite().length();
 
   bool is_async = peek() == Token::ASYNC &&
                   !scanner()->HasAnyLineTerminatorAfterNext() &&
@@ -2732,6 +2750,7 @@ ParserBase<Impl>::ParseAssignmentExpression(bool accept_IN, bool* ok) {
     this->scope()->PropagateUsageFlagsToScope(scope);
 
     scope_snapshot.Reparent(scope);
+    function_state_->SetDestructuringAssignmentsScope(rewritable_length, scope);
 
     FormalParametersT parameters(scope);
     if (!classifier()->is_simple_parameter_list()) {
@@ -2748,7 +2767,8 @@ ParserBase<Impl>::ParseAssignmentExpression(bool accept_IN, bool* ok) {
     if (duplicate_loc.IsValid()) {
       classifier()->RecordDuplicateFormalParameterError(duplicate_loc);
     }
-    expression = ParseArrowFunctionLiteral(accept_IN, parameters, CHECK_OK);
+    expression = ParseArrowFunctionLiteral(accept_IN, parameters,
+                                           rewritable_length, CHECK_OK);
     impl()->Discard();
     classifier()->RecordPatternError(arrow_loc,
                                      MessageTemplate::kUnexpectedToken,
@@ -4097,7 +4117,8 @@ bool ParserBase<Impl>::IsTrivialExpression() {
 template <typename Impl>
 typename ParserBase<Impl>::ExpressionT
 ParserBase<Impl>::ParseArrowFunctionLiteral(
-    bool accept_IN, const FormalParametersT& formal_parameters, bool* ok) {
+    bool accept_IN, const FormalParametersT& formal_parameters,
+    int rewritable_length, bool* ok) {
   const RuntimeCallStats::CounterId counters[2][2] = {
       {&RuntimeCallStats::ParseBackgroundArrowFunctionLiteral,
        &RuntimeCallStats::ParseArrowFunctionLiteral},
@@ -4228,6 +4249,14 @@ ParserBase<Impl>::ParseArrowFunctionLiteral(
     }
     impl()->CheckConflictingVarDeclarations(formal_parameters.scope, CHECK_OK);
 
+    if (is_lazy_top_level_function) {
+      FunctionState* parent_state = function_state.outer();
+      DCHECK_NOT_NULL(parent_state);
+      DCHECK_GE(parent_state->destructuring_assignments_to_rewrite().length(),
+                rewritable_length);
+      parent_state->RewindDestructuringAssignments(rewritable_length);
+    }
+
     impl()->RewriteDestructuringAssignments();
   }
 

diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc
@@ -880,6 +880,8 @@ FunctionLiteral* Parser::DoParseFunction(ParseInfo* info,
       scope->set_start_position(info->start_position());
       ExpressionClassifier formals_classifier(this);
       ParserFormalParameters formals(scope);
+      int rewritable_length =
+          function_state.destructuring_assignments_to_rewrite().length();
       Checkpoint checkpoint(this);
       {
         // Parsing patterns as variable reference expression creates
@@ -916,7 +918,8 @@ FunctionLiteral* Parser::DoParseFunction(ParseInfo* info,
 
         // Pass `accept_IN=true` to ParseArrowFunctionLiteral --- This should
         // not be observable, or else the preparser would have failed.
-        Expression* expression = ParseArrowFunctionLiteral(true, formals, &ok);
+        Expression* expression =
+            ParseArrowFunctionLiteral(true, formals, rewritable_length, &ok);
         if (ok) {
           // Scanning must end at the same position that was recorded
           // previously. If not, parsing has been interrupted due to a stack
@@ -929,6 +932,10 @@ FunctionLiteral* Parser::DoParseFunction(ParseInfo* info,
             // must produce a FunctionLiteral.
             DCHECK(expression->IsFunctionLiteral());
             result = expression->AsFunctionLiteral();
+            // Rewrite destructuring assignments in the parameters. (The ones
+            // inside the function body are rewritten by
+            // ParseArrowFunctionLiteral.)
+            RewriteDestructuringAssignments();
           } else {
             ok = false;
           }

diff --git a/test/mjsunit/regress/regress-704811.js b/test/mjsunit/regress/regress-704811.js
@@ -0,0 +1,88 @@
+// 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.
+
+// The bug was that destructuring assignments which occur inside a lazy arrow
+// function parameter list were not rewritten.
+
+// Repro from the bug (slightly modified so that it doesn't produce a run-time
+// exception).
+(({x = {} = {}}) => {})({});
+
+// ... and without the parens.
+let a0 = ({x = {} = {}}) => {};
+a0({});
+
+// Testing that the destructuring assignments also work properly. The semantics
+// are: The value of the destructuring assignment is an object {myprop: 2115}
+// and 2115 also gets assigned to global_side_assignment. So the default value
+// for x is {myprop: 2115}. This is the value which x will have if the function
+// is called with an object which doesn't have property x.
+let called = false;
+let global_side_assignment = undefined;
+(({x = {myprop: global_side_assignment} = {myprop: 2115}}) => {
+  assertTrue('myprop' in x);
+  assertEquals(2115, x.myprop);
+  called = true;
+})({});
+assertTrue(called);
+assertEquals(2115, global_side_assignment);
+
+// If the parameter is an object which has property x, the default value is not
+// used.
+called = false;
+global_side_assignment = undefined;
+(({x = {myprop: global_side_assignment} = {myprop: 2115}}) => {
+  assertEquals(3000, x);
+  called = true;
+})({x: 3000});
+assertTrue(called);
+// Global side assignment doesn't happen, since the default value was not used.
+assertEquals(undefined, global_side_assignment);
+
+// Different kinds of lazy arrow functions (it's actually a bit weird that the
+// above functions are lazy, since they are parenthesized).
+called = false;
+global_side_assignment = undefined;
+let a1 = ({x = {myprop: global_side_assignment} = {myprop: 2115}}) => {
+  assertTrue('myprop' in x);
+  assertEquals(2115, x.myprop);
+  called = true;
+}
+a1({});
+assertTrue(called);
+assertEquals(2115, global_side_assignment);
+
+called = false;
+global_side_assignment = undefined;
+let a2 = ({x = {myprop: global_side_assignment} = {myprop: 2115}}) => {
+  assertEquals(3000, x);
+  called = true;
+}
+a2({x: 3000});
+assertTrue(called);
+assertEquals(undefined, global_side_assignment);
+
+// We never had a problem with non-arrow functions, but testing them too for
+// completeness.
+called = false;
+global_side_assignment = undefined;
+function f1({x = {myprop: global_side_assignment} = {myprop: 2115}}) {
+  assertTrue('myprop' in x);
+  assertEquals(2115, x.myprop);
+  assertEquals(2115, global_side_assignment);
+  called = true;
+}
+f1({});
+assertTrue(called);
+assertEquals(2115, global_side_assignment);
+
+called = false;
+global_side_assignment = undefined;
+function f2({x = {myprop: global_side_assignment} = {myprop: 2115}}) {
+  assertEquals(3000, x);
+  called = true;
+}
+f2({x: 3000});
+assertTrue(called);
+assertEquals(undefined, global_side_assignment);
diff --git a/test/mjsunit/regress/regress-706234-2.js b/test/mjsunit/regress/regress-706234-2.js
@@ -0,0 +1,37 @@
+// 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.
+
+// Lazy top-level arrow function which must be re-parsed and eagerly compiled.
+var f = ({ x } = { x: 1 }) => {
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
+};
+
+f();
diff --git a/test/mjsunit/regress/regress-706234.js b/test/mjsunit/regress/regress-706234.js
@@ -0,0 +1,8 @@
+// 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.
+
+var fn = ({foo = {} = {}}) => { return foo; }
+if (true) {
+  fn({});
+}