[ast][parsing] Variable declaration cleanups.

src/parsing/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/parsing/parser.h"
 
 #include <memory>
 
 #include "src/api.h"
 #include "src/ast/ast.h"
@@ skipping 1505 matching lines @@
                              parsing_module_)) {
       *ok = false;
       ReportMessage(MessageTemplate::kUnexpectedReserved);
       return nullptr;
     } else if (IsEvalOrArguments(local_name)) {
       *ok = false;
       ReportMessage(MessageTemplate::kStrictEvalArguments);
       return nullptr;
     }
 
-    DeclareConstVariable(local_name, kNeedsInitialization, position(),
-                         CHECK_OK);
+    DeclareVariable(local_name, CONST, kNeedsInitialization, position(),
+                    CHECK_OK);
 
     NamedImport* import = new (zone()) NamedImport(
         import_name, local_name, scanner()->location());
     result->Add(import, zone());
 
     if (peek() == Token::RBRACE) break;
     Expect(Token::COMMA, CHECK_OK);
   }
 
   Expect(Token::RBRACE, CHECK_OK);
@@ skipping 29 matching lines @@
     return;
   }
 
   // Parse ImportedDefaultBinding if present.
   const AstRawString* import_default_binding = nullptr;
   Scanner::Location import_default_binding_loc;
   if (tok != Token::MUL && tok != Token::LBRACE) {
     import_default_binding =
         ParseIdentifier(kDontAllowRestrictedIdentifiers, CHECK_OK_VOID);
     import_default_binding_loc = scanner()->location();
-    DeclareConstVariable(import_default_binding, kNeedsInitialization, pos,
-                         CHECK_OK_VOID);
+    DeclareVariable(import_default_binding, CONST, kNeedsInitialization, pos,
+                    CHECK_OK_VOID);
   }
 
   // Parse NameSpaceImport or NamedImports if present.
   const AstRawString* module_namespace_binding = nullptr;
   Scanner::Location module_namespace_binding_loc;
   const ZoneList<const NamedImport*>* named_imports = nullptr;
   if (import_default_binding == nullptr || Check(Token::COMMA)) {
     switch (peek()) {
       case Token::MUL: {
         Consume(Token::MUL);
         ExpectContextualKeyword(CStrVector("as"), CHECK_OK_VOID);
         module_namespace_binding =
             ParseIdentifier(kDontAllowRestrictedIdentifiers, CHECK_OK_VOID);
         module_namespace_binding_loc = scanner()->location();
-        DeclareConstVariable(module_namespace_binding, kCreatedInitialized, pos,
-                             CHECK_OK_VOID);
+        DeclareVariable(module_namespace_binding, CONST, kCreatedInitialized,
+                        pos, CHECK_OK_VOID);
         break;
       }
 
       case Token::LBRACE:
         named_imports = ParseNamedImports(pos, CHECK_OK_VOID);
         break;
 
       default:
         *ok = false;
         ReportUnexpectedToken(scanner()->current_token());
         return;
     }
   }
 
   ExpectContextualKeyword(CStrVector("from"), CHECK_OK_VOID);
   const AstRawString* module_specifier = ParseModuleSpecifier(CHECK_OK_VOID);
   ExpectSemicolon(CHECK_OK_VOID);
 
   // Now that we have all the information, we can make the appropriate
   // declarations.
 
