[es6] Implement proper TDZ for parameters

src/parser.cc

Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index 73cf136cd2c549b46b28495c78ea18b2e8ecf897..6488f9f38f545f82bebf1445f1e946fd4f5fe927 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -1203,6 +1203,9 @@ FunctionLiteral* Parser::ParseLazy(Isolate* isolate, ParseInfo* info,
           // 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);
         }
       }
 
@@ -2216,10 +2219,7 @@ Statement* Parser::ParseFunctionDeclaration(
       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);
@@ -3902,10 +3902,26 @@ void ParserTraits::ParseArrowFunctionFormalParameters(
   }
 
   ++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;
+    }
   }
 }
 
@@ -4317,7 +4333,8 @@ Block* Parser::BuildParameterInitializationBlock(
       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;