Detect conflicting variable bindings in harmony mode.

src/parser.cc

Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index cf907ce0a5c88d081be37a3b07bafa2e1d1c99ff..b367c2846c45432c7103d24fa60d266e268936dc 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -647,6 +647,11 @@ FunctionLiteral* Parser::DoParseProgram(Handle<String> source,
     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(),
@@ -1345,12 +1350,24 @@ VariableProxy* Parser::Declare(Handle<String> name,
     } 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 =
@@ -1380,7 +1397,8 @@ VariableProxy* Parser::Declare(Handle<String> name,
   // 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()) {
@@ -2207,7 +2225,9 @@ TryStatement* Parser::ParseTryStatement(bool* ok) {
       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_;
@@ -3736,7 +3756,10 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> function_name,
         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",
@@ -3863,6 +3886,10 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> function_name,
     }
   }
 
+  if (harmony_block_scoping_) {
+    CheckConflictingVarDeclarations(scope, CHECK_OK);
+  }
+
   FunctionLiteral* function_literal =
       new(zone()) FunctionLiteral(isolate(),
                                   function_name,
@@ -4069,6 +4096,21 @@ void Parser::CheckOctalLiteral(int beg_pos, int end_pos, bool* ok) {
 }
 
 
+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,