Revert "Revert "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 3623 matching lines @@
   // that case, we don't have a function name (it's empty).
   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;
   }
 
   int num_parameters = 0;
-  Scope* scope = NewScope(top_scope_, Scope::FUNCTION_SCOPE, inside_with());
+  // 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());
@@ skipping 53 matching lines @@
           top_scope_->NewUnresolved(function_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 be PARSE_LAZILY if the --lazy flag is true.
+    // Determine if the function will be lazily compiled. The mode can only
+    // be PARSE_LAZILY if the --lazy flag is true.  We will not lazily
+    // compile if we do not have preparser data for the function.
     bool is_lazily_compiled = (mode() == PARSE_LAZILY &&
                                top_scope_->outer_scope()->is_global_scope() &&
                                top_scope_->HasTrivialOuterContext() &&
-                               !parenthesized_function_);
+                               !parenthesized_function_ &&
+                               pre_data() != NULL);
     parenthesized_function_ = false;  // The bit was set for this function only.
 
-    int function_block_pos = scanner().location().beg_pos;
-    if (is_lazily_compiled && pre_data() != NULL) {
+    if (is_lazily_compiled) {
+      int function_block_pos = scanner().location().beg_pos;
       FunctionEntry entry = pre_data()->GetFunctionEntry(function_block_pos);
       if (!entry.is_valid()) {
-        ReportInvalidPreparseData(name, CHECK_OK);
+        // There is no preparser data for the function, we will not lazily
+        // compile after all.
+        is_lazily_compiled = false;

[Lasse Reichstein, 2011/08/03 09:05:17]

This handles false positives (from the parser's perspective) only. It also
affects false negatives, if they can occur.  It will completely block all
further laziness, since the ScriptDataImpl will never match (and thereby skip)
the unused function entry.

That's probably fine for now.

[Kevin Millikin (Chromium), 2011/08/03 09:09:49]

Thanks for pointing that out, I hadn't realized it.  I want to commit this as
is, but we should clean it up soon.

+      } 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);
+        }
+        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);
       }
-      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);
-      }
-      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);
-    } else {
+    }
+
+    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;
@@ skipping 1353 matching lines @@
                                    info->is_global(),
                                    info->StrictMode());
     }
   }
 
   info->SetFunction(result);
   return (result != NULL);
 }
 
 } }  // namespace v8::internal