[parser] Skipping inner funcs: Fix related to classes.

- Default constructor scopes won't need the scope data for deciding the scope
allocation of variables inside them. Also, PreParser doesn't construct them. So
they should be just skipped when applying the scope data.

- PreParser needs to declare the class name + have a proper end position for
the class scope.

- This makes all mjsunit tests pass with --experimental-preparser-scope-analysis.

- Also added several DCHECKs which were useful for debugging.

BUG=v8:5516

Change-Id: I5b3e6c60ed79efe25f33576a3547d707c700c6dd
Reviewed-on: https://chromium-review.googlesource.com/503208
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45303}

src/parsing/parser-base.h

@@ -4478,8 +4478,10 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseClassLiteral(
   }
 
   Expect(Token::RBRACE, CHECK_OK);
+  int end_pos = scanner()->location().end_pos;
+  block_scope->set_end_position(end_pos);
   return impl()->RewriteClassLiteral(block_scope, name, &class_info,
-                                     class_token_pos, ok);
+                                     class_token_pos, end_pos, ok);
 }
 
 template <typename Impl>

src/parsing/parser.cc

@@ -3320,11 +3320,10 @@ void Parser::DeclareClassProperty(const AstRawString* class_name,
 Expression* Parser::RewriteClassLiteral(Scope* block_scope,
                                         const AstRawString* name,
                                         ClassInfo* class_info, int pos,
-                                        bool* ok) {
+                                        int end_pos, bool* ok) {
   DCHECK_NOT_NULL(block_scope);
   DCHECK_EQ(block_scope->scope_type(), BLOCK_SCOPE);
   DCHECK_EQ(block_scope->language_mode(), STRICT);
-  int end_pos = scanner()->location().end_pos;
 
   bool has_extends = class_info->extends != nullptr;
   bool has_default_constructor = class_info->constructor == nullptr;
@@ -3333,8 +3332,6 @@ Expression* Parser::RewriteClassLiteral(Scope* block_scope,
         DefaultConstructor(name, has_extends, pos, end_pos);
   }
 
-  block_scope->set_end_position(end_pos);
-
   if (name != nullptr) {
     DCHECK_NOT_NULL(class_info->proxy);
     class_info->proxy->var()->set_initializer_position(end_pos);

src/parsing/parser.h

@@ -376,7 +376,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   V8_INLINE Expression* RewriteClassLiteral(Scope* block_scope,
                                             const AstRawString* name,
                                             ClassInfo* class_info, int pos,
-                                            bool* ok);
+                                            int end_pos, bool* ok);
   V8_INLINE Statement* DeclareNative(const AstRawString* name, int pos,
                                      bool* ok);
 

src/parsing/preparse-data.h

@@ -102,7 +102,7 @@ class PreParseData final {
     LanguageMode language_mode;
     bool uses_super_property : 1;
 
-    FunctionData() : end(-1) {}
+    FunctionData() : end(kNoSourcePosition) {}
 
     FunctionData(int end, int num_parameters, int num_inner_functions,
                  LanguageMode language_mode, bool uses_super_property)
@@ -112,7 +112,10 @@ class PreParseData final {
           language_mode(language_mode),
           uses_super_property(uses_super_property) {}
 
-    bool is_valid() const { return end > 0; }
+    bool is_valid() const {
+      DCHECK_IMPLIES(end < 0, end == kNoSourcePosition);
+      return end != kNoSourcePosition;
+    }
   };
 
   FunctionData GetFunctionData(int start) const;

src/parsing/preparsed-scope-data.cc

@@ -51,6 +51,13 @@ const int kFunctionDataSize = 8;
 
 void PreParsedScopeData::SaveData(Scope* scope) {
   DCHECK(!has_data_);
+  DCHECK_NE(scope->end_position(), kNoSourcePosition);
+
+  // We're not trying to save data for default constructors because the
+  // PreParser doesn't construct them.
+  DCHECK_IMPLIES(scope->scope_type() == ScopeType::FUNCTION_SCOPE,
+                 (scope->AsDeclarationScope()->function_kind() &
+                  kDefaultConstructor) == 0);
 
   if (scope->scope_type() == ScopeType::FUNCTION_SCOPE &&
       !scope->AsDeclarationScope()->is_arrow_scope()) {
@@ -96,6 +103,7 @@ void PreParsedScopeData::AddSkippableFunction(
 
 void PreParsedScopeData::AddFunction(
     int start_position, const PreParseData::FunctionData& function_data) {
+  DCHECK(function_data.is_valid());
   function_index_.AddFunctionData(start_position, function_data);
 }
 
@@ -127,6 +135,7 @@ void PreParsedScopeData::RestoreData(Scope* scope, uint32_t* index_ptr) const {
       !scope->AsDeclarationScope()->is_arrow_scope()) {
     const PreParseData::FunctionData& data =
         function_index_.GetFunctionData(scope->start_position());
+    DCHECK(data.is_valid());
     DCHECK_EQ(data.end, scope->end_position());
     // FIXME(marja): unify num_parameters too and DCHECK here.
     DCHECK_EQ(data.language_mode, scope->language_mode());
@@ -148,6 +157,7 @@ void PreParsedScopeData::RestoreData(Scope* scope, uint32_t* index_ptr) const {
     return;
   }
 
+  DCHECK_GE(backing_store_.size(), index + 3);
   DCHECK_EQ(backing_store_[index++], scope->scope_type());
 
   if (backing_store_[index++]) {
@@ -266,11 +276,13 @@ void PreParsedScopeData::RestoreDataForVariable(Variable* var,
   uint32_t& index = *index_ptr;
 #ifdef DEBUG
   const AstRawString* name = var->raw_name();
+  DCHECK_GT(backing_store_.size(), index + name->length());
   DCHECK_EQ(backing_store_[index++], static_cast<uint32_t>(name->length()));
   for (int i = 0; i < name->length(); ++i) {
     DCHECK_EQ(backing_store_[index++], name->raw_data()[i]);
   }
 #endif
+  DCHECK_GT(backing_store_.size(), index);
   byte variable_data = backing_store_[index++];
   if (VariableIsUsedField::decode(variable_data)) {
     var->set_is_used();
@@ -321,7 +333,10 @@ bool PreParsedScopeData::FindFunctionData(int start_pos,
 
 bool PreParsedScopeData::ScopeNeedsData(Scope* scope) {
   if (scope->scope_type() == ScopeType::FUNCTION_SCOPE) {
-    return true;
+    // Default constructors don't need data (they cannot contain inner functions
+    // defined by the user). Other functions do.
+    return (scope->AsDeclarationScope()->function_kind() &
+            kDefaultConstructor) == 0;
   }
   if (!scope->is_hidden()) {
     for (Variable* var : *scope->locals()) {

src/parsing/preparser.h

@@ -1127,7 +1127,12 @@ class PreParser : public ParserBase<PreParser> {
   }
   V8_INLINE void DeclareClassVariable(PreParserIdentifier name,
                                       ClassInfo* class_info,
-                                      int class_token_pos, bool* ok) {}
+                                      int class_token_pos, bool* ok) {
+    if (name.string_ != nullptr) {
+      DCHECK(track_unresolved_variables_);
+      scope()->DeclareVariableName(name.string_, CONST);
+    }
+  }
   V8_INLINE void DeclareClassProperty(PreParserIdentifier class_name,
                                       PreParserExpression property,
                                       ClassLiteralProperty::Kind kind,
@@ -1137,7 +1142,8 @@ class PreParser : public ParserBase<PreParser> {
   V8_INLINE PreParserExpression RewriteClassLiteral(Scope* scope,
                                                     PreParserIdentifier name,
                                                     ClassInfo* class_info,
-                                                    int pos, bool* ok) {
+                                                    int pos, int end_pos,
+                                                    bool* ok) {
     bool has_default_constructor = !class_info->has_seen_constructor;
     // Account for the default constructor.
     if (has_default_constructor) GetNextFunctionLiteralId();

test/cctest/parsing/test-preparser.cc

@@ -547,6 +547,10 @@ TEST(PreParserScopeAnalysis) {
       // Shadowing the catch variable
       {"try { } catch(var1) { var var1 = 3; }"},
       {"try { } catch(var1) { var var1 = 3; function f() { var1 = 3; } }"},
+
+      // Classes
+      // FIXME(marja): Add more complex class cases.
+      {"class MyClass {}"},
   };
 
   for (unsigned outer_ix = 0; outer_ix < arraysize(outers); ++outer_ix) {