Revision: 6475
Author: [email protected]
Date: Tue Jan 25 10:42:35 2011
Log: Strict mode object property validation.
Review URL: http://codereview.chromium.org/6335010/
http://code.google.com/p/v8/source/detail?r=6475
Modified:
/branches/bleeding_edge/src/messages.js
/branches/bleeding_edge/src/parser.cc
/branches/bleeding_edge/test/mjsunit/strict-mode.js
=======================================
--- /branches/bleeding_edge/src/messages.js Tue Jan 25 00:48:59 2011
+++ /branches/bleeding_edge/src/messages.js Tue Jan 25 10:42:35 2011
@@ -233,6 +233,9 @@
strict_var_name: "Variable name may not be eval or
arguments in strict mode",
strict_function_name: "Function name may not be eval or
arguments in strict mode",
strict_octal_literal: "Octal literals are not allowed in
strict mode.",
+ strict_duplicate_property: "Duplicate data property in object
literal not allowed in strict mode",
+ accessor_data_property: "Object literal may not have data and
accessor property with the same name",
+ accessor_get_set: "Object literal may not have multiple
get/set accessors with the same name",
};
}
var format = kMessages[message.type];
=======================================
--- /branches/bleeding_edge/src/parser.cc Tue Jan 25 09:21:45 2011
+++ /branches/bleeding_edge/src/parser.cc Tue Jan 25 10:42:35 2011
@@ -3016,6 +3016,126 @@
return Factory::undefined_value();
}
+// Defined in ast.cc
+bool IsEqualString(void* first, void* second);
+bool IsEqualSmi(void* first, void* second);
+
+
+// Validation per 11.1.5 Object Initialiser
+class ObjectLiteralPropertyChecker {
+ public:
+ ObjectLiteralPropertyChecker(Parser* parser, bool strict) :
+ props(&IsEqualString),
+ elems(&IsEqualSmi),
+ parser_(parser),
+ strict_(strict) {
+ }
+
+ void CheckProperty(
+ ObjectLiteral::Property* property,
+ Scanner::Location loc,
+ bool* ok);
+
+ private:
+ enum PropertyKind {
+ kGetAccessor = 0x01,
+ kSetAccessor = 0x02,
+ kAccessor = kGetAccessor | kSetAccessor,
+ kData = 0x04
+ };
+
+ static intptr_t GetPropertyKind(ObjectLiteral::Property* property) {
+ switch (property->kind()) {
+ case ObjectLiteral::Property::GETTER:
+ return kGetAccessor;
+ case ObjectLiteral::Property::SETTER:
+ return kSetAccessor;
+ default:
+ return kData;
+ }
+ }
+
+ HashMap props;
+ HashMap elems;
+ Parser* parser_;
+ bool strict_;
+};
+
+
+void ObjectLiteralPropertyChecker::CheckProperty(
+ ObjectLiteral::Property* property,
+ Scanner::Location loc,
+ bool* ok) {
+
+ ASSERT(property != NULL);
+
+ Literal *lit = property->key();
+ Handle<Object> handle = lit->handle();
+
+ uint32_t hash;
+ HashMap* map;
+ void* key;
+ Smi* smi_key_location;
+
+ if (handle->IsSymbol()) {
+ Handle<String> name(String::cast(*handle));
+ if (name->AsArrayIndex(&hash)) {
+ smi_key_location = Smi::FromInt(hash);
+ key = &smi_key_location;
+ map = &elems;
+ } else {
+ key = handle.location();
+ hash = name->Hash();
+ map = &props;
+ }
+ } else if (handle->ToArrayIndex(&hash)) {
+ key = handle.location();
+ map = &elems;
+ } else {
+ ASSERT(handle->IsNumber());
+ double num = handle->Number();
+ char arr[100];
+ Vector<char> buffer(arr, ARRAY_SIZE(arr));
+ const char* str = DoubleToCString(num, buffer);
+ Handle<String> name = Factory::NewStringFromAscii(CStrVector(str));
+ key = name.location();
+ hash = name->Hash();
+ map = &props;
+ }
+
+ // Lookup property previously defined, if any.
+ HashMap::Entry* entry = map->Lookup(key, hash, true);
+ intptr_t prev = reinterpret_cast<intptr_t> (entry->value);
+ intptr_t curr = GetPropertyKind(property);
+
+ // Duplicate data properties are illegal in strict mode.
+ if (strict_ && (curr & prev & kData) != 0) {
+ parser_->ReportMessageAt(loc, "strict_duplicate_property",
+ Vector<const char*>::empty());
+ *ok = false;
+ return;
+ }
+ // Data property conflicting with an accessor.
+ if (((curr & kData) && (prev & kAccessor)) ||
+ ((prev & kData) && (curr & kAccessor))) {
+ parser_->ReportMessageAt(loc, "accessor_data_property",
+ Vector<const char*>::empty());
+ *ok = false;
+ return;
+ }
+ // Two accessors of the same type conflicting
+ if ((curr & prev & kAccessor) != 0) {
+ parser_->ReportMessageAt(loc, "accessor_get_set",
+ Vector<const char*>::empty());
+ *ok = false;
+ return;
+ }
+
+ // Update map
+ entry->value = reinterpret_cast<void*> (prev | curr);
+ *ok = true;
+}
+
void Parser::BuildObjectLiteralConstantProperties(
ZoneList<ObjectLiteral::Property*>* properties,
@@ -3121,12 +3241,20 @@
new ZoneList<ObjectLiteral::Property*>(4);
int number_of_boilerplate_properties = 0;
+ ObjectLiteralPropertyChecker checker(this, temp_scope_->StrictMode());
+
Expect(Token::LBRACE, CHECK_OK);
+ Scanner::Location loc = scanner().location();
+
while (peek() != Token::RBRACE) {
if (fni_ != NULL) fni_->Enter();
Literal* key = NULL;
Token::Value next = peek();
+
+ // Location of the property name token
+ Scanner::Location loc = scanner().peek_location();
+
switch (next) {
case Token::IDENTIFIER: {
bool is_getter = false;
@@ -3136,11 +3264,15 @@
if (fni_ != NULL) fni_->PushLiteralName(id);
if ((is_getter || is_setter) && peek() != Token::COLON) {
+ // Update loc to point to the identifier
+ loc = scanner().peek_location();
ObjectLiteral::Property* property =
ParseObjectLiteralGetSet(is_getter, CHECK_OK);
if (IsBoilerplateProperty(property)) {
number_of_boilerplate_properties++;
}
+ // Validate the property.
+ checker.CheckProperty(property, loc, CHECK_OK);
properties->Add(property);
if (peek() != Token::RBRACE) Expect(Token::COMMA, CHECK_OK);
@@ -3197,6 +3329,8 @@
// Count CONSTANT or COMPUTED properties to maintain the enumeration
order.
if (IsBoilerplateProperty(property))
number_of_boilerplate_properties++;
+ // Validate the property
+ checker.CheckProperty(property, loc, CHECK_OK);
properties->Add(property);
// TODO(1240767): Consider allowing trailing comma.
@@ -3208,6 +3342,7 @@
}
}
Expect(Token::RBRACE, CHECK_OK);
+
// Computation of literal_index must happen before pre parse bailout.
int literal_index = temp_scope_->NextMaterializedLiteralIndex();
=======================================
--- /branches/bleeding_edge/test/mjsunit/strict-mode.js Tue Jan 25 09:21:45
2011
+++ /branches/bleeding_edge/test/mjsunit/strict-mode.js Tue Jan 25 10:42:35
2011
@@ -129,3 +129,52 @@
"octal\\032directive";\
"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";
+ var x = { 123 : 1, "0123" : 2 };
+ var x = { 123:
1, '123.00000000000000000000000000000000000000000000000000000000000000000001' :
2 }
+}
+
+//CheckStrictMode("", SyntaxError)
+
+// 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)
+
+// 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)
+
+// 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);
+
+// 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);
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
