Fix a bug in named getter/setter compilation.

Because these are function literals that have an associated name, we were
compiling them as if they were named function expressions.  This is
incorrect, the property name should not be in scope.

R=vegorov@chromium.org
BUG=
TEST=

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@8863 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
This commit is contained in:
kmillikin@chromium.org 2011-08-09 12:43:08 +00:00
parent 2f826c2b89
commit 7adb10a48e
5 changed files with 38 additions and 23 deletions

View File

@ -1711,6 +1711,12 @@ class Throw: public Expression {
class FunctionLiteral: public Expression { class FunctionLiteral: public Expression {
public: public:
enum Type {
ANONYMOUS_EXPRESSION,
NAMED_EXPRESSION,
DECLARATION
};
FunctionLiteral(Isolate* isolate, FunctionLiteral(Isolate* isolate,
Handle<String> name, Handle<String> name,
Scope* scope, Scope* scope,
@ -1722,7 +1728,7 @@ class FunctionLiteral: public Expression {
int num_parameters, int num_parameters,
int start_position, int start_position,
int end_position, int end_position,
bool is_expression, Type type,
bool has_duplicate_parameters) bool has_duplicate_parameters)
: Expression(isolate), : Expression(isolate),
name_(name), name_(name),
@ -1738,7 +1744,8 @@ class FunctionLiteral: public Expression {
end_position_(end_position), end_position_(end_position),
function_token_position_(RelocInfo::kNoPosition), function_token_position_(RelocInfo::kNoPosition),
inferred_name_(HEAP->empty_string()), inferred_name_(HEAP->empty_string()),
is_expression_(is_expression), is_expression_(type != DECLARATION),
is_anonymous_(type == ANONYMOUS_EXPRESSION),
pretenure_(false), pretenure_(false),
has_duplicate_parameters_(has_duplicate_parameters) { has_duplicate_parameters_(has_duplicate_parameters) {
} }
@ -1753,6 +1760,7 @@ class FunctionLiteral: public Expression {
int start_position() const { return start_position_; } int start_position() const { return start_position_; }
int end_position() const { return end_position_; } int end_position() const { return end_position_; }
bool is_expression() const { return is_expression_; } bool is_expression() const { return is_expression_; }
bool is_anonymous() const { return is_anonymous_; }
bool strict_mode() const; bool strict_mode() const;
int materialized_literal_count() { return materialized_literal_count_; } int materialized_literal_count() { return materialized_literal_count_; }
@ -1797,6 +1805,7 @@ class FunctionLiteral: public Expression {
int function_token_position_; int function_token_position_;
Handle<String> inferred_name_; Handle<String> inferred_name_;
bool is_expression_; bool is_expression_;
bool is_anonymous_;
bool pretenure_; bool pretenure_;
bool has_duplicate_parameters_; bool has_duplicate_parameters_;
}; };

View File

@ -736,7 +736,7 @@ void Compiler::SetFunctionInfo(Handle<SharedFunctionInfo> function_info,
function_info->set_start_position(lit->start_position()); function_info->set_start_position(lit->start_position());
function_info->set_end_position(lit->end_position()); function_info->set_end_position(lit->end_position());
function_info->set_is_expression(lit->is_expression()); function_info->set_is_expression(lit->is_expression());
function_info->set_is_anonymous(lit->name()->length() == 0); function_info->set_is_anonymous(lit->is_anonymous());
function_info->set_is_toplevel(is_toplevel); function_info->set_is_toplevel(is_toplevel);
function_info->set_inferred_name(*lit->inferred_name()); function_info->set_inferred_name(*lit->inferred_name());
function_info->SetThisPropertyAssignmentsInfo( function_info->SetThisPropertyAssignmentsInfo(

View File

@ -659,8 +659,8 @@ FunctionLiteral* Parser::DoParseProgram(Handle<String> source,
0, 0,
0, 0,
source->length(), source->length(),
false, FunctionLiteral::ANONYMOUS_EXPRESSION,
false); false); // Does not have duplicate parameters.
} else if (stack_overflow_) { } else if (stack_overflow_) {
isolate()->StackOverflow(); isolate()->StackOverflow();
} }
@ -729,12 +729,13 @@ FunctionLiteral* Parser::ParseLazy(CompilationInfo* info,
top_scope_->EnableStrictMode(); top_scope_->EnableStrictMode();
} }
FunctionLiteralType type = FunctionLiteral::Type type = shared_info->is_expression()
shared_info->is_expression() ? EXPRESSION : DECLARATION; ? (shared_info->is_anonymous()
Handle<String> function_name = ? FunctionLiteral::ANONYMOUS_EXPRESSION
shared_info->is_anonymous() ? Handle<String>::null() : name; : FunctionLiteral::NAMED_EXPRESSION)
: FunctionLiteral::DECLARATION;
bool ok = true; bool ok = true;
result = ParseFunctionLiteral(function_name, result = ParseFunctionLiteral(name,
false, // Strict mode name already checked. false, // Strict mode name already checked.
RelocInfo::kNoPosition, RelocInfo::kNoPosition,
type, type,
@ -1475,7 +1476,7 @@ Statement* Parser::ParseFunctionDeclaration(bool* ok) {
FunctionLiteral* fun = ParseFunctionLiteral(name, FunctionLiteral* fun = ParseFunctionLiteral(name,
is_strict_reserved, is_strict_reserved,
function_token_position, function_token_position,
DECLARATION, FunctionLiteral::DECLARATION,
CHECK_OK); CHECK_OK);
// Even if we're not at the top-level of the global or a function // Even if we're not at the top-level of the global or a function
// scope, we treat is as such and introduce the function with it's // scope, we treat is as such and introduce the function with it's
@ -2846,10 +2847,13 @@ Expression* Parser::ParseMemberWithNewPrefixesExpression(PositionStack* stack,
name = ParseIdentifierOrStrictReservedWord(&is_strict_reserved_name, name = ParseIdentifierOrStrictReservedWord(&is_strict_reserved_name,
CHECK_OK); CHECK_OK);
} }
FunctionLiteral::Type type = name.is_null()
? FunctionLiteral::ANONYMOUS_EXPRESSION
: FunctionLiteral::NAMED_EXPRESSION;
result = ParseFunctionLiteral(name, result = ParseFunctionLiteral(name,
is_strict_reserved_name, is_strict_reserved_name,
function_token_position, function_token_position,
EXPRESSION, type,
CHECK_OK); CHECK_OK);
} else { } else {
result = ParsePrimaryExpression(CHECK_OK); result = ParsePrimaryExpression(CHECK_OK);
@ -3419,7 +3423,7 @@ ObjectLiteral::Property* Parser::ParseObjectLiteralGetSet(bool is_getter,
ParseFunctionLiteral(name, ParseFunctionLiteral(name,
false, // reserved words are allowed here false, // reserved words are allowed here
RelocInfo::kNoPosition, RelocInfo::kNoPosition,
EXPRESSION, FunctionLiteral::ANONYMOUS_EXPRESSION,
CHECK_OK); CHECK_OK);
// Allow any number of parameters for compatiabilty with JSC. // Allow any number of parameters for compatiabilty with JSC.
// Specification only allows zero parameters for get and one for set. // Specification only allows zero parameters for get and one for set.
@ -3629,7 +3633,7 @@ ZoneList<Expression*>* Parser::ParseArguments(bool* ok) {
FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> function_name, FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> function_name,
bool name_is_strict_reserved, bool name_is_strict_reserved,
int function_token_position, int function_token_position,
FunctionLiteralType type, FunctionLiteral::Type type,
bool* ok) { bool* ok) {
// Function :: // Function ::
// '(' FormalParameterList? ')' '{' FunctionBody '}' // '(' FormalParameterList? ')' '{' FunctionBody '}'
@ -3646,7 +3650,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> function_name,
int num_parameters = 0; int num_parameters = 0;
// Function declarations are hoisted. // Function declarations are hoisted.
Scope* scope = (type == DECLARATION) Scope* scope = (type == FunctionLiteral::DECLARATION)
? NewScope(top_scope_->DeclarationScope(), Scope::FUNCTION_SCOPE, false) ? NewScope(top_scope_->DeclarationScope(), Scope::FUNCTION_SCOPE, false)
: NewScope(top_scope_, Scope::FUNCTION_SCOPE, inside_with()); : NewScope(top_scope_, Scope::FUNCTION_SCOPE, inside_with());
ZoneList<Statement*>* body = new(zone()) ZoneList<Statement*>(8); ZoneList<Statement*>* body = new(zone()) ZoneList<Statement*>(8);
@ -3709,7 +3713,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> function_name,
// NOTE: We create a proxy and resolve it here so that in the // NOTE: We create a proxy and resolve it here so that in the
// future we can change the AST to only refer to VariableProxies // future we can change the AST to only refer to VariableProxies
// instead of Variables and Proxis as is the case now. // instead of Variables and Proxis as is the case now.
if (type == EXPRESSION && function_name->length() > 0) { if (type == FunctionLiteral::NAMED_EXPRESSION) {
Variable* fvar = top_scope_->DeclareFunctionVar(function_name); Variable* fvar = top_scope_->DeclareFunctionVar(function_name);
VariableProxy* fproxy = VariableProxy* fproxy =
top_scope_->NewUnresolved(function_name, inside_with()); top_scope_->NewUnresolved(function_name, inside_with());
@ -3827,7 +3831,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> function_name,
num_parameters, num_parameters,
start_pos, start_pos,
end_pos, end_pos,
type == EXPRESSION, type,
has_duplicate_parameters); has_duplicate_parameters);
function_literal->set_function_token_position(function_token_position); function_literal->set_function_token_position(function_token_position);

View File

@ -552,16 +552,11 @@ class Parser {
// in the object literal boilerplate. // in the object literal boilerplate.
Handle<Object> GetBoilerplateValue(Expression* expression); Handle<Object> GetBoilerplateValue(Expression* expression);
enum FunctionLiteralType {
EXPRESSION,
DECLARATION
};
ZoneList<Expression*>* ParseArguments(bool* ok); ZoneList<Expression*>* ParseArguments(bool* ok);
FunctionLiteral* ParseFunctionLiteral(Handle<String> var_name, FunctionLiteral* ParseFunctionLiteral(Handle<String> var_name,
bool name_is_reserved, bool name_is_reserved,
int function_token_position, int function_token_position,
FunctionLiteralType type, FunctionLiteral::Type type,
bool* ok); bool* ok);

View File

@ -48,3 +48,10 @@ assertEquals('hest', o.m());
assertEquals('hest', o.m()); assertEquals('hest', o.m());
%OptimizeFunctionOnNextCall(o.m); %OptimizeFunctionOnNextCall(o.m);
assertEquals('hest', o.m()); assertEquals('hest', o.m());
// Fixing the bug above introduced (revealed?) an inconsistency in named
// getters and setters. The property name was also treated as a function
// name.
var global = 'horse';
var p = { get global() { return global; }};
assertEquals('horse', p.global);