Refactor ParseFunctionLiteral.

It was a pretty monstrous 500 line function.

R=mstarzinger@chromium.org

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20752 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/parser.cc

@@ -3258,9 +3258,6 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
   FunctionLiteral::IsParenthesizedFlag parenthesized = parenthesized_function_
       ? FunctionLiteral::kIsParenthesized
       : FunctionLiteral::kNotParenthesized;
-  FunctionLiteral::IsGeneratorFlag generator = is_generator
-      ? FunctionLiteral::kIsGenerator
-      : FunctionLiteral::kNotGenerator;
   DeferredFeedbackSlotProcessor* slot_processor;
   AstProperties ast_properties;
   BailoutReason dont_optimize_reason = kNoReason;
@@ -3389,44 +3386,131 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
     parenthesized_function_ = false;  // The bit was set for this function only.
 
     if (is_lazily_parsed) {
+      SkipLazyFunctionBody(function_name, &materialized_literal_count,
+                           &expected_property_count, CHECK_OK);
+    } else {
+      body = ParseEagerFunctionBody(function_name, pos, fvar, fvar_init_op,
+                                    is_generator, CHECK_OK);
+      materialized_literal_count = function_state.materialized_literal_count();
+      expected_property_count = function_state.expected_property_count();
+      handler_count = function_state.handler_count();
+    }
+
+    // Validate strict mode. We can do this only after parsing the function,
+    // since the function can declare itself strict.
+    if (strict_mode() == STRICT) {
+      if (IsEvalOrArguments(function_name)) {
+        ReportMessageAt(function_name_location, "strict_eval_arguments");
+        *ok = false;
+        return NULL;
+      }
+      if (name_is_strict_reserved) {
+        ReportMessageAt(function_name_location, "unexpected_strict_reserved");
+        *ok = false;
+        return NULL;
+      }
+      if (eval_args_error_log.IsValid()) {
+        ReportMessageAt(eval_args_error_log, "strict_eval_arguments");
+        *ok = false;
+        return NULL;
+      }
+      if (dupe_error_loc.IsValid()) {
+        ReportMessageAt(dupe_error_loc, "strict_param_dupe");
+        *ok = false;
+        return NULL;
+      }
+      if (reserved_loc.IsValid()) {
+        ReportMessageAt(reserved_loc, "unexpected_strict_reserved");
+        *ok = false;
+        return NULL;
+      }
+      CheckOctalLiteral(scope->start_position(),
+                        scope->end_position(),
+                        CHECK_OK);
+    }
+    ast_properties = *factory()->visitor()->ast_properties();
+    slot_processor = factory()->visitor()->slot_processor();
+    dont_optimize_reason = factory()->visitor()->dont_optimize_reason();
+  }
+
+  if (allow_harmony_scoping() && strict_mode() == STRICT) {
+    CheckConflictingVarDeclarations(scope, CHECK_OK);
+  }
+
+  FunctionLiteral::IsGeneratorFlag generator = is_generator
+      ? FunctionLiteral::kIsGenerator
+      : FunctionLiteral::kNotGenerator;
+  FunctionLiteral* function_literal =
+      factory()->NewFunctionLiteral(function_name,
+                                    scope,
+                                    body,
+                                    materialized_literal_count,
+                                    expected_property_count,
+                                    handler_count,
+                                    num_parameters,
+                                    duplicate_parameters,
+                                    function_type,
+                                    FunctionLiteral::kIsFunction,
+                                    parenthesized,
+                                    generator,
+                                    pos);
+  function_literal->set_function_token_position(function_token_pos);
+  function_literal->set_ast_properties(&ast_properties);
+  function_literal->set_slot_processor(slot_processor);
+  function_literal->set_dont_optimize_reason(dont_optimize_reason);
+
+  if (fni_ != NULL && should_infer_name) fni_->AddFunction(function_literal);
+  return function_literal;
+}
+
+
+void Parser::SkipLazyFunctionBody(Handle<String> function_name,
+                                  int* materialized_literal_count,
+                                  int* expected_property_count,
+                                  bool* ok) {
   int function_block_pos = position();
-      FunctionEntry entry;
   if (cached_data_mode_ == CONSUME_CACHED_DATA) {
-        // If we have cached data, we use it to skip parsing the function body.
+    // If we have cached data, we use it to skip parsing the function body. The
-        // The data contains the information we need to construct the lazy
+    // data contains the information we need to construct the lazy function.
-        // function.
+    FunctionEntry entry =
-        entry = (*cached_data())->GetFunctionEntry(function_block_pos);
+        (*cached_data())->GetFunctionEntry(function_block_pos);
     if (entry.is_valid()) {
       if (entry.end_pos() <= function_block_pos) {
-            // End position greater than end of stream is safe, and hard
+        // End position greater than end of stream is safe, and hard to check.
-            // to check.
+        ReportInvalidCachedData(function_name, ok);
-            ReportInvalidCachedData(function_name, CHECK_OK);
+        if (!*ok) {
+          return;
+        }
       }
       scanner()->SeekForward(entry.end_pos() - 1);
 
-          scope->set_end_position(entry.end_pos());
+      scope_->set_end_position(entry.end_pos());
-          Expect(Token::RBRACE, CHECK_OK);
+      Expect(Token::RBRACE, ok);
+      if (!*ok) {
+        return;
+      }
       isolate()->counters()->total_preparse_skipped()->Increment(
-              scope->end_position() - function_block_pos);
+          scope_->end_position() - function_block_pos);
-          materialized_literal_count = entry.literal_count();
+      *materialized_literal_count = entry.literal_count();
-          expected_property_count = entry.property_count();
+      *expected_property_count = entry.property_count();
       scope_->SetStrictMode(entry.strict_mode());
     } else {
-          // This case happens when we have preparse data but it doesn't contain
+      // This case happens when we have preparse data but it doesn't contain an
-          // an entry for the function. Fail the compilation.
+      // entry for the function. Fail the compilation.
-          ReportInvalidCachedData(function_name, CHECK_OK);
+      ReportInvalidCachedData(function_name, ok);
+      return;
     }
   } else {
-        // With no cached data, we partially parse the function, without
+    // With no cached data, we partially parse the function, without building an
-        // building an AST. This gathers the data needed to build a lazy
+    // AST. This gathers the data needed to build a lazy function.
-        // function.
     SingletonLogger logger;
-        PreParser::PreParseResult result = LazyParseFunctionLiteral(&logger);
+    PreParser::PreParseResult result =
+        ParseLazyFunctionBodyWithPreParser(&logger);
     if (result == PreParser::kPreParseStackOverflow) {
       // Propagate stack overflow.
       set_stack_overflow();
       *ok = false;
-          return NULL;
+      return;
     }
     if (logger.has_error()) {
       const char* arg = logger.argument_opt();
@@ -3438,32 +3522,38 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
           Scanner::Location(logger.start(), logger.end()),
           logger.message(), args, logger.is_reference_error());
       *ok = false;
-          return NULL;
+      return;
+    }
+    scope_->set_end_position(logger.end());
+    Expect(Token::RBRACE, ok);
+    if (!*ok) {
+      return;
     }
