Do not create unresolved variables when parsing arrow functions lazily

src/parser.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/v8.h"
 
 #include "src/api.h"
 #include "src/ast.h"
 #include "src/bailout-reason.h"
 #include "src/base/platform/platform.h"
@@ skipping 705 matching lines @@
 Expression* ParserTraits::ExpressionFromIdentifier(const AstRawString* name,
                                                    int pos, Scope* scope,
                                                    AstNodeFactory* factory) {
   if (parser_->fni_ != NULL) parser_->fni_->PushVariableName(name);
   // The name may refer to a module instance object, so its type is unknown.
 #ifdef DEBUG
   if (FLAG_print_interface_details)
     PrintF("# Variable %.*s ", name->length(), name->raw_data());
 #endif
   Interface* interface = Interface::NewUnknown(parser_->zone());
-  return scope->NewUnresolved(factory, name, interface, pos);
+
+  // Arrow function parameters are parsed as an expression. When
+  // parsing lazily, it is enough to create a VariableProxy in order
+  // for Traits::DeclareArrowParametersFromExpression() to be able to
+  // pick the names of the parameters.
+  return parser_->parsing_lazy_arrow_parameters_
+      ? factory->NewVariableProxy(name, false, interface, pos)
+      : scope->NewUnresolved(factory, name, interface, pos);
 }
 
 
 Expression* ParserTraits::ExpressionFromString(int pos, Scanner* scanner,
                                                AstNodeFactory* factory) {
   const AstRawString* symbol = GetSymbol(scanner);
   if (parser_->fni_ != NULL) parser_->fni_->PushLiteralName(symbol);
   return factory->NewStringLiteral(symbol, pos);
 }
 
@@ skipping 44 matching lines @@
 Parser::Parser(CompilationInfo* info, ParseInfo* parse_info)
     : ParserBase<ParserTraits>(info->isolate(), info->zone(), &scanner_,
                                parse_info->stack_limit, info->extension(), NULL,
                                this),
       scanner_(parse_info->unicode_cache),
       reusable_preparser_(NULL),
       original_scope_(NULL),
       target_stack_(NULL),
       cached_parse_data_(NULL),
       info_(info),
+      parsing_lazy_arrow_parameters_(false),
       has_pending_error_(false),
       pending_error_message_(NULL),
       pending_error_arg_(NULL),
       pending_error_char_arg_(NULL),
       total_preparse_skipped_(0),
       pre_parse_timer_(NULL) {
   DCHECK(!script().is_null() || info->source_stream() != NULL);
   set_allow_lazy(false);  // Must be explicitly enabled.
   set_allow_natives(FLAG_allow_natives_syntax || info->is_native());
   set_allow_harmony_scoping(!info->is_native() && FLAG_harmony_scoping);
@@ skipping 253 matching lines @@
     DCHECK(info()->strict_mode() == shared_info->strict_mode());
     scope->SetStrictMode(shared_info->strict_mode());
     FunctionLiteral::FunctionType function_type = shared_info->is_expression()
         ? (shared_info->is_anonymous()
               ? FunctionLiteral::ANONYMOUS_EXPRESSION
               : FunctionLiteral::NAMED_EXPRESSION)
         : FunctionLiteral::DECLARATION;
     bool ok = true;
 
     if (shared_info->is_arrow()) {
+      // The first expression being parsed is the parameter list of the arrow
+      // function. Setting this avoids prevents ExpressionFromIdentifier()
+      // from creating unresolved variables in already-resolved scopes.
+      parsing_lazy_arrow_parameters_ = true;
       Expression* expression = ParseExpression(false, &ok);
       DCHECK(expression->IsFunctionLiteral());
       result = expression->AsFunctionLiteral();

[marja, 2015/01/29 09:53:01]

Why are setting parsing_lazy_arrow_parameters_ to true and false so far apart
(conceptually)? Can we set it to false right after ParseExpression here? (Then
it would be trivial to see that it is actually set back to false after we're
done.)

[aperez, 2015/01/29 14:38:03]

That does not work.

ParseExpression() here parses the whole arrow function, including the body,
which can itself en up calling ParseExpression().

The flag has to be active only for the “expressions” inside the parameter list
of the arrow function, that's why the flag is set to “false” in
DeclareArrowParametersFromExpression(): at that point it's certain that at least
one entire arrow function parameter list has been parsed.

Other arrow functions contained inside the top-level arrow function being parsed
lazily are always parsed eagerly, so for those the flag will be always “false”
because their parsing has not been started by ParseLazy().

I hope it is clearer now why the flag is reset inside
DeclareArrowParametersFromExpression().

     } else if (shared_info->is_default_constructor()) {
       result = DefaultConstructor(shared_info->uses_super_constructor_call(),
                                   scope, shared_info->start_position(),
                                   shared_info->end_position());
     } else {
       result = ParseFunctionLiteral(raw_name, Scanner::Location::invalid(),
                                     false,  // Strict mode name already checked.
                                     shared_info->kind(), RelocInfo::kNoPosition,
                                     function_type,
                                     FunctionLiteral::NORMAL_ARITY, &ok);
@@ skipping 2257 matching lines @@
 
   // Any other kind of expression is not a valid parameter list.
   return false;
 }
 
 
 int ParserTraits::DeclareArrowParametersFromExpression(
     Expression* expression, Scope* scope, Scanner::Location* dupe_loc,
     bool* ok) {
   int num_params = 0;
+  // Always reset the flag: It only needs to be set for the first expression
+  // parsed as arrow function parameter list, becauseonly top-level functions
+  // are parsed lazily.
+  parser_->parsing_lazy_arrow_parameters_ = false;
   *ok = CheckAndDeclareArrowParameter(this, expression, scope, &num_params,
                                       dupe_loc);
   return num_params;
 }
 
 
 FunctionLiteral* Parser::ParseFunctionLiteral(
     const AstRawString* function_name, Scanner::Location function_name_location,
     bool name_is_strict_reserved, FunctionKind kind, int function_token_pos,
     FunctionLiteral::FunctionType function_type,
@@ skipping 1789 matching lines @@
     } else {
       const uc16* data = reinterpret_cast<const uc16*>(raw_string->raw_data());
       running_hash = StringHasher::ComputeRunningHash(running_hash, data,
                                                       raw_string->length());
     }
   }
 
   return running_hash;
 }
 } }  // namespace v8::internal