Revert "[parser] Skipping inner funcs: Use less memory for variables."

This reverts commit 97ead4338e.

Reason for revert: makes the PreParserScopeAnalysis test much slower.

Original change's description:
> [parser] Skipping inner funcs: Use less memory for variables.
> 
> - Make it possible to store quarter-bytes instead of full bytes.
> 
> - Don't store is_used; it can be recovered correctly based on the actual full
>   parse (when a lazy function is eventually called) and
>   has_forced_scope_allocation.
> 
> - With the is_used change, the old testing approach (which compared a scope for
>   which we didn't do scope allocation to the baseline) no longer made
>   sense. Replaced it with a new testing approach, which is also closer to the
>   actual usage.
> 
> BUG=v8:5516
> 
> Change-Id: I02bac24e482126689dcdbabe8b3a04977be29b0c
> Reviewed-on: https://chromium-review.googlesource.com/725422
> Commit-Queue: Marja Hölttä <marja@chromium.org>
> Reviewed-by: Toon Verwaest <verwaest@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#48828}

TBR=marja@chromium.org,verwaest@chromium.org

Change-Id: I8cb87bcd55462b1cef4444dabb5cbfa2ecb24c7c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:5516
Reviewed-on: https://chromium-review.googlesource.com/732878
Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48829}

src/parsing/preparsed-scope-data.cc

@@ -20,16 +20,17 @@ class ScopeCallsSloppyEvalField : public BitField<bool, 0, 1> {};
 class InnerScopeCallsEvalField
     : public BitField<bool, ScopeCallsSloppyEvalField::kNext, 1> {};
 
-class VariableMaybeAssignedField : public BitField8<bool, 0, 1> {};
+class VariableIsUsedField : public BitField16<bool, 0, 1> {};
+class VariableMaybeAssignedField
+    : public BitField16<bool, VariableIsUsedField::kNext, 1> {};
 class VariableContextAllocatedField
-    : public BitField8<bool, VariableMaybeAssignedField::kNext, 1> {};
+    : public BitField16<bool, VariableMaybeAssignedField::kNext, 1> {};
 
 const int kMagicValue = 0xc0de0de;
 
 #ifdef DEBUG
 const size_t kUint32Size = 5;
 const size_t kUint8Size = 2;
-const size_t kQuarterMarker = 0;
 #else
 const size_t kUint32Size = 4;
 const size_t kUint8Size = 1;
@@ -38,8 +39,8 @@ const size_t kUint8Size = 1;
 const int kPlaceholderSize = kUint32Size;
 const int kSkippableFunctionDataSize = 4 * kUint32Size + 1 * kUint8Size;
 
-class LanguageField : public BitField8<LanguageMode, 0, 1> {};
+class LanguageField : public BitField<LanguageMode, 0, 1> {};
-class UsesSuperField : public BitField8<bool, LanguageField::kNext, 1> {};
+class UsesSuperField : public BitField<bool, LanguageField::kNext, 1> {};
 STATIC_ASSERT(LanguageModeSize <= LanguageField::kNumValues);
 
 }  // namespace
