From 0a12f5dcf83cb6b7b6b61129bfa7a1719aec29e3 Mon Sep 17 00:00:00 2001 From: "mikhail.naganov@gmail.com" Date: Wed, 22 Jun 2011 20:23:48 +0000 Subject: [PATCH] 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 | 44 ++++++++++++++++--------- src/func-name-inferrer.h | 28 +++++++++++----- src/heap.h | 4 ++- src/parser.cc | 20 ++++++++--- test/cctest/cctest.status | 8 ----- test/cctest/test-func-name-inference.cc | 15 +++++++-- 6 files changed, 79 insertions(+), 40 deletions(-) diff --git a/src/func-name-inferrer.cc b/src/func-name-inferrer.cc index ebac4b9bff..239358dfa6 100644 --- a/src/func-name-inferrer.cc +++ b/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 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 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 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 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 FuncNameInferrer::MakeNameFromStackHelper(int pos, Handle 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 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 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); + } } } diff --git a/src/func-name-inferrer.h b/src/func-name-inferrer.h index 5aa2b35bb1..bec3a5cf9a 100644 --- a/src/func-name-inferrer.h +++ b/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 name, NameType type) : name(name), type(type) { } + Handle name; + NameType type; + }; + + Isolate* isolate() { return isolate_; } + // Constructs a full name in dotted notation from gathered names. Handle MakeNameFromStack(); @@ -97,10 +107,10 @@ class FuncNameInferrer : public ZoneObject { // Performs name inferring for added functions. void InferFunctionsNames(); + Isolate* isolate_; ZoneList entries_stack_; - ZoneList > names_stack_; + ZoneList names_stack_; ZoneList funcs_to_infer_; - Handle dot_; DISALLOW_COPY_AND_ASSIGN(FuncNameInferrer); }; diff --git a/src/heap.h b/src/heap.h index 4e7846d73e..7d7a236d87 100644 --- a/src/heap.h +++ b/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; diff --git a/src/parser.cc b/src/parser.cc index b04a2f93b2..a8e20443e4 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -595,7 +595,7 @@ FunctionLiteral* Parser::ParseProgram(Handle 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 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; } diff --git a/test/cctest/cctest.status b/test/cctest/cctest.status index 4e4f01754e..6e7824f2f2 100644 --- a/test/cctest/cctest.status +++ b/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 ] diff --git a/test/cctest/test-func-name-inference.cc b/test/cctest/test-func-name-inference.cc index e0d99ec139..4d993af4a3 100644 --- a/test/cctest/test-func-name-inference.cc +++ b/test/cctest/test-func-name-inference.cc @@ -288,19 +288,28 @@ TEST(MultipleAssignments) { v8::HandleScope scope; v8::Handle 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 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", ""); }