Strict mode parameter validation.

src/parser.cc

 // Copyright 2010 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 3278 matching lines @@
         NewScope(top_scope_, Scope::FUNCTION_SCOPE, inside_with());
     LexicalScope lexical_scope(&this->top_scope_, &this->with_nesting_level_,
                                scope);
     TemporaryScope temp_scope(&this->temp_scope_);
     top_scope_->SetScopeName(name);
 
     //  FormalParameterList ::
     //    '(' (Identifier)*[','] ')'
     Expect(Token::LPAREN, CHECK_OK);
     int start_pos = scanner().location().beg_pos;
+    Scanner::Location name_loc(RelocInfo::kNoPosition, RelocInfo::kNoPosition);

[Kevin Millikin (Chromium), 2011/01/21 11:41:56]

This might be more readable with a factory function for "no location" in the
scanner class so you can write:

Scanner::Location name_loc = Scanner::NoLocation();

[Martin Maly, 2011/01/21 23:08:39]

Done.

+    Scanner::Location dupe_loc(RelocInfo::kNoPosition, RelocInfo::kNoPosition);
 
     bool done = (peek() == Token::RPAREN);
     while (!done) {
       Handle<String> param_name = ParseIdentifier(CHECK_OK);
+
+      // Store locations for possible future error reports.
+      if (name_loc.beg_pos == RelocInfo::kNoPosition &&

[Kevin Millikin (Chromium), 2011/01/21 11:41:56]

Likewise, struct Scanner::Location could have a predicate for whether the
location is set so you could write:

if (name_loc.HasLocation() ...

If it seems to strange to ask a location if it has a location, we could try
something like Scanner::InvalidLocation() and Location::IsValid().

[Martin Maly, 2011/01/21 23:08:39]

Done. The Location::IsValid felt best so I went with that one.
The only slight glitch is that the scanner.h doesn't see assembler.h where
kNoPosition is located so the factory simply uses -1 literal.

+          IsEvalOrArguments(param_name)) {
+        // Store location for later

[Kevin Millikin (Chromium), 2011/01/21 11:41:56]

This comment is probably redundant and so's the one in the body of the next if.

[Martin Maly, 2011/01/21 23:08:39]

Done.

+        name_loc = scanner().location();
+      }
+      if (dupe_loc.beg_pos == RelocInfo::kNoPosition &&
+        top_scope_->IsDeclared(param_name)) {

[Kevin Millikin (Chromium), 2011/01/21 11:41:56]

Indentation is off here.

[Martin Maly, 2011/01/21 23:08:39]

Done.

+        // Store location for later
+        dupe_loc = scanner().location();
+      }
+
       Variable* parameter = top_scope_->DeclareLocal(param_name, Variable::VAR);
       top_scope_->AddParameter(parameter);
       num_parameters++;
       done = (peek() == Token::RPAREN);
       if (!done) Expect(Token::COMMA, CHECK_OK);
     }
     Expect(Token::RPAREN, CHECK_OK);
 
     Expect(Token::LBRACE, CHECK_OK);
     ZoneList<Statement*>* body = new ZoneList<Statement*>(8);
@@ skipping 57 matching lines @@
       this_property_assignments = temp_scope.this_property_assignments();
 
       Expect(Token::RBRACE, CHECK_OK);
       end_pos = scanner().location().end_pos;
     }
 
     // Validate strict mode.
     if (temp_scope_->StrictMode()) {
       if (IsEvalOrArguments(name)) {
         int position = function_token_position != RelocInfo::kNoPosition
                          ? function_token_position

[Kevin Millikin (Chromium), 2011/01/21 11:41:56]

We usually indent these continuations 4 spaces from the line above so save them
running away to the right:

int position = function_token_position != RelocInfo::kNoPosition
    ? function_token_position ....

[Martin Maly, 2011/01/21 23:08:39]

Done.

                          : (start_pos > 0 ? start_pos - 1 : start_pos);
         ReportMessageAt(Scanner::Location(position, start_pos),
                         "strict_function_name", Vector<const char*>::empty());
         *ok = false;
         return NULL;
       }
+      if (name_loc.beg_pos != RelocInfo::kNoPosition) {
+        ReportMessageAt(name_loc, "strict_param_name",
+                        Vector<const char*>::empty());
+        *ok = false;
+        return NULL;
+      }
+      if (dupe_loc.beg_pos != RelocInfo::kNoPosition) {
+        ReportMessageAt(dupe_loc, "strict_param_dupe",
+                        Vector<const char*>::empty());
+        *ok = false;
+        return NULL;
+      }
       // TODO(mmaly): Check for octal escape sequence here.
     }
 
     FunctionLiteral* function_literal =
         new FunctionLiteral(name,
                             top_scope_,
                             body,
                             materialized_literal_count,
                             expected_property_count,
                             only_simple_this_property_assignments,
@@ skipping 1421 matching lines @@
       Handle<String> source = Handle<String>(String::cast(script->source()));
       result = parser.ParseProgram(source, info->is_global());
     }
   }
 
   info->SetFunction(result);
   return (result != NULL);
 }
 
 } }  // namespace v8::internal