[strong] Declaration-after-use errors.

src/parser.cc

Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index b1d427e2529f4ec23a4ac702094ba4f874657443..c0836182a34724b0c39647319b53cc1958db4ad1 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -608,12 +608,9 @@ void ParserTraits::ReportMessageAt(Scanner::Location source_location,
     // and we want to report the stack overflow later.
     return;
   }
-  parser_->has_pending_error_ = true;
-  parser_->pending_error_location_ = source_location;
-  parser_->pending_error_message_ = message;
-  parser_->pending_error_char_arg_ = arg;
-  parser_->pending_error_arg_ = NULL;
-  parser_->pending_error_type_ = error_type;
+  parser_->pending_error_handler_.ReportMessageAt(source_location.beg_pos,
+                                                  source_location.end_pos,
+                                                  message, arg, error_type);
 }
 
 
@@ -640,12 +637,9 @@ void ParserTraits::ReportMessageAt(Scanner::Location source_location,
     // and we want to report the stack overflow later.
     return;
   }
-  parser_->has_pending_error_ = true;
-  parser_->pending_error_location_ = source_location;
-  parser_->pending_error_message_ = message;
-  parser_->pending_error_char_arg_ = NULL;
-  parser_->pending_error_arg_ = arg;
-  parser_->pending_error_type_ = error_type;
+  parser_->pending_error_handler_.ReportMessageAt(source_location.beg_pos,
+                                                  source_location.end_pos,
+                                                  message, arg, error_type);
 }
 
 
