Detect conflicting variable bindings in harmony mode.

src/parser.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 629 matching lines @@
     if (strict_mode == kStrictMode) {
       top_scope_->EnableStrictMode();
     }
     ZoneList<Statement*>* body = new(zone()) ZoneList<Statement*>(16);
     bool ok = true;
     int beg_loc = scanner().location().beg_pos;
     ParseSourceElements(body, Token::EOS, &ok);
     if (ok && top_scope_->is_strict_mode()) {
       CheckOctalLiteral(beg_loc, scanner().location().end_pos, &ok);
     }
+
+    if (ok && harmony_block_scoping_) {
+      CheckConflictingVarDeclarations(scope, &ok);
+    }
+
     if (ok) {
       result = new(zone()) FunctionLiteral(
           isolate(),
           no_name,
           top_scope_,
           body,
           lexical_scope.materialized_literal_count(),
           lexical_scope.expected_property_count(),
           lexical_scope.only_simple_this_property_assignments(),
           lexical_scope.this_property_assignments(),
@@ skipping 678 matching lines @@
       declaration_scope->is_strict_mode_eval_scope() ||
       declaration_scope->is_block_scope()) {
     // Declare the variable in the function scope.
     var = declaration_scope->LocalLookup(name);
     if (var == NULL) {
       // Declare the name.
       var = declaration_scope->DeclareLocal(name, mode);
     } else {
       // The name was declared before; check for conflicting re-declarations.
       // We have a conflict if either of the declarations is not a var. There
-      // is similar code in runtime.cc in the Declare functions.
+      // is similar code in runtime.cc in the Declare functions. Also the
+      // function CheckNonConflictingScope checks for conflicting hoisting
+      // of var declared variables.

[Lasse Reichstein, 2011/09/01 07:34:04]

This checks only for a redeclaration in the same scope, right?

[Steven, 2011/09/01 15:01:33]

Yes. Clarified it in the comment.
On 2011/09/01 07:34:04, Lasse Reichstein wrote:

       if ((mode != Variable::VAR) || (var->mode() != Variable::VAR)) {
         // We only have vars, consts and lets in declarations.
         ASSERT(var->mode() == Variable::VAR ||
                var->mode() == Variable::CONST ||
                var->mode() == Variable::LET);
+        if (harmony_block_scoping_) {
+          // In harmony mode we treat re-declarations as early errors. See
+          // ES5 16 for a definition of early errors.
+          SmartPointer<char> c_string = name->ToCString(DISALLOW_NULLS);
+          const char* elms[2] = { "Variable", *c_string };
+          Vector<const char*> args(elms, 2);
+          ReportMessage("redeclaration", args);
+          *ok = false;
+          return NULL;
+        }
         const char* type = (var->mode() == Variable::VAR) ? "var" :
                            (var->mode() == Variable::CONST) ? "const" : "let";
         Handle<String> type_string =
             isolate()->factory()->NewStringFromUtf8(CStrVector(type), TENURED);
         Expression* expression =
             NewThrowTypeError(isolate()->factory()->redeclaration_symbol(),
                               type_string, name);
         declaration_scope->SetIllegalRedeclaration(expression);
       }
     }
   }
 
   // We add a declaration node for every declaration. The compiler
   // will only generate code if necessary. In particular, declarations
   // for inner local variables that do not represent functions won't
   // result in any generated code.
   //
   // Note that we always add an unresolved proxy even if it's not
   // used, simply because we don't know in this method (w/o extra
   // parameters) if the proxy is needed or not. The proxy will be
   // bound during variable resolution time unless it was pre-bound
   // below.
   //
   // WARNING: This will lead to multiple declaration nodes for the
   // same variable if it is declared several times. This is not a
   // semantic issue as long as we keep the source order, but it may be
   // a performance issue since it may lead to repeated
   // Runtime::DeclareContextSlot() calls.
   VariableProxy* proxy = declaration_scope->NewUnresolved(name, false);
-  declaration_scope->AddDeclaration(new(zone()) Declaration(proxy, mode, fun));
+  declaration_scope->AddDeclaration(
+      new(zone()) Declaration(proxy, mode, fun, top_scope_));
 
   // For global const variables we bind the proxy to a variable.
   if (mode == Variable::CONST && declaration_scope->is_global_scope()) {
     ASSERT(resolve);  // should be set by all callers
     Variable::Kind kind = Variable::NORMAL;
     var = new(zone()) Variable(declaration_scope,
                                name,
                                Variable::CONST,
                                true,
                                kind);
@@ skipping 806 matching lines @@
     Expect(Token::RPAREN, CHECK_OK);
 
     if (peek() == Token::LBRACE) {
       // Rewrite the catch body { B } to a block:
       // { { B } ExitContext; }.
       Target target(&this->target_stack_, &catch_collector);
       catch_scope = NewScope(top_scope_, Scope::CATCH_SCOPE, inside_with());
       if (top_scope_->is_strict_mode()) {
         catch_scope->EnableStrictMode();
       }
-      catch_variable = catch_scope->DeclareLocal(name, Variable::VAR);
+      Variable::Mode mode = harmony_block_scoping_
+          ? Variable::LET : Variable::VAR;
+      catch_variable = catch_scope->DeclareLocal(name, mode);
       catch_block = new(zone()) Block(isolate(), NULL, 2, false);
 
       Scope* saved_scope = top_scope_;
       top_scope_ = catch_scope;
       Block* catch_body = ParseBlock(NULL, CHECK_OK);
       top_scope_ = saved_scope;
       catch_block->AddStatement(catch_body);
       catch_block->AddStatement(new(zone()) ExitContextStatement());
     } else {
       Expect(Token::LBRACE, CHECK_OK);
@@ skipping 1508 matching lines @@
         name_loc = scanner().location();
       }
       if (!dupe_loc.IsValid() && top_scope_->IsDeclared(param_name)) {
         has_duplicate_parameters = true;
         dupe_loc = scanner().location();
       }
       if (!reserved_loc.IsValid() && is_strict_reserved) {
         reserved_loc = scanner().location();
       }
 
-      top_scope_->DeclareParameter(param_name);
+      top_scope_->DeclareParameter(param_name,
+                                   harmony_block_scoping_
+                                   ? Variable::LET
+                                   : Variable::VAR);
       num_parameters++;
       if (num_parameters > kMaxNumFunctionParameters) {
         ReportMessageAt(scanner().location(), "too_many_parameters",
                         Vector<const char*>::empty());
         *ok = false;
         return NULL;
       }
       done = (peek() == Token::RPAREN);
       if (!done) Expect(Token::COMMA, CHECK_OK);
     }
@@ skipping 106 matching lines @@
       if (reserved_loc.IsValid()) {
         ReportMessageAt(reserved_loc, "strict_reserved_word",
                         Vector<const char*>::empty());
         *ok = false;
         return NULL;
       }
       CheckOctalLiteral(start_pos, end_pos, CHECK_OK);
     }
   }
 
