Issue 117 - strict mode and future reserved words

src/parser.cc

Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index c0976988ea3c077800842816c03981ee5cf7b248..720f1e7d7d7840efc3d33b025f3cb10b43d23bff 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -750,7 +750,8 @@ FunctionLiteral* Parser::ParseLazy(Handle<SharedFunctionInfo> info,
     FunctionLiteralType type =
         info->is_expression() ? EXPRESSION : DECLARATION;
     bool ok = true;
-    result = ParseFunctionLiteral(name, RelocInfo::kNoPosition, type, &ok);
+    result = ParseFunctionLiteral(name, false, // strict mode identifier already checked???

[Lasse Reichstein, 2011/02/03 11:42:48]

The SharedFunctionInfo should (eventually) contain the strict-mode bit of the
surrounding context.
It might even know whether the function contains the "use strict" declarative if
it was provided by the preparser/parser that first scanned the function source.

So, definitely inherit strictness from the context, and probably have the body
already checked.

[Peter Hallam, 2011/02/04 18:32:41]

If I understand correctly, we can only get here after the function to be lazy
parsed has successfully passed a regular full parse. In that case there can be
no strict mode errors left in the function when we get here. Passing false
disables the future reserved word checking for the function name, which should
be ok because it has been checked in the earlier full parse.

On 2011/02/03 11:42:48, Lasse Reichstein wrote:

[Lasse Reichstein, 2011/02/07 08:05:36]

True. Maybe at some point add an ASSERT that the name isn't reserved, and give
the false value a name (e.g., is_reserved_name).

+                                  RelocInfo::kNoPosition, type, &ok);
     // Make sure the results agree.
     ASSERT(ok == (result != NULL));
     // The only errors should be stack overflows.
@@ -1439,8 +1440,10 @@ Statement* Parser::ParseFunctionDeclaration(bool* ok) {
   //   'function' Identifier '(' FormalParameterListopt ')' '{' FunctionBody '}'
   Expect(Token::FUNCTION, CHECK_OK);
   int function_token_position = scanner().location().beg_pos;
-  Handle<String> name = ParseIdentifier(CHECK_OK);
+  bool is_reserved = false;
+  Handle<String> name = ParseIdentifierOrReservedWord(&is_reserved, CHECK_OK);
   FunctionLiteral* fun = ParseFunctionLiteral(name,
+                                              is_reserved,
                                               function_token_position,
                                               DECLARATION,
                                               CHECK_OK);
@@ -1699,7 +1702,7 @@ Statement* Parser::ParseExpressionOrLabelledStatement(ZoneStringList* labels,
   // ExpressionStatement | LabelledStatement ::
   //   Expression ';'
   //   Identifier ':' Statement
-  bool starts_with_idenfifier = (peek() == Token::IDENTIFIER);
+  bool starts_with_idenfifier = peek_any_identifier();
   Expression* expr = ParseExpression(true, CHECK_OK);
   if (peek() == Token::COLON && starts_with_idenfifier && expr &&
       expr->AsVariableProxy() != NULL &&
@@ -2688,9 +2691,12 @@ Expression* Parser::ParseMemberWithNewPrefixesExpression(PositionStack* stack,
     Expect(Token::FUNCTION, CHECK_OK);
     int function_token_position = scanner().location().beg_pos;
     Handle<String> name;
-    if (peek() == Token::IDENTIFIER) name = ParseIdentifier(CHECK_OK);
-    result = ParseFunctionLiteral(name, function_token_position,
-                                  NESTED, CHECK_OK);
+    bool is_reserved_name = false;
+    if (peek_any_identifier()) {
+        name = ParseIdentifierOrReservedWord(&is_reserved_name, CHECK_OK);
+    }
+    result = ParseFunctionLiteral(name, is_reserved_name,
+                                  function_token_position, NESTED, CHECK_OK);
   } else {
     result = ParsePrimaryExpression(CHECK_OK);
   }
@@ -2759,6 +2765,11 @@ void Parser::ReportUnexpectedToken(Token::Value token) {
     case Token::IDENTIFIER:
       return ReportMessage("unexpected_token_identifier",
                            Vector<const char*>::empty());
+    case Token::IDENTIFIER_OR_FUTURE_RESERVED_WORD:
+      return ReportMessage(temp_scope_->StrictMode() ?
+                               "unexpected_strict_reserved" :
+                               "unexpected_token_identifier",
+                           Vector<const char*>::empty());
     default:
       const char* name = Token::String(token);
       ASSERT(name != NULL);
@@ -2814,7 +2825,8 @@ Expression* Parser::ParsePrimaryExpression(bool* ok) {
       result = new Literal(Factory::false_value());
       break;
 
-    case Token::IDENTIFIER: {
+    case Token::IDENTIFIER:
+    case Token::IDENTIFIER_OR_FUTURE_RESERVED_WORD: {

[Lasse Reichstein, 2011/02/03 11:42:48]

This seems odd, since both tokens seem to be able to represent an identifier. If
IDENTIFIER_OR_FUTURE_RESERVED_WORD only represent future reserved words, it
should be named just FUTURE_RESERVED_WORD. The parser can then choose to handle
a future reserved word as an identifier.

On a side note, they seem to have removed "native" as a reserved word. Which is
annoying, since we are actually using it in our natives files to declare
extension backed functions.

[Peter Hallam, 2011/02/04 18:32:41]

Done.
Done - added issue 1097.

       Handle<String> name = ParseIdentifier(CHECK_OK);
       if (fni_ != NULL) fni_->PushVariableName(name);
       result = top_scope_->NewUnresolved(name, inside_with());
@@ -3222,6 +3234,7 @@ ObjectLiteral::Property* Parser::ParseObjectLiteralGetSet(bool is_getter,
   Token::Value next = Next();
   bool is_keyword = Token::IsKeyword(next);
   if (next == Token::IDENTIFIER || next == Token::NUMBER ||
+      next == Token::IDENTIFIER_OR_FUTURE_RESERVED_WORD ||
       next == Token::STRING || is_keyword) {
     Handle<String> name;
     if (is_keyword) {
@@ -3231,6 +3244,7 @@ ObjectLiteral::Property* Parser::ParseObjectLiteralGetSet(bool is_getter,
     }
     FunctionLiteral* value =
         ParseFunctionLiteral(name,
+                             false, // reserved words are allowed here
                              RelocInfo::kNoPosition,
                              DECLARATION,
                              CHECK_OK);
@@ -3273,6 +3287,7 @@ Expression* Parser::ParseObjectLiteral(bool* ok) {
     Scanner::Location loc = scanner().peek_location();
 
     switch (next) {
+      case Token::IDENTIFIER_OR_FUTURE_RESERVED_WORD:
       case Token::IDENTIFIER: {
         bool is_getter = false;
         bool is_setter = false;
@@ -3421,6 +3436,7 @@ ZoneList<Expression*>* Parser::ParseArguments(bool* ok) {
 
 
 FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name,
+                                              bool name_is_reserved,
                                               int function_token_position,
                                               FunctionLiteralType type,
                                               bool* ok) {
@@ -3454,10 +3470,13 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name,
     int start_pos = scanner().location().beg_pos;
     Scanner::Location name_loc = Scanner::NoLocation();
     Scanner::Location dupe_loc = Scanner::NoLocation();
+    Scanner::Location reserved_loc = Scanner::NoLocation();
 
     bool done = (peek() == Token::RPAREN);
     while (!done) {
-      Handle<String> param_name = ParseIdentifier(CHECK_OK);
+      bool is_reserved = false;
+      Handle<String> param_name = 
+          ParseIdentifierOrReservedWord(&is_reserved, CHECK_OK);
 
       // Store locations for possible future error reports.
       if (!name_loc.IsValid() && IsEvalOrArguments(param_name)) {
@@ -3466,6 +3485,9 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name,
       if (!dupe_loc.IsValid() && top_scope_->IsDeclared(param_name)) {
         dupe_loc = scanner().location();
       }
+      if (!reserved_loc.IsValid() && is_reserved) {
+        reserved_loc = scanner().location();
+      }
 
       Variable* parameter = top_scope_->DeclareLocal(param_name, Variable::VAR);
       top_scope_->AddParameter(parameter);
@@ -3546,7 +3568,8 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name,
         int position = function_token_position != RelocInfo::kNoPosition
             ? function_token_position
             : (start_pos > 0 ? start_pos - 1 : start_pos);
-        ReportMessageAt(Scanner::Location(position, start_pos),
+        Scanner::Location location = Scanner::Location(position, start_pos);
+        ReportMessageAt(location,
                         "strict_function_name", Vector<const char*>::empty());
         *ok = false;
         return NULL;
@@ -3563,6 +3586,22 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name,
         *ok = false;
         return NULL;
       }
+      if (name_is_reserved) {
+        int position = function_token_position != RelocInfo::kNoPosition
+            ? function_token_position
+            : (start_pos > 0 ? start_pos - 1 : start_pos);
+        Scanner::Location location = Scanner::Location(position, start_pos);
+        ReportMessageAt(location, "strict_reserved_word",
+                        Vector<const char*>::empty());
+        *ok = false;
+        return NULL;
+      }
+      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);
     }
 
@@ -3634,6 +3673,13 @@ Expression* Parser::ParseV8Intrinsic(bool* ok) {
 }
 
 
+bool Parser::peek_any_identifier() {
+  Token::Value next = peek();
+  return next == Token::IDENTIFIER ||
+         next == Token::IDENTIFIER_OR_FUTURE_RESERVED_WORD;
+}
+
+
 void Parser::Consume(Token::Value token) {
   Token::Value next = Next();
   USE(next);
@@ -3693,7 +3739,22 @@ Literal* Parser::GetLiteralNumber(double value) {
 
 
 Handle<String> Parser::ParseIdentifier(bool* ok) {
-  Expect(Token::IDENTIFIER, ok);
+  bool is_reserved;
+  return ParseIdentifierOrReservedWord(&is_reserved, ok);
+}
+
+
+Handle<String> Parser::ParseIdentifierOrReservedWord(bool* is_reserved, 
+                                                     bool* ok) {
+  *is_reserved = false;
+  if (temp_scope_->StrictMode()) {
+    Expect(Token::IDENTIFIER, ok);
+  } else {
+    if (!Check(Token::IDENTIFIER)) {
+      Expect(Token::IDENTIFIER_OR_FUTURE_RESERVED_WORD, ok);
+      *is_reserved = true;
+    }
+  }
   if (!*ok) return Handle<String>();
   return GetSymbol(ok);
 }
@@ -3701,7 +3762,9 @@ Handle<String> Parser::ParseIdentifier(bool* ok) {
 
 Handle<String> Parser::ParseIdentifierName(bool* ok) {
   Token::Value next = Next();
-  if (next != Token::IDENTIFIER && !Token::IsKeyword(next)) {
+  if (next != Token::IDENTIFIER &&
+          next != Token::IDENTIFIER_OR_FUTURE_RESERVED_WORD &&
+          !Token::IsKeyword(next)) {

[Lasse Reichstein, 2011/02/03 11:42:48]

Maybe future reserved words should be considered keywords, so you only need to
check IDENTIFIER or IsKeyword.
We only use IsKeyword to check for something that is a sequence of letters, but
which isn't scanned as an identifier. Future reserved words would match that
categorization. 
(But we need to check that we don't assume that IsKeyword(t) => Token::String(t)
works).

     ReportUnexpectedToken(next);
     *ok = false;
     return Handle<String>();
@@ -3741,20 +3804,20 @@ void Parser::CheckOctalLiteral(int beg_pos, int end_pos, bool* ok) {
 
 
 // This function reads an identifier and determines whether or not it
-// is 'get' or 'set'.  The reason for not using ParseIdentifier and
-// checking on the output is that this involves heap allocation which
+// is 'get' or 'set'.  The reason for not checking the result of
+// ParseIdentifier is that this involves heap allocation which
 // we can't do during preparsing.

[Lasse Reichstein, 2011/02/03 11:42:48]

This code isn't used for preparsing any more, so the comment is out of date.
(The identical comment in perparser.cc is also out of date, for the opposite
reason).

[Peter Hallam, 2011/02/04 18:32:41]

Done.

 Handle<String> Parser::ParseIdentifierOrGetOrSet(bool* is_get,
                                                  bool* is_set,
                                                  bool* ok) {
-  Expect(Token::IDENTIFIER, ok);
+  Handle<String> result = ParseIdentifier(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;
     *is_set = !*is_get && strncmp(token, "set", 3) == 0;
   }
-  return GetSymbol(ok);
+  return result;
 }
 
 
@@ -3896,6 +3959,7 @@ Handle<Object> JsonParser::ParseJson(Handle<String> script,
           message = "unexpected_token_string";
           break;
         case Token::IDENTIFIER:
+        case Token::IDENTIFIER_OR_FUTURE_RESERVED_WORD:
           message = "unexpected_token_identifier";
           break;
         default: