[parser] Simplify FuncNameInferrer juggling in the parser

Rather than allocating one in the zone and swapping them on discardable zone
swap, we simply swap the zone in the inferrer and allow the stacks to grow in
the outer zone. The inner segments will be dropped anyway.

This also introduces a PreParserFuncNameInferrer that just has dummy
implementations. That way we can avoid checking whether fni_ is nullptr at
runtime.

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

src/parsing/func-name-inferrer.h

@@ -36,12 +36,8 @@ class FuncNameInferrer : public ZoneObject {
   // on the stack.
   class State {
    public:
-    explicit State(FuncNameInferrer* fni) : fni_(fni) {
-      if (fni_ != nullptr) fni_->Enter();
-    }
-    ~State() {
-      if (fni_ != nullptr) fni_->Leave();
-    }
+    explicit State(FuncNameInferrer* fni) : fni_(fni) { fni_->Enter(); }
+    ~State() { fni_->Leave(); }
 
    private:
     FuncNameInferrer* fni_;
@@ -83,6 +79,8 @@ class FuncNameInferrer : public ZoneObject {
     }
   }
 
+  void set_zone(Zone* new_zone) { zone_ = new_zone; }
+
  private:
   enum NameType {
     kEnclosingConstructorName,

src/parsing/parser-base.h

@@ -243,6 +243,8 @@ class ParserBase {
   typedef typename Types::ForStatement ForStatementT;
   typedef typename v8::internal::ExpressionClassifier<Types>
       ExpressionClassifier;
+  typedef typename Types::FuncNameInferrer FuncNameInferrer;
+  typedef typename Types::FuncNameInferrer::State FuncNameInferrerState;
 
   // All implementation-specific methods must be called through this.
   Impl* impl() { return static_cast<Impl*>(this); }
@@ -257,7 +259,7 @@ class ParserBase {
         original_scope_(nullptr),
         function_state_(nullptr),
         extension_(extension),
-        fni_(nullptr),
+        fni_(ast_value_factory, zone),
         ast_value_factory_(ast_value_factory),
         ast_node_factory_(ast_value_factory, zone),
         runtime_call_stats_(runtime_call_stats),
@@ -1519,7 +1521,7 @@ class ParserBase {
   Scope* original_scope_;  // The top scope for the current parsing item.
   FunctionState* function_state_;  // Function state stack.
   v8::Extension* extension_;
-  FuncNameInferrer* fni_;
+  FuncNameInferrer fni_;
   AstValueFactory* ast_value_factory_;  // Not owned.
   typename Types::Factory ast_node_factory_;
   RuntimeCallStats* runtime_call_stats_;
@@ -2712,7 +2714,7 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseObjectLiteral(
   Consume(Token::LBRACE);
 
   while (peek() != Token::RBRACE) {
-    FuncNameInferrer::State fni_state(fni_);
+    FuncNameInferrerState fni_state(&fni_);
 
     bool is_computed_name = false;
     bool is_rest_property = false;
@@ -2740,7 +2742,7 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseObjectLiteral(
       Expect(Token::COMMA, CHECK_OK);
     }
 
-    if (fni_ != nullptr) fni_->Infer();
+    fni_.Infer();
   }
   Expect(Token::RBRACE, CHECK_OK);
 
@@ -2849,7 +2851,7 @@ ParserBase<Impl>::ParseAssignmentExpression(bool accept_IN, bool* ok) {
     return ParseYieldExpression(accept_IN, ok);
   }
 
-  FuncNameInferrer::State fni_state(fni_);
+  FuncNameInferrerState fni_state(&fni_);
   ExpressionClassifier arrow_formals_classifier(
       this, classifier()->duplicate_finder());
 
@@ -2881,10 +2883,8 @@ ParserBase<Impl>::ParseAssignmentExpression(bool accept_IN, bool* ok) {
     IdentifierT name = ParseAndClassifyIdentifier(CHECK_OK);
     expression =
         impl()->ExpressionFromIdentifier(name, position(), InferName::kNo);
-    if (fni_) {
-      // Remove `async` keyword from inferred name stack.
-      fni_->RemoveAsyncKeywordFromEnd();
-    }
+    // Remove `async` keyword from inferred name stack.
+    fni_.RemoveAsyncKeywordFromEnd();
   }
 
   if (peek() == Token::ARROW) {
@@ -2928,7 +2928,7 @@ ParserBase<Impl>::ParseAssignmentExpression(bool accept_IN, bool* ok) {
                                      MessageTemplate::kUnexpectedToken,
                                      Token::String(Token::ARROW));
 
-    if (fni_ != nullptr) fni_->Infer();
+    fni_.Infer();
 
     return expression;
   }
@@ -2993,15 +2993,13 @@ ParserBase<Impl>::ParseAssignmentExpression(bool accept_IN, bool* ok) {
 
   impl()->CheckAssigningFunctionLiteralToProperty(expression, right);
 
-  if (fni_ != nullptr) {
-    // Check if the right hand side is a call to avoid inferring a
-    // name if we're dealing with "a = function(){...}();"-like
-    // expression.
-    if (op == Token::ASSIGN && !right->IsCall() && !right->IsCallNew()) {
-      fni_->Infer();
-    } else {
-      fni_->RemoveLastFunction();
-    }
+  // Check if the right hand side is a call to avoid inferring a
+  // name if we're dealing with "a = function(){...}();"-like
+  // expression.
+  if (op == Token::ASSIGN && !right->IsCall() && !right->IsCallNew()) {
+    fni_.Infer();
+  } else {
+    fni_.RemoveLastFunction();
   }
 
   if (op == Token::ASSIGN) {
@@ -3364,9 +3362,7 @@ ParserBase<Impl>::ParseLeftHandSideExpression(bool* ok) {
           args = ParseArguments(&spread_pos, true, &is_simple_parameter_list,
                                 CHECK_OK);
           if (peek() == Token::ARROW) {
-            if (fni_) {
-              fni_->RemoveAsyncKeywordFromEnd();
-            }
+            fni_.RemoveAsyncKeywordFromEnd();
             ValidateBindingPattern(CHECK_OK);
             ValidateFormalParameterInitializer(CHECK_OK);
             if (!classifier()->is_valid_async_arrow_formal_parameters()) {
@@ -3409,7 +3405,7 @@ ParserBase<Impl>::ParseLeftHandSideExpression(bool* ok) {
           result = factory()->NewCall(result, args, pos, is_possibly_eval);
         }
 
-        if (fni_ != nullptr) fni_->RemoveLastFunction();
+        fni_.RemoveLastFunction();
         break;
       }
 
@@ -3746,7 +3742,7 @@ void ParserBase<Impl>::ParseFormalParameter(FormalParametersT* parameters,
   //   BindingElement[?Yield, ?GeneratorParameter]
   bool is_rest = parameters->has_rest;
 
-  FuncNameInferrer::State fni_state(fni_);
+  FuncNameInferrerState fni_state(&fni_);
   ExpressionT pattern = ParsePrimaryExpression(CHECK_OK_CUSTOM(Void));
   ValidateBindingPattern(CHECK_OK_CUSTOM(Void));
 
@@ -3874,7 +3870,7 @@ typename ParserBase<Impl>::BlockT ParserBase<Impl>::ParseVariableDeclarations(
   int bindings_start = peek_position();
   do {
     // Parse binding pattern.
-    FuncNameInferrer::State fni_state(fni_);
+    FuncNameInferrerState fni_state(&fni_);
 
     ExpressionT pattern = impl()->NullExpression();
     int decl_pos = peek_position();
@@ -3912,11 +3908,11 @@ typename ParserBase<Impl>::BlockT ParserBase<Impl>::ParseVariableDeclarations(
       }
 
       // Don't infer if it is "a = function(){...}();"-like expression.
-      if (single_name && fni_ != nullptr) {
+      if (single_name) {
         if (!value->IsCall() && !value->IsCallNew()) {
-          fni_->Infer();
+          fni_.Infer();
         } else {
-          fni_->RemoveLastFunction();
+          fni_.RemoveLastFunction();
         }
       }
 
@@ -4039,7 +4035,7 @@ ParserBase<Impl>::ParseHoistableDeclaration(
     variable_name = name;
   }
 
-  FuncNameInferrer::State fni_state(fni_);
+  FuncNameInferrerState fni_state(&fni_);
   impl()->PushEnclosingName(name);
 
   FunctionKind kind = FunctionKindFor(flags);
@@ -4494,7 +4490,7 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseClassLiteral(
 
   scope()->set_start_position(scanner()->location().end_pos);
   if (Check(Token::EXTENDS)) {
-    FuncNameInferrer::State fni_state(fni_);
+    FuncNameInferrerState fni_state(&fni_);
     ExpressionClassifier extends_classifier(this);
     class_info.extends = ParseLeftHandSideExpression(CHECK_OK);
     ValidateExpression(CHECK_OK);
@@ -4508,7 +4504,7 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseClassLiteral(
   const bool has_extends = !impl()->IsNull(class_info.extends);
   while (peek() != Token::RBRACE) {
     if (Check(Token::SEMICOLON)) continue;
-    FuncNameInferrer::State fni_state(fni_);
+    FuncNameInferrerState fni_state(&fni_);
     bool is_computed_name = false;  // Classes do not care about computed
                                     // property names here.
     bool is_static;

src/parsing/parser.cc

@@ -36,9 +36,7 @@ namespace internal {
 class DiscardableZoneScope {
  public:
   DiscardableZoneScope(Parser* parser, Zone* temp_zone, bool use_temp_zone)
-      : fni_(parser->ast_value_factory_, temp_zone),
-        parser_(parser),
-        prev_fni_(parser->fni_),
+      : parser_(parser),
         prev_zone_(parser->zone_),
         prev_allow_lazy_(parser->allow_lazy_),
         prev_temp_zoned_(parser->temp_zoned_) {
@@ -46,7 +44,7 @@ class DiscardableZoneScope {
       DCHECK(!parser_->temp_zoned_);
       parser_->allow_lazy_ = false;
       parser_->temp_zoned_ = true;
-      parser_->fni_ = &fni_;
+      parser_->fni_.set_zone(temp_zone);
       parser_->zone_ = temp_zone;
       parser_->factory()->set_zone(temp_zone);
       if (parser_->reusable_preparser_ != nullptr) {
@@ -56,7 +54,7 @@ class DiscardableZoneScope {
     }
   }
   void Reset() {
-    parser_->fni_ = prev_fni_;
+    parser_->fni_.set_zone(prev_zone_);
     parser_->zone_ = prev_zone_;
     parser_->factory()->set_zone(prev_zone_);
     parser_->allow_lazy_ = prev_allow_lazy_;
@@ -69,9 +67,7 @@ class DiscardableZoneScope {
   ~DiscardableZoneScope() { Reset(); }
 
  private:
-  FuncNameInferrer fni_;
   Parser* parser_;
-  FuncNameInferrer* prev_fni_;
   Zone* prev_zone_;
   bool prev_allow_lazy_;
   bool prev_temp_zoned_;
@@ -508,7 +504,6 @@ FunctionLiteral* Parser::ParseProgram(Isolate* isolate, ParseInfo* info) {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"), "V8.ParseProgram");
   base::ElapsedTimer timer;
   if (V8_UNLIKELY(FLAG_log_function_events)) timer.Start();
-  fni_ = new (zone()) FuncNameInferrer(ast_value_factory(), zone());
 
   // Initialize parser state.
   DeserializeScopeChain(isolate, info, info->maybe_outer_scope_info());
@@ -754,8 +749,7 @@ FunctionLiteral* Parser::DoParseFunction(Isolate* isolate, ParseInfo* info,
   DCHECK_NULL(target_stack_);
 
   DCHECK(ast_value_factory());
-  fni_ = new (zone()) FuncNameInferrer(ast_value_factory(), zone());
-  fni_->PushEnclosingName(raw_name);
+  fni_.PushEnclosingName(raw_name);
 
   ResetFunctionLiteralId();
   DCHECK_LT(0, info->function_literal_id());
@@ -2710,8 +2704,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
   function_literal->set_suspend_count(suspend_count);
 
   if (should_infer_name) {
-    DCHECK_NOT_NULL(fni_);
-    fni_->AddFunction(function_literal);
+    fni_.AddFunction(function_literal);
   }
   return function_literal;
 }
@@ -3464,7 +3457,6 @@ void Parser::ParseOnBackground(ParseInfo* info) {
   // position set at the end of the script (the top scope and possible eval
   // scopes) and set their end position after we know the script length.
   if (info->is_toplevel()) {
-    fni_ = new (zone()) FuncNameInferrer(ast_value_factory(), zone());
     result = DoParseProgram(/* isolate = */ nullptr, info);
   } else {
     result =

src/parsing/parser.h

@@ -135,6 +135,7 @@ struct ParserTypes<Parser> {
   typedef v8::internal::BreakableStatement* BreakableStatement;
   typedef v8::internal::ForStatement* ForStatement;
   typedef v8::internal::IterationStatement* IterationStatement;
+  typedef v8::internal::FuncNameInferrer FuncNameInferrer;
 
   // For constructing objects returned by the traversing functions.
   typedef AstNodeFactory Factory;
@@ -665,38 +666,30 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   // Functions for encapsulating the differences between parsing and preparsing;
   // operations interleaved with the recursive descent.
   V8_INLINE void PushLiteralName(const AstRawString* id) {
-    DCHECK_NOT_NULL(fni_);
-    fni_->PushLiteralName(id);
+    fni_.PushLiteralName(id);
   }
 
   V8_INLINE void PushVariableName(const AstRawString* id) {
-    DCHECK_NOT_NULL(fni_);
-    fni_->PushVariableName(id);
+    fni_.PushVariableName(id);
   }
 
   V8_INLINE void PushPropertyName(Expression* expression) {
-    DCHECK_NOT_NULL(fni_);
     if (expression->IsPropertyName()) {
-      fni_->PushLiteralName(expression->AsLiteral()->AsRawPropertyName());
+      fni_.PushLiteralName(expression->AsLiteral()->AsRawPropertyName());
     } else {
-      fni_->PushLiteralName(ast_value_factory()->anonymous_function_string());
+      fni_.PushLiteralName(ast_value_factory()->anonymous_function_string());
     }
   }
 
   V8_INLINE void PushEnclosingName(const AstRawString* name) {
-    DCHECK_NOT_NULL(fni_);
-    fni_->PushEnclosingName(name);
+    fni_.PushEnclosingName(name);
   }
 
   V8_INLINE void AddFunctionForNameInference(FunctionLiteral* func_to_infer) {
-    DCHECK_NOT_NULL(fni_);
-    fni_->AddFunction(func_to_infer);
+    fni_.AddFunction(func_to_infer);
   }
 
-  V8_INLINE void InferFunctionName() {
-    DCHECK_NOT_NULL(fni_);
-    fni_->Infer();
-  }
+  V8_INLINE void InferFunctionName() { fni_.Infer(); }
 
   // If we assign a function literal to a property we pretenure the
   // literal so it can be added as a constant function property.
@@ -859,14 +852,14 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
       const AstRawString* name, int start_position,
       InferName infer = InferName::kYes) {
     if (infer == InferName::kYes) {
-      fni_->PushVariableName(name);
+      fni_.PushVariableName(name);
     }
     return NewUnresolved(name, start_position);
   }
 
   V8_INLINE Expression* ExpressionFromString(int pos) {
     const AstRawString* symbol = GetSymbol();
-    fni_->PushLiteralName(symbol);
+    fni_.PushLiteralName(symbol);
     return factory()->NewStringLiteral(symbol, pos);
   }
 

src/parsing/preparser.h

@@ -887,6 +887,25 @@ class PreParserTargetScope {
   explicit PreParserTargetScope(ParserBase<PreParser>* preparser) {}
 };
 
+class PreParserFuncNameInferrer {
+ public:
+  PreParserFuncNameInferrer(AstValueFactory* avf, Zone* zone) {}
+  void RemoveAsyncKeywordFromEnd() {}
+  void Infer() {}
+  void RemoveLastFunction() {}
+
+  class State {
+   public:
+    explicit State(PreParserFuncNameInferrer* fni) {}
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(State);
+  };
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PreParserFuncNameInferrer);
+};
+
 template <>
 struct ParserTypes<PreParser> {
   typedef ParserBase<PreParser> Base;
@@ -916,6 +935,7 @@ struct ParserTypes<PreParser> {
 
   typedef PreParserTarget Target;
   typedef PreParserTargetScope TargetScope;
+  typedef PreParserFuncNameInferrer FuncNameInferrer;
 };