+  if (harmony_block_scoping_) {
+    CheckConflictingVarDeclarations(scope, CHECK_OK);
+  }
+
   FunctionLiteral* function_literal =
       new(zone()) FunctionLiteral(isolate(),
                                   function_name,
                                   scope,
                                   body,
                                   materialized_literal_count,
                                   expected_property_count,
                                   only_simple_this_property_assignments,
                                   this_property_assignments,
                                   num_parameters,
@@ skipping 186 matching lines @@
       beg_pos <= octal.beg_pos &&
       octal.end_pos <= end_pos) {
     ReportMessageAt(octal, "strict_octal_literal",
                     Vector<const char*>::empty());
     scanner().clear_octal_position();
     *ok = false;
   }
 }
 
 
+void Parser::CheckConflictingVarDeclarations(Scope* scope, bool* ok) {
+  Declaration* decl = scope->CheckConflictingVarDeclarations();
+  if (decl != NULL) {
+    // In harmony mode we treat conflicting variable bindinds as early
+    // errors. See ES5 16 for a definition of early errors.
+    Handle<String> name = decl->proxy()->name();
+    SmartPointer<char> c_string = name->ToCString(DISALLOW_NULLS);
+    const char* elms[2] = { "Variable", *c_string };
+    Vector<const char*> args(elms, 2);
+    ReportMessage("redeclaration", args);

[Steven, 2011/08/31 21:33:00]

This ReportMessage should rather be a ReportMessageAt. The proxy does store a
position but what is the correct way to turn it into a Scanner::Location?

+    *ok = false;
+  }
+}
+
+
 // This function reads an identifier name and determines whether or not it
 // is 'get' or 'set'.
 Handle<String> Parser::ParseIdentifierNameOrGetOrSet(bool* is_get,
                                                      bool* is_set,
                                                      bool* ok) {
   Handle<String> result = ParseIdentifierName(ok);
   if (!*ok) return Handle<String>();
   if (scanner().is_literal_ascii() && scanner().literal_length() == 3) {
     const char* token = scanner().literal_ascii_string().start();
     *is_get = strncmp(token, "get", 3) == 0;
@@ skipping 1101 matching lines @@
       result = parser.ParseProgram(source,
                                    info->is_global(),
                                    info->StrictMode());
     }
   }
   info->SetFunction(result);
   return (result != NULL);
 }
 
 } }  // namespace v8::internal