@@ -712,7 +706,9 @@ Literal* ParserTraits::ExpressionFromLiteral(Token::Value token, int pos,
 
 
 Expression* ParserTraits::ExpressionFromIdentifier(const AstRawString* name,
-                                                   int pos, Scope* scope,
+                                                   int start_position,
+                                                   int end_position,
+                                                   Scope* scope,
                                                    AstNodeFactory* factory) {
   if (parser_->fni_ != NULL) parser_->fni_->PushVariableName(name);
 
@@ -721,8 +717,10 @@ Expression* ParserTraits::ExpressionFromIdentifier(const AstRawString* name,
   // for Traits::DeclareArrowParametersFromExpression() to be able to
   // pick the names of the parameters.
   return parser_->parsing_lazy_arrow_parameters_
-             ? factory->NewVariableProxy(name, false, pos)
-             : scope->NewUnresolved(factory, name, pos);
+             ? factory->NewVariableProxy(name, false, start_position,
+                                         end_position)
+             : scope->NewUnresolved(factory, name, start_position,
+                                    end_position);
 }
 
 
@@ -789,11 +787,6 @@ Parser::Parser(CompilationInfo* info, uintptr_t stack_limit, uint32_t hash_seed,
       compile_options_(info->compile_options()),
       cached_parse_data_(NULL),
       parsing_lazy_arrow_parameters_(false),
-      has_pending_error_(false),
-      pending_error_message_(NULL),
-      pending_error_arg_(NULL),
-      pending_error_char_arg_(NULL),
-      pending_error_type_(kSyntaxError),
       total_preparse_skipped_(0),
       pre_parse_timer_(NULL),
       parsing_on_main_thread_(true) {
@@ -1792,7 +1785,9 @@ VariableProxy* Parser::NewUnresolved(const AstRawString* name,
   // scope.
   // Let/const variables in harmony mode are always added to the immediately
   // enclosing scope.
-  return DeclarationScope(mode)->NewUnresolved(factory(), name, position());
+  return DeclarationScope(mode)->NewUnresolved(factory(), name,
+                                               scanner()->location().beg_pos,
+                                               scanner()->location().end_pos);
 }
 
 
@@ -1822,10 +1817,12 @@ void Parser::Declare(Declaration* declaration, bool resolve, bool* ok) {
     if (var == NULL) {
       // Declare the name.
       var = declaration_scope->DeclareLocal(
-          name, mode, declaration->initialization(), kNotAssigned);
-    } else if (IsLexicalVariableMode(mode) || IsLexicalVariableMode(var->mode())
-               || ((mode == CONST_LEGACY || var->mode() == CONST_LEGACY) &&
-                   !declaration_scope->is_script_scope())) {
+          name, mode, declaration->initialization(), declaration->position(),

[rossberg, 2015/02/24 15:52:40]

Hm, I'm confused, why is this the right init position?

[marja, 2015/02/24 18:05:40]

Argh, this was only working because the position is fixed later w/
set_initializer_position. The only case where this position actually got used
(was not overwritten) was for "var", and for those, there were (are?) no test
cases where the difference would matter. (Like, var x = x + 1; is ok and we
don't allow those in strong mode anyway.)

So I removed transmitting the declaration position altogether and made var
variables also have their initializer positions set. This is needed so that we
can make this an error: function foo() {"use strong"; x; } var x = 0;

+          declaration->IsFunctionDeclaration(), kNotAssigned);
+    } else if (IsLexicalVariableMode(mode) ||
+               IsLexicalVariableMode(var->mode()) ||
+               ((mode == CONST_LEGACY || var->mode() == CONST_LEGACY) &&
+                !declaration_scope->is_script_scope())) {
       // The name was declared in this scope before; check for conflicting
       // re-declarations. We have a conflict if either of the declarations is
       // not a var (in script scope, we also have to ignore legacy const for
@@ -2302,11 +2299,15 @@ Block* Parser::ParseVariableDeclarations(
         fni_->RemoveLastFunction();
       }
       if (decl_props != NULL) *decl_props = kHasInitializers;
-    }
-
-    // Record the end position of the initializer.
-    if (proxy->is_resolved()) {
-      proxy->var()->set_initializer_position(position());
+      // Record the end position of the initializer.
+      if (proxy->is_resolved()) {
+        proxy->var()->set_initializer_position(scanner()->location().end_pos);
+      }
+    } else {
+      // Record the end position of the initializer.
+      if (proxy->is_resolved()) {
+        proxy->var()->set_initializer_position(position());
+      }
     }
 
     // Make sure that 'const x' and 'let x' initialize 'x' to undefined.
@@ -2811,7 +2812,8 @@ TryStatement* Parser::ParseTryStatement(bool* ok) {
 
     Expect(Token::RPAREN, CHECK_OK);
 
-    catch_variable = catch_scope->DeclareLocal(name, VAR, kCreatedInitialized);
+    catch_variable = catch_scope->DeclareLocal(
+        name, VAR, kCreatedInitialized, scanner()->location().beg_pos, false);
     BlockState block_state(&scope_, catch_scope);
     catch_block = ParseBlock(NULL, CHECK_OK);
 
@@ -3103,16 +3105,16 @@ Statement* Parser::DesugarLetBindingsInForStatement(
   //    make statement: let x = temp_x.
   for (int i = 0; i < names->length(); i++) {
     VariableProxy* proxy = NewUnresolved(names->at(i), LET);
-    Declaration* declaration =
-        factory()->NewVariableDeclaration(proxy, LET, scope_, pos);
+    Declaration* declaration = factory()->NewVariableDeclaration(
+        proxy, LET, scope_, RelocInfo::kNoPosition);
     Declare(declaration, true, CHECK_OK);
     inner_vars.Add(declaration->proxy()->var(), zone());
     VariableProxy* temp_proxy = factory()->NewVariableProxy(temps.at(i));
     Assignment* assignment = factory()->NewAssignment(
         Token::INIT_LET, proxy, temp_proxy, pos);
-    Statement* assignment_statement = factory()->NewExpressionStatement(
-        assignment, pos);
-    proxy->var()->set_initializer_position(pos);
+    Statement* assignment_statement =
+        factory()->NewExpressionStatement(assignment, RelocInfo::kNoPosition);
+    proxy->var()->set_initializer_position(init->position());
     inner_block->AddStatement(assignment_statement, zone());
   }
 
@@ -3260,7 +3262,8 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
                                     CHECK_OK);
       bool accept_OF = decl_props == kHasNoInitializers;
       ForEachStatement::VisitMode mode;
-      int each_pos = position();
+      int each_beg_pos = scanner()->location().beg_pos;
+      int each_end_pos = scanner()->location().end_pos;
 
       if (name != NULL && CheckInOrOf(accept_OF, &mode, ok)) {
         if (!*ok) return nullptr;
@@ -3271,7 +3274,8 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
         Expression* enumerable = ParseExpression(true, CHECK_OK);
         Expect(Token::RPAREN, CHECK_OK);
 
-        VariableProxy* each = scope_->NewUnresolved(factory(), name, each_pos);
+        VariableProxy* each =
+            scope_->NewUnresolved(factory(), name, each_beg_pos, each_end_pos);
         Statement* body = ParseSubStatement(NULL, CHECK_OK);
         InitializeForEachStatement(loop, each, enumerable, body);
         Block* result =
@@ -3298,7 +3302,8 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
       bool accept_IN = name != NULL && decl_props != kHasInitializers;
       bool accept_OF = decl_props == kHasNoInitializers;
       ForEachStatement::VisitMode mode;
-      int each_pos = position();
+      int each_beg_pos = scanner()->location().beg_pos;
+      int each_end_pos = scanner()->location().end_pos;
 
       if (accept_IN && CheckInOrOf(accept_OF, &mode, ok)) {
         if (!*ok) return nullptr;
@@ -3320,7 +3325,8 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
         // implementing stack allocated block scoped variables.
         Variable* temp = scope_->DeclarationScope()->NewTemporary(
             ast_value_factory()->dot_for_string());
-        VariableProxy* temp_proxy = factory()->NewVariableProxy(temp, each_pos);
+        VariableProxy* temp_proxy =
+            factory()->NewVariableProxy(temp, each_beg_pos, each_end_pos);
         ForEachStatement* loop =
             factory()->NewForEachStatement(mode, labels, stmt_pos);
         Target target(&this->target_stack_, loop);
@@ -3331,7 +3337,8 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
         scope_ = for_scope;
         Expect(Token::RPAREN, CHECK_OK);
 
-        VariableProxy* each = scope_->NewUnresolved(factory(), name, each_pos);
+        VariableProxy* each =
+            scope_->NewUnresolved(factory(), name, each_beg_pos, each_end_pos);
         Statement* body = ParseSubStatement(NULL, CHECK_OK);
         Block* body_block =
             factory()->NewBlock(NULL, 3, false, RelocInfo::kNoPosition);
@@ -4115,6 +4122,8 @@ ClassLiteral* Parser::ParseClassLiteral(const AstRawString* name,
   bool has_seen_constructor = false;
 
   Expect(Token::LBRACE, CHECK_OK);
+  int body_beg_pos = scanner()->location().beg_pos;
+
   const bool has_extends = extends != nullptr;
   while (peek() != Token::RBRACE) {
     if (Check(Token::SEMICOLON)) continue;
@@ -4154,7 +4163,7 @@ ClassLiteral* Parser::ParseClassLiteral(const AstRawString* name,
   if (name != NULL) {
     DCHECK_NOT_NULL(proxy);
     DCHECK_NOT_NULL(block_scope);
-    proxy->var()->set_initializer_position(end_pos);
+    proxy->var()->set_initializer_position(body_beg_pos);
   }
 
   return factory()->NewClassLiteral(name, block_scope, proxy, extends,
@@ -4293,57 +4302,6 @@ void Parser::HandleSourceURLComments(Isolate* isolate, Handle<Script> script) {
 }
 
 
-void Parser::ThrowPendingError(Isolate* isolate, Handle<Script> script) {
-  DCHECK(ast_value_factory()->IsInternalized());
-  if (has_pending_error_) {
-    MessageLocation location(script, pending_error_location_.beg_pos,
-                             pending_error_location_.end_pos);
-    Factory* factory = isolate->factory();
-    bool has_arg =
-        pending_error_arg_ != NULL || pending_error_char_arg_ != NULL;
-    Handle<FixedArray> elements = factory->NewFixedArray(has_arg ? 1 : 0);
-    if (pending_error_arg_ != NULL) {
-      Handle<String> arg_string = pending_error_arg_->string();
-      elements->set(0, *arg_string);
-    } else if (pending_error_char_arg_ != NULL) {
-      Handle<String> arg_string =
-          factory->NewStringFromUtf8(CStrVector(pending_error_char_arg_))
-          .ToHandleChecked();
-      elements->set(0, *arg_string);
-    }
-    isolate->debug()->OnCompileError(script);
-
-    Handle<JSArray> array = factory->NewJSArrayWithElements(elements);
-    Handle<Object> error;
-    switch (pending_error_type_) {
-      case kReferenceError:
-        error = factory->NewReferenceError(pending_error_message_, array);
-        break;
-      case kSyntaxError:
-        error = factory->NewSyntaxError(pending_error_message_, array);
-        break;
-    }
-
-    Handle<JSObject> jserror = Handle<JSObject>::cast(error);
-
-    Handle<Name> key_start_pos = factory->error_start_pos_symbol();
-    JSObject::SetProperty(jserror, key_start_pos,
-                          handle(Smi::FromInt(location.start_pos()), isolate),
-                          SLOPPY).Check();
-
-    Handle<Name> key_end_pos = factory->error_end_pos_symbol();
-    JSObject::SetProperty(jserror, key_end_pos,
-                          handle(Smi::FromInt(location.end_pos()), isolate),
-                          SLOPPY).Check();
-
-    Handle<Name> key_script = factory->error_script_symbol();
-    JSObject::SetProperty(jserror, key_script, script, SLOPPY).Check();
-
-    isolate->Throw(*error, &location);
-  }
-}
-
-
 void Parser::Internalize(Isolate* isolate, Handle<Script> script, bool error) {
   // Internalize strings.
   ast_value_factory()->Internalize(isolate);
@@ -4353,7 +4311,8 @@ void Parser::Internalize(Isolate* isolate, Handle<Script> script, bool error) {
     if (stack_overflow()) {
       isolate->StackOverflow();
     } else {
-      ThrowPendingError(isolate, script);
+      DCHECK(pending_error_handler_.has_pending_error());
+      pending_error_handler_.ThrowPendingError(isolate, script);
     }
   }