Fix a bug in scope analysis.

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 2824 matching lines @@
   Expression* result = NULL;
   if (peek() == Token::FUNCTION) {
     Expect(Token::FUNCTION, CHECK_OK);
     int function_token_position = scanner().location().beg_pos;
     Handle<String> name;
     bool is_strict_reserved_name = false;
     if (peek_any_identifier()) {
       name = ParseIdentifierOrStrictReservedWord(&is_strict_reserved_name,
                                                  CHECK_OK);
     }
-    result = ParseFunctionLiteral(name, is_strict_reserved_name,
-                                  function_token_position, NESTED, CHECK_OK);
+    result = ParseFunctionLiteral(name,
+                                  is_strict_reserved_name,
+                                  function_token_position,
+                                  EXPRESSION,
+                                  CHECK_OK);
   } else {
     result = ParsePrimaryExpression(CHECK_OK);
   }
 
   while (true) {
     switch (peek()) {
       case Token::LBRACK: {
         Consume(Token::LBRACK);
         int pos = scanner().location().beg_pos;
         Expression* index = ParseExpression(true, CHECK_OK);
@@ skipping 548 matching lines @@
     Handle<String> name;
     if (is_keyword) {
       name = isolate_->factory()->LookupAsciiSymbol(Token::String(next));
     } else {
       name = GetSymbol(CHECK_OK);
     }
     FunctionLiteral* value =
         ParseFunctionLiteral(name,
                              false,   // reserved words are allowed here
                              RelocInfo::kNoPosition,
-                             DECLARATION,
+                             EXPRESSION,
                              CHECK_OK);
     // Allow any number of parameters for compatiabilty with JSC.
     // Specification only allows zero parameters for get and one for set.
     ObjectLiteral::Property* property =
         new(zone()) ObjectLiteral::Property(is_getter, value);
     return property;
   } else {
     ReportUnexpectedToken(next);
     *ok = false;
     return NULL;
@@ skipping 186 matching lines @@
       return NULL;
     }
     done = (peek() == Token::RPAREN);
     if (!done) Expect(Token::COMMA, CHECK_OK);
   }
   Expect(Token::RPAREN, CHECK_OK);
   return result;
 }
 
 
-FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name,
+FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> function_name,
                                               bool name_is_strict_reserved,
                                               int function_token_position,
                                               FunctionLiteralType type,
                                               bool* ok) {
   // Function ::
   //   '(' FormalParameterList? ')' '{' FunctionBody '}'
-  bool is_named = !var_name.is_null();
+  bool has_name = !function_name.is_null();
+  // The function's name, empty if the function is anonymous.
+  if (!has_name) {
+    function_name = isolate()->factory()->empty_symbol();
+  }
 
-  // The name associated with this function. If it's a function expression,
-  // this is the actual function name, otherwise this is the name of the
-  // variable declared and initialized with the function (expression). In
-  // that case, we don't have a function name (it's empty).

[Kevin Millikin (Chromium), 2011/08/04 16:32:09]

I loved this comment, but I couldn't find a way to keep it with the restructured
code, mostly because it makes no sense at all.

-  Handle<String> name =
-      is_named ? var_name : isolate()->factory()->empty_symbol();
-  // The function name, if any.
-  Handle<String> function_name = isolate()->factory()->empty_symbol();
-  if (is_named && (type == EXPRESSION || type == NESTED)) {
-    function_name = name;
+  // The name of a named function expression, otherwise an empty handle.
+  Handle<String> self_name;
+  if (type == EXPRESSION && function_name->length() > 0) {
+    self_name = function_name;
   }
 
   int num_parameters = 0;
   // Function declarations are hoisted.
   Scope* scope = (type == DECLARATION)
       ? NewScope(top_scope_->DeclarationScope(), Scope::FUNCTION_SCOPE, false)
       : NewScope(top_scope_, Scope::FUNCTION_SCOPE, inside_with());
   ZoneList<Statement*>* body = new(zone()) ZoneList<Statement*>(8);
   int materialized_literal_count;
   int expected_property_count;
   int start_pos;
   int end_pos;
   bool only_simple_this_property_assignments;
   Handle<FixedArray> this_property_assignments;
   bool has_duplicate_parameters = false;
   // Parse function body.
   { LexicalScope lexical_scope(this, scope, isolate());
-    top_scope_->SetScopeName(name);
+    top_scope_->SetScopeName(function_name);
 
     //  FormalParameterList ::
     //    '(' (Identifier)*[','] ')'
     Expect(Token::LPAREN, CHECK_OK);
     start_pos = scanner().location().beg_pos;
     Scanner::Location name_loc = Scanner::Location::invalid();
     Scanner::Location dupe_loc = Scanner::Location::invalid();
     Scanner::Location reserved_loc = Scanner::Location::invalid();
 
     bool done = (peek() == Token::RPAREN);
@@ skipping 29 matching lines @@
     Expect(Token::RPAREN, CHECK_OK);
 
     Expect(Token::LBRACE, CHECK_OK);
 
     // If we have a named function expression, we add a local variable
     // declaration to the body of the function with the name of the
     // function and let it refer to the function itself (closure).
     // NOTE: We create a proxy and resolve it here so that in the
     // future we can change the AST to only refer to VariableProxies
     // instead of Variables and Proxis as is the case now.
-    if (!function_name.is_null() && function_name->length() > 0) {
-      Variable* fvar = top_scope_->DeclareFunctionVar(function_name);
+    if (!self_name.is_null()) {
+      Variable* fvar = top_scope_->DeclareFunctionVar(self_name);
       VariableProxy* fproxy =
-          top_scope_->NewUnresolved(function_name, inside_with());
+          top_scope_->NewUnresolved(self_name, inside_with());
       fproxy->BindTo(fvar);
       body->Add(new(zone()) ExpressionStatement(
           new(zone()) Assignment(isolate(),
                                  Token::INIT_CONST,
                                  fproxy,
                                  new(zone()) ThisFunction(isolate()),
                                  RelocInfo::kNoPosition)));
     }
 
     // Determine if the function will be lazily compiled. The mode can only
@@ skipping 10 matching lines @@
       int function_block_pos = scanner().location().beg_pos;
       FunctionEntry entry = pre_data()->GetFunctionEntry(function_block_pos);
       if (!entry.is_valid()) {
         // There is no preparser data for the function, we will not lazily
         // compile after all.
         is_lazily_compiled = false;
       } else {
         end_pos = entry.end_pos();
         if (end_pos <= function_block_pos) {
           // End position greater than end of stream is safe, and hard to check.
-          ReportInvalidPreparseData(name, CHECK_OK);
+          ReportInvalidPreparseData(function_name, CHECK_OK);
         }
         isolate()->counters()->total_preparse_skipped()->Increment(
             end_pos - function_block_pos);
         // Seek to position just before terminal '}'.
         scanner().SeekForward(end_pos - 1);
         materialized_literal_count = entry.literal_count();
         expected_property_count = entry.property_count();
         if (entry.strict_mode()) top_scope_->EnableStrictMode();
         only_simple_this_property_assignments = false;
         this_property_assignments = isolate()->factory()->empty_fixed_array();
         Expect(Token::RBRACE, CHECK_OK);
       }
     }
 
     if (!is_lazily_compiled) {
       ParseSourceElements(body, Token::RBRACE, CHECK_OK);
 
       materialized_literal_count = lexical_scope.materialized_literal_count();
       expected_property_count = lexical_scope.expected_property_count();
       only_simple_this_property_assignments =
           lexical_scope.only_simple_this_property_assignments();
       this_property_assignments = lexical_scope.this_property_assignments();
 
       Expect(Token::RBRACE, CHECK_OK);
       end_pos = scanner().location().end_pos;
     }
 
     // Validate strict mode.
     if (top_scope_->is_strict_mode()) {
-      if (IsEvalOrArguments(name)) {
+      if (IsEvalOrArguments(function_name)) {
         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_function_name", Vector<const char*>::empty());
         *ok = false;
         return NULL;
       }
       if (name_loc.IsValid()) {
@@ skipping 23 matching lines @@
                         Vector<const char*>::empty());
         *ok = false;
         return NULL;
       }
       CheckOctalLiteral(start_pos, end_pos, CHECK_OK);
     }
   }
 
   FunctionLiteral* function_literal =
       new(zone()) FunctionLiteral(isolate(),
-                                  name,
+                                  function_name,
                                   scope,
                                   body,
                                   materialized_literal_count,
                                   expected_property_count,
                                   only_simple_this_property_assignments,
                                   this_property_assignments,
                                   num_parameters,
                                   start_pos,
                                   end_pos,
-                                  (function_name->length() > 0),

[Kevin Millikin (Chromium), 2011/08/04 16:32:09]

This is the is_expression flag.  It incorrectly categorized anonymous function
expressions as !is_expression.

+                                  type == EXPRESSION,
                                   has_duplicate_parameters);
   function_literal->set_function_token_position(function_token_position);
 
-  if (fni_ != NULL && !is_named) fni_->AddFunction(function_literal);
+  if (fni_ != NULL && !has_name) fni_->AddFunction(function_literal);
   return function_literal;
 }
 
 
 Expression* Parser::ParseV8Intrinsic(bool* ok) {
   // CallRuntime ::
   //   '%' Identifier Arguments
 
   Expect(Token::MOD, CHECK_OK);
   Handle<String> name = ParseIdentifier(CHECK_OK);
@@ skipping 1280 matching lines @@
                                    info->is_global(),
                                    info->StrictMode());
     }
   }
 
   info->SetFunction(result);
   return (result != NULL);
 }
 
 } }  // namespace v8::internal