Revision: 8383
Author: [email protected]
Date: Wed Jun 22 13:23:48 2011
Log: Fix issue 1354: Bad function name inference.
[email protected], [email protected]
BUG=1354
TEST=test-func-name-inference
Review URL: http://codereview.chromium.org/7206015
http://code.google.com/p/v8/source/detail?r=8383
Modified:
/branches/bleeding_edge/src/func-name-inferrer.cc
/branches/bleeding_edge/src/func-name-inferrer.h
/branches/bleeding_edge/src/heap.h
/branches/bleeding_edge/src/parser.cc
/branches/bleeding_edge/test/cctest/cctest.status
/branches/bleeding_edge/test/cctest/test-func-name-inference.cc
=======================================
--- /branches/bleeding_edge/src/func-name-inferrer.cc Tue May 3 01:23:58
2011
+++ /branches/bleeding_edge/src/func-name-inferrer.cc Wed Jun 22 13:23:48
2011
@@ -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);
+ }
}
}
=======================================
--- /branches/bleeding_edge/src/func-name-inferrer.h Fri Mar 18 13:35:07
2011
+++ /branches/bleeding_edge/src/func-name-inferrer.h Wed Jun 22 13:23:48
2011
@@ -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 @@
// 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 @@
}
}
- // 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 @@
// 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);
};
=======================================
--- /branches/bleeding_edge/src/heap.h Mon Jun 20 03:20:57 2011
+++ /branches/bleeding_edge/src/heap.h Wed Jun 22 13:23:48 2011
@@ -218,7 +218,9 @@
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;
=======================================
--- /branches/bleeding_edge/src/parser.cc Wed Jun 22 02:06:03 2011
+++ /branches/bleeding_edge/src/parser.cc Wed Jun 22 13:23:48 2011
@@ -595,7 +595,7 @@
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 @@
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 @@
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 @@
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 @@
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;
}
=======================================
--- /branches/bleeding_edge/test/cctest/cctest.status Fri Jun 17 01:40:30
2011
+++ /branches/bleeding_edge/test/cctest/cctest.status Wed Jun 22 13:23:48
2011
@@ -38,14 +38,6 @@
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 ]
=======================================
--- /branches/bleeding_edge/test/cctest/test-func-name-inference.cc Mon
May 2 04:08:44 2011
+++ /branches/bleeding_edge/test/cctest/test-func-name-inference.cc Wed Jun
22 13:23:48 2011
@@ -288,19 +288,28 @@
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", "");
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
