Revert of Don't use different function scopes when parsing with temp zones

Revert of Don't use different function scopes when parsing with temp zones (patchset #11 id:200001 of https://codereview.chromium.org/2368313002/ )

Reason for revert:
Revert due to asm.js slowdown

Original issue's description:
> Don't use different function scopes when parsing with temp zones
>
> Previously we'd have a scope in the main zone, and another in the temp zone. Then we carefully copied back data to the main zone. This CL changes it so that the scope is just fixed up to only contain data from the main zone. That avoids additional copies and additional allocations; while not increasing the care that needs to be taken. This will also make it easier to abort preparsing while parsing using a temp zone.
>
> BUG=
>
> Committed: https://crrev.com/f41e7ebd62b32e861b6aa14ad8bfce3018d03c3c
> Cr-Commit-Position: refs/heads/master@{#39800}

TBR=marja@chromium.org,adamk@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

Committed: https://crrev.com/9e2b40aa8702f5f8b4ae7ec2763cb7adbd402c25
Cr-Commit-Position: refs/heads/master@{#39821}

[Toon Verwaest, 2016-09-28 09:21:16]

The CQ bit was checked by verwaest@chromium.org

[Toon Verwaest, 2016-09-28 09:21:17]

Created Revert of Don't use different function scopes when parsing with temp
zones

[commit-bot: I haz the power, 2016-09-28 09:21:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-28 09:22:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-28 09:22:02]

Failed to apply patch for src/parsing/parser.cc:
While running git apply --index -3 -p1;
  error: patch failed: src/parsing/parser.cc:2889
  Falling back to three-way merge...
  Applied patch to 'src/parsing/parser.cc' with conflicts.
  U src/parsing/parser.cc

Patch:       src/parsing/parser.cc
Index: src/parsing/parser.cc
diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc
index
c93b62fd5553f5e6c78d84fe61bf5ea9481830db..864b3c2fe1ad48d40b7682bbeb1a205cd01aefff
100644
--- a/src/parsing/parser.cc
+++ b/src/parsing/parser.cc
@@ -2745,9 +2745,11 @@
       !(FLAG_validate_asm && scope()->IsAsmModule());
   bool is_lazy_inner_function = use_temp_zone && FLAG_lazy_inner_functions;
 
-  // This Scope lives in the main zone. We'll migrate data into that zone
later.
-  DeclarationScope* scope = NewFunctionScope(kind);
-  SetLanguageMode(scope, language_mode);
+  DeclarationScope* main_scope = nullptr;
+  if (use_temp_zone) {
+    // This Scope lives in the main Zone; we'll migrate data into it later.
+    main_scope = NewFunctionScope(kind);
+  }
 
   ZoneList<Statement*>* body = nullptr;
   int arity = -1;
@@ -2767,14 +2769,21 @@
     // new temporary zone if the preconditions are satisfied, and ensures that
     // the previous zone is always restored after parsing the body. To be able
     // to do scope analysis correctly after full parsing, we migrate needed
-    // information when the function is parsed.
+    // information from scope into main_scope when the function has been
parsed.
     Zone temp_zone(zone()->allocator());
     DiscardableZoneScope zone_scope(this, &temp_zone, use_temp_zone);
+
+    DeclarationScope* scope = NewFunctionScope(kind);
+    SetLanguageMode(scope, language_mode);
+    if (!use_temp_zone) {
+      main_scope = scope;
+    } else {
+      DCHECK(main_scope->zone() != scope->zone());
+    }
 
     FunctionState function_state(&function_state_, &scope_state_, scope);
 #ifdef DEBUG
     scope->SetScopeName(function_name);
-    if (use_temp_zone) scope->set_needs_migration();
 #endif
     ExpressionClassifier formals_classifier(this, &duplicate_finder);
 
@@ -2836,23 +2845,24 @@
         // used once.
         eager_compile_hint = FunctionLiteral::kShouldEagerCompile;
         should_be_used_once_hint = true;
-        scope->ResetAfterPreparsing(true);
+      } else if (is_lazy_inner_function) {
+        DCHECK(main_scope != scope);
+        scope->AnalyzePartially(main_scope, &previous_zone_ast_node_factory);
       }
     }
