Revision: 19750
Author: [email protected]
Date: Mon Mar 10 11:42:17 2014 UTC
Log: Unify (Pre)Parser::ParseObjectLiteral and add tests.
Notes:
- The regexp in the ParseObjectLiteralComment was wrong, made it less wrong
(
it's still wrong since trailing commas are not required / allowed).
- Change in logic: In case we have "get somekeyword() { }",
the "somekeyword"
was not logged as a symbol by PreParser and not expected in the preparser
data
by Parser. This is unnecessary complication; in other contexts where
keywords
are allowed as identifiers, they are logged as symbols (see
ParseIdentifierName).
BUG=v8:3126
LOG=N
[email protected]
Review URL: https://codereview.chromium.org/173273006
http://code.google.com/p/v8/source/detail?r=19750
Modified:
/branches/bleeding_edge/src/parser.cc
/branches/bleeding_edge/src/preparser.cc
/branches/bleeding_edge/test/cctest/test-parsing.cc
=======================================
--- /branches/bleeding_edge/src/parser.cc Thu Feb 20 11:35:37 2014 UTC
+++ /branches/bleeding_edge/src/parser.cc Mon Mar 10 11:42:17 2014 UTC
@@ -3492,10 +3492,11 @@
Expression* Parser::ParseObjectLiteral(bool* ok) {
// ObjectLiteral ::
- // '{' (
- // ((IdentifierName | String | Number) ':' AssignmentExpression)
- // | (('get' | 'set') (IdentifierName | String | Number)
FunctionLiteral)
- // )*[','] '}'
+ // '{' ((
+ // ((IdentifierName | String | Number) ':' AssignmentExpression) |
+ // (('get' | 'set') (IdentifierName | String | Number)
FunctionLiteral)
+ // ) ',')* '}'
+ // (Except that trailing comma is not required and not allowed.)
int pos = peek_position();
ZoneList<ObjectLiteral::Property*>* properties =
@@ -3529,14 +3530,12 @@
// { ... , get foo() { ... }, ... , set foo(v) { ... v ...
} , ... }
// We have already read the "get" or "set" keyword.
Token::Value next = Next();
- bool is_keyword = Token::IsKeyword(next);
if (next != i::Token::IDENTIFIER &&
next != i::Token::FUTURE_RESERVED_WORD &&
next != i::Token::FUTURE_STRICT_RESERVED_WORD &&
next != i::Token::NUMBER &&
next != i::Token::STRING &&
- !is_keyword) {
- // Unexpected token.
+ !Token::IsKeyword(next)) {
ReportUnexpectedToken(next);
*ok = false;
return NULL;
@@ -3544,9 +3543,7 @@
// Validate the property.
PropertyKind type = is_getter ? kGetterProperty :
kSetterProperty;
checker.CheckProperty(next, type, CHECK_OK);
- Handle<String> name = is_keyword
- ?
isolate_->factory()->InternalizeUtf8String(Token::String(next))
- : GetSymbol();
+ Handle<String> name = GetSymbol();
FunctionLiteral* value =
ParseFunctionLiteral(name,
scanner()->location(),
@@ -3571,8 +3568,8 @@
}
continue; // restart the while
}
- // Failed to parse as get/set property, so it's just a property
- // called "get" or "set".
+ // Failed to parse as get/set property, so it's just a normal
property
+ // (which might be called "get" or "set" or something else).
key = factory()->NewLiteral(id, next_pos);
break;
}
@@ -3604,7 +3601,6 @@
Handle<String> string = GetSymbol();
key = factory()->NewLiteral(string, next_pos);
} else {
- // Unexpected token.
Token::Value next = Next();
ReportUnexpectedToken(next);
*ok = false;
=======================================
--- /branches/bleeding_edge/src/preparser.cc Thu Feb 20 11:35:37 2014 UTC
+++ /branches/bleeding_edge/src/preparser.cc Mon Mar 10 11:42:17 2014 UTC
@@ -1124,10 +1124,11 @@
PreParser::Expression PreParser::ParseObjectLiteral(bool* ok) {
// ObjectLiteral ::
- // '{' (
- // ((IdentifierName | String | Number) ':' AssignmentExpression)
- // | (('get' | 'set') (IdentifierName | String | Number)
FunctionLiteral)
- // )*[','] '}'
+ // '{' ((
+ // ((IdentifierName | String | Number) ':' AssignmentExpression) |
+ // (('get' | 'set') (IdentifierName | String | Number)
FunctionLiteral)
+ // ) ',')* '}'
+ // (Except that trailing comma is not required and not allowed.)
ObjectLiteralChecker checker(this, scope_->language_mode());
@@ -1142,23 +1143,22 @@
bool is_setter = false;
ParseIdentifierNameOrGetOrSet(&is_getter, &is_setter, CHECK_OK);
if ((is_getter || is_setter) && peek() != Token::COLON) {
- Token::Value name = Next();
- bool is_keyword = Token::IsKeyword(name);
- if (name != Token::IDENTIFIER &&
- name != Token::FUTURE_RESERVED_WORD &&
- name != Token::FUTURE_STRICT_RESERVED_WORD &&
- name != Token::NUMBER &&
- name != Token::STRING &&
- !is_keyword) {
+ Token::Value next = Next();
+ if (next != Token::IDENTIFIER &&
+ next != Token::FUTURE_RESERVED_WORD &&
+ next != Token::FUTURE_STRICT_RESERVED_WORD &&
+ next != Token::NUMBER &&
+ next != Token::STRING &&
+ !Token::IsKeyword(next)) {
+ ReportUnexpectedToken(next);
*ok = false;
return Expression::Default();
}
- if (!is_keyword) {
- LogSymbol();
- }
+ // Validate the property
PropertyKind type = is_getter ? kGetterProperty :
kSetterProperty;
- checker.CheckProperty(name, type, CHECK_OK);
- ParseFunctionLiteral(Identifier::Default(),
+ checker.CheckProperty(next, type, CHECK_OK);
+ PreParserIdentifier name = GetSymbol(scanner());
+ ParseFunctionLiteral(name,
scanner()->location(),
false, // reserved words are allowed here
false, // not a generator
@@ -1168,29 +1168,29 @@
}
continue; // restart the while
}
- checker.CheckProperty(next, kValueProperty, CHECK_OK);
break;
}
case Token::STRING:
Consume(next);
- checker.CheckProperty(next, kValueProperty, CHECK_OK);
LogSymbol();
break;
case Token::NUMBER:
Consume(next);
- checker.CheckProperty(next, kValueProperty, CHECK_OK);
break;
default:
if (Token::IsKeyword(next)) {
Consume(next);
- checker.CheckProperty(next, kValueProperty, CHECK_OK);
LogSymbol();
} else {
- // Unexpected token.
+ Token::Value next = Next();
+ ReportUnexpectedToken(next);
*ok = false;
return Expression::Default();
}
}
+
+ // Validate the property
+ checker.CheckProperty(next, kValueProperty, CHECK_OK);
Expect(Token::COLON, CHECK_OK);
ParseAssignmentExpression(true, CHECK_OK);
=======================================
--- /branches/bleeding_edge/test/cctest/test-parsing.cc Fri Mar 7 08:43:54
2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-parsing.cc Mon Mar 10 11:42:17
2014 UTC
@@ -1962,7 +1962,6 @@
// These tests make sure that PreParser doesn't start producing less
data.
v8::V8::Initialize();
-
int marker;
CcTest::i_isolate()->stack_guard()->SetStackLimit(
reinterpret_cast<uintptr_t>(&marker) - 128 * 1024);
@@ -1972,9 +1971,18 @@
int symbols;
int functions;
} test_cases[] = {
- // Labels, variables and functions are recorded as symbols.
+ // Labels and variables are recorded as symbols.
{"{label: 42}", 1, 0}, {"{label: 42; label2: 43}", 2, 0},
{"var x = 42;", 1, 0}, {"var x = 42, y = 43;", 2, 0},
+ {"var x = {y: 1};", 2, 0},
+ {"var x = {}; x.y = 1", 2, 0},
+ // "get" is recorded as a symbol too.
+ {"var x = {get foo(){} };", 3, 1},
+ // When keywords are used as identifiers, they're logged as symbols,
too:
+ {"var x = {if: 1};", 2, 0},
+ {"var x = {}; x.if = 1", 2, 0},
+ {"var x = {get if(){} };", 3, 1},
+ // Functions
{"function foo() {}", 1, 1}, {"function foo() {} function bar() {}",
2, 2},
// Labels, variables and functions insize lazy functions are not
recorded.
{"function lazy() { var a, b, c; }", 1, 1},
@@ -2202,3 +2210,111 @@
RunParserSyncTest(context_data, statement_data, kError);
}
+
+
+TEST(StrictObjectLiteralChecking) {
+ const char* strict_context_data[][2] = {
+ {"\"use strict\"; var myobject = {", "};"},
+ { NULL, NULL }
+ };
+ const char* non_strict_context_data[][2] = {
+ {"var myobject = {", "};"},
+ { NULL, NULL }
+ };
+
+ // These are only errors in strict mode.
+ const char* statement_data[] = {
+ "foo: 1, foo: 2",
+ "\"foo\": 1, \"foo\": 2",
+ "foo: 1, \"foo\": 2",
+ "1: 1, 1: 2",
+ "1: 1, \"1\": 2",
+ "get: 1, get: 2", // Not a getter for real, just a property called
get.
+ "set: 1, set: 2", // Not a setter for real, just a property called
set.
+ NULL
+ };
+
+ RunParserSyncTest(non_strict_context_data, statement_data, kSuccess);
+ RunParserSyncTest(strict_context_data, statement_data, kError);
+}
+
+
+TEST(ErrorsObjectLiteralChecking) {
+ const char* context_data[][2] = {
+ {"\"use strict\"; var myobject = {", "};"},
+ {"var myobject = {", "};"},
+ { NULL, NULL }
+ };
+
+ const char* statement_data[] = {
+ "foo: 1, get foo() {}",
+ "foo: 1, set foo() {}",
+ "\"foo\": 1, get \"foo\"() {}",
+ "\"foo\": 1, set \"foo\"() {}",
+ "1: 1, get 1() {}",
+ "1: 1, set 1() {}",
+ // It's counter-intuitive, but these collide too (even in classic
+ // mode). Note that we can have "foo" and foo as properties in classic
mode,
+ // but we cannot have "foo" and get foo, or foo and get "foo".
+ "foo: 1, get \"foo\"() {}",
+ "foo: 1, set \"foo\"() {}",
+ "\"foo\": 1, get foo() {}",
+ "\"foo\": 1, set foo() {}",
+ "1: 1, get \"1\"() {}",
+ "1: 1, set \"1\"() {}",
+ "\"1\": 1, get 1() {}"
+ "\"1\": 1, set 1() {}"
+ // Parsing FunctionLiteral for getter or setter fails
+ "get foo( +",
+ "get foo() \"error\"",
+ NULL
+ };
+
+ RunParserSyncTest(context_data, statement_data, kError);
+}
+
+
+TEST(NoErrorsObjectLiteralChecking) {
+ const char* context_data[][2] = {
+ {"var myobject = {", "};"},
+ {"\"use strict\"; var myobject = {", "};"},
+ { NULL, NULL }
+ };
+
+ const char* statement_data[] = {
+ "foo: 1, bar: 2",
+ "\"foo\": 1, \"bar\": 2",
+ "1: 1, 2: 2",
+ // Syntax: IdentifierName ':' AssignmentExpression
+ "foo: bar = 5 + baz",
+ // Syntax: 'get' (IdentifierName | String | Number) FunctionLiteral
+ "get foo() {}",
+ "get \"foo\"() {}",
+ "get 1() {}",
+ // Syntax: 'set' (IdentifierName | String | Number) FunctionLiteral
+ "set foo() {}",
+ "set \"foo\"() {}",
+ "set 1() {}",
+ // Non-colliding getters and setters -> no errors
+ "foo: 1, get bar() {}",
+ "foo: 1, set bar(b) {}",
+ "\"foo\": 1, get \"bar\"() {}",
+ "\"foo\": 1, set \"bar\"() {}",
+ "1: 1, get 2() {}",
+ "1: 1, set 2() {}",
+ // Weird number of parameters -> no errors
+ "get bar() {}, set bar() {}",
+ "get bar(x) {}, set bar(x) {}",
+ "get bar(x, y) {}, set bar(x, y) {}",
+ // Keywords, future reserved and strict future reserved are also
allowed as
+ // property names.
+ "if: 4",
+ "interface: 5",
+ "super: 6",
+ "eval: 7",
+ "arguments: 8",
+ NULL
+ };
+
+ RunParserSyncTest(context_data, statement_data, kSuccess);
+}
--
--
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.
