[es6] Implement proper TDZ for parameters

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/ast-literal-reindexer.h"
 #include "src/bailout-reason.h"
@@ skipping 1184 matching lines @@
         // NewUnresolved references in current scope. Entrer arrow function
         // scope for formal parameter parsing.
         BlockState block_state(&scope_, scope);
         if (Check(Token::LPAREN)) {
           // '(' StrictFormalParameters ')'
           ParseFormalParameterList(&formals, &formals_classifier, &ok);
           if (ok) ok = Check(Token::RPAREN);
         } else {
           // BindingIdentifier
           const bool is_rest = false;
           ParseFormalParameter(is_rest, &formals, &formals_classifier, &ok);

[adamk, 2015/08/03 18:43:51]

Check ok before declaring?

[rossberg, 2015/08/04 15:31:19]

Done.

+          DeclareFormalParameter(
+              formals.scope, formals.at(0), formals.is_simple,
+              &formals_classifier);
         }
       }
 
       if (ok) {
         checkpoint.Restore(&formals.materialized_literals_count);
         Expression* expression =
             ParseArrowFunctionLiteral(formals, formals_classifier, &ok);
         if (ok) {
           // Scanning must end at the same position that was recorded
           // previously. If not, parsing has been interrupted due to a stack
@@ skipping 993 matching lines @@
       language_mode(), CHECK_OK);
   // Even if we're not at the top-level of the global or a function
   // scope, we treat it as such and introduce the function with its
   // initial value upon entering the corresponding scope.
   // 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 =
       is_strong(language_mode())
           ? CONST
           : (is_strict(language_mode()) || allow_harmony_sloppy()) &&
-                    !(scope_->is_script_scope() || scope_->is_eval_scope() ||
-                      scope_->is_function_scope())
-                ? LET
-                : VAR;
+              !scope_->is_declaration_scope() ? LET : VAR;

[adamk, 2015/08/03 18:43:51]

This looks unrelated to the rest of the change; maybe it's further fixup from
the broadening of declaration scope definition?

[rossberg, 2015/08/04 15:31:19]

You are right, I overlooked this one when splitting up the CL. It is needed
because there was some test started failing with this CL otherwise. I can
separate it out if you want.

[adamk, 2015/08/04 17:25:03]

Given that the rest of this change is needed to exercise it (apparently), I
guess splitting it out is more trouble than it's worth. Just keep an eye on it
if this change has to be reverted for other reasons.

   VariableProxy* proxy = NewUnresolved(name, mode);
   Declaration* declaration =
       factory()->NewFunctionDeclaration(proxy, mode, fun, scope_, pos);
   Declare(declaration, DeclarationDescriptor::NORMAL, true, CHECK_OK);
   if (names) names->Add(name, zone());
   return factory()->NewEmptyStatement(RelocInfo::kNoPosition);
 }
 
 
 Statement* Parser::ParseClassDeclaration(ZoneList<const AstRawString*>* names,
@@ skipping 1662 matching lines @@
   if (expr->IsVariableProxy()) {
     // When the formal parameter was originally seen, it was parsed as a
     // VariableProxy and recorded as unresolved in the scope.  Here we undo that
     // parse-time side-effect for parameters that are single-names (not
     // patterns; for patterns that happens uniformly in
     // PatternRewriter::VisitVariableProxy).
     parser_->scope_->RemoveUnresolved(expr->AsVariableProxy());
   }
 
   ++parameters->arity;
-  ExpressionClassifier classifier;
-  DeclareFormalParameter(parameters, expr, is_rest, &classifier);
-  if (!duplicate_loc->IsValid()) {
-    *duplicate_loc = classifier.duplicate_formal_parameter_error().location;
+  AddFormalParameter(parameters, expr, is_rest);
+}
+
+
+void ParserTraits::ParseArrowFunctionFormalParameterList(
+    ParserFormalParameters* parameters, Expression* expr,
+    const Scanner::Location& params_loc,
+    Scanner::Location* duplicate_loc, bool* ok) {
+  ParseArrowFunctionFormalParameters(parameters, expr, params_loc,
+                                     duplicate_loc, ok);
+  if (!*ok) return;
+
+  for (int i = 0; i < parameters->arity; ++i) {

[adamk, 2015/08/03 18:43:51]

Seems scary to use arity for this instead of the params. As in the other change,
this separation of arity handling from the actual list of parameters feels
brittle, and I'd feel better if there were something other than a DCHECK keeping
the arity and params.size() in lock-step.

+    auto parameter = parameters->at(i);
+    ExpressionClassifier classifier;
+    DeclareFormalParameter(
+        parameters->scope, parameter, parameters->is_simple, &classifier);
+    if (!duplicate_loc->IsValid()) {
+      *duplicate_loc = classifier.duplicate_formal_parameter_error().location;
+    }
   }
 }
 
 
 void ParserTraits::ReindexLiterals(const ParserFormalParameters& parameters) {
   if (parser_->function_state_->materialized_literal_count() > 0) {
     AstLiteralReindexer reindexer;
 
     for (const auto p : parameters.params) {
       if (p.pattern != nullptr) reindexer.Reindex(p.pattern);
@@ skipping 391 matching lines @@
 
 
 Block* Parser::BuildParameterInitializationBlock(
     const ParserFormalParameters& parameters, bool* ok) {
   DCHECK(!parameters.is_simple);
   DCHECK(scope_->is_function_scope());
   Block* init_block =
       factory()->NewBlock(NULL, 1, true, RelocInfo::kNoPosition);
   for (int i = 0; i < parameters.params.length(); ++i) {
     auto parameter = parameters.params[i];
-    if (parameter.pattern == nullptr) continue;
+    // TODO(caitp,rossberg): Remove special handling for rest once desugared.
+    if (parameter.is_rest) break;
     DeclarationDescriptor descriptor;
     descriptor.declaration_kind = DeclarationDescriptor::PARAMETER;
     descriptor.parser = this;
     descriptor.declaration_scope = scope_;
     descriptor.scope = scope_;
     descriptor.mode = LET;
     descriptor.is_const = false;
     descriptor.needs_init = true;
     descriptor.declaration_pos = parameter.pattern->position();
     descriptor.initialization_pos = parameter.pattern->position();
@@ skipping 1655 matching lines @@
 Expression* Parser::SpreadCallNew(Expression* function,
                                   ZoneList<v8::internal::Expression*>* args,
                                   int pos) {
   args->InsertAt(0, function, zone());
 
   return factory()->NewCallRuntime(
       ast_value_factory()->reflect_construct_string(), NULL, args, pos);
 }
 }  // namespace internal
 }  // namespace v8