-
     if (!is_lazy_top_level_function && !is_lazy_inner_function) {
       body = ParseEagerFunctionBody(function_name, pos, formals, kind,
                                     function_type, CHECK_OK);
 
       materialized_literal_count = function_state.materialized_literal_count();
       expected_property_count = function_state.expected_property_count();
-    }
-
-    if (use_temp_zone || is_lazy_top_level_function) {
-      // If the preconditions are correct the function body should never be
-      // accessed, but do this anyway for better behaviour if they're wrong.
-      body = nullptr;
-      scope->AnalyzePartially(&previous_zone_ast_node_factory);
+      if (use_temp_zone) {
+        // If the preconditions are correct the function body should never be
+        // accessed, but do this anyway for better behaviour if they're wrong.
+        body = nullptr;
+        DCHECK(main_scope != scope);
+        scope->AnalyzePartially(main_scope, &previous_zone_ast_node_factory);
+      }
     }
 
     // Parsing the body may change the language mode in our scope.
@@ -2889,7 +2899,7 @@
 
   // Note that the FunctionLiteral needs to be created in the main Zone again.
   FunctionLiteral* function_literal = factory()->NewFunctionLiteral(
-      function_name, scope, body, materialized_literal_count,
+      function_name, main_scope, body, materialized_literal_count,
       expected_property_count, arity, duplicate_parameters, function_type,
       eager_compile_hint, kind, pos);
   function_literal->set_function_token_position(function_token_pos);
@@ -2937,6 +2947,7 @@
   int function_block_pos = position();
   DeclarationScope* scope = function_state_->scope();
   DCHECK(scope->is_function_scope());
