Revision: 2994
Author: [email protected]
Date: Wed Sep 30 02:49:36 2009
Log: Two changes, one a refactoring and one that affects V8's JS semantics.
1. Change the AST node type CallNew to be a subclass of Expression
rather than Call. It's not really a call but it just happens to
have the same fields.
2. Change our error reporting for invalid left-hand sides in for-in
statements, pre- and postfix count expressions, and assignments.
Before we signaled a syntax error at compile time *unless* the LHS
was a function call or 'new' expression, in which case we signaled
a reference error at runtime. Now we signal a reference error at
runtime in all cases. This matches the JSC behavior in Safari 4.
Review URL: http://codereview.chromium.org/249039
http://code.google.com/p/v8/source/detail?r=2994
Modified:
/branches/bleeding_edge/src/ast.h
/branches/bleeding_edge/src/parser.cc
/branches/bleeding_edge/src/usage-analyzer.cc
/branches/bleeding_edge/test/mjsunit/invalid-lhs.js
=======================================
--- /branches/bleeding_edge/src/ast.h Tue Sep 29 06:28:30 2009
+++ /branches/bleeding_edge/src/ast.h Wed Sep 30 02:49:36 2009
@@ -954,12 +954,8 @@
class Call: public Expression {
public:
- Call(Expression* expression,
- ZoneList<Expression*>* arguments,
- int pos)
- : expression_(expression),
- arguments_(arguments),
- pos_(pos) { }
+ Call(Expression* expression, ZoneList<Expression*>* arguments, int pos)
+ : expression_(expression), arguments_(arguments), pos_(pos) { }
virtual void Accept(AstVisitor* v);
@@ -981,12 +977,21 @@
};
-class CallNew: public Call {
+class CallNew: public Expression {
public:
CallNew(Expression* expression, ZoneList<Expression*>* arguments, int
pos)
- : Call(expression, arguments, pos) { }
+ : expression_(expression), arguments_(arguments), pos_(pos) { }
virtual void Accept(AstVisitor* v);
+
+ Expression* expression() const { return expression_; }
+ ZoneList<Expression*>* arguments() const { return arguments_; }
+ int position() { return pos_; }
+
+ private:
+ Expression* expression_;
+ ZoneList<Expression*>* arguments_;
+ int pos_;
};
=======================================
--- /branches/bleeding_edge/src/parser.cc Tue Sep 29 06:28:30 2009
+++ /branches/bleeding_edge/src/parser.cc Wed Sep 30 02:49:36 2009
@@ -2660,25 +2660,13 @@
} else {
Expression* expression = ParseExpression(false, CHECK_OK);
if (peek() == Token::IN) {
- // Report syntax error if the expression is an invalid
- // left-hand side expression.
+ // Signal a reference error if the expression is an invalid
+ // left-hand side expression. We could report this as a syntax
+ // error here but for compatibility with JSC we choose to report
+ // the error at runtime.
if (expression == NULL || !expression->IsValidLeftHandSide()) {
- if (expression != NULL && expression->AsCall() != NULL) {
- // According to ECMA-262 host function calls are permitted to
- // return references. This cannot happen in our system so we
- // will always get an error. We could report this as a syntax
- // error here but for compatibility with KJS and SpiderMonkey
we
- // choose to report the error at runtime.
- Handle<String> type = Factory::invalid_lhs_in_for_in_symbol();
- expression = NewThrowReferenceError(type);
- } else {
- // Invalid left hand side expressions that are not function
- // calls are reported as syntax errors at compile time.
- ReportMessage("invalid_lhs_in_for_in",
- Vector<const char*>::empty());
- *ok = false;
- return NULL;
- }
+ Handle<String> type = Factory::invalid_lhs_in_for_in_symbol();
+ expression = NewThrowReferenceError(type);
}
ForInStatement* loop = NEW(ForInStatement(labels));
Target target(this, loop);
@@ -2755,29 +2743,14 @@
return expression;
}
+ // Signal a reference error if the expression is an invalid left-hand
+ // side expression. We could report this as a syntax error here but
+ // for compatibility with JSC we choose to report the error at
+ // runtime.
if (expression == NULL || !expression->IsValidLeftHandSide()) {
- if (expression != NULL && expression->AsCall() != NULL) {
- // According to ECMA-262 host function calls are permitted to
- // return references. This cannot happen in our system so we
- // will always get an error. We could report this as a syntax
- // error here but for compatibility with KJS and SpiderMonkey we
- // choose to report the error at runtime.
- Handle<String> type = Factory::invalid_lhs_in_assignment_symbol();
- expression = NewThrowReferenceError(type);
- } else {
- // Invalid left hand side expressions that are not function
- // calls are reported as syntax errors at compile time.
- //
- // NOTE: KJS sometimes delay the error reporting to runtime. If
- // we want to be completely compatible we should do the same.
- // For example: "(x++) = 42" gives a reference error at runtime
- // with KJS whereas we report a syntax error at compile time.
- ReportMessage("invalid_lhs_in_assignment", Vector<const
char*>::empty());
- *ok = false;
- return NULL;
- }
- }
-
+ Handle<String> type = Factory::invalid_lhs_in_assignment_symbol();
+ expression = NewThrowReferenceError(type);
+ }
Token::Value op = Next(); // Get assignment operator.
int pos = scanner().location().beg_pos;
@@ -2951,45 +2924,37 @@
Token::Value op = peek();
if (Token::IsUnaryOp(op)) {
op = Next();
- Expression* x = ParseUnaryExpression(CHECK_OK);
+ Expression* expression = ParseUnaryExpression(CHECK_OK);
// Compute some expressions involving only number literals.
- if (x && x->AsLiteral() && x->AsLiteral()->handle()->IsNumber()) {
- double x_val = x->AsLiteral()->handle()->Number();
+ if (expression != NULL && expression->AsLiteral() &&
+ expression->AsLiteral()->handle()->IsNumber()) {
+ double value = expression->AsLiteral()->handle()->Number();
switch (op) {
case Token::ADD:
- return x;
+ return expression;
case Token::SUB:
- return NewNumberLiteral(-x_val);
+ return NewNumberLiteral(-value);
case Token::BIT_NOT:
- return NewNumberLiteral(~DoubleToInt32(x_val));
+ return NewNumberLiteral(~DoubleToInt32(value));
default: break;
}
}
- return NEW(UnaryOperation(op, x));
+ return NEW(UnaryOperation(op, expression));
} else if (Token::IsCountOp(op)) {
op = Next();
- Expression* x = ParseUnaryExpression(CHECK_OK);
- if (x == NULL || !x->IsValidLeftHandSide()) {
- if (x != NULL && x->AsCall() != NULL) {
- // According to ECMA-262 host function calls are permitted to
- // return references. This cannot happen in our system so we
- // will always get an error. We could report this as a syntax
- // error here but for compatibility with KJS and SpiderMonkey we
- // choose to report the error at runtime.
- Handle<String> type = Factory::invalid_lhs_in_prefix_op_symbol();
- x = NewThrowReferenceError(type);
- } else {
- // Invalid left hand side expressions that are not function
- // calls are reported as syntax errors at compile time.
- ReportMessage("invalid_lhs_in_prefix_op", Vector<const
char*>::empty());
- *ok = false;
- return NULL;
- }
- }
- return NEW(CountOperation(true /* prefix */, op, x));
+ Expression* expression = ParseUnaryExpression(CHECK_OK);
+ // Signal a reference error if the expression is an invalid
+ // left-hand side expression. We could report this as a syntax
+ // error here but for compatibility with JSC we choose to report the
+ // error at runtime.
+ if (expression == NULL || !expression->IsValidLeftHandSide()) {
+ Handle<String> type = Factory::invalid_lhs_in_prefix_op_symbol();
+ expression = NewThrowReferenceError(type);
+ }
+ return NEW(CountOperation(true /* prefix */, op, expression));
} else {
return ParsePostfixExpression(ok);
@@ -3001,30 +2966,20 @@
// PostfixExpression ::
// LeftHandSideExpression ('++' | '--')?
- Expression* result = ParseLeftHandSideExpression(CHECK_OK);
+ Expression* expression = ParseLeftHandSideExpression(CHECK_OK);
if (!scanner_.has_line_terminator_before_next() &&
Token::IsCountOp(peek())) {
- if (result == NULL || !result->IsValidLeftHandSide()) {
- if (result != NULL && result->AsCall() != NULL) {
- // According to ECMA-262 host function calls are permitted to
- // return references. This cannot happen in our system so we
- // will always get an error. We could report this as a syntax
- // error here but for compatibility with KJS and SpiderMonkey we
- // choose to report the error at runtime.
- Handle<String> type = Factory::invalid_lhs_in_postfix_op_symbol();
- result = NewThrowReferenceError(type);
- } else {
- // Invalid left hand side expressions that are not function
- // calls are reported as syntax errors at compile time.
- ReportMessage("invalid_lhs_in_postfix_op",
- Vector<const char*>::empty());
- *ok = false;
- return NULL;
- }
+ // Signal a reference error if the expression is an invalid
+ // left-hand side expression. We could report this as a syntax
+ // error here but for compatibility with JSC we choose to report the
+ // error at runtime.
+ if (expression == NULL || !expression->IsValidLeftHandSide()) {
+ Handle<String> type = Factory::invalid_lhs_in_postfix_op_symbol();
+ expression = NewThrowReferenceError(type);
}
Token::Value next = Next();
- result = NEW(CountOperation(false /* postfix */, next, result));
- }
- return result;
+ expression = NEW(CountOperation(false /* postfix */, next,
expression));
+ }
+ return expression;
}
=======================================
--- /branches/bleeding_edge/src/usage-analyzer.cc Tue Sep 29 06:28:30 2009
+++ /branches/bleeding_edge/src/usage-analyzer.cc Wed Sep 30 02:49:36 2009
@@ -44,44 +44,12 @@
public:
static bool Traverse(AstNode* node);
- void VisitBlock(Block* node);
- void VisitDeclaration(Declaration* node);
- void VisitExpressionStatement(ExpressionStatement* node);
- void VisitEmptyStatement(EmptyStatement* node);
- void VisitIfStatement(IfStatement* node);
- void VisitContinueStatement(ContinueStatement* node);
- void VisitBreakStatement(BreakStatement* node);
- void VisitReturnStatement(ReturnStatement* node);
- void VisitWithEnterStatement(WithEnterStatement* node);
- void VisitWithExitStatement(WithExitStatement* node);
- void VisitSwitchStatement(SwitchStatement* node);
- void VisitLoopStatement(LoopStatement* node);
- void VisitForInStatement(ForInStatement* node);
- void VisitTryCatch(TryCatch* node);
- void VisitTryFinally(TryFinally* node);
- void VisitDebuggerStatement(DebuggerStatement* node);
- void VisitFunctionLiteral(FunctionLiteral* node);
- void VisitFunctionBoilerplateLiteral(FunctionBoilerplateLiteral* node);
- void VisitConditional(Conditional* node);
- void VisitSlot(Slot* node);
- void VisitVariable(Variable* node);
- void VisitVariableProxy(VariableProxy* node);
- void VisitLiteral(Literal* node);
- void VisitRegExpLiteral(RegExpLiteral* node);
- void VisitObjectLiteral(ObjectLiteral* node);
- void VisitArrayLiteral(ArrayLiteral* node);
- void VisitCatchExtensionObject(CatchExtensionObject* node);
- void VisitAssignment(Assignment* node);
- void VisitThrow(Throw* node);
- void VisitProperty(Property* node);
- void VisitCall(Call* node);
- void VisitCallNew(CallNew* node);
- void VisitCallRuntime(CallRuntime* node);
- void VisitUnaryOperation(UnaryOperation* node);
- void VisitCountOperation(CountOperation* node);
- void VisitBinaryOperation(BinaryOperation* node);
- void VisitCompareOperation(CompareOperation* node);
- void VisitThisFunction(ThisFunction* node);
+ // AST node visit functions.
+#define DECLARE_VISIT(type) void Visit##type(type* node);
+ AST_NODE_LIST(DECLARE_VISIT)
+#undef DECLARE_VISIT
+
+ void VisitVariable(Variable* var);
private:
int weight_;
@@ -329,7 +297,8 @@
void UsageComputer::VisitCallNew(CallNew* node) {
- VisitCall(node);
+ Read(node->expression());
+ ReadList(node->arguments());
}
=======================================
--- /branches/bleeding_edge/test/mjsunit/invalid-lhs.js Tue Sep 9 13:08:45
2008
+++ /branches/bleeding_edge/test/mjsunit/invalid-lhs.js Wed Sep 30 02:49:36
2009
@@ -25,9 +25,8 @@
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-// Test that we get exceptions for invalid left-hand sides. Also
-// tests that if the invalid left-hand side is a function call, the
-// exception is delayed until runtime.
+// Test that we get exceptions for invalid left-hand sides. The
+// exceptions are delayed until runtime.
// Normal assignments:
assertThrows("12 = 12");
@@ -57,12 +56,10 @@
// Assignments to 'this'.
assertThrows("this = 42");
-assertThrows("function f() { this = 12; }");
-assertThrows("for (this in Array) ;");
+assertDoesNotThrow("function f() { this = 12; }");
+assertThrows("for (this in {x:3, y:4, z:5}) ;");
assertThrows("for (this = 0;;) ;");
assertThrows("this++");
assertThrows("++this");
assertThrows("this--");
assertThrows("--this");
-
-
--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---