-  // TODO(neis): Would prefer to call DeclareConstVariable for each case below
-  // rather than above and in ParseNamedImports, but then a possible error
-  // message would point to the wrong location.  Maybe have a DeclareAt version
-  // of Declare that takes a location?
+  // TODO(neis): Would prefer to call DeclareVariable for each case below rather
+  // than above and in ParseNamedImports, but then a possible error message
+  // would point to the wrong location.  Maybe have a DeclareAt version of
+  // Declare that takes a location?
 
   if (module_namespace_binding != nullptr) {
     module()->AddStarImport(module_namespace_binding, module_specifier,
                             module_namespace_binding_loc, zone());
   }
 
   if (import_default_binding != nullptr) {
     module()->AddImport(ast_value_factory()->default_string(),
                         import_default_binding, module_specifier,
                         import_default_binding_loc, zone());
@@ skipping 50 matching lines @@
           ParseAssignmentExpression(true, &classifier, CHECK_OK);
       RewriteNonPattern(&classifier, CHECK_OK);
       SetFunctionName(value, ast_value_factory()->default_string());
 
       const AstRawString* local_name =
           ast_value_factory()->star_default_star_string();
       local_names.Add(local_name, zone());
 
       // It's fine to declare this as CONST because the user has no way of
       // writing to it.
-      VariableProxy* proxy = NewUnresolved(local_name, CONST);
-      Declaration* declaration =
-          factory()->NewVariableDeclaration(proxy, CONST, scope(), pos);
-      Declare(declaration, DeclarationDescriptor::NORMAL, true, CHECK_OK);
-      proxy->var()->set_initializer_position(position());
+      Declaration* decl = DeclareVariable(local_name, CONST, pos, CHECK_OK);
+      decl->proxy()->var()->set_initializer_position(position());
 
       Assignment* assignment = factory()->NewAssignment(
-          Token::INIT, proxy, value, kNoSourcePosition);
+          Token::INIT, decl->proxy(), value, kNoSourcePosition);
       result = factory()->NewExpressionStatement(assignment, kNoSourcePosition);
 
       ExpectSemicolon(CHECK_OK);
       break;
     }
   }
 
   DCHECK_EQ(local_names.length(), 1);
   module()->AddExport(local_names.first(),
                       ast_value_factory()->default_string(), default_loc,
@@ skipping 259 matching lines @@
   // scope.
   // Let/const variables are always added to the immediately enclosing scope.
   Scope* scope = IsLexicalVariableMode(mode)
                      ? this->scope()
                      : this->scope()->GetDeclarationScope();
   return scope->NewUnresolved(factory(), name, Variable::NORMAL,
                               scanner()->location().beg_pos,
                               scanner()->location().end_pos);
 }
 
