Reviewers: Dmitry Lomov (chromium), marja,
Message:
PTAL
Another option would be to make the checker handle this but since the
checker is
going away I didn't want to put the logic in there.
Description:
Allow duplicate property names in classes
ES6 no longer makes duplicate properties an error. However, we
continue to treat duplicate properties in strict mode object
literals as errors. With this change we allow duplicate properties
in class bodies. We continue to flag duplicate constructors as an
error as required by ES6.
BUG=v8:3330
LOG=Y
Please review this at https://codereview.chromium.org/677953004/
Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+36, -25 lines):
M src/messages.js
M src/preparser.h
M test/cctest/test-parsing.cc
Index: src/messages.js
diff --git a/src/messages.js b/src/messages.js
index
6578e8dff41044f88133083db03ea4c6ea8c5b91..513ffdc2a9b63e3b49a414886055f35d236322c3
100644
--- a/src/messages.js
+++ b/src/messages.js
@@ -176,7 +176,8 @@ var kMessages = {
module_export_undefined: ["Export '", "%0", "' is not defined in
module"],
unexpected_super: ["'super' keyword unexpected here"],
extends_value_not_a_function: ["Class extends value ", "%0", " is not a
function or null"],
- prototype_parent_not_an_object: ["Class extends value does not have
valid prototype property ", "%0"]
+ prototype_parent_not_an_object: ["Class extends value does not have
valid prototype property ", "%0"],
+ duplicate_constructor: ["A class may only have one constructor"]
};
Index: src/preparser.h
diff --git a/src/preparser.h b/src/preparser.h
index
a53abfe4ff3af732cd2cd46ac79db8c35264f698..95e6e440b64f06ced8b7158977615a2ad0c50f9e
100644
--- a/src/preparser.h
+++ b/src/preparser.h
@@ -476,6 +476,7 @@ class ParserBase : public Traits {
ExpressionT ParseObjectLiteral(bool* ok);
ObjectLiteralPropertyT ParsePropertyDefinition(ObjectLiteralChecker*
checker,
bool in_class, bool
is_static,
+ bool*
has_seen_constructor,
bool* ok);
typename Traits::Type::ExpressionList ParseArguments(bool* ok);
ExpressionT ParseAssignmentExpression(bool accept_IN, bool* ok);
@@ -1183,8 +1184,6 @@ class PreParserTraits {
return false;
}
- bool IsConstructorProperty(PreParserExpression property) { return false;
}
-
static PreParserExpression GetPropertyValue(PreParserExpression
property) {
UNREACHABLE();
return PreParserExpression::Default();
@@ -1925,7 +1924,8 @@ typename ParserBase<Traits>::IdentifierT
ParserBase<Traits>::ParsePropertyName(
template <class Traits>
typename ParserBase<Traits>::ObjectLiteralPropertyT ParserBase<
Traits>::ParsePropertyDefinition(ObjectLiteralChecker* checker,
- bool in_class, bool is_static, bool*
ok) {
+ bool in_class, bool is_static,
+ bool* has_seen_constructor, bool* ok)
{
ExpressionT value = this->EmptyExpression();
bool is_get = false;
bool is_set = false;
@@ -1942,8 +1942,10 @@ typename ParserBase<Traits>::ObjectLiteralPropertyT
ParserBase<
if (!in_class && !is_generator && peek() == Token::COLON) {
// PropertyDefinition : PropertyName ':' AssignmentExpression
- checker->CheckProperty(name_token, kValueProperty,
- CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+ if (checker != NULL) {
+ checker->CheckProperty(name_token, kValueProperty,
+ CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+ }
Consume(Token::COLON);
value = this->ParseAssignmentExpression(
true, CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
@@ -1968,11 +1970,20 @@ typename ParserBase<Traits>::ObjectLiteralPropertyT
ParserBase<
return this->EmptyObjectLiteralProperty();
}
+ if (*has_seen_constructor) {
+ ReportMessageAt(scanner()->location(), "duplicate_constructor");
+ *ok = false;
+ return this->EmptyObjectLiteralProperty();
+ }
+
+ *has_seen_constructor = true;
kind = FunctionKind::kNormalFunction;
}
- checker->CheckProperty(name_token, kValueProperty,
- CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+ if (checker != NULL) {
+ checker->CheckProperty(name_token, kValueProperty,
+ CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+ }
value = this->ParseFunctionLiteral(
name, scanner()->location(),
@@ -1983,7 +1994,7 @@ typename ParserBase<Traits>::ObjectLiteralPropertyT
ParserBase<
} else if (in_class && name_is_static && !is_static) {
// static MethodDefinition
- return ParsePropertyDefinition(checker, true, true, ok);
+ return ParsePropertyDefinition(checker, true, true, NULL, ok);
} else if (is_get || is_set) {
// Accessor
@@ -1998,16 +2009,15 @@ typename ParserBase<Traits>::ObjectLiteralPropertyT
ParserBase<
*ok = false;
return this->EmptyObjectLiteralProperty();
} else if (in_class && !is_static && this->IsConstructor(name)) {
- // ES6, spec draft rev 27, treats static get constructor as an error
too.
- // https://bugs.ecmascript.org/show_bug.cgi?id=3223
- // TODO(arv): Update when bug is resolved.
ReportMessageAt(scanner()->location(), "constructor_special_method");
*ok = false;
return this->EmptyObjectLiteralProperty();
}
- checker->CheckProperty(name_token,
- is_get ? kGetterProperty : kSetterProperty,
- CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+ if (checker != NULL) {
+ checker->CheckProperty(name_token,
+ is_get ? kGetterProperty : kSetterProperty,
+ CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+ }
typename Traits::Type::FunctionLiteral value =
this->ParseFunctionLiteral(
name, scanner()->location(),
@@ -2061,8 +2071,8 @@ typename ParserBase<Traits>::ExpressionT
ParserBase<Traits>::ParseObjectLiteral(
const bool in_class = false;
const bool is_static = false;
- ObjectLiteralPropertyT property =
- this->ParsePropertyDefinition(&checker, in_class, is_static,
CHECK_OK);
+ ObjectLiteralPropertyT property = this->ParsePropertyDefinition(
+ &checker, in_class, is_static, NULL, CHECK_OK);
// Mark top-level object literals that contain function literals and
// pretenure the literal so it can be added as a constant function
@@ -2744,22 +2754,22 @@ typename ParserBase<Traits>::ExpressionT
ParserBase<Traits>::ParseClassLiteral(
scope_->SetStrictMode(STRICT);
scope_->SetScopeName(name);
- ObjectLiteralChecker checker(this, STRICT);
typename Traits::Type::PropertyList properties =
this->NewPropertyList(4, zone_);
ExpressionT constructor = this->EmptyExpression();
+ bool has_seen_constructor = false;
Expect(Token::LBRACE, CHECK_OK);
while (peek() != Token::RBRACE) {
if (Check(Token::SEMICOLON)) continue;
if (fni_ != NULL) fni_->Enter();
-
const bool in_class = true;
const bool is_static = false;
- ObjectLiteralPropertyT property =
- this->ParsePropertyDefinition(&checker, in_class, is_static,
CHECK_OK);
+ bool old_has_seen_constructor = has_seen_constructor;
+ ObjectLiteralPropertyT property = this->ParsePropertyDefinition(
+ NULL, in_class, is_static, &has_seen_constructor, CHECK_OK);
- if (this->IsConstructorProperty(property)) {
+ if (has_seen_constructor != old_has_seen_constructor) {
constructor = this->GetPropertyValue(property);
} else {
properties->Add(property, zone());
Index: test/cctest/test-parsing.cc
diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc
index
c1bb363a03a867938678fe04a436ae1b6e2c4043..6dccef848373a7ab63737d802a5a64ffb0de9e8e
100644
--- a/test/cctest/test-parsing.cc
+++ b/test/cctest/test-parsing.cc
@@ -4059,9 +4059,7 @@ TEST(ClassMultipleConstructorErrors) {
}
-// TODO(arv): We should allow duplicate property names.
-// https://code.google.com/p/v8/issues/detail?id=3570
-DISABLED_TEST(ClassMultiplePropertyNamesNoErrors) {
+TEST(ClassMultiplePropertyNamesNoErrors) {
const char* context_data[][2] = {{"class C {", "}"},
{"(class {", "});"},
{NULL, NULL}};
@@ -4070,6 +4068,8 @@ DISABLED_TEST(ClassMultiplePropertyNamesNoErrors) {
"constructor() {}; static constructor() {}",
"m() {}; static m() {}",
"m() {}; m() {}",
+ "static m() {}; static m() {}",
+ "get m() {}; set m(_) {}; get m() {}; set m(_) {};",
NULL};
static const ParserFlag always_flags[] = {
--
--
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.