@@ -98,7 +99,6 @@ void ProducedPreParsedScopeData::ByteData::WriteUint32(uint32_t data) {
   for (int i = 0; i < 4; ++i) {
     backing_store_.push_back(*d++);
   }
-  free_quarters_in_last_byte_ = 0;
 }
 
 void ProducedPreParsedScopeData::ByteData::OverwriteFirstUint32(uint32_t data) {
@@ -121,25 +121,6 @@ void ProducedPreParsedScopeData::ByteData::WriteUint8(uint8_t data) {
   backing_store_.push_back(kUint8Size);
 #endif
   backing_store_.push_back(data);
-  free_quarters_in_last_byte_ = 0;
-}
-
-void ProducedPreParsedScopeData::ByteData::WriteQuarter(uint8_t data) {
-  DCHECK_LE(data, 3);
-  if (free_quarters_in_last_byte_ == 0) {
-#ifdef DEBUG
-    // Save a marker in debug mode.
-    backing_store_.push_back(kQuarterMarker);
-#endif
-    backing_store_.push_back(0);
-    free_quarters_in_last_byte_ = 3;
-  } else {
-    --free_quarters_in_last_byte_;
-  }
-
-  uint8_t shift_amount = free_quarters_in_last_byte_ * 2;
-  DCHECK_EQ(backing_store_.back() & (3 << shift_amount), 0);
-  backing_store_.back() |= (data << shift_amount);
 }
 
 Handle<PodArray<uint8_t>> ProducedPreParsedScopeData::ByteData::Serialize(
@@ -235,7 +216,7 @@ void ProducedPreParsedScopeData::AddSkippableFunction(
   uint8_t language_and_super = LanguageField::encode(language_mode) |
                                UsesSuperField::encode(uses_super_property);
 
-  byte_data_->WriteQuarter(language_and_super);
+  byte_data_->WriteUint8(language_and_super);
 }
 
 void ProducedPreParsedScopeData::SaveScopeAllocationData(
@@ -401,11 +382,14 @@ void ProducedPreParsedScopeData::SaveDataForVariable(Variable* var) {
     byte_data_->WriteUint8(name->raw_data()[i]);
   }
 #endif
-  byte variable_data = VariableMaybeAssignedField::encode(
+  // FIXME(marja): Only 3 bits needed, not a full byte.
+  byte variable_data = VariableIsUsedField::encode(var->is_used()) |
+                       VariableMaybeAssignedField::encode(
                            var->maybe_assigned() == kMaybeAssigned) |
                        VariableContextAllocatedField::encode(
                            var->has_forced_context_allocation());
-  byte_data_->WriteQuarter(variable_data);
+
+  byte_data_->WriteUint8(variable_data);
 }
 
 void ProducedPreParsedScopeData::SaveDataForInnerScopes(Scope* scope) {
@@ -445,7 +429,6 @@ int32_t ConsumedPreParsedScopeData::ByteData::ReadUint32() {
   for (int i = 0; i < 4; ++i) {
     *p++ = data_->get(index_++);
   }
-  stored_quarters_ = 0;
   return result;
 }
 
@@ -456,29 +439,9 @@ uint8_t ConsumedPreParsedScopeData::ByteData::ReadUint8() {
   // Check that there indeed is a byte following.
   DCHECK_EQ(data_->get(index_++), kUint8Size);
 #endif
-  stored_quarters_ = 0;
   return data_->get(index_++);
 }
 
-uint8_t ConsumedPreParsedScopeData::ByteData::ReadQuarter() {
-  DCHECK_NOT_NULL(data_);
-  if (stored_quarters_ == 0) {
-    DCHECK_GE(RemainingBytes(), kUint8Size);
-#ifdef DEBUG
-    // Check that there indeed are quarters following.
-    DCHECK_EQ(data_->get(index_++), kQuarterMarker);
-#endif
-    stored_byte_ = data_->get(index_++);
-    stored_quarters_ = 4;
-  }
-  // Read the first 2 bits from stored_byte_.
-  uint8_t result = (stored_byte_ >> 6) & 3;
-  DCHECK_LE(result, 3);
-  --stored_quarters_;
-  stored_byte_ <<= 2;
-  return result;
-}
-
 ConsumedPreParsedScopeData::ConsumedPreParsedScopeData()
     : scope_data_(new ByteData()), child_index_(0) {}
 
@@ -514,7 +477,7 @@ ConsumedPreParsedScopeData::GetDataForSkippableFunction(
   *num_parameters = scope_data_->ReadUint32();
   *num_inner_functions = scope_data_->ReadUint32();
 
-  uint8_t language_and_super = scope_data_->ReadQuarter();
+  uint8_t language_and_super = scope_data_->ReadUint8();
   *language_mode = LanguageMode(LanguageField::decode(language_and_super));
   *uses_super_property = UsesSuperField::decode(language_and_super);
 
@@ -555,6 +518,13 @@ void ConsumedPreParsedScopeData::RestoreScopeAllocationData(
   DCHECK_EQ(scope_data_->RemainingBytes(), 0);
 }
 
+void ConsumedPreParsedScopeData::SkipFunctionDataForTesting() {
+  ByteData::ReadingScope reading_scope(this);
+  scope_data_->SetPosition(0);
+  uint32_t scope_data_start = scope_data_->ReadUint32();
+  scope_data_->SetPosition(scope_data_start);
+}
+
 void ConsumedPreParsedScopeData::RestoreData(Scope* scope) {
   if (scope->is_declaration_scope() &&
       scope->AsDeclarationScope()->is_skipped_function()) {
@@ -611,12 +581,15 @@ void ConsumedPreParsedScopeData::RestoreDataForVariable(Variable* var) {
     DCHECK_EQ(scope_data_->ReadUint8(), name->raw_data()[i]);
   }
 #endif
-  uint8_t variable_data = scope_data_->ReadQuarter();
+  CHECK_GE(scope_data_->RemainingBytes(), kUint8Size);
+  uint8_t variable_data = scope_data_->ReadUint8();
+  if (VariableIsUsedField::decode(variable_data)) {
+    var->set_is_used();
+  }
   if (VariableMaybeAssignedField::decode(variable_data)) {
     var->set_maybe_assigned();
   }
   if (VariableContextAllocatedField::decode(variable_data)) {
-    var->set_is_used();
     var->ForceContextAllocation();
   }
 }

src/parsing/preparsed-scope-data.h

@@ -69,12 +69,10 @@ class ProducedPreParsedScopeData : public ZoneObject {
  public:
   class ByteData : public ZoneObject {
    public:
-    explicit ByteData(Zone* zone)
+    explicit ByteData(Zone* zone) : backing_store_(zone) {}
-        : backing_store_(zone), free_quarters_in_last_byte_(0) {}
 
     void WriteUint32(uint32_t data);
     void WriteUint8(uint8_t data);
-    void WriteQuarter(uint8_t data);
 
     // For overwriting previously written data at position 0.
     void OverwriteFirstUint32(uint32_t data);
@@ -85,7 +83,6 @@ class ProducedPreParsedScopeData : public ZoneObject {
 
    private:
     ZoneDeque<uint8_t> backing_store_;
-    uint8_t free_quarters_in_last_byte_;
   };
 
   // Create a ProducedPreParsedScopeData object which will collect data as we
@@ -184,8 +181,7 @@ class ConsumedPreParsedScopeData {
  public:
   class ByteData {
    public:
-    ByteData()
+    ByteData() : data_(nullptr), index_(0) {}
-        : data_(nullptr), index_(0), stored_quarters_(0), stored_byte_(0) {}
 
     // Reading from the ByteData is only allowed when a ReadingScope is on the
     // stack. This ensures that we have a DisallowHeapAllocation in place
@@ -208,7 +204,6 @@ class ConsumedPreParsedScopeData {
 
     int32_t ReadUint32();
     uint8_t ReadUint8();
-    uint8_t ReadQuarter();
 
     size_t RemainingBytes() const {
       DCHECK_NOT_NULL(data_);
@@ -218,8 +213,6 @@ class ConsumedPreParsedScopeData {
     // private:
     PodArray<uint8_t>* data_;
     int index_;
-    uint8_t stored_quarters_;
-    uint8_t stored_byte_;
   };
 
   ConsumedPreParsedScopeData();
@@ -238,6 +231,11 @@ class ConsumedPreParsedScopeData {
   // subscopes') variables.
   void RestoreScopeAllocationData(DeclarationScope* scope);
 
+  // Skips the data about skippable functions, moves straight to the scope
+  // allocation data. Useful for tests which don't want to verify only the scope
+  // allocation data.
+  void SkipFunctionDataForTesting();
+
  private:
   void RestoreData(Scope* scope);
   void RestoreDataForVariable(Variable* var);

test/cctest/parsing/test-preparser.cc

@@ -2,7 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "src/api.h"
 #include "src/ast/ast.h"
 #include "src/compiler.h"
 #include "src/objects-inl.h"
@@ -38,52 +37,85 @@ TEST(PreParserScopeAnalysis) {
   i::FLAG_aggressive_lazy_inner_functions = true;
   i::Isolate* isolate = CcTest::i_isolate();
   i::Factory* factory = isolate->factory();
+  i::HandleScope scope(isolate);
+  LocalContext env;
 
   struct {
     const char* code;
     bool strict_outer;
     bool strict_test_function;
     bool arrow;
+    std::vector<unsigned> location;  // "Directions" to the relevant scope.
   } outers[] = {
       // Normal case (test function at the laziness boundary):
-      {"function test(%s) { %s function skippable() { } } test;", false, false,
+      {"(function outer() { function test(%s) { %s \n"
-       false},
+       "function skippable() { } } })();",
+       false,
+       false,
+       false,
+       {0, 0}},
 
-      {"let test2 = function test(%s) { %s function skippable() { } }; test2",
+      {"(function outer() { let test2 = function test(%s) { %s \n"
-       false, false, false},
+       "function skippable() { } } })();",
+       false,
+       false,
+       false,
+       {0, 0}},
 
       // Arrow functions (they can never be at the laziness boundary):
-      {"function test() { (%s) => { %s }; function skippable() { } } test;",
+      {"(function outer() { function inner() { (%s) => { %s } \n"
-       false, false, true},
+       "function skippable() { } } })();",
+       false,
+       false,
+       true,
+       {0, 0}},
 
-      // Repeat the above mentioned cases with global 'use strict'
+      // Repeat the above mentioned cases w/ outer function declaring itself
-      {"'use strict'; function test(%s) { %s function skippable() { } } test;",
+      // strict:
-       true, false, false},
+      {"(function outer() { 'use strict'; function test(%s) { %s \n"
+       "function skippable() { } } })();",
+       true,
+       false,
+       false,
+       {0, 0}},
 
-      {"'use strict'; let test2 = function test(%s) { %s \n"
+      {"(function outer() { 'use strict'; function inner() { "
-       "function skippable() { } }; test2",
+       "(%s) => { %s } \nfunction skippable() { } } })();",
-       true, false, false},
+       true,
-
+       false,
-      {"'use strict'; function test() { (%s) => { %s };\n"
+       true,
-       "function skippable() { } } test;",
+       {0, 0}},
-       true, false, true},
 
       // ... and with the test function declaring itself strict:
-      {"function test(%s) { 'use strict'; %s function skippable() { } } test;",
+      {"(function outer() { function test(%s) { 'use strict'; %s \n"
-       false, true, false},
+       "function skippable() { } } })();",
+       false,
+       true,
+       false,
+       {0, 0}},
 
-      {"let test2 = function test(%s) { 'use strict'; %s \n"
+      {"(function outer() { function inner() { "
-       "function skippable() { } }; test2",
+       "(%s) => { 'use strict'; %s } \nfunction skippable() { } } })();",
-       false, true, false},
+       false,
-
+       true,
-      {"function test() { 'use strict'; (%s) => { %s };\n"
+       true,
-       "function skippable() { } } test;",
+       {0, 0}},
-       false, true, true},
 
       // Methods containing skippable functions.
-      {"class MyClass { test_method(%s) { %s \n"
+      {"class MyClass { constructor(%s) { %s \n"
-       "function skippable() { } } } let o = new MyClass(); o.test_method;",
+       "function skippable() { } } }",
-       true, true, false},
+       true,
+       true,
+       false,
+       {0, 0}},
+
+      {"class MyClass { test(%s) { %s \n"
+       "function skippable() { } } }",
+       true,
+       true,
+       false,
+       // The default constructor is scope 0 inside the class.
+       {0, 1}},
 
       // FIXME(marja): Generators and async functions
   };
@@ -143,20 +175,20 @@ TEST(PreParserScopeAnalysis) {
 
       // Functions.
       {"function f1() { let var2; }"},
-      {"var var1 = function f1() { let var2; };"},
+      {"var var1 = function f1() { let var2; }"},
-      {"let var1 = function f1() { let var2; };"},
+      {"let var1 = function f1() { let var2; }"},
-      {"const var1 = function f1() { let var2; };"},
+      {"const var1 = function f1() { let var2; }"},
-      {"var var1 = function() { let var2; };"},
+      {"var var1 = function() { let var2; }"},
-      {"let var1 = function() { let var2; };"},
+      {"let var1 = function() { let var2; }"},
-      {"const var1 = function() { let var2; };"},
+      {"const var1 = function() { let var2; }"},
 
       {"function *f1() { let var2; }"},
-      {"let var1 = function *f1() { let var2; };"},
+      {"let var1 = function *f1() { let var2; }"},
-      {"let var1 = function*() { let var2; };"},
+      {"let var1 = function*() { let var2; }"},
 
       {"async function f1() { let var2; }"},
-      {"let var1 = async function f1() { let var2; };"},
+      {"let var1 = async function f1() { let var2; }"},
-      {"let var1 = async function() { let var2; };"},
+      {"let var1 = async function() { let var2; }"},
 
       // Redeclarations.
       {"var var1; var var1;"},
@@ -184,15 +216,15 @@ TEST(PreParserScopeAnalysis) {
       {"arguments = 5;", SKIP_STRICT},
       {"if (true) { arguments; }"},
       {"if (true) { arguments = 5; }", SKIP_STRICT},
-      {"() => { arguments; };"},
+      {"() => { arguments; }"},
       {"var1, var2, var3", "arguments;"},
       {"var1, var2, var3", "arguments = 5;", SKIP_STRICT},
-      {"var1, var2, var3", "() => { arguments; };"},
+      {"var1, var2, var3", "() => { arguments; }"},
-      {"var1, var2, var3", "() => { arguments = 5; };", SKIP_STRICT},
+      {"var1, var2, var3", "() => { arguments = 5; }", SKIP_STRICT},
 
       {"this;"},
       {"if (true) { this; }"},
-      {"() => { this; };"},
+      {"() => { this; }"},
 
       // Variable called "arguments"
       {"var arguments;", SKIP_STRICT},
@@ -500,21 +532,21 @@ TEST(PreParserScopeAnalysis) {
       {"{name1: var1}", "name1 = 16;", SKIP_STRICT_FUNCTION},
       {"{var1}", "var1 = 16;", SKIP_STRICT_FUNCTION},
 
-      {"[var1]", "() => { var1; };", SKIP_STRICT_FUNCTION},
+      {"[var1]", "() => { var1; }", SKIP_STRICT_FUNCTION},
-      {"{name1: var1}", "() => { var1; };", SKIP_STRICT_FUNCTION},
+      {"{name1: var1}", "() => { var1; }", SKIP_STRICT_FUNCTION},
-      {"{name1: var1}", "() => { name1; };", SKIP_STRICT_FUNCTION},
+      {"{name1: var1}", "() => { name1; }", SKIP_STRICT_FUNCTION},
-      {"{var1}", "() => { var1; };", SKIP_STRICT_FUNCTION},
+      {"{var1}", "() => { var1; }", SKIP_STRICT_FUNCTION},
 
       {"[var1, var2, var3]", "", SKIP_STRICT_FUNCTION},
       {"{name1: var1, name2: var2, name3: var3}", "", SKIP_STRICT_FUNCTION},
       {"{var1, var2, var3}", "", SKIP_STRICT_FUNCTION},
 
-      {"[var1, var2, var3]", "() => { var2 = 16;};", SKIP_STRICT_FUNCTION},
+      {"[var1, var2, var3]", "() => { var2 = 16;}", SKIP_STRICT_FUNCTION},
-      {"{name1: var1, name2: var2, name3: var3}", "() => { var2 = 16;};",
+      {"{name1: var1, name2: var2, name3: var3}", "() => { var2 = 16;}",
        SKIP_STRICT_FUNCTION},
-      {"{name1: var1, name2: var2, name3: var3}", "() => { name2 = 16;};",
+      {"{name1: var1, name2: var2, name3: var3}", "() => { name2 = 16;}",
        SKIP_STRICT_FUNCTION},
-      {"{var1, var2, var3}", "() => { var2 = 16;};", SKIP_STRICT_FUNCTION},
+      {"{var1, var2, var3}", "() => { var2 = 16;}", SKIP_STRICT_FUNCTION},
 
       // Nesting destructuring.
       {"[var1, [var2, var3], {var4, name5: [var5, var6]}]", "",
@@ -530,9 +562,9 @@ TEST(PreParserScopeAnalysis) {
 
       // Destructuring rest. Because we can.
       {"var1, ...[var2]", "", SKIP_STRICT_FUNCTION},
-      {"var1, ...[var2]", "() => { var2; };", SKIP_STRICT_FUNCTION},
+      {"var1, ...[var2]", "() => { var2; }", SKIP_STRICT_FUNCTION},
       {"var1, ...{0: var2}", "", SKIP_STRICT_FUNCTION},
-      {"var1, ...{0: var2}", "() => { var2; };", SKIP_STRICT_FUNCTION},
+      {"var1, ...{0: var2}", "() => { var2; }", SKIP_STRICT_FUNCTION},
       {"var1, ...[]", "", SKIP_STRICT_FUNCTION},
       {"var1, ...{}", "", SKIP_STRICT_FUNCTION},
       {"var1, ...[var2, var3]", "", SKIP_STRICT_FUNCTION},
@@ -551,16 +583,16 @@ TEST(PreParserScopeAnalysis) {
        PreciseMaybeAssigned::NO},
 
       // Locals shadowing parameters.
-      {"var1, var2", "var var1 = 16; () => { var1 = 17; };"},
+      {"var1, var2", "var var1 = 16; () => { var1 = 17; }"},
 
       // Locals shadowing destructuring parameters and the rest parameter.
-      {"[var1, var2]", "var var1 = 16; () => { var1 = 17; };",
+      {"[var1, var2]", "var var1 = 16; () => { var1 = 17; }",
        SKIP_STRICT_FUNCTION},
-      {"{var1, var2}", "var var1 = 16; () => { var1 = 17; };",
+      {"{var1, var2}", "var var1 = 16; () => { var1 = 17; }",
        SKIP_STRICT_FUNCTION},
-      {"var1, var2, ...var3", "var var3 = 16; () => { var3 = 17; };",
+      {"var1, var2, ...var3", "var var3 = 16; () => { var3 = 17; }",
        SKIP_STRICT_FUNCTION},
-      {"var1, var2 = var1", "var var1 = 16; () => { var1 = 17; };",
+      {"var1, var2 = var1", "var var1 = 16; () => { var1 = 17; }",
        SKIP_STRICT_FUNCTION, PreciseMaybeAssigned::NO},
 
       // Hoisted sloppy block function shadowing a parameter.
@@ -600,13 +632,13 @@ TEST(PreParserScopeAnalysis) {
 
       // Classes
       {"class MyClass {}"},
-      {"var1 = class MyClass {};"},
+      {"var1 = class MyClass {}"},
-      {"var var1 = class MyClass {};"},
+      {"var var1 = class MyClass {}"},
-      {"let var1 = class MyClass {};"},
+      {"let var1 = class MyClass {}"},
-      {"const var1 = class MyClass {};"},
+      {"const var1 = class MyClass {}"},
-      {"var var1 = class {};"},
+      {"var var1 = class {}"},
-      {"let var1 = class {};"},
+      {"let var1 = class {}"},
-      {"const var1 = class {};"},
+      {"const var1 = class {}"},
 
       {"class MyClass { constructor() {} }"},
       {"class MyClass { constructor() { var var1; } }"},
@@ -673,65 +705,79 @@ TEST(PreParserScopeAnalysis) {
       i::SNPrintF(program, code, inners[inner_ix].params,
                   inners[inner_ix].source);
 
-      i::HandleScope scope(isolate);
-      LocalContext env;
-
       i::Handle<i::String> source =
           factory->InternalizeUtf8String(program.start());
       source->PrintOn(stdout);
       printf("\n");
 
-      // Compile and run the script to get a pointer to the lazy function.
+      // First compile with the lazy inner function and extract the scope data.
-      v8::Local<v8::Value> v = CompileRun(program.start());
+      i::Handle<i::Script> script = factory->NewScript(source);
-      i::Handle<i::Object> o = v8::Utils::OpenHandle(*v);
+      i::ParseInfo lazy_info(script);
-      i::Handle<i::JSFunction> f = i::Handle<i::JSFunction>::cast(o);
-      i::Handle<i::SharedFunctionInfo> shared = i::handle(f->shared());
 
+      // No need to run scope analysis; preparser scope data is produced when
+      // parsing.
+      CHECK(i::parsing::ParseProgram(&lazy_info, isolate));
+
+      // Retrieve the scope data we produced.
+      i::Scope* scope_with_data = i::ScopeTestHelper::FindScope(
+          lazy_info.literal()->scope(), outers[outer_ix].location);
+      i::ProducedPreParsedScopeData* produced_data =
+          scope_with_data->AsDeclarationScope()
+              ->produced_preparsed_scope_data();
+      i::MaybeHandle<i::PreParsedScopeData> maybe_produced_data_on_heap =
+          produced_data->Serialize(isolate);
       if (inners[inner_ix].bailout == Bailout::BAILOUT_IF_OUTER_SLOPPY &&
           !outers[outer_ix].strict_outer) {
-        CHECK(!shared->HasPreParsedScopeData());
+        DCHECK(maybe_produced_data_on_heap.is_null());
         continue;
       }
+      DCHECK(!maybe_produced_data_on_heap.is_null());
+      i::Handle<i::PreParsedScopeData> produced_data_on_heap =
+          maybe_produced_data_on_heap.ToHandleChecked();
 
-      CHECK(shared->HasPreParsedScopeData());
+      // Then parse eagerly and check against the scope data.
-      i::Handle<i::PreParsedScopeData> produced_data_on_heap(
+      script = factory->NewScript(source);
-          i::PreParsedScopeData::cast(shared->preparsed_scope_data()));
 
-      // Parse the lazy function using the scope data.
+      i::ParseInfo eager_normal(script);
-      i::ParseInfo using_scope_data(shared);
+      eager_normal.set_allow_lazy_parsing(false);
-      using_scope_data.set_lazy_compile();
-      using_scope_data.consumed_preparsed_scope_data()->SetData(
-          produced_data_on_heap);
-      CHECK(i::parsing::ParseFunction(&using_scope_data, shared, isolate));
 
-      // Verify that we skipped at least one function inside that scope.
+      CHECK(i::parsing::ParseProgram(&eager_normal, isolate));
-      i::DeclarationScope* scope_with_skipped_functions =
+      CHECK(i::Compiler::Analyze(&eager_normal));
-          using_scope_data.literal()->scope();
-      CHECK(i::ScopeTestHelper::HasSkippedFunctionInside(
-          scope_with_skipped_functions));
 
-      // Do scope allocation (based on the preparsed scope data).
+      // Compare the allocation of the variables in two cases: 1) normal scope
-      i::DeclarationScope::Analyze(&using_scope_data);
+      // allocation 2) allocation based on the preparse data.
 
-      // Parse the lazy function again eagerly to produce baseline data.
+      i::Scope* normal_scope = i::ScopeTestHelper::FindScope(
-      i::ParseInfo not_using_scope_data(shared);
+          eager_normal.literal()->scope(), outers[outer_ix].location);
-      not_using_scope_data.set_lazy_compile();
+      CHECK_NULL(normal_scope->sibling());
-      CHECK(i::parsing::ParseFunction(&not_using_scope_data, shared, isolate));
+      CHECK(normal_scope->is_function_scope());
 
-      // Verify that we didn't skip anything (there's no preparsed scope data,
+      i::ParseInfo eager_using_scope_data(script);
-      // so we cannot skip).
+      eager_using_scope_data.set_allow_lazy_parsing(false);
-      i::DeclarationScope* scope_without_skipped_functions =
-          not_using_scope_data.literal()->scope();
-      CHECK(!i::ScopeTestHelper::HasSkippedFunctionInside(
-          scope_without_skipped_functions));
 
-      // Do normal scope allocation.
+      CHECK(i::parsing::ParseProgram(&eager_using_scope_data, isolate));
-      i::DeclarationScope::Analyze(&not_using_scope_data);
+      // Don't run scope analysis (that would obviously decide the correct
+      // allocation for the variables).
+
+      i::Scope* unallocated_scope = i::ScopeTestHelper::FindScope(
+          eager_using_scope_data.literal()->scope(), outers[outer_ix].location);
+      CHECK_NULL(unallocated_scope->sibling());
+      CHECK(unallocated_scope->is_function_scope());
+
+      // Mark all inner functions as "skipped", so that we don't try to restore
+      // data for them. No test should contain eager functions, because we
+      // cannot properly decide whether we have or don't have data for them.
+      i::ScopeTestHelper::MarkInnerFunctionsAsSkipped(unallocated_scope);
+      i::ConsumedPreParsedScopeData* consumed_preparsed_scope_data =
+          lazy_info.consumed_preparsed_scope_data();
+      consumed_preparsed_scope_data->SetData(produced_data_on_heap);
+      consumed_preparsed_scope_data->SkipFunctionDataForTesting();
+      consumed_preparsed_scope_data->RestoreScopeAllocationData(
+          unallocated_scope->AsDeclarationScope());
+      i::ScopeTestHelper::AllocateWithoutVariableResolution(unallocated_scope);
 
-      // Verify that scope allocation gave the same results when parsing w/ the
-      // scope data (and skipping functions), and when parsing without.
       i::ScopeTestHelper::CompareScopes(
-          scope_without_skipped_functions, scope_with_skipped_functions,
+          normal_scope, unallocated_scope,
           inners[inner_ix].precise_maybe_assigned == PreciseMaybeAssigned::YES);
     }
   }
@@ -773,20 +819,6 @@ TEST(ProducingAndConsumingByteData) {
   bytes.WriteUint8(0);
   bytes.OverwriteFirstUint32(2017);
   bytes.WriteUint8(100);
-  // Write quarter bytes between uint8s and uint32s to verify they're stored
-  // correctly.
-  bytes.WriteQuarter(3);
-  bytes.WriteQuarter(0);
-  bytes.WriteQuarter(2);
-  bytes.WriteQuarter(1);
-  bytes.WriteQuarter(0);
-  bytes.WriteUint8(50);
-  bytes.WriteQuarter(0);
-  bytes.WriteQuarter(1);
-  bytes.WriteQuarter(2);
-  bytes.WriteUint32(50);
-  // End with a lonely quarter.
-  bytes.WriteQuarter(2);
 
   i::Handle<i::PodArray<uint8_t>> data_on_heap = bytes.Serialize(isolate);
   i::ConsumedPreParsedScopeData::ByteData bytes_for_reading;
@@ -801,15 +833,4 @@ TEST(ProducingAndConsumingByteData) {
   CHECK_EQ(bytes_for_reading.ReadUint32(), 0);
   CHECK_EQ(bytes_for_reading.ReadUint8(), 0);
   CHECK_EQ(bytes_for_reading.ReadUint8(), 100);
-  CHECK_EQ(bytes_for_reading.ReadQuarter(), 3);
-  CHECK_EQ(bytes_for_reading.ReadQuarter(), 0);
-  CHECK_EQ(bytes_for_reading.ReadQuarter(), 2);
-  CHECK_EQ(bytes_for_reading.ReadQuarter(), 1);
-  CHECK_EQ(bytes_for_reading.ReadQuarter(), 0);
-  CHECK_EQ(bytes_for_reading.ReadUint8(), 50);
-  CHECK_EQ(bytes_for_reading.ReadQuarter(), 0);
-  CHECK_EQ(bytes_for_reading.ReadQuarter(), 1);
-  CHECK_EQ(bytes_for_reading.ReadQuarter(), 2);
-  CHECK_EQ(bytes_for_reading.ReadUint32(), 50);
-  CHECK_EQ(bytes_for_reading.ReadQuarter(), 2);
 }

test/cctest/scope-test-helper.h

@@ -17,6 +17,10 @@ class ScopeTestHelper {
     return var->scope()->MustAllocateInContext(var);
   }
 
+  static void AllocateWithoutVariableResolution(Scope* scope) {
+    scope->AllocateVariablesRecursively();
+  }
+
   static void CompareScopes(Scope* baseline, Scope* scope,
                             bool precise_maybe_assigned) {
     CHECK_EQ(baseline->scope_type(), scope->scope_type());
@@ -105,20 +109,6 @@ class ScopeTestHelper {
       MarkInnerFunctionsAsSkipped(inner);
     }
   }
-
-  static bool HasSkippedFunctionInside(Scope* scope) {
-    if (scope->scope_type() == ScopeType::FUNCTION_SCOPE &&
-        scope->AsDeclarationScope()->is_skipped_function()) {
-      return true;
-    }
-    for (Scope* inner = scope->inner_scope(); inner != nullptr;
-         inner = inner->sibling()) {
-      if (HasSkippedFunctionInside(inner)) {
-        return true;
-      }
-    }
-    return false;
-  }
 };
 }  // namespace internal
 }  // namespace v8