Fix issue 1354: Bad function name inference.

R=kmillikin@chromium.org, vitalyr@chromium.org
BUG=1354
TEST=test-func-name-inference

Review URL: http://codereview.chromium.org/7206015

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@8383 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/func-name-inferrer.cc

@@ -34,48 +34,62 @@
 namespace v8 {
 namespace internal {
 
+FuncNameInferrer::FuncNameInferrer(Isolate* isolate)
+    : isolate_(isolate),
+      entries_stack_(10),
+      names_stack_(5),
+      funcs_to_infer_(4) {
+}
+
 
 void FuncNameInferrer::PushEnclosingName(Handle<String> name) {
   // Enclosing name is a name of a constructor function. To check
   // that it is really a constructor, we check that it is not empty
   // and starts with a capital letter.
   if (name->length() > 0 && Runtime::IsUpperCaseChar(
-      Isolate::Current()->runtime_state(), name->Get(0))) {
-    names_stack_.Add(name);
+          isolate()->runtime_state(), name->Get(0))) {
+    names_stack_.Add(Name(name, kEnclosingConstructorName));
   }
 }
 
 
 void FuncNameInferrer::PushLiteralName(Handle<String> name) {
-  if (IsOpen() && !HEAP->prototype_symbol()->Equals(*name)) {
-    names_stack_.Add(name);
+  if (IsOpen() && !isolate()->heap()->prototype_symbol()->Equals(*name)) {
+    names_stack_.Add(Name(name, kLiteralName));
   }
 }
 
 
 void FuncNameInferrer::PushVariableName(Handle<String> name) {
-  if (IsOpen() && !HEAP->result_symbol()->Equals(*name)) {
-    names_stack_.Add(name);
+  if (IsOpen() && !isolate()->heap()->result_symbol()->Equals(*name)) {
+    names_stack_.Add(Name(name, kVariableName));
   }
 }
 
 
 Handle<String> FuncNameInferrer::MakeNameFromStack() {
-  if (names_stack_.is_empty()) {
-    return FACTORY->empty_string();
-  } else {
-    return MakeNameFromStackHelper(1, names_stack_.at(0));
-  }
+  return MakeNameFromStackHelper(0, isolate()->factory()->empty_string());
 }
 
 
 Handle<String> FuncNameInferrer::MakeNameFromStackHelper(int pos,
                                                          Handle<String> prev) {
-  if (pos >= names_stack_.length()) {
-    return prev;
+  if (pos >= names_stack_.length()) return prev;
+  if (pos < names_stack_.length() - 1 &&
+      names_stack_.at(pos).type == kVariableName &&
+      names_stack_.at(pos + 1).type == kVariableName) {
+    // Skip consecutive variable declarations.
+    return MakeNameFromStackHelper(pos + 1, prev);
   } else {
-    Handle<String> curr = FACTORY->NewConsString(dot_, names_stack_.at(pos));
-    return MakeNameFromStackHelper(pos + 1, FACTORY->NewConsString(prev, curr));
+    if (prev->length() > 0) {
+      Factory* factory = isolate()->factory();
+      Handle<String> curr = factory->NewConsString(
+          factory->dot_symbol(), names_stack_.at(pos).name);
+      return MakeNameFromStackHelper(pos + 1,
+                                     factory->NewConsString(prev, curr));
+    } else {
+      return MakeNameFromStackHelper(pos + 1, names_stack_.at(pos).name);
+    }
   }
 }
 

src/func-name-inferrer.h

@@ -31,6 +31,8 @@
 namespace v8 {
 namespace internal {
 
+class Isolate;
+
 // FuncNameInferrer is a stateful class that is used to perform name
 // inference for anonymous functions during static analysis of source code.
 // Inference is performed in cases when an anonymous function is assigned
@@ -43,12 +45,7 @@ namespace internal {
 // a name.
 class FuncNameInferrer : public ZoneObject {
  public:
-  FuncNameInferrer()
-      : entries_stack_(10),
-        names_stack_(5),
-        funcs_to_infer_(4),
-        dot_(FACTORY->NewStringFromAscii(CStrVector("."))) {
-  }
+  explicit FuncNameInferrer(Isolate* isolate);
 
   // Returns whether we have entered name collection state.
   bool IsOpen() const { return !entries_stack_.is_empty(); }
@@ -81,13 +78,26 @@ class FuncNameInferrer : public ZoneObject {
     }
   }
 
-  // Infers a function name and leaves names collection state.
+  // Leaves names collection state.
   void Leave() {
     ASSERT(IsOpen());
     names_stack_.Rewind(entries_stack_.RemoveLast());
   }
 
  private:
+  enum NameType {
+    kEnclosingConstructorName,
+    kLiteralName,
+    kVariableName
+  };
+  struct Name {
+    Name(Handle<String> name, NameType type) : name(name), type(type) { }
+    Handle<String> name;
+    NameType type;
+  };
+
+  Isolate* isolate() { return isolate_; }
+
   // Constructs a full name in dotted notation from gathered names.
   Handle<String> MakeNameFromStack();
 
@@ -97,10 +107,10 @@ class FuncNameInferrer : public ZoneObject {
   // Performs name inferring for added functions.
   void InferFunctionsNames();
 
+  Isolate* isolate_;
   ZoneList<int> entries_stack_;
-  ZoneList<Handle<String> > names_stack_;
+  ZoneList<Name> names_stack_;
   ZoneList<FunctionLiteral*> funcs_to_infer_;
-  Handle<String> dot_;
 
   DISALLOW_COPY_AND_ASSIGN(FuncNameInferrer);
 };

src/heap.h

@@ -218,7 +218,9 @@ inline Heap* _inline_get_heap_();
   V(global_eval_symbol, "GlobalEval")                                    \
   V(identity_hash_symbol, "v8::IdentityHash")                            \
   V(closure_symbol, "(closure)")                                         \
-  V(use_strict, "use strict")
+  V(use_strict, "use strict")                                            \
+  V(dot_symbol, ".")                                                     \
+  V(anonymous_function_symbol, "(anonymous function)")
 
 // Forward declarations.
 class GCTracer;

src/parser.cc

@@ -595,7 +595,7 @@ FunctionLiteral* Parser::ParseProgram(Handle<String> source,
 
   HistogramTimerScope timer(isolate()->counters()->parse());
   isolate()->counters()->total_parse_size()->Increment(source->length());
-  fni_ = new(zone()) FuncNameInferrer();
+  fni_ = new(zone()) FuncNameInferrer(isolate());
 
   // Initialize parser state.
   source->TryFlatten();
@@ -707,7 +707,7 @@ FunctionLiteral* Parser::ParseLazy(CompilationInfo* info,
   ASSERT(target_stack_ == NULL);
 
   Handle<String> name(String::cast(shared_info->name()));
-  fni_ = new(zone()) FuncNameInferrer();
+  fni_ = new(zone()) FuncNameInferrer(isolate());
   fni_->PushEnclosingName(name);
 
   mode_ = PARSE_EAGERLY;
@@ -1613,7 +1613,11 @@ Block* Parser::ParseVariableDeclarations(bool accept_IN,
       position = scanner().location().beg_pos;
       value = ParseAssignmentExpression(accept_IN, CHECK_OK);
       // Don't infer if it is "a = function(){...}();"-like expression.
-      if (fni_ != NULL && value->AsCall() == NULL) fni_->Infer();
+      if (fni_ != NULL &&
+          value->AsCall() == NULL &&
+          value->AsCallNew() == NULL) {
+        fni_->Infer();
+      }
     }
 
     // Make sure that 'const c' actually initializes 'c' to undefined
@@ -2380,7 +2384,7 @@ Expression* Parser::ParseAssignmentExpression(bool accept_IN, bool* ok) {
     if ((op == Token::INIT_VAR
          || op == Token::INIT_CONST
          || op == Token::ASSIGN)
-        && (right->AsCall() == NULL)) {
+        && (right->AsCall() == NULL && right->AsCallNew() == NULL)) {
       fni_->Infer();
     }
     fni_->Leave();
@@ -2787,6 +2791,14 @@ Expression* Parser::ParseMemberWithNewPrefixesExpression(PositionStack* stack,
         int pos = scanner().location().beg_pos;
         Expression* index = ParseExpression(true, CHECK_OK);
         result = new(zone()) Property(result, index, pos);
+        if (fni_ != NULL) {
+          if (index->IsPropertyName()) {
+            fni_->PushLiteralName(index->AsLiteral()->AsPropertyName());
+          } else {
+            fni_->PushLiteralName(
+                isolate()->factory()->anonymous_function_symbol());
+          }
+        }
         Expect(Token::RBRACK, CHECK_OK);
         break;
       }

test/cctest/cctest.status

@@ -38,14 +38,6 @@ test-api/ApplyInterruption: PASS || TIMEOUT
 test-serialize/TestThatAlwaysFails: FAIL
 test-serialize/DependentTestThatAlwaysFails: FAIL
 
-##############################################################################
-# BUG(1354): Bad function name inference
-test-func-name-inference/FactoryHashmapConditional: FAIL
-test-func-name-inference/FactoryHashmapVariable: FAIL
-test-func-name-inference/FactoryHashmap: FAIL
-test-func-name-inference/MultipleAssignments: FAIL
-test-func-name-inference/PassedAsConstructorParameter: FAIL
-
 ##############################################################################
 [ $arch == arm ]
 

test/cctest/test-func-name-inference.cc

@@ -288,19 +288,28 @@ TEST(MultipleAssignments) {
   v8::HandleScope scope;
 
   v8::Handle<v8::Script> script = Compile(
-      "var fun1 = fun2 = function () { return 1; }");
+      "var fun1 = fun2 = function () { return 1; }\n"
+      "var bar1 = bar2 = bar3 = function () { return 2; }\n"
+      "foo1 = foo2 = function () { return 3; }\n"
+      "baz1 = baz2 = baz3 = function () { return 4; }");
   CheckFunctionName(script, "return 1", "fun2");
+  CheckFunctionName(script, "return 2", "bar3");
+  CheckFunctionName(script, "return 3", "foo2");
+  CheckFunctionName(script, "return 4", "baz3");
 }
 
 
-TEST(PassedAsConstructorParameter) {
+TEST(AsConstructorParameter) {
   InitializeVM();
   v8::HandleScope scope;
 
   v8::Handle<v8::Script> script = Compile(
       "function Foo() {}\n"
-      "var foo = new Foo(function() { return 1; })");
+      "var foo = new Foo(function() { return 1; })\n"
+      "var bar = new Foo(function() { return 2; }, function() { return 3; })");
   CheckFunctionName(script, "return 1", "");
+  CheckFunctionName(script, "return 2", "");
+  CheckFunctionName(script, "return 3", "");
 }