-        scope->set_end_position(logger.end());
-        Expect(Token::RBRACE, CHECK_OK);
     isolate()->counters()->total_preparse_skipped()->Increment(
-            scope->end_position() - function_block_pos);
+        scope_->end_position() - function_block_pos);
-        materialized_literal_count = logger.literals();
+    *materialized_literal_count = logger.literals();
-        expected_property_count = logger.properties();
+    *expected_property_count = logger.properties();
     scope_->SetStrictMode(logger.strict_mode());
     if (cached_data_mode_ == PRODUCE_CACHED_DATA) {
       ASSERT(log_);
       // Position right after terminal '}'.
       int body_end = scanner()->location().end_pos;
       log_->LogFunction(function_block_pos, body_end,
-                            materialized_literal_count,
+                        *materialized_literal_count,
-                            expected_property_count,
+                        *expected_property_count,
                         scope_->strict_mode());
     }
   }
-    }
+}
 
-    if (!is_lazily_parsed) {
+
+ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
+    Handle<String> function_name, int pos, Variable* fvar,
+    Token::Value fvar_init_op, bool is_generator, bool* ok) {
   // Everything inside an eagerly parsed function will be parsed eagerly
   // (see comment above).
   ParsingModeScope parsing_mode(this, PARSE_EAGERLY);
-      body = new(zone()) ZoneList<Statement*>(8, zone());
+  ZoneList<Statement*>* body = new(zone()) ZoneList<Statement*>(8, zone());
   if (fvar != NULL) {
     VariableProxy* fproxy = scope_->NewUnresolved(
         factory(), function_name, Interface::NewConst());
@@ -3509,80 +3599,14 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
         yield, RelocInfo::kNoPosition), zone());
   }
 
