[parser] early error when declaration Pattern missing Initializer

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/parser.h"
 
 #include "src/api.h"
 #include "src/ast.h"
 #include "src/ast-literal-reindexer.h"
 #include "src/bailout-reason.h"
@@ skipping 2513 matching lines @@
   bool first_declaration = true;
   int bindings_start = peek_position();
   bool is_for_iteration_variable;
   do {
     if (fni_ != NULL) fni_->Enter();
 
     // Parse name.
     if (!first_declaration) Consume(Token::COMMA);
 
     Expression* pattern;
+    bool is_pattern = false;

[rossberg, 2015/11/04 10:52:35]

Nit: declare this after the block and initialise directly.

+    int decl_pos = peek_position();
     {
       ExpressionClassifier pattern_classifier;
       Token::Value next = peek();
       pattern = ParsePrimaryExpression(&pattern_classifier, ok);
       if (!*ok) return;
       ValidateBindingPattern(&pattern_classifier, ok);
       if (!*ok) return;
       if (IsLexicalVariableMode(parsing_result->descriptor.mode)) {
         ValidateLetPattern(&pattern_classifier, ok);
         if (!*ok) return;
       }
       if (!allow_harmony_destructuring() && !pattern->IsVariableProxy()) {
         ReportUnexpectedToken(next);
         *ok = false;
         return;
       }
+      is_pattern = pattern->IsObjectLiteral() || pattern->IsArrayLiteral();
     }
 
     Scanner::Location variable_loc = scanner()->location();
     const AstRawString* single_name =
         pattern->IsVariableProxy() ? pattern->AsVariableProxy()->raw_name()
                                    : nullptr;
     if (single_name != nullptr) {
       if (fni_ != NULL) fni_->PushVariableName(single_name);
     }
 
     is_for_iteration_variable =
         var_context == kForStatement &&
         (peek() == Token::IN || PeekContextualKeyword(CStrVector("of")));
     if (is_for_iteration_variable &&
         (parsing_result->descriptor.mode == CONST ||
          parsing_result->descriptor.mode == CONST_LEGACY)) {
       parsing_result->descriptor.needs_init = false;
     }
 
     Expression* value = NULL;
     // Harmony consts have non-optional initializers.
     int initializer_position = RelocInfo::kNoPosition;
-    if (peek() == Token::ASSIGN || (parsing_result->descriptor.mode == CONST &&
-                                    !is_for_iteration_variable)) {
-      Expect(Token::ASSIGN, ok);
-      if (!*ok) return;
+    if (peek() == Token::ASSIGN ||
+        ((parsing_result->descriptor.mode == CONST || is_pattern) &&

[rossberg, 2015/11/04 10:52:35]

Why not test these in the else branch and move the error generation there?

[caitp (gmail), 2015/11/04 15:07:28]

I think I've done what you're asking for here --- it looks a bit cleaner and
passes tests, at any rate

+         !is_for_iteration_variable)) {
+      if (peek() != Token::ASSIGN) {

[adamk, 2015/11/04 00:37:54]

You can also consume here to avoid mentioning ASSIGN once more below:

if (!Check(Token::ASSIGN)) {
  ...
}

[caitp (gmail), 2015/11/04 00:50:02]

Yeah, it messes up the positioning a bit if i do that. I doubt anyone other than
d8/node users would notice, but still

[adamk, 2015/11/04 00:55:10]

Ah, right, feel free to leave as-is.

+        ParserTraits::ReportMessageAt(
+            Scanner::Location(decl_pos, scanner()->location().end_pos),

[adamk, 2015/11/04 00:37:54]

I'm not sure this location makes any more sense than just pointing at where the
missing ASSIGN token should have been (as the old code did). Seems weird to
point at the syntactically-valid pattern or variable name when it's what doesn't
come after it that's the problem.

[caitp (gmail), 2015/11/04 00:50:02]

If the devtools actually showed the positions, I'd personally get more context
from seeing the object that needs the initializer. But since devtools don't do
this, it doesn't matter a whole lot. I'll change it to be simpler i guess

[adamk, 2015/11/04 00:55:10]

I too have been wondering why devtools doesn't show the positions.

+            MessageTemplate::kDeclarationMissingInitializer,
+            is_pattern ? "destructuring" : "const");
+        *ok = false;
+        return;
+      }
+      Consume(Token::ASSIGN);
       ExpressionClassifier classifier;
       value = ParseAssignmentExpression(var_context != kForStatement,
                                         &classifier, ok);
       if (!*ok) return;
       ValidateExpression(&classifier, ok);
       if (!*ok) return;
       variable_loc.end_pos = scanner()->location().end_pos;
 
       if (!parsing_result->first_initializer_loc.IsValid()) {
         parsing_result->first_initializer_loc = variable_loc;
@@ skipping 3795 matching lines @@
 
 Expression* Parser::SpreadCallNew(Expression* function,
                                   ZoneList<v8::internal::Expression*>* args,
                                   int pos) {
   args->InsertAt(0, function, zone());
 
   return factory()->NewCallRuntime(Context::REFLECT_CONSTRUCT_INDEX, args, pos);
 }
 }  // namespace internal
 }  // namespace v8