[es6] Refactor FormalParameter

src/parser.h

 // 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.
 
 #ifndef V8_PARSER_H_
 #define V8_PARSER_H_
 
 #include "src/allocation.h"
 #include "src/ast.h"
 #include "src/compiler.h"  // TODO(titzer): remove this include dependency
@@ skipping 523 matching lines @@
 
 // ----------------------------------------------------------------------------
 // JAVASCRIPT PARSING
 
 class Parser;
 class SingletonLogger;
 
 
 struct ParserFormalParameters : public PreParserFormalParameters {
   struct Parameter {
-    Parameter(Variable* var, Expression* pattern)
-        : var(var), pattern(pattern) {}
-    Variable* var;
+    Parameter(const AstRawString* name, Expression* pattern, bool is_rest)
+        : name(name), pattern(pattern), is_rest(is_rest) {}
+    const AstRawString* name;
     Expression* pattern;
+    bool is_rest;
   };
 
   explicit ParserFormalParameters(Scope* scope)
       : PreParserFormalParameters(scope), params(4, scope->zone()) {}
 
   ZoneList<Parameter> params;
 
-  void AddParameter(Variable* var, Expression* pattern) {
-    params.Add(Parameter(var, pattern), scope->zone());
+  void AddParameter(
+      const AstRawString* name, Expression* pattern, bool is_rest) {
+    params.Add(Parameter(name, pattern, is_rest), scope->zone());
+    DCHECK(arity == params.length());

[adamk, 2015/08/03 18:20:23]

DCHECK_EQ.

Also, it will probably be more clear after the followup, but it seems sad that
the only way to check this consistency is a DCHECK. Especially since the
incrementing code is now duplicated between preparser and parser.

[rossberg, 2015/08/04 13:44:22]

Done.
My thinking was that the parser/preparser duplication should go away eventually,
once statement parsing is unified. But the FormalParameter will stay separate.
So you want to move as little of the logic there as possible.

But maybe you are right, I'll reconsider in the follow-up CL.

   }
 };
 
 
 class ParserTraits {
  public:
   struct Type {
     // TODO(marja): To be removed. The Traits object should contain all the data
     // it needs.
     typedef v8::internal::Parser* Parser;
 
     typedef Variable GeneratorVariable;
 
     typedef v8::internal::AstProperties AstProperties;
 
     // Return types for traversing functions.
     typedef const AstRawString* Identifier;
     typedef v8::internal::Expression* Expression;
     typedef Yield* YieldExpression;
     typedef v8::internal::FunctionLiteral* FunctionLiteral;
     typedef v8::internal::ClassLiteral* ClassLiteral;
     typedef v8::internal::Literal* Literal;
     typedef ObjectLiteral::Property* ObjectLiteralProperty;
     typedef ZoneList<v8::internal::Expression*>* ExpressionList;
     typedef ZoneList<ObjectLiteral::Property*>* PropertyList;
-    typedef const v8::internal::AstRawString* FormalParameter;
+    typedef ParserFormalParameters::Parameter FormalParameter;
     typedef ParserFormalParameters FormalParameters;
     typedef ZoneList<v8::internal::Statement*>* StatementList;
 
     // For constructing objects returned by the traversing functions.
     typedef AstNodeFactory Factory;
   };
 
   explicit ParserTraits(Parser* parser) : parser_(parser) {}
 
   // Helper functions for recursive descent.
@@ skipping 720 matching lines @@
     ExpressionClassifier* classifier) {
   bool is_duplicate = false;
   bool is_simple = pattern->IsVariableProxy();
   DCHECK(parser_->allow_harmony_destructuring() || is_simple);
 
   const AstRawString* name = is_simple
                                  ? pattern->AsVariableProxy()->raw_name()
                                  : parser_->ast_value_factory()->empty_string();
   Variable* var =
       parameters->scope->DeclareParameter(name, VAR, is_rest, &is_duplicate);
-  parameters->AddParameter(var, is_simple ? nullptr : pattern);
+  parameters->AddParameter(name, is_simple ? nullptr : pattern, is_rest);
   if (is_duplicate) {
     classifier->RecordDuplicateFormalParameterError(
         parser_->scanner()->location());
   }
   if (is_sloppy(parameters->scope->language_mode())) {
     // TODO(sigurds) Mark every parameter as maybe assigned. This is a
     // conservative approximation necessary to account for parameters
     // that are assigned via the arguments array.
     var->set_maybe_assigned();
   }
 }
 
 
 void ParserTraits::AddParameterInitializationBlock(
     const ParserFormalParameters& parameters,
     ZoneList<v8::internal::Statement*>* body, bool* ok) {
   if (!parameters.is_simple) {
     auto* init_block =
         parser_->BuildParameterInitializationBlock(parameters, ok);
     if (!*ok) return;
     if (init_block != nullptr) {
       body->Add(init_block, parser_->zone());
     }
   }
 }
 } }  // namespace v8::internal
 
 #endif  // V8_PARSER_H_