+  scope->set_is_lazily_parsed(true);
   // Inner functions are not part of the cached data.
   if (!is_inner_function && consume_cached_parse_data() &&
       !cached_parse_data_->rejected()) {
@@ -2969,7 +2980,10 @@
       ParseLazyFunctionBodyWithPreParser(&logger, is_inner_function,
may_abort);
 
   // Return immediately if pre-parser decided to abort parsing.
-  if (result == PreParser::kPreParseAbort) return kLazyParsingAborted;
+  if (result == PreParser::kPreParseAbort) {
+    scope->set_is_lazily_parsed(false);
+    return kLazyParsingAborted;
+  }
   if (result == PreParser::kPreParseStackOverflow) {
     // Propagate stack overflow.
     set_stack_overflow();
@@ -3470,6 +3484,11 @@
   PreParser::PreParseResult result = reusable_preparser_->PreParseLazyFunction(
       function_scope, parsing_module_, logger, is_inner_function, may_abort,
       use_counts_);
+  // Detaching the scopes created by PreParser from the Scope chain must be
done
+  // above (see ParseFunctionLiteral & AnalyzePartially).
+  if (!is_inner_function) {
+    function_scope->ResetAfterPreparsing(result == PreParser::kPreParseAbort);
+  }
   if (pre_parse_timer_ != NULL) {
     pre_parse_timer_->Stop();
   }
@@ -3616,7 +3635,7 @@
     return nullptr;
   }
 
-  BlockState block_state(zone(), &scope_state_);
+  BlockState block_state(&scope_state_);
   RaiseLanguageMode(STRICT);
 #ifdef DEBUG
   scope()->SetScopeName(name);

[Toon Verwaest, 2016-09-28 09:56:31]

The CQ bit was checked by verwaest@chromium.org

[commit-bot: I haz the power, 2016-09-28 09:56:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-28 09:56:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-28 09:56:41]

Failed to apply patch for src/parsing/parser.cc:
While running git apply --index -3 -p1;
  error: patch failed: src/parsing/parser.cc:2889
  Falling back to three-way merge...
  Applied patch to 'src/parsing/parser.cc' with conflicts.
  U src/parsing/parser.cc

Patch:       src/parsing/parser.cc
Index: src/parsing/parser.cc
diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc
index
c93b62fd5553f5e6c78d84fe61bf5ea9481830db..864b3c2fe1ad48d40b7682bbeb1a205cd01aefff
100644
--- a/src/parsing/parser.cc
+++ b/src/parsing/parser.cc
@@ -2745,9 +2745,11 @@
       !(FLAG_validate_asm && scope()->IsAsmModule());
   bool is_lazy_inner_function = use_temp_zone && FLAG_lazy_inner_functions;
 
-  // This Scope lives in the main zone. We'll migrate data into that zone
later.
-  DeclarationScope* scope = NewFunctionScope(kind);
-  SetLanguageMode(scope, language_mode);
+  DeclarationScope* main_scope = nullptr;
+  if (use_temp_zone) {
+    // This Scope lives in the main Zone; we'll migrate data into it later.
+    main_scope = NewFunctionScope(kind);
+  }
 
   ZoneList<Statement*>* body = nullptr;
   int arity = -1;
@@ -2767,14 +2769,21 @@
     // new temporary zone if the preconditions are satisfied, and ensures that
     // the previous zone is always restored after parsing the body. To be able
     // to do scope analysis correctly after full parsing, we migrate needed
-    // information when the function is parsed.
+    // information from scope into main_scope when the function has been
parsed.
     Zone temp_zone(zone()->allocator());
     DiscardableZoneScope zone_scope(this, &temp_zone, use_temp_zone);
+
+    DeclarationScope* scope = NewFunctionScope(kind);
+    SetLanguageMode(scope, language_mode);
+    if (!use_temp_zone) {
+      main_scope = scope;
+    } else {
+      DCHECK(main_scope->zone() != scope->zone());
+    }
 
     FunctionState function_state(&function_state_, &scope_state_, scope);
 #ifdef DEBUG
     scope->SetScopeName(function_name);
-    if (use_temp_zone) scope->set_needs_migration();
 #endif
     ExpressionClassifier formals_classifier(this, &duplicate_finder);
 
@@ -2836,23 +2845,24 @@
         // used once.
         eager_compile_hint = FunctionLiteral::kShouldEagerCompile;
         should_be_used_once_hint = true;
-        scope->ResetAfterPreparsing(true);
+      } else if (is_lazy_inner_function) {
+        DCHECK(main_scope != scope);
+        scope->AnalyzePartially(main_scope, &previous_zone_ast_node_factory);
       }
     }
-
     if (!is_lazy_top_level_function && !is_lazy_inner_function) {
       body = ParseEagerFunctionBody(function_name, pos, formals, kind,
                                     function_type, CHECK_OK);
 
       materialized_literal_count = function_state.materialized_literal_count();
       expected_property_count = function_state.expected_property_count();
-    }
-
-    if (use_temp_zone || is_lazy_top_level_function) {
-      // If the preconditions are correct the function body should never be
-      // accessed, but do this anyway for better behaviour if they're wrong.
-      body = nullptr;
-      scope->AnalyzePartially(&previous_zone_ast_node_factory);
+      if (use_temp_zone) {
+        // If the preconditions are correct the function body should never be
+        // accessed, but do this anyway for better behaviour if they're wrong.
+        body = nullptr;
+        DCHECK(main_scope != scope);
+        scope->AnalyzePartially(main_scope, &previous_zone_ast_node_factory);
+      }
     }
 
     // Parsing the body may change the language mode in our scope.
@@ -2889,7 +2899,7 @@
 
   // Note that the FunctionLiteral needs to be created in the main Zone again.
   FunctionLiteral* function_literal = factory()->NewFunctionLiteral(
-      function_name, scope, body, materialized_literal_count,
+      function_name, main_scope, body, materialized_literal_count,
       expected_property_count, arity, duplicate_parameters, function_type,
       eager_compile_hint, kind, pos);
   function_literal->set_function_token_position(function_token_pos);
@@ -2937,6 +2947,7 @@
   int function_block_pos = position();
   DeclarationScope* scope = function_state_->scope();
   DCHECK(scope->is_function_scope());
