[parser] Skip checks in the parser that are guaranteed by the preparser

Change-Id: I8971d1e2ab47599bba4db8cac8631bcf39058593
Reviewed-on: https://chromium-review.googlesource.com/c/1436024Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59128}

src/ast/scopes.cc

@@ -240,6 +240,7 @@ void DeclarationScope::SetDefaults() {
   force_eager_compilation_ = false;
   has_arguments_parameter_ = false;
   scope_uses_super_property_ = false;
+  has_checked_syntax_ = false;
   has_rest_ = false;
   receiver_ = nullptr;
   new_target_ = nullptr;
@@ -1084,7 +1085,8 @@ Variable* Scope::NewTemporary(const AstRawString* name,
   return var;
 }
 
-Declaration* Scope::CheckConflictingVarDeclarations() {
+Declaration* DeclarationScope::CheckConflictingVarDeclarations() {
+  if (has_checked_syntax_) return nullptr;
   for (Declaration* decl : decls_) {
     // Lexical vs lexical conflicts within the same scope have already been
     // captured in Parser::Declare. The only conflicts we still need to check

src/ast/scopes.h

@@ -250,14 +250,6 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   // TODO(verwaest): Move to DeclarationScope?
   Variable* NewTemporary(const AstRawString* name);
 
-  // ---------------------------------------------------------------------------
-  // Illegal redeclaration support.
-
-  // Check if the scope has conflicting var
-  // declarations, i.e. a var declaration that has been hoisted from a nested
-  // scope over a let binding of the same name.
-  Declaration* CheckConflictingVarDeclarations();
-
   // Find variable with (variable->mode() <= |mode_limit|) that was declared in
   // |scope|. This is used to catch patterns like `try{}catch(e){let e;}` and
   // function([e]) { let e }, which are errors even though the two 'e's are each
@@ -751,6 +743,19 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
     zone_ = zone;
   }
 
+  // ---------------------------------------------------------------------------
+  // Illegal redeclaration support.
+
+  // Check if the scope has conflicting var
+  // declarations, i.e. a var declaration that has been hoisted from a nested
+  // scope over a let binding of the same name.
+  Declaration* CheckConflictingVarDeclarations();
+
+  void set_has_checked_syntax(bool has_checked_syntax) {
+    has_checked_syntax_ = has_checked_syntax;
+  }
+  bool has_checked_syntax() const { return has_checked_syntax_; }
+
   bool ShouldEagerCompile() const {
     return force_eager_compilation_ || should_eager_compile_;
   }
@@ -1025,6 +1030,7 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
 #endif
   bool is_skipped_function_ : 1;
   bool has_inferred_function_name_ : 1;
+  bool has_checked_syntax_ : 1;
 
   int num_parameters_ = 0;
 

src/parsing/parser-base.h

@@ -1088,7 +1088,7 @@ class ParserBase {
   // The var declarations are hoisted to the function scope, but originate from
   // a scope where the name has also been let bound or the var declaration is
   // hoisted over such a scope.
-  void CheckConflictingVarDeclarations(Scope* scope) {
+  void CheckConflictingVarDeclarations(DeclarationScope* scope) {
     if (has_error()) return;
     Declaration* decl = scope->CheckConflictingVarDeclarations();
     if (decl != nullptr) {
@@ -3853,10 +3853,12 @@ void ParserBase<Impl>::ParseFunctionBody(
       inner_body.Rewind();
       inner_body.Add(inner_block);
       inner_block->set_scope(inner_scope);
-      const AstRawString* conflict = inner_scope->FindVariableDeclaredIn(
-          function_scope, VariableMode::kLastLexicalVariableMode);
-      if (conflict != nullptr) {
-        impl()->ReportVarRedeclarationIn(conflict, inner_scope);
+      if (!impl()->HasCheckedSyntax()) {
+        const AstRawString* conflict = inner_scope->FindVariableDeclaredIn(
+            function_scope, VariableMode::kLastLexicalVariableMode);
+        if (conflict != nullptr) {
+          impl()->ReportVarRedeclarationIn(conflict, inner_scope);
+        }
       }
       impl()->InsertShadowingVarBindingInitializers(inner_block);
     }
@@ -3883,6 +3885,7 @@ void ParserBase<Impl>::CheckArityRestrictions(int param_count,
                                               bool has_rest,
                                               int formals_start_pos,
                                               int formals_end_pos) {
+  if (impl()->HasCheckedSyntax()) return;
   if (IsGetterFunction(function_kind)) {
     if (param_count != 0) {
       impl()->ReportMessageAt(
@@ -3944,7 +3947,7 @@ ParserBase<Impl>::ParseArrowFunctionLiteral(
   if (V8_UNLIKELY(FLAG_log_function_events)) timer.Start();
 
   DCHECK_IMPLIES(!has_error(), peek() == Token::ARROW);
-  if (scanner_->HasLineTerminatorBeforeNext()) {
+  if (!impl()->HasCheckedSyntax() && scanner_->HasLineTerminatorBeforeNext()) {
     // ASI inserts `;` after arrow parameters if a line terminator is found.
     // `=> ...` is never a valid expression, so report as syntax error.
     // If next token is not `=>`, it's a syntax error anyways.
@@ -4100,7 +4103,7 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseClassLiteral(
   bool is_anonymous = impl()->IsNull(name);
 
   // All parts of a ClassDeclaration and ClassExpression are strict code.
-  if (!is_anonymous) {
+  if (!impl()->HasCheckedSyntax() && !is_anonymous) {
     if (name_is_strict_reserved) {
       impl()->ReportMessageAt(class_name_location,
                               MessageTemplate::kUnexpectedStrictReserved);
@@ -4636,7 +4639,8 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseStatement(
     case Token::VAR:
       return ParseVariableStatement(kStatement, nullptr);
     case Token::ASYNC:
-      if (!scanner()->HasLineTerminatorAfterNext() &&
+      if (!impl()->HasCheckedSyntax() &&
+          !scanner()->HasLineTerminatorAfterNext() &&
           PeekAhead() == Token::FUNCTION) {
         impl()->ReportMessageAt(
             scanner()->peek_location(),
@@ -5237,18 +5241,20 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseTryStatement() {
             catch_statements.Add(inner_block);
 
             // Check for `catch(e) { let e; }` and similar errors.
-            Scope* inner_scope = inner_block->scope();
-            if (inner_scope != nullptr) {
-              const AstRawString* conflict = nullptr;
-              if (impl()->IsNull(catch_info.pattern)) {
-                const AstRawString* name = catch_info.variable->raw_name();
-                if (inner_scope->LookupLocal(name)) conflict = name;
-              } else {
-                conflict = inner_scope->FindVariableDeclaredIn(
-                    scope(), VariableMode::kVar);
-              }
-              if (conflict != nullptr) {
-                impl()->ReportVarRedeclarationIn(conflict, inner_scope);
+            if (!impl()->HasCheckedSyntax()) {
+              Scope* inner_scope = inner_block->scope();
+              if (inner_scope != nullptr) {
+                const AstRawString* conflict = nullptr;
+                if (impl()->IsNull(catch_info.pattern)) {
+                  const AstRawString* name = catch_info.variable->raw_name();
+                  if (inner_scope->LookupLocal(name)) conflict = name;
+                } else {
+                  conflict = inner_scope->FindVariableDeclaredIn(
+                      scope(), VariableMode::kVar);
+                }
+                if (conflict != nullptr) {
+                  impl()->ReportVarRedeclarationIn(conflict, inner_scope);
+                }
               }
             }
 

src/parsing/parser.cc

@@ -786,6 +786,7 @@ FunctionLiteral* Parser::DoParseFunction(Isolate* isolate, ParseInfo* info,
 
       // TODO(adamk): We should construct this scope from the ScopeInfo.
       DeclarationScope* scope = NewFunctionScope(kind);
+      scope->set_has_checked_syntax(true);
 
       // This bit only needs to be explicitly set because we're
       // not passing the ScopeInfo to the Scope constructor.

src/parsing/parser.h

@@ -373,6 +373,10 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
 
   Scope* NewHiddenCatchScope();
 
+  bool HasCheckedSyntax() {
+    return scope()->GetDeclarationScope()->has_checked_syntax();
+  }
+
   // PatternRewriter and associated methods defined in pattern-rewriter.cc.
   friend class PatternRewriter;
   void InitializeVariables(

src/parsing/preparser.h

@@ -1028,6 +1028,8 @@ class PreParser : public ParserBase<PreParser> {
     return literal;
   }
 
+  bool HasCheckedSyntax() { return false; }
+
   void ParseStatementListAndLogFunction(PreParserFormalParameters* formals);
 
   struct TemplateLiteralState {};