-void Parser::DeclareConstVariable(const AstRawString* name,
-                                  InitializationFlag init, int pos, bool* ok) {
-  DCHECK_NOT_NULL(name);
-  VariableProxy* proxy = NewUnresolved(name, CONST);
-  Declaration* declaration =
-      factory()->NewVariableDeclaration(proxy, CONST, scope(), init, pos);
-  Declare(declaration, DeclarationDescriptor::NORMAL, true, CHECK_OK_VOID);
+InitializationFlag Parser::DefaultInitializationFlag(VariableMode mode) {

[adamk, 2016/08/08 17:32:28]

What about just putting this with the InitializationFlag enum definition in
globals.h? Its placement as an instance method of Parser makes it look like it
might depend on something _about_ parser, but it doesn't.

[neis, 2016/08/09 08:17:52]

Oops, this was of course supposed to be a static method.  I'm leaving it in the
parser for now, but I want to discuss this again.

[adamk, 2016/08/09 16:27:09]

Ah, forgot PatternRewriter is an inner class, this looks fine now.

+  return mode == VAR ? kCreatedInitialized : kNeedsInitialization;

[adamk, 2016/08/08 17:32:28]

Maybe add a DCHECK(IsDeclaredVariableMode(mode)) above this?

[neis, 2016/08/09 08:17:52]

Done.

 }
 
+Declaration* Parser::DeclareVariable(const AstRawString* name,
+                                     VariableMode mode, int pos, bool* ok) {
+  return DeclareVariable(name, mode, DefaultInitializationFlag(mode), pos, ok);
+}
+
+Declaration* Parser::DeclareVariable(const AstRawString* name,
+                                     VariableMode mode, InitializationFlag init,
+                                     int pos, bool* ok) {
+  DCHECK_NOT_NULL(name);
+  VariableProxy* proxy = NewUnresolved(name, mode);
+  Declaration* declaration =
+      factory()->NewVariableDeclaration(proxy, mode, scope(), pos);
+  Declare(declaration, DeclarationDescriptor::NORMAL, init, true, CHECK_OK);
+  return declaration;
+}
 
 Variable* Parser::Declare(Declaration* declaration,
                           DeclarationDescriptor::Kind declaration_kind,
-                          bool resolve, bool* ok, Scope* scope) {
+                          InitializationFlag init, bool resolve, bool* ok,
+                          Scope* scope) {
   VariableProxy* proxy = declaration->proxy();
   DCHECK(proxy->raw_name() != NULL);
   const AstRawString* name = proxy->raw_name();
   VariableMode mode = declaration->mode();
   DCHECK(IsDeclaredVariableMode(mode) && mode != CONST_LEGACY);
   bool is_function_declaration = declaration->IsFunctionDeclaration();
   if (scope == nullptr) scope = this->scope();
   Scope* declaration_scope =
       IsLexicalVariableMode(mode) ? scope : scope->GetDeclarationScope();
   Variable* var = NULL;
@@ skipping 14 matching lines @@
        (is_strict(declaration_scope->language_mode()) ||
         IsLexicalVariableMode(mode)))) {
     // Declare the variable in the declaration scope.
     var = declaration_scope->LookupLocal(name);
     if (var == NULL) {
       // Declare the name.
       Variable::Kind kind = Variable::NORMAL;
       if (is_function_declaration) {
         kind = Variable::FUNCTION;
       }
-      var = declaration_scope->DeclareLocal(
-          name, mode, declaration->initialization(), kind, kNotAssigned);
+      var =
+          declaration_scope->DeclareLocal(name, mode, init, kind, kNotAssigned);
     } else if (IsLexicalVariableMode(mode) ||
                IsLexicalVariableMode(var->mode())) {
       // Allow duplicate function decls for web compat, see bug 4693.
       bool duplicate_allowed = false;
       if (is_sloppy(language_mode()) && is_function_declaration &&
           var->is_function()) {
         DCHECK(IsLexicalVariableMode(mode) &&
                IsLexicalVariableMode(var->mode()));
         // If the duplication is allowed, then the var will show up
         // in the SloppyBlockFunctionMap and the new FunctionKind
@@ skipping 39 matching lines @@
     }
   } else if (declaration_scope->is_eval_scope() &&
              is_sloppy(declaration_scope->language_mode()) &&
              !IsLexicalVariableMode(mode)) {
     // In a var binding in a sloppy direct eval, pollute the enclosing scope
     // with this new binding by doing the following:
     // The proxy is bound to a lookup variable to force a dynamic declaration
     // using the DeclareEvalVar or DeclareEvalFunction runtime functions.
     Variable::Kind kind = Variable::NORMAL;
     // TODO(sigurds) figure out if kNotAssigned is OK here
-    var = new (zone()) Variable(declaration_scope, name, mode, kind,
-                                declaration->initialization(), kNotAssigned);
+    var = new (zone())
+        Variable(declaration_scope, name, mode, kind, init, kNotAssigned);
     var->AllocateTo(VariableLocation::LOOKUP, -1);
     resolve = true;
   }
 
 
   // We add a declaration node for every declaration. The compiler
   // will only generate code if necessary. In particular, declarations
   // for inner local variables that do not represent functions won't
   // result in any generated code.
   //
@@ skipping 66 matching lines @@
   // Make sure that the function containing the native declaration
   // isn't lazily compiled. The extension structures are only
   // accessible while parsing the first time not when reparsing
   // because of lazy compilation.
   // TODO(adamk): Should this be GetClosureScope()?
   scope()->GetDeclarationScope()->ForceEagerCompilation();
 
   // TODO(1240846): It's weird that native function declarations are
   // introduced dynamically when we meet their declarations, whereas
   // other functions are set up when entering the surrounding scope.
-  VariableProxy* proxy = NewUnresolved(name, VAR);
-  Declaration* declaration =
-      factory()->NewVariableDeclaration(proxy, VAR, scope(), pos);
-  Declare(declaration, DeclarationDescriptor::NORMAL, true, CHECK_OK);
+  Declaration* decl = DeclareVariable(name, VAR, pos, CHECK_OK);
   NativeFunctionLiteral* lit =
       factory()->NewNativeFunctionLiteral(name, extension_, kNoSourcePosition);
   return factory()->NewExpressionStatement(
-      factory()->NewAssignment(Token::INIT, proxy, lit, kNoSourcePosition),
+      factory()->NewAssignment(Token::INIT, decl->proxy(), lit,
+                               kNoSourcePosition),
       pos);
 }
 
 Statement* Parser::ParseHoistableDeclaration(
     ZoneList<const AstRawString*>* names, bool default_export, bool* ok) {
   Expect(Token::FUNCTION, CHECK_OK);
   int pos = position();
   ParseFunctionFlags flags = ParseFunctionFlags::kIsNormal;
   if (Check(Token::MUL)) {
     flags |= ParseFunctionFlags::kIsGenerator;
@@ skipping 65 matching lines @@
       pos, FunctionLiteral::kDeclaration, language_mode(), CHECK_OK);
 
   // In ES6, a function behaves as a lexical binding, except in
   // a script scope, or the initial scope of eval or another function.
   VariableMode mode =
       (!scope()->is_declaration_scope() || scope()->is_module_scope()) ? LET
                                                                        : VAR;
   VariableProxy* proxy = NewUnresolved(variable_name, mode);
   Declaration* declaration =
       factory()->NewFunctionDeclaration(proxy, mode, fun, scope(), pos);
-  Declare(declaration, DeclarationDescriptor::NORMAL, true, CHECK_OK);
+  Declare(declaration, DeclarationDescriptor::NORMAL, kCreatedInitialized, true,
+          CHECK_OK);
   if (names) names->Add(variable_name, zone());
   EmptyStatement* empty = factory()->NewEmptyStatement(kNoSourcePosition);
   // Async functions don't undergo sloppy mode block scoped hoisting, and don't
   // allow duplicates in a block. Both are represented by the
   // sloppy_block_function_map. Don't add them to the map for async functions.
   // Generators are also supposed to be prohibited; currently doing this behind
   // a flag and UseCounting violations to assess web compatibility.
   if (is_sloppy(language_mode()) && !scope()->is_declaration_scope() &&
       !is_async && !(allow_harmony_restrictive_generators() && is_generator)) {
     SloppyBlockFunctionStatement* delegate =
@@ skipping 35 matching lines @@
     is_strict_reserved = false;
     variable_name = ast_value_factory()->star_default_star_string();
   } else {
     name = ParseIdentifierOrStrictReservedWord(&is_strict_reserved, CHECK_OK);
     variable_name = name;
   }
 
   Expression* value = ParseClassLiteral(nullptr, name, scanner()->location(),
                                         is_strict_reserved, pos, CHECK_OK);
 
-  VariableProxy* proxy = NewUnresolved(variable_name, LET);
-  Declaration* declaration =
-      factory()->NewVariableDeclaration(proxy, LET, scope(), pos);
-  Declare(declaration, DeclarationDescriptor::NORMAL, true, CHECK_OK);
-  proxy->var()->set_initializer_position(position());
+  Declaration* decl = DeclareVariable(variable_name, LET, pos, CHECK_OK);
+  decl->proxy()->var()->set_initializer_position(position());
   Assignment* assignment =
-      factory()->NewAssignment(Token::INIT, proxy, value, pos);
+      factory()->NewAssignment(Token::INIT, decl->proxy(), value, pos);
   Statement* assignment_statement =
       factory()->NewExpressionStatement(assignment, kNoSourcePosition);
   if (names) names->Add(variable_name, zone());
   return assignment_statement;
 }
 
 Block* Parser::ParseBlock(ZoneList<const AstRawString*>* labels, bool* ok) {
   // The harmony mode uses block elements instead of statements.
   //
   // Block ::
@@ skipping 1206 matching lines @@
   Block* inner_block = factory()->NewBlock(NULL, 3, false, kNoSourcePosition);
   {
     BlockState block_state(&scope_state_, inner_scope);
 
     Block* ignore_completion_block =
         factory()->NewBlock(NULL, names->length() + 3, true, kNoSourcePosition);
     ZoneList<Variable*> inner_vars(names->length(), zone());
     // For each let variable x:
     //    make statement: let/const x = temp_x.
     for (int i = 0; i < names->length(); i++) {
-      VariableProxy* proxy = NewUnresolved(names->at(i), mode);
-      Declaration* declaration = factory()->NewVariableDeclaration(
-          proxy, mode, scope(), kNoSourcePosition);
-      Declare(declaration, DeclarationDescriptor::NORMAL, true, CHECK_OK);
-      inner_vars.Add(declaration->proxy()->var(), zone());
+      Declaration* decl =
+          DeclareVariable(names->at(i), mode, kNoSourcePosition, CHECK_OK);
+      inner_vars.Add(decl->proxy()->var(), zone());
       VariableProxy* temp_proxy = factory()->NewVariableProxy(temps.at(i));
       Assignment* assignment = factory()->NewAssignment(
-          Token::INIT, proxy, temp_proxy, kNoSourcePosition);
+          Token::INIT, decl->proxy(), temp_proxy, kNoSourcePosition);
       Statement* assignment_statement =
           factory()->NewExpressionStatement(assignment, kNoSourcePosition);
       DCHECK(init->position() != kNoSourcePosition);
-      proxy->var()->set_initializer_position(init->position());
+      decl->proxy()->var()->set_initializer_position(init->position());
       ignore_completion_block->statements()->Add(assignment_statement, zone());
     }
 
     // Make statement: if (first == 1) { first = 0; } else { next; }
     if (next) {
       DCHECK(first);
       Expression* compare = NULL;
       // Make compare expression: first == 1.
       {
         Expression* const1 = factory()->NewSmiLiteral(1, kNoSourcePosition);
@@ skipping 319 matching lines @@
         if (bound_names_are_lexical) {
           DCHECK_NULL(init_block);
 
           init_block =
               factory()->NewBlock(nullptr, 1, false, kNoSourcePosition);
 
           for (int i = 0; i < bound_names.length(); ++i) {
             // TODO(adamk): This needs to be some sort of special
             // INTERNAL variable that's invisible to the debugger
             // but visible to everything else.
-            VariableProxy* tdz_proxy = NewUnresolved(bound_names[i], LET);
-            Declaration* tdz_decl = factory()->NewVariableDeclaration(
-                tdz_proxy, LET, scope(), kNoSourcePosition);
-            Variable* tdz_var = Declare(
-                tdz_decl, DeclarationDescriptor::NORMAL, true, CHECK_OK);
-            tdz_var->set_initializer_position(position());
+            Declaration* tdz_decl = DeclareVariable(
+                bound_names[i], LET, kNoSourcePosition, CHECK_OK);
+            tdz_decl->proxy()->var()->set_initializer_position(position());
           }
         }
 
         for_state.set_end_position(scanner()->location().end_pos);
         Scope* for_scope = for_state.FinalizedBlockScope();
         // Parsed for-in loop w/ variable declarations.
         if (init_block != nullptr) {
           init_block->statements()->Add(final_loop, zone());
           init_block->set_scope(for_scope);
           return init_block;
@@ skipping 1159 matching lines @@
 #ifdef DEBUG
   scope()->SetScopeName(name);
 #endif
 
   VariableProxy* proxy = nullptr;
   if (name != nullptr) {
     proxy = NewUnresolved(name, CONST);
     // TODO(verwaest): declare via block_state.
     Declaration* declaration = factory()->NewVariableDeclaration(
         proxy, CONST, block_state.scope(), pos);
-    Declare(declaration, DeclarationDescriptor::NORMAL, true, CHECK_OK);
+    Declare(declaration, DeclarationDescriptor::NORMAL,
+            DefaultInitializationFlag(CONST), true, CHECK_OK);
   }
 
   Expression* extends = nullptr;
   if (Check(Token::EXTENDS)) {
     block_state.set_start_position(scanner()->location().end_pos);
     ExpressionClassifier extends_classifier(this);
     extends = ParseLeftHandSideExpression(&extends_classifier, CHECK_OK);
     CheckNoTailCallExpressions(&extends_classifier, CHECK_OK);
     RewriteNonPattern(&extends_classifier, CHECK_OK);
     if (classifier != nullptr) {
@@ skipping 257 matching lines @@
       } while (query_scope != outer_scope);
 
       if (!should_hoist) continue;
 
       // Declare a var-style binding for the function in the outer scope
       if (!var_created) {
         var_created = true;
         VariableProxy* proxy = scope->NewUnresolved(factory(), name);
         Declaration* declaration = factory()->NewVariableDeclaration(
             proxy, VAR, scope, kNoSourcePosition);
-        Declare(declaration, DeclarationDescriptor::NORMAL, true, ok, scope);
+        Declare(declaration, DeclarationDescriptor::NORMAL,
+                DefaultInitializationFlag(VAR), true, ok, scope);
         DCHECK(ok);  // Based on the preceding check, this should not fail
         if (!ok) return;
       }
 
       // Read from the local lexical scope and write to the function scope
       VariableProxy* to = scope->NewUnresolved(factory(), name);
       VariableProxy* from = delegate->scope()->NewUnresolved(factory(), name);
       Expression* assignment =
           factory()->NewAssignment(Token::ASSIGN, to, from, kNoSourcePosition);
       Statement* statement =
@@ skipping 1785 matching lines @@
   node->Print(Isolate::Current());
 }
 #endif  // DEBUG
 
 #undef CHECK_OK
 #undef CHECK_OK_VOID
 #undef CHECK_FAILED
 
 }  // namespace internal
 }  // namespace v8