+  scope->set_is_lazily_parsed(true);
   // Inner functions are not part of the cached data.
   if (!is_inner_function && consume_cached_parse_data() &&
       !cached_parse_data_->rejected()) {
@@ -2969,7 +2980,10 @@
       ParseLazyFunctionBodyWithPreParser(&logger, is_inner_function,
may_abort);
 
   // Return immediately if pre-parser decided to abort parsing.
-  if (result == PreParser::kPreParseAbort) return kLazyParsingAborted;
+  if (result == PreParser::kPreParseAbort) {
+    scope->set_is_lazily_parsed(false);
+    return kLazyParsingAborted;
+  }
   if (result == PreParser::kPreParseStackOverflow) {
     // Propagate stack overflow.
     set_stack_overflow();
@@ -3470,6 +3484,11 @@
   PreParser::PreParseResult result = reusable_preparser_->PreParseLazyFunction(
       function_scope, parsing_module_, logger, is_inner_function, may_abort,
       use_counts_);
+  // Detaching the scopes created by PreParser from the Scope chain must be
done
+  // above (see ParseFunctionLiteral & AnalyzePartially).
+  if (!is_inner_function) {
+    function_scope->ResetAfterPreparsing(result == PreParser::kPreParseAbort);
+  }
   if (pre_parse_timer_ != NULL) {
     pre_parse_timer_->Stop();
   }
@@ -3616,7 +3635,7 @@
     return nullptr;
   }
 
-  BlockState block_state(zone(), &scope_state_);
+  BlockState block_state(&scope_state_);
   RaiseLanguageMode(STRICT);
 #ifdef DEBUG
   scope()->SetScopeName(name);

[Toon Verwaest, 2016-09-28 11:16:24]

The CQ bit was checked by verwaest@chromium.org

[commit-bot: I haz the power, 2016-09-28 11:16:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-28 11:17:22]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-09-28 11:17:41]

Description was changed from

==========
Revert of Don't use different function scopes when parsing with temp zones
(patchset #11 id:200001 of https://codereview.chromium.org/2368313002/ )

Reason for revert:
Revert due to asm.js slowdown

Original issue's description:
> Don't use different function scopes when parsing with temp zones
> 
> Previously we'd have a scope in the main zone, and another in the temp zone.
Then we carefully copied back data to the main zone. This CL changes it so that
the scope is just fixed up to only contain data from the main zone. That avoids
additional copies and additional allocations; while not increasing the care that
needs to be taken. This will also make it easier to abort preparsing while
parsing using a temp zone.
> 
> BUG=
> 
> Committed: https://crrev.com/f41e7ebd62b32e861b6aa14ad8bfce3018d03c3c
> Cr-Commit-Position: refs/heads/master@{#39800}

TBR=marja@chromium.org,adamk@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=
==========

to

==========
Revert of Don't use different function scopes when parsing with temp zones
(patchset #11 id:200001 of https://codereview.chromium.org/2368313002/ )

Reason for revert:
Revert due to asm.js slowdown

Original issue's description:
> Don't use different function scopes when parsing with temp zones
>
> Previously we'd have a scope in the main zone, and another in the temp zone.
Then we carefully copied back data to the main zone. This CL changes it so that
the scope is just fixed up to only contain data from the main zone. That avoids
additional copies and additional allocations; while not increasing the care that
needs to be taken. This will also make it easier to abort preparsing while
parsing using a temp zone.
>
> BUG=
>
> Committed: https://crrev.com/f41e7ebd62b32e861b6aa14ad8bfce3018d03c3c
> Cr-Commit-Position: refs/heads/master@{#39800}

TBR=marja@chromium.org,adamk@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

Committed: https://crrev.com/9e2b40aa8702f5f8b4ae7ec2763cb7adbd402c25
Cr-Commit-Position: refs/heads/master@{#39821}
==========

[commit-bot: I haz the power, 2016-09-28 11:17:42]

Patchset 1 (id:??) landed as
https://crrev.com/9e2b40aa8702f5f8b4ae7ec2763cb7adbd402c25
Cr-Commit-Position: refs/heads/master@{#39821}