Revision: 23284
Author: [email protected]
Date: Thu Aug 21 15:32:22 2014 UTC
Log: Add back the duplicate property checker
We're not quite ready to make this change.
BUG=v8:3498
LOG=Y
[email protected]
Review URL: https://codereview.chromium.org/491053002
http://code.google.com/p/v8/source/detail?r=23284
Modified:
/branches/bleeding_edge/src/preparser.h
/branches/bleeding_edge/test/cctest/test-parsing.cc
/branches/bleeding_edge/test/mjsunit/strict-mode.js
/branches/bleeding_edge/test/test262/test262.status
/branches/bleeding_edge/test/webkit/object-literal-syntax-expected.txt
/branches/bleeding_edge/test/webkit/object-literal-syntax.js
=======================================
--- /branches/bleeding_edge/src/preparser.h Thu Aug 21 09:22:08 2014 UTC
+++ /branches/bleeding_edge/src/preparser.h Thu Aug 21 15:32:22 2014 UTC
@@ -130,6 +130,7 @@
};
class CheckpointBase;
+ class ObjectLiteralChecker;
//
---------------------------------------------------------------------------
// FunctionState and BlockState together implement the parser's scope
stack.
@@ -473,7 +474,8 @@
ExpressionT ParseExpression(bool accept_IN, bool* ok);
ExpressionT ParseArrayLiteral(bool* ok);
ExpressionT ParseObjectLiteral(bool* ok);
- ObjectLiteralPropertyT ParsePropertyDefinition(bool* ok);
+ ObjectLiteralPropertyT ParsePropertyDefinition(ObjectLiteralChecker*
checker,
+ bool* ok);
typename Traits::Type::ExpressionList ParseArguments(bool* ok);
ExpressionT ParseAssignmentExpression(bool accept_IN, bool* ok);
ExpressionT ParseYieldExpression(bool* ok);
@@ -496,6 +498,60 @@
ExpressionT expression,
Scanner::Location location, const char* message, bool* ok);
+ // Used to detect duplicates in object literals. Each of the values
+ // kGetterProperty, kSetterProperty and kValueProperty represents
+ // a type of object literal property. When parsing a property, its
+ // type value is stored in the DuplicateFinder for the property name.
+ // Values are chosen so that having intersection bits means the there is
+ // an incompatibility.
+ // I.e., you can add a getter to a property that already has a setter,
since
+ // kGetterProperty and kSetterProperty doesn't intersect, but not if it
+ // already has a getter or a value. Adding the getter to an existing
+ // setter will store the value (kGetterProperty | kSetterProperty), which
+ // is incompatible with adding any further properties.
+ enum PropertyKind {
+ kNone = 0,
+ // Bit patterns representing different object literal property types.
+ kGetterProperty = 1,
+ kSetterProperty = 2,
+ kValueProperty = 7,
+ // Helper constants.
+ kValueFlag = 4
+ };
+
+ // Validation per ECMA 262 - 11.1.5 "Object Initializer".
+ class ObjectLiteralChecker {
+ public:
+ ObjectLiteralChecker(ParserBase* parser, StrictMode strict_mode)
+ : parser_(parser),
+ finder_(scanner()->unicode_cache()),
+ strict_mode_(strict_mode) {}
+
+ void CheckProperty(Token::Value property, PropertyKind type, bool* ok);
+
+ private:
+ ParserBase* parser() const { return parser_; }
+ Scanner* scanner() const { return parser_->scanner(); }
+
+ // Checks the type of conflict based on values coming from
PropertyType.
+ bool HasConflict(PropertyKind type1, PropertyKind type2) {
+ return (type1 & type2) != 0;
+ }
+ bool IsDataDataConflict(PropertyKind type1, PropertyKind type2) {
+ return ((type1 & type2) & kValueFlag) != 0;
+ }
+ bool IsDataAccessorConflict(PropertyKind type1, PropertyKind type2) {
+ return ((type1 ^ type2) & kValueFlag) != 0;
+ }
+ bool IsAccessorAccessorConflict(PropertyKind type1, PropertyKind
type2) {
+ return ((type1 | type2) & kValueFlag) == 0;
+ }
+
+ ParserBase* parser_;
+ DuplicateFinder finder_;
+ StrictMode strict_mode_;
+ };
+
// If true, the next (and immediately following) function literal is
// preceded by a parenthesis.
// Heuristically that means that the function will be called immediately,
@@ -1793,8 +1849,8 @@
template <class Traits>
-typename ParserBase<Traits>::ObjectLiteralPropertyT
-ParserBase<Traits>::ParsePropertyDefinition(bool* ok) {
+typename ParserBase<Traits>::ObjectLiteralPropertyT ParserBase<
+ Traits>::ParsePropertyDefinition(ObjectLiteralChecker* checker, bool*
ok) {
LiteralT key = this->EmptyLiteral();
Token::Value next = peek();
int next_pos = peek_position();
@@ -1846,6 +1902,10 @@
name = ParseIdentifierName(
CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
}
+ // Validate the property.
+ PropertyKind type = is_getter ? kGetterProperty : kSetterProperty;
+ checker->CheckProperty(next, type,
+
CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
typename Traits::Type::FunctionLiteral value =
this->ParseFunctionLiteral(
name, scanner()->location(),
@@ -1862,6 +1922,10 @@
key = factory()->NewStringLiteral(id, next_pos);
}
}
+
+ // Validate the property
+ checker->CheckProperty(next, kValueProperty,
+ CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
Expect(Token::COLON, CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
ExpressionT value = this->ParseAssignmentExpression(
@@ -1883,12 +1947,15 @@
int number_of_boilerplate_properties = 0;
bool has_function = false;
+ ObjectLiteralChecker checker(this, strict_mode());
+
Expect(Token::LBRACE, CHECK_OK);
while (peek() != Token::RBRACE) {
if (fni_ != NULL) fni_->Enter();
- ObjectLiteralPropertyT property =
this->ParsePropertyDefinition(CHECK_OK);
+ ObjectLiteralPropertyT property =
+ this->ParsePropertyDefinition(&checker, CHECK_OK);
// Mark top-level object literals that contain function literals and
// pretenure the literal so it can be added as a constant function
@@ -2572,6 +2639,32 @@
#undef CHECK_OK_CUSTOM
+template <typename Traits>
+void ParserBase<Traits>::ObjectLiteralChecker::CheckProperty(
+ Token::Value property, PropertyKind type, bool* ok) {
+ int old;
+ if (property == Token::NUMBER) {
+ old = scanner()->FindNumber(&finder_, type);
+ } else {
+ old = scanner()->FindSymbol(&finder_, type);
+ }
+ PropertyKind old_type = static_cast<PropertyKind>(old);
+ if (HasConflict(old_type, type)) {
+ if (IsDataDataConflict(old_type, type)) {
+ // Both are data properties.
+ if (strict_mode_ == SLOPPY) return;
+ parser()->ReportMessage("strict_duplicate_property");
+ } else if (IsDataAccessorConflict(old_type, type)) {
+ // Both a data and an accessor property with the same name.
+ parser()->ReportMessage("accessor_data_property");
+ } else {
+ DCHECK(IsAccessorAccessorConflict(old_type, type));
+ // Both accessors of the same type.
+ parser()->ReportMessage("accessor_get_set");
+ }
+ *ok = false;
+ }
+}
} } // v8::internal
#endif // V8_PREPARSER_H
=======================================
--- /branches/bleeding_edge/test/cctest/test-parsing.cc Wed Aug 20 14:25:48
2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-parsing.cc Thu Aug 21 15:32:22
2014 UTC
@@ -2486,7 +2486,7 @@
{ NULL, NULL }
};
- // ES6 allows duplicate properties even in strict mode.
+ // These are only errors in strict mode.
const char* statement_data[] = {
"foo: 1, foo: 2",
"\"foo\": 1, \"foo\": 2",
@@ -2499,7 +2499,7 @@
};
RunParserSyncTest(non_strict_context_data, statement_data, kSuccess);
- RunParserSyncTest(strict_context_data, statement_data, kSuccess);
+ RunParserSyncTest(strict_context_data, statement_data, kError);
}
@@ -2511,17 +2511,23 @@
};
const char* statement_data[] = {
- ",",
- // Wrong number of parameters
- "get bar(x) {}",
- "get bar(x, y) {}",
- "set bar() {}",
- "set bar(x, y) {}",
- // Parsing FunctionLiteral for getter or setter fails
- "get foo( +",
- "get foo() \"error\"",
- NULL
- };
+ ",", "foo: 1, get foo() {}", "foo: 1, set foo(v) {}",
+ "\"foo\": 1, get \"foo\"() {}", "\"foo\": 1, set \"foo\"(v) {}",
+ "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\"(v) {}",
+ "\"foo\": 1, get foo() {}", "\"foo\": 1, set foo(v) {}",
+ "1: 1, get \"1\"() {}", "1: 1, set \"1\"() {}",
+ "\"1\": 1, get 1() {}"
+ "\"1\": 1, set 1(v) {}"
+ // Wrong number of parameters
+ "get bar(x) {}",
+ "get bar(x, y) {}", "set bar() {}", "set bar(x, y) {}",
+ // Parsing FunctionLiteral for getter or setter fails
+ "get foo( +", "get foo() \"error\"", NULL};
RunParserSyncTest(context_data, statement_data, kError);
}
@@ -2565,24 +2571,6 @@
"super: 6",
"eval: 7",
"arguments: 8",
- // Duplicate property names are allowed in ES6.
- "foo: 1, get foo() {}",
- "foo: 1, set foo(v) {}",
- "\"foo\": 1, get \"foo\"() {}",
- "\"foo\": 1, set \"foo\"(v) {}",
- "1: 1, get 1() {}",
- "1: 1, set 1(v) {}",
- // 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\"(v) {}",
- "\"foo\": 1, get foo() {}",
- "\"foo\": 1, set foo(v) {}",
- "1: 1, get \"1\"() {}",
- "1: 1, set \"1\"(v) {}",
- "\"1\": 1, get 1() {}",
- "\"1\": 1, set 1(v) {}",
NULL
};
=======================================
--- /branches/bleeding_edge/test/mjsunit/strict-mode.js Wed Aug 20 14:25:48
2014 UTC
+++ /branches/bleeding_edge/test/mjsunit/strict-mode.js Thu Aug 21 15:32:22
2014 UTC
@@ -166,6 +166,17 @@
"use strict";\
}', SyntaxError);
+// Duplicate data properties.
+CheckStrictMode("var x = { dupe : 1, nondupe: 3, dupe : 2 };",
SyntaxError);
+CheckStrictMode("var x = { '1234' : 1, '2345' : 2, '1234' : 3 };",
SyntaxError);
+CheckStrictMode("var x = { '1234' : 1, '2345' : 2, 1234 : 3 };",
SyntaxError);
+CheckStrictMode("var x = { 3.14 : 1, 2.71 : 2, 3.14 : 3 };", SyntaxError);
+CheckStrictMode("var x = { 3.14 : 1, '3.14' : 2 };", SyntaxError);
+CheckStrictMode("var x = { \
+ 123: 1, \
+
123.00000000000000000000000000000000000000000000000000000000000000000001: 2
\
+}", SyntaxError);
+
// Non-conflicting data properties.
(function StrictModeNonDuplicate() {
"use strict";
@@ -177,52 +188,37 @@
};
})();
-// Duplicate properties are no longer errors in ES6.
-(function Duplicates() {
- "use strict";
+// Two getters (non-strict)
+assertThrows("var x = { get foo() { }, get foo() { } };", SyntaxError);
+assertThrows("var x = { get foo(){}, get 'foo'(){}};", SyntaxError);
+assertThrows("var x = { get 12(){}, get '12'(){}};", SyntaxError);
- ({ dupe : 1, nondupe: 3, dupe : 2 });
- ({ '1234' : 1, '2345' : 2, '1234' : 3 });
- ({ '1234' : 1, '2345' : 2, 1234 : 3 });
- ({ 3.14 : 1, 2.71 : 2, 3.14 : 3 });
- ({ 3.14 : 1, '3.14' : 2 });
- ({
- 123: 1,
-
123.00000000000000000000000000000000000000000000000000000000000000000001: 2
- });
+// Two setters (non-strict)
+assertThrows("var x = { set foo(v) { }, set foo(v) { } };", SyntaxError);
+assertThrows("var x = { set foo(v) { }, set 'foo'(v) { } };", SyntaxError);
+assertThrows("var x = { set 13(v) { }, set '13'(v) { } };", SyntaxError);
- // Two getters
- ({ get foo() { }, get foo() { } });
- ({ get foo(){}, get 'foo'(){}});
- ({ get 12(){}, get '12'(){}});
+// Setter and data (non-strict)
+assertThrows("var x = { foo: 'data', set foo(v) { } };", SyntaxError);
+assertThrows("var x = { set foo(v) { }, foo: 'data' };", SyntaxError);
+assertThrows("var x = { foo: 'data', set 'foo'(v) { } };", SyntaxError);
+assertThrows("var x = { set foo(v) { }, 'foo': 'data' };", SyntaxError);
+assertThrows("var x = { 'foo': 'data', set foo(v) { } };", SyntaxError);
+assertThrows("var x = { set 'foo'(v) { }, foo: 'data' };", SyntaxError);
+assertThrows("var x = { 'foo': 'data', set 'foo'(v) { } };", SyntaxError);
+assertThrows("var x = { set 'foo'(v) { }, 'foo': 'data' };", SyntaxError);
+assertThrows("var x = { 12: 1, set '12'(v){}};", SyntaxError);
+assertThrows("var x = { 12: 1, set 12(v){}};", SyntaxError);
+assertThrows("var x = { '12': 1, set '12'(v){}};", SyntaxError);
+assertThrows("var x = { '12': 1, set 12(v){}};", SyntaxError);
- // Two setters
- ({ set foo(v) { }, set foo(v) { } });
- ({ set foo(v) { }, set 'foo'(v) { } });
- ({ set 13(v) { }, set '13'(v) { } });
-
- // Setter and data
- ({ foo: 'data', set foo(v) { } });
- ({ set foo(v) { }, foo: 'data' });
- ({ foo: 'data', set 'foo'(v) { } });
- ({ set foo(v) { }, 'foo': 'data' });
- ({ 'foo': 'data', set foo(v) { } });
- ({ set 'foo'(v) { }, foo: 'data' });
- ({ 'foo': 'data', set 'foo'(v) { } });
- ({ set 'foo'(v) { }, 'foo': 'data' });
- ({ 12: 1, set '12'(v){}});
- ({ 12: 1, set 12(v){}});
- ({ '12': 1, set '12'(v){}});
- ({ '12': 1, set 12(v){}});
-
- // Getter and data
- ({ foo: 'data', get foo() { } });
- ({ get foo() { }, foo: 'data' });
- ({ 'foo': 'data', get foo() { } });
- ({ get 'foo'() { }, 'foo': 'data' });
- ({ '12': 1, get '12'(){}});
- ({ '12': 1, get 12(){}});
-})();
+// Getter and data (non-strict)
+assertThrows("var x = { foo: 'data', get foo() { } };", SyntaxError);
+assertThrows("var x = { get foo() { }, foo: 'data' };", SyntaxError);
+assertThrows("var x = { 'foo': 'data', get foo() { } };", SyntaxError);
+assertThrows("var x = { get 'foo'() { }, 'foo': 'data' };", SyntaxError);
+assertThrows("var x = { '12': 1, get '12'(){}};", SyntaxError);
+assertThrows("var x = { '12': 1, get 12(){}};", SyntaxError);
// Assignment to eval or arguments
CheckStrictMode("function strict() { eval = undefined; }", SyntaxError);
=======================================
--- /branches/bleeding_edge/test/test262/test262.status Wed Aug 20 15:24:53
2014 UTC
+++ /branches/bleeding_edge/test/test262/test262.status Thu Aug 21 15:32:22
2014 UTC
@@ -77,17 +77,6 @@
'S8.5_A2.1': [PASS, FAIL_OK],
'S8.5_A2.2': [PASS, FAIL_OK],
- # ES6 allows duplicate properties
- '11.1.5-4-4-a-1-s': [FAIL],
- '11.1.5_4-4-b-1': [FAIL],
- '11.1.5_4-4-b-2': [FAIL],
- '11.1.5_4-4-c-1': [FAIL],
- '11.1.5_4-4-c-2': [FAIL],
- '11.1.5_4-4-d-1': [FAIL],
- '11.1.5_4-4-d-2': [FAIL],
- '11.1.5_4-4-d-3': [FAIL],
- '11.1.5_4-4-d-4': [FAIL],
-
############################ INVALID TESTS #############################
# The reference value calculated by Test262 is incorrect if you run these
=======================================
--- /branches/bleeding_edge/test/webkit/object-literal-syntax-expected.txt
Wed Aug 20 14:25:48 2014 UTC
+++ /branches/bleeding_edge/test/webkit/object-literal-syntax-expected.txt
Thu Aug 21 15:32:22 2014 UTC
@@ -26,20 +26,20 @@
On success, you will see a series of "PASS" messages, followed by "TEST
COMPLETE".
-PASS ({a:1, get a(){}}), true is true
-PASS ({a:1, set a(v){}}), true is true
-PASS ({get a(){}, a:1}), true is true
-PASS ({set a(v){}, a:1}), true is true
-PASS ({get a(){}, get a(){}}), true is true
-PASS ({set a(v){}, set a(v){}}), true is true
-PASS ({set a(v){}, get a(){}, set a(v){}}), true is true
-PASS (function(){({a:1, get a(){}})}), true is true
-PASS (function(){({a:1, set a(v){}})}), true is true
-PASS (function(){({get a(){}, a:1})}), true is true
-PASS (function(){({set a(v){}, a:1})}), true is true
-PASS (function(){({get a(){}, get a(){}})}), true is true
-PASS (function(){({set a(v){}, set a(v){}})}), true is true
-PASS (function(){({set a(v){}, get a(){}, set a(v){}})}), true is true
+PASS ({a:1, get a(){}}) threw exception SyntaxError: Object literal may
not have data and accessor property with the same name.
+PASS ({a:1, set a(v){}}) threw exception SyntaxError: Object literal may
not have data and accessor property with the same name.
+PASS ({get a(){}, a:1}) threw exception SyntaxError: Object literal may
not have data and accessor property with the same name.
+PASS ({set a(v){}, a:1}) threw exception SyntaxError: Object literal may
not have data and accessor property with the same name.
+PASS ({get a(){}, get a(){}}) threw exception SyntaxError: Object literal
may not have multiple get/set accessors with the same name.
+PASS ({set a(v){}, set a(v){}}) threw exception SyntaxError: Object
literal may not have multiple get/set accessors with the same name.
+PASS ({set a(v){}, get a(){}, set a(v){}}) threw exception SyntaxError:
Object literal may not have multiple get/set accessors with the same name.
+PASS (function(){({a:1, get a(){}})}) threw exception SyntaxError: Object
literal may not have data and accessor property with the same name.
+PASS (function(){({a:1, set a(v){}})}) threw exception SyntaxError: Object
literal may not have data and accessor property with the same name.
+PASS (function(){({get a(){}, a:1})}) threw exception SyntaxError: Object
literal may not have data and accessor property with the same name.
+PASS (function(){({set a(v){}, a:1})}) threw exception SyntaxError: Object
literal may not have data and accessor property with the same name.
+PASS (function(){({get a(){}, get a(){}})}) threw exception SyntaxError:
Object literal may not have multiple get/set accessors with the same name.
+PASS (function(){({set a(v){}, set a(v){}})}) threw exception SyntaxError:
Object literal may not have multiple get/set accessors with the same name.
+PASS (function(){({set a(v){}, get a(){}, set a(v){}})}) threw exception
SyntaxError: Object literal may not have multiple get/set accessors with
the same name.
PASS ({a:1, a:1, a:1}), true is true
PASS ({get a(){}, set a(v){}}), true is true
PASS ({set a(v){}, get a(){}}), true is true
@@ -48,6 +48,5 @@
PASS (function(){({set a(v){}, get a(){}})}), true is true
PASS successfullyParsed is true
-
TEST COMPLETE
=======================================
--- /branches/bleeding_edge/test/webkit/object-literal-syntax.js Wed Aug 20
14:25:48 2014 UTC
+++ /branches/bleeding_edge/test/webkit/object-literal-syntax.js Thu Aug 21
15:32:22 2014 UTC
@@ -23,20 +23,20 @@
description("Make sure that we correctly identify parse errors in object
literals");
-shouldBeTrue("({a:1, get a(){}}), true");
-shouldBeTrue("({a:1, set a(v){}}), true");
-shouldBeTrue("({get a(){}, a:1}), true");
-shouldBeTrue("({set a(v){}, a:1}), true");
-shouldBeTrue("({get a(){}, get a(){}}), true");
-shouldBeTrue("({set a(v){}, set a(v){}}), true");
-shouldBeTrue("({set a(v){}, get a(){}, set a(v){}}), true");
-shouldBeTrue("(function(){({a:1, get a(){}})}), true");
-shouldBeTrue("(function(){({a:1, set a(v){}})}), true");
-shouldBeTrue("(function(){({get a(){}, a:1})}), true");
-shouldBeTrue("(function(){({set a(v){}, a:1})}), true");
-shouldBeTrue("(function(){({get a(){}, get a(){}})}), true");
-shouldBeTrue("(function(){({set a(v){}, set a(v){}})}), true");
-shouldBeTrue("(function(){({set a(v){}, get a(){}, set a(v){}})}), true");
+shouldThrow("({a:1, get a(){}})");
+shouldThrow("({a:1, set a(v){}})");
+shouldThrow("({get a(){}, a:1})");
+shouldThrow("({set a(v){}, a:1})");
+shouldThrow("({get a(){}, get a(){}})");
+shouldThrow("({set a(v){}, set a(v){}})");
+shouldThrow("({set a(v){}, get a(){}, set a(v){}})");
+shouldThrow("(function(){({a:1, get a(){}})})");
+shouldThrow("(function(){({a:1, set a(v){}})})");
+shouldThrow("(function(){({get a(){}, a:1})})");
+shouldThrow("(function(){({set a(v){}, a:1})})");
+shouldThrow("(function(){({get a(){}, get a(){}})})");
+shouldThrow("(function(){({set a(v){}, set a(v){}})})");
+shouldThrow("(function(){({set a(v){}, get a(){}, set a(v){}})})");
shouldBeTrue("({a:1, a:1, a:1}), true");
shouldBeTrue("({get a(){}, set a(v){}}), true");
shouldBeTrue("({set a(v){}, get a(){}}), true");
--
--
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.
