Reviewers: adamk, rossberg,
Message:
Hi, PTAL, I wanted to take on a smaller bug since it's been a few weeks.
Going to add a 2nd patch shortly to address the self-review I've done
https://codereview.chromium.org/1292393002/diff/1/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/1292393002/diff/1/src/preparser.h#newcode3982
src/preparser.h:3982: : this->NewThrowSyntaxError(message, pos);
I'm not sure if this distinction is necessary or good here --- getting
rid of this does make the patch a lot smaller.
SpiderMonkey does not make the distinction, at any rate.
Description:
[parser] make kInvalidLhsInFor a SyntaxError
Second item in section 13.7.5.1 states that the error should be a
SyntaxError, when previously CheckAndRewriteReferenceExpression
would always emit a ReferenceError.
BUG=v8:4373
R=adamk, rossberg
LOG=N
Please review this at https://codereview.chromium.org/1292393002/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+40, -13 lines):
M src/parser.h
M src/parser.cc
M src/preparser.h
M src/preparser.cc
M test/message/for-loop-invalid-lhs.out
M test/message/new-target-for-loop.out
M test/mjsunit/harmony/new-target.js
M test/mjsunit/invalid-lhs.js
Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index
301bda94c4011987aff1a704a5f2e6f84d79c137..b7ccff4dd03d0d316e7aa106ba54b2dcbf2eb7a5
100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -658,6 +658,13 @@ Expression*
ParserTraits::NewThrowSyntaxError(MessageTemplate::Template message,
}
+Expression* ParserTraits::NewThrowSyntaxError(MessageTemplate::Template
message,
+ int pos) {
+ return NewThrowError(Runtime::kNewSyntaxError, message,
+ parser_->ast_value_factory()->empty_string(), pos);
+}
+
+
Expression* ParserTraits::NewThrowTypeError(MessageTemplate::Template
message,
const AstRawString* arg, int
pos) {
return NewThrowError(Runtime::kNewTypeError, message, arg, pos);
@@ -3693,7 +3700,7 @@ Statement* Parser::ParseForStatement(ZoneList<const
AstRawString*>* labels,
if (!*ok) return nullptr;
expression = this->CheckAndRewriteReferenceExpression(
expression, lhs_beg_pos, lhs_end_pos,
- MessageTemplate::kInvalidLhsInFor, CHECK_OK);
+ MessageTemplate::kInvalidLhsInFor, kSyntaxError, CHECK_OK);
ForEachStatement* loop =
factory()->NewForEachStatement(mode, labels, stmt_pos);
Index: src/parser.h
diff --git a/src/parser.h b/src/parser.h
index
4a77d91b0ac859aeb6b938c40c36e4b00d5d37f1..81ddf73482ff77da4ff8d5229db548489660aea8
100644
--- a/src/parser.h
+++ b/src/parser.h
@@ -688,6 +688,7 @@ class ParserTraits {
// which case no arguments are passed to the constructor.
Expression* NewThrowSyntaxError(MessageTemplate::Template message,
const AstRawString* arg, int pos);
+ Expression* NewThrowSyntaxError(MessageTemplate::Template message, int
pos);
// Generate AST node that throws a TypeError with the given
// type. Both arguments must be non-null (in the handle sense).
Index: src/preparser.cc
diff --git a/src/preparser.cc b/src/preparser.cc
index
9a30ca58cc71b18031b17ff9cd152e95826a50f1..b1541f616afe133554881e99aec4f71a9f5e45e0
100644
--- a/src/preparser.cc
+++ b/src/preparser.cc
@@ -922,7 +922,7 @@ PreParser::Statement PreParser::ParseForStatement(bool*
ok) {
if (!*ok) return Statement::Default();
lhs = CheckAndRewriteReferenceExpression(
lhs, lhs_beg_pos, lhs_end_pos,
MessageTemplate::kInvalidLhsInFor,
- CHECK_OK);
+ kSyntaxError, CHECK_OK);
ParseExpression(true, CHECK_OK);
Expect(Token::RPAREN, CHECK_OK);
ParseSubStatement(CHECK_OK);
Index: src/preparser.h
diff --git a/src/preparser.h b/src/preparser.h
index
249eed983c4f4c08485073b60f67651f5a3cb407..1051f97b8af2554510dc0ffd6fe589958511a1b3
100644
--- a/src/preparser.h
+++ b/src/preparser.h
@@ -714,6 +714,9 @@ class ParserBase : public Traits {
ExpressionT CheckAndRewriteReferenceExpression(
ExpressionT expression, int beg_pos, int end_pos,
MessageTemplate::Template message, bool* ok);
+ ExpressionT CheckAndRewriteReferenceExpression(
+ ExpressionT expression, int beg_pos, int end_pos,
+ MessageTemplate::Template message, ParseErrorType type, bool* ok);
// Used to validate property names in object literals and class literals
enum PropertyKind {
@@ -1473,6 +1476,10 @@ class PreParserTraits {
Handle<Object> arg, int pos) {
return PreParserExpression::Default();
}
+ PreParserExpression NewThrowSyntaxError(MessageTemplate::Template
message,
+ int pos) {
+ return PreParserExpression::Default();
+ }
PreParserExpression NewThrowTypeError(MessageTemplate::Template message,
Handle<Object> arg, int pos) {
return PreParserExpression::Default();
@@ -3937,6 +3944,16 @@ typename ParserBase<Traits>::ExpressionT
ParserBase<Traits>::CheckAndRewriteReferenceExpression(
ExpressionT expression, int beg_pos, int end_pos,
MessageTemplate::Template message, bool* ok) {
+ return this->CheckAndRewriteReferenceExpression(expression, beg_pos,
end_pos,
+ message,
kReferenceError, ok);
+}
+
+
+template <typename Traits>
+typename ParserBase<Traits>::ExpressionT
+ParserBase<Traits>::CheckAndRewriteReferenceExpression(
+ ExpressionT expression, int beg_pos, int end_pos,
+ MessageTemplate::Template message, ParseErrorType type, bool* ok) {
Scanner::Location location(beg_pos, end_pos);
if (this->IsIdentifier(expression)) {
if (is_strict(language_mode()) &&
@@ -3960,10 +3977,12 @@
ParserBase<Traits>::CheckAndRewriteReferenceExpression(
// If it is a call, make it a runtime error for legacy web
compatibility.
// Rewrite `expr' to `expr[throw ReferenceError]'.
int pos = location.beg_pos;
- ExpressionT error = this->NewThrowReferenceError(message, pos);
+ ExpressionT error = type == kReferenceError
+ ? this->NewThrowReferenceError(message, pos)
+ : this->NewThrowSyntaxError(message, pos);
return factory()->NewProperty(expression, error, pos);
} else {
- this->ReportMessageAt(location, message, kReferenceError);
+ this->ReportMessageAt(location, message, type);
*ok = false;
return this->EmptyExpression();
}
Index: test/message/for-loop-invalid-lhs.out
diff --git a/test/message/for-loop-invalid-lhs.out
b/test/message/for-loop-invalid-lhs.out
index
105eb6461cb11e54f4e8f5c8176eea4b70a520ef..441ba3b60c680a729d986ae92312326b874852be
100644
--- a/test/message/for-loop-invalid-lhs.out
+++ b/test/message/for-loop-invalid-lhs.out
@@ -1,5 +1,5 @@
-*%(basename)s:9: ReferenceError: Invalid left-hand side in for-loop
+*%(basename)s:9: SyntaxError: Invalid left-hand side in for-loop
function f() { for ("unassignable" in {}); }
^^^^^^^^^^^^^^
-ReferenceError: Invalid left-hand side in for-loop
+SyntaxError: Invalid left-hand side in for-loop
Index: test/message/new-target-for-loop.out
diff --git a/test/message/new-target-for-loop.out
b/test/message/new-target-for-loop.out
index
30f1f75d7e33b048904cf242b32f14e510a1ed1a..342b1315e999897bccbf857c872f1eba531cc6e6
100644
--- a/test/message/new-target-for-loop.out
+++ b/test/message/new-target-for-loop.out
@@ -1,4 +1,4 @@
-*%(basename)s:7: ReferenceError: Invalid left-hand side in for-loop
+*%(basename)s:7: SyntaxError: Invalid left-hand side in for-loop
function f() { for (new.target in {}); }
^^^^^^^^^^
-ReferenceError: Invalid left-hand side in for-loop
+SyntaxError: Invalid left-hand side in for-loop
Index: test/mjsunit/harmony/new-target.js
diff --git a/test/mjsunit/harmony/new-target.js
b/test/mjsunit/harmony/new-target.js
index
a1f020b00546bfa54c740fb17da35261aec8231d..d98f5f809860d2e9854aeb18326593ec6af1fce9
100644
--- a/test/mjsunit/harmony/new-target.js
+++ b/test/mjsunit/harmony/new-target.js
@@ -394,5 +394,5 @@
assertThrows(function() { Function("--new.target"); }, ReferenceError);
assertThrows(function() { Function("(new.target)++"); }, ReferenceError);
assertThrows(function() { Function("++(new.target)"); }, ReferenceError);
- assertThrows(function() { Function("for (new.target of {});"); },
ReferenceError);
+ assertThrows(function() { Function("for (new.target of {});"); },
SyntaxError);
})();
Index: test/mjsunit/invalid-lhs.js
diff --git a/test/mjsunit/invalid-lhs.js b/test/mjsunit/invalid-lhs.js
index
52ee89582a4d059bbf3b92cb05a627ed36a39aa0..61636f5e7eb7d72f636a6f4dd1d95b56ec29e410
100644
--- a/test/mjsunit/invalid-lhs.js
+++ b/test/mjsunit/invalid-lhs.js
@@ -50,9 +50,9 @@ assertDoesNotThrow("if (false) ++(eval('12'))",
ReferenceError);
assertDoesNotThrow("if (false) (eval('12'))++", ReferenceError);
// For in:
-assertThrows("for (12 in [1]) print(12);", ReferenceError);
-assertThrows("for (eval('var x') in [1]) print(12);", ReferenceError);
-assertThrows("if (false) for (12 in [1]) print(12);", ReferenceError);
+assertThrows("for (12 in [1]) print(12);", SyntaxError);
+assertThrows("for (eval('var x') in [1]) print(12);", SyntaxError);
+assertThrows("if (false) for (12 in [1]) print(12);", SyntaxError);
assertDoesNotThrow("if (false) for (eval('0') in [1]) print(12);",
ReferenceError);
// For:
@@ -64,7 +64,7 @@ assertDoesNotThrow("if (false) for (eval('var x') = 1;;)
print(12);", ReferenceE
// Assignments to 'this'.
assertThrows("this = 42", ReferenceError);
assertThrows("function f() { this = 12; }", ReferenceError);
-assertThrows("for (this in {x:3, y:4, z:5}) ;", ReferenceError);
+assertThrows("for (this in {x:3, y:4, z:5}) ;", SyntaxError);
assertThrows("for (this = 0;;) ;", ReferenceError);
assertThrows("this++", ReferenceError);
assertThrows("++this", ReferenceError);
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