-      materialized_literal_count = function_state.materialized_literal_count();
-      expected_property_count = function_state.expected_property_count();
-      handler_count = function_state.handler_count();
-
   Expect(Token::RBRACE, CHECK_OK);
-      scope->set_end_position(scanner()->location().end_pos);
+  scope_->set_end_position(scanner()->location().end_pos);
-    }
 
-    // Validate strict mode. We can do this only after parsing the function,
+  return body;
-    // since the function can declare itself strict.
-    if (strict_mode() == STRICT) {
-      if (IsEvalOrArguments(function_name)) {
-        ReportMessageAt(function_name_location, "strict_eval_arguments");
-        *ok = false;
-        return NULL;
-      }
-      if (name_is_strict_reserved) {
-        ReportMessageAt(function_name_location, "unexpected_strict_reserved");
-        *ok = false;
-        return NULL;
-      }
-      if (eval_args_error_log.IsValid()) {
-        ReportMessageAt(eval_args_error_log, "strict_eval_arguments");
-        *ok = false;
-        return NULL;
-      }
-      if (dupe_error_loc.IsValid()) {
-        ReportMessageAt(dupe_error_loc, "strict_param_dupe");
-        *ok = false;
-        return NULL;
-      }
-      if (reserved_loc.IsValid()) {
-        ReportMessageAt(reserved_loc, "unexpected_strict_reserved");
-        *ok = false;
-        return NULL;
-      }
-      CheckOctalLiteral(scope->start_position(),
-                        scope->end_position(),
-                        CHECK_OK);
-    }
-    ast_properties = *factory()->visitor()->ast_properties();
-    slot_processor = factory()->visitor()->slot_processor();
-    dont_optimize_reason = factory()->visitor()->dont_optimize_reason();
-  }
-
-  if (allow_harmony_scoping() && strict_mode() == STRICT) {
-    CheckConflictingVarDeclarations(scope, CHECK_OK);
-  }
-
-  FunctionLiteral* function_literal =
-      factory()->NewFunctionLiteral(function_name,
-                                    scope,
-                                    body,
-                                    materialized_literal_count,
-                                    expected_property_count,
-                                    handler_count,
-                                    num_parameters,
-                                    duplicate_parameters,
-                                    function_type,
-                                    FunctionLiteral::kIsFunction,
-                                    parenthesized,
-                                    generator,
-                                    pos);
-  function_literal->set_function_token_position(function_token_pos);
-  function_literal->set_ast_properties(&ast_properties);
-  function_literal->set_slot_processor(slot_processor);
-  function_literal->set_dont_optimize_reason(dont_optimize_reason);
-
-  if (fni_ != NULL && should_infer_name) fni_->AddFunction(function_literal);
-  return function_literal;
 }
 
 
-PreParser::PreParseResult Parser::LazyParseFunctionLiteral(
+PreParser::PreParseResult Parser::ParseLazyFunctionBodyWithPreParser(
     SingletonLogger* logger) {
   HistogramTimerScope preparse_scope(isolate()->counters()->pre_parse());
   ASSERT_EQ(Token::LBRACE, scanner()->current_token());

src/parser.h

@@ -790,9 +790,24 @@ class Parser : public ParserBase<ParserTraits> {
 
   Handle<String> LookupCachedSymbol(int symbol_id);
 
-  PreParser::PreParseResult LazyParseFunctionLiteral(
+  // Skip over a lazy function, either using cached data if we have it, or
+  // by parsing the function with PreParser. Consumes the ending }.
+  void SkipLazyFunctionBody(Handle<String> function_name,
+                            int* materialized_literal_count,
+                            int* expected_property_count,
+                            bool* ok);
+
+  PreParser::PreParseResult ParseLazyFunctionBodyWithPreParser(
       SingletonLogger* logger);
 
+  // Consumes the ending }.
+  ZoneList<Statement*>* ParseEagerFunctionBody(Handle<String> function_name,
+                                               int pos,
+                                               Variable* fvar,
+                                               Token::Value fvar_init_op,
+                                               bool is_generator,
+                                               bool* ok);
+
   Isolate* isolate_;
   ZoneList<Handle<String> > symbol_cache_;