[parser] Fix single-expression arrow function scoping

Always parse through ParseFunctionBody to avoid bugs with parameter/scope
handling.

Change-Id: Ia0e78c6b3127e99f92a6c772ba2be509f6379f5a
Reviewed-on: https://chromium-review.googlesource.com/c/1268236
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56445}

src/parsing/parser-base.h

@@ -1150,8 +1150,6 @@ class ParserBase {
   ExpressionT ParseArrowFunctionLiteral(bool accept_IN,
                                         const FormalParametersT& parameters,
                                         int rewritable_length, bool* ok);
-  void ParseSingleExpressionFunctionBody(StatementListT body, bool is_async,
-                                         bool accept_IN, bool* ok);
   void ParseAsyncFunctionBody(Scope* scope, StatementListT body, bool* ok);
   ExpressionT ParseAsyncFunctionLiteral(bool* ok);
   ExpressionT ParseClassLiteral(IdentifierT name,
@@ -1186,11 +1184,14 @@ class ParserBase {
                                    bool default_export, bool* ok);
   StatementT ParseNativeDeclaration(bool* ok);
 
+  // Whether we're parsing a single-expression arrow function or something else.
+  enum class FunctionBodyType { kExpression, kBlock };
   // Consumes the ending }.
   void ParseFunctionBody(StatementListT result, IdentifierT function_name,
                          int pos, const FormalParametersT& parameters,
                          FunctionKind kind,
-                         FunctionLiteral::FunctionType function_type, bool* ok);
+                         FunctionLiteral::FunctionType function_type,
+                         FunctionBodyType body_type, bool accept_IN, bool* ok);
 
   // Under some circumstances, we allow preparsing to abort if the preparsed
   // function is "long and trivial", and fully parse instead. Our current
@@ -4186,7 +4187,8 @@ template <typename Impl>
 void ParserBase<Impl>::ParseFunctionBody(
     typename ParserBase<Impl>::StatementListT result, IdentifierT function_name,
     int pos, const FormalParametersT& parameters, FunctionKind kind,
-    FunctionLiteral::FunctionType function_type, bool* ok) {
+    FunctionLiteral::FunctionType function_type, FunctionBodyType body_type,
+    bool accept_IN, bool* ok) {
   DeclarationScope* function_scope = scope()->AsDeclarationScope();
   DeclarationScope* inner_scope = function_scope;
   BlockT inner_block = impl()->NullStatement();
@@ -4200,34 +4202,53 @@ void ParserBase<Impl>::ParseFunctionBody(
     body = inner_block->statements();
   }
 
-  // If we are parsing the source as if it is wrapped in a function, the source
-  // ends without a closing brace.
-  Token::Value closing_token =
-      function_type == FunctionLiteral::kWrapped ? Token::EOS : Token::RBRACE;
-
   {
     BlockState block_state(&scope_, inner_scope);
 
     if (IsResumableFunction(kind)) impl()->PrepareGeneratorVariables();
 
-    if (IsAsyncGeneratorFunction(kind)) {
-      impl()->ParseAndRewriteAsyncGeneratorFunctionBody(pos, kind, body, ok);
-    } else if (IsGeneratorFunction(kind)) {
-      impl()->ParseAndRewriteGeneratorFunctionBody(pos, kind, body, ok);
-    } else if (IsAsyncFunction(kind)) {
-      ParseAsyncFunctionBody(inner_scope, body, CHECK_OK_VOID);
+    if (body_type == FunctionBodyType::kExpression) {
+      ExpressionClassifier classifier(this);
+      ExpressionT expression =
+          ParseAssignmentExpression(accept_IN, CHECK_OK_VOID);
+      ValidateExpression(CHECK_OK_VOID);
+
+      if (IsAsyncFunction(kind)) {
+        BlockT block = factory()->NewBlock(1, true);
+        impl()->RewriteAsyncFunctionBody(body, block, expression,
+                                         CHECK_OK_VOID);
+      } else {
+        body->Add(BuildReturnStatement(expression, expression->position()),
+                  zone());
+      }
     } else {
-      ParseStatementList(body, closing_token, CHECK_OK_VOID);
-    }
+      DCHECK(accept_IN);
+      DCHECK_EQ(FunctionBodyType::kBlock, body_type);
+      // If we are parsing the source as if it is wrapped in a function, the
+      // source ends without a closing brace.
+      Token::Value closing_token = function_type == FunctionLiteral::kWrapped
+                                       ? Token::EOS
+                                       : Token::RBRACE;
+
+      if (IsAsyncGeneratorFunction(kind)) {
+        impl()->ParseAndRewriteAsyncGeneratorFunctionBody(pos, kind, body, ok);
+      } else if (IsGeneratorFunction(kind)) {
+        impl()->ParseAndRewriteGeneratorFunctionBody(pos, kind, body, ok);
+      } else if (IsAsyncFunction(kind)) {
+        ParseAsyncFunctionBody(inner_scope, body, CHECK_OK_VOID);
+      } else {
+        ParseStatementList(body, closing_token, CHECK_OK_VOID);
+      }
 
-    if (IsDerivedConstructor(kind)) {
-      body->Add(factory()->NewReturnStatement(impl()->ThisExpression(),
-                                              kNoSourcePosition),
-                zone());
+      if (IsDerivedConstructor(kind)) {
+        body->Add(factory()->NewReturnStatement(impl()->ThisExpression(),
+                                                kNoSourcePosition),
+                  zone());
+      }
+      Expect(closing_token, CHECK_OK_VOID);
     }
   }
 
-  Expect(closing_token, CHECK_OK_VOID);
   scope()->set_end_position(end_position());
 
   if (!parameters.is_simple) {
@@ -4428,7 +4449,8 @@ ParserBase<Impl>::ParseArrowFunctionLiteral(
           body = impl()->NewStatementList(8);
           ParseFunctionBody(body, impl()->NullIdentifier(), kNoSourcePosition,
                             formal_parameters, kind,
-                            FunctionLiteral::kAnonymousExpression, ok);
+                            FunctionLiteral::kAnonymousExpression,
+                            FunctionBodyType::kBlock, true, ok);
           CHECK(!*ok);
           return impl()->NullExpression();
         }
@@ -4437,17 +4459,18 @@ ParserBase<Impl>::ParseArrowFunctionLiteral(
         body = impl()->NewStatementList(8);
         ParseFunctionBody(body, impl()->NullIdentifier(), kNoSourcePosition,
                           formal_parameters, kind,
-                          FunctionLiteral::kAnonymousExpression, CHECK_OK);
+                          FunctionLiteral::kAnonymousExpression,
+                          FunctionBodyType::kBlock, true, CHECK_OK);
         expected_property_count = function_state.expected_property_count();
       }
     } else {
       // Single-expression body
       has_braces = false;
-      const bool is_async = IsAsyncFunction(kind);
       body = impl()->NewStatementList(1);
-      impl()->AddParameterInitializationBlock(formal_parameters, body, is_async,
-                                              CHECK_OK);
-      ParseSingleExpressionFunctionBody(body, is_async, accept_IN, CHECK_OK);
+      ParseFunctionBody(body, impl()->NullIdentifier(), kNoSourcePosition,
+                        formal_parameters, kind,
+                        FunctionLiteral::kAnonymousExpression,
+                        FunctionBodyType::kExpression, accept_IN, CHECK_OK);
       expected_property_count = function_state.expected_property_count();
     }
 
@@ -4584,25 +4607,6 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseClassLiteral(
                                      class_token_pos, end_pos, ok);
 }
 
-template <typename Impl>
-void ParserBase<Impl>::ParseSingleExpressionFunctionBody(StatementListT body,
-                                                         bool is_async,
-                                                         bool accept_IN,
-                                                         bool* ok) {
-  if (is_async) impl()->PrepareGeneratorVariables();
-
-  ExpressionClassifier classifier(this);
-  ExpressionT expression = ParseAssignmentExpression(accept_IN, CHECK_OK_VOID);
-  ValidateExpression(CHECK_OK_VOID);
-
-  if (is_async) {
-    BlockT block = factory()->NewBlock(1, true);
-    impl()->RewriteAsyncFunctionBody(body, block, expression, CHECK_OK_VOID);
-  } else {
-    body->Add(BuildReturnStatement(expression, expression->position()), zone());
-  }
-}
-
 template <typename Impl>
 void ParserBase<Impl>::ParseAsyncFunctionBody(Scope* scope, StatementListT body,
                                               bool* ok) {

src/parsing/parser.cc

@@ -3052,7 +3052,8 @@ ZonePtrList<Statement>* Parser::ParseFunction(
   *function_length = formals.function_length;
 
   ZonePtrList<Statement>* body = new (zone()) ZonePtrList<Statement>(8, zone());
-  ParseFunctionBody(body, function_name, pos, formals, kind, function_type, ok);
+  ParseFunctionBody(body, function_name, pos, formals, kind, function_type,
+                    FunctionBodyType::kBlock, true, ok);
 
   // Validate parameter names. We can do this only after parsing the function,
   // since the function can declare itself strict.

src/parsing/parser.h

@@ -902,18 +902,6 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
         factory()->NewThrow(exception, pos), pos);
   }
 
-  V8_INLINE void AddParameterInitializationBlock(
-      const ParserFormalParameters& parameters, ZonePtrList<Statement>* body,
-      bool is_async, bool* ok) {
-    if (parameters.is_simple) return;
-    auto* init_block = BuildParameterInitializationBlock(parameters, ok);
-    if (!*ok) return;
-    if (is_async) {
-      init_block = BuildRejectPromiseOnException(init_block);
-    }
-    body->Add(init_block, zone());
-  }
-
   V8_INLINE void AddFormalParameter(ParserFormalParameters* parameters,
                                     Expression* pattern,
                                     Expression* initializer,

src/parsing/preparser.cc

@@ -344,7 +344,7 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
   int pos = function_token_pos == kNoSourcePosition ? peek_position()
                                                     : function_token_pos;
   ParseFunctionBody(body, function_name, pos, formals, kind, function_type,
-                    CHECK_OK);
+                    FunctionBodyType::kBlock, true, CHECK_OK);
 
   // Parsing the body may change the language mode in our scope.
   language_mode = function_scope->language_mode();

src/parsing/preparser.h

@@ -1697,14 +1697,6 @@ class PreParser : public ParserBase<PreParser> {
     return PreParserStatement::Jump();
   }
 
-  V8_INLINE void AddParameterInitializationBlock(
-      const PreParserFormalParameters& parameters, PreParserStatementList body,
-      bool is_async, bool* ok) {
-    if (!parameters.is_simple) {
-      BuildParameterInitializationBlock(parameters, ok);
-    }
-  }
-
   V8_INLINE void AddFormalParameter(PreParserFormalParameters* parameters,
                                     const PreParserExpression& pattern,
                                     const PreParserExpression& initializer,

test/mjsunit/regress/regress-arrow-single-expression-eval.js

+// Copyright 2018 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.
+
+((x=1) => eval("var x = 10"))();