Skip to content

Commit 5bbc92d

Browse files
author
marja@chromium.org
committed
Throw syntax error when a getter/setter has the wrong number of params
We used to allow any number of parameters in getters and setters to match JSC. This is a violation of ES5.1 and both SpiderMonkey and Chakra throw on these syntax errors. BUG=v8:3371 LOG=Y R=marja@chromium.org Review URL: https://codereview.chromium.org/329413002 Patch from Erik Arvidsson <arv@chromium.org>. git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21868 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
1 parent e056708 commit 5bbc92d

14 files changed

Lines changed: 98 additions & 59 deletions

‎src/ast.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2304,6 +2304,12 @@ class FunctionLiteral V8_FINAL : public Expression {
23042304
kNotGenerator
23052305
};
23062306

2307+
enum ArityRestriction {
2308+
NORMAL_ARITY,
2309+
GETTER_ARITY,
2310+
SETTER_ARITY
2311+
};
2312+
23072313
DECLARE_NODE_TYPE(FunctionLiteral)
23082314

23092315
Handle<String> name() const { return raw_name_->string(); }

‎src/parser.cc‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -746,10 +746,12 @@ FunctionLiteral* ParserTraits::ParseFunctionLiteral(
746746
bool is_generator,
747747
int function_token_position,
748748
FunctionLiteral::FunctionType type,
749+
FunctionLiteral::ArityRestriction arity_restriction,
749750
bool* ok) {
750751
return parser_->ParseFunctionLiteral(name, function_name_location,
751752
name_is_strict_reserved, is_generator,
752-
function_token_position, type, ok);
753+
function_token_position, type,
754+
arity_restriction, ok);
753755
}
754756

755757

@@ -1020,6 +1022,7 @@ FunctionLiteral* Parser::ParseLazy(Utf16CharacterStream* source) {
10201022
shared_info->is_generator(),
10211023
RelocInfo::kNoPosition,
10221024
function_type,
1025+
FunctionLiteral::NORMAL_ARITY,
10231026
&ok);
10241027
// Make sure the results agree.
10251028
ASSERT(ok == (result != NULL));
@@ -1871,6 +1874,7 @@ Statement* Parser::ParseFunctionDeclaration(ZoneList<const AstString*>* names,
18711874
is_generator,
18721875
pos,
18731876
FunctionLiteral::DECLARATION,
1877+
FunctionLiteral::NORMAL_ARITY,
18741878
CHECK_OK);
18751879
// Even if we're not at the top-level of the global or a function
18761880
// scope, we treat it as such and introduce the function with its
@@ -3303,9 +3307,16 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
33033307
bool is_generator,
33043308
int function_token_pos,
33053309
FunctionLiteral::FunctionType function_type,
3310+
FunctionLiteral::ArityRestriction arity_restriction,
33063311
bool* ok) {
33073312
// Function ::
33083313
// '(' FormalParameterList? ')' '{' FunctionBody '}'
3314+
//
3315+
// Getter ::
3316+
// '(' ')' '{' FunctionBody '}'
3317+
//
3318+
// Setter ::
3319+
// '(' PropertySetParameterList ')' '{' FunctionBody '}'
33093320

33103321
int pos = function_token_pos == RelocInfo::kNoPosition
33113322
? peek_position() : function_token_pos;
@@ -3400,7 +3411,9 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
34003411
Scanner::Location dupe_error_loc = Scanner::Location::invalid();
34013412
Scanner::Location reserved_loc = Scanner::Location::invalid();
34023413

3403-
bool done = (peek() == Token::RPAREN);
3414+
bool done = arity_restriction == FunctionLiteral::GETTER_ARITY ||
3415+
(peek() == Token::RPAREN &&
3416+
arity_restriction != FunctionLiteral::SETTER_ARITY);
34043417
while (!done) {
34053418
bool is_strict_reserved = false;
34063419
const AstString* param_name =
@@ -3425,6 +3438,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
34253438
*ok = false;
34263439
return NULL;
34273440
}
3441+
if (arity_restriction == FunctionLiteral::SETTER_ARITY) break;
34283442
done = (peek() == Token::RPAREN);
34293443
if (!done) Expect(Token::COMMA, CHECK_OK);
34303444
}

‎src/parser.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,7 @@ class ParserTraits {
582582
bool is_generator,
583583
int function_token_position,
584584
FunctionLiteral::FunctionType type,
585+
FunctionLiteral::ArityRestriction arity_restriction,
585586
bool* ok);
586587

587588
private:
@@ -739,6 +740,7 @@ class Parser : public ParserBase<ParserTraits> {
739740
bool is_generator,
740741
int function_token_position,
741742
FunctionLiteral::FunctionType type,
743+
FunctionLiteral::ArityRestriction arity_restriction,
742744
bool* ok);
743745

744746
// Magical syntax support.

‎src/preparser.cc‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,11 @@ PreParserExpression PreParserTraits::ParseFunctionLiteral(
9595
bool is_generator,
9696
int function_token_position,
9797
FunctionLiteral::FunctionType type,
98+
FunctionLiteral::ArityRestriction arity_restriction,
9899
bool* ok) {
99100
return pre_parser_->ParseFunctionLiteral(
100101
name, function_name_location, name_is_strict_reserved, is_generator,
101-
function_token_position, type, ok);
102+
function_token_position, type, arity_restriction, ok);
102103
}
103104

104105

@@ -320,6 +321,7 @@ PreParser::Statement PreParser::ParseFunctionDeclaration(bool* ok) {
320321
is_generator,
321322
pos,
322323
FunctionLiteral::DECLARATION,
324+
FunctionLiteral::NORMAL_ARITY,
323325
CHECK_OK);
324326
return Statement::FunctionDeclaration();
325327
}
@@ -799,6 +801,7 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
799801
bool is_generator,
800802
int function_token_pos,
801803
FunctionLiteral::FunctionType function_type,
804+
FunctionLiteral::ArityRestriction arity_restriction,
802805
bool* ok) {
803806
// Function ::
804807
// '(' FormalParameterList? ')' '{' FunctionBody '}'
@@ -812,14 +815,17 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
812815
// '(' (Identifier)*[','] ')'
813816
Expect(Token::LPAREN, CHECK_OK);
814817
int start_position = position();
815-
bool done = (peek() == Token::RPAREN);
816818
DuplicateFinder duplicate_finder(scanner()->unicode_cache());
817819
// We don't yet know if the function will be strict, so we cannot yet produce
818820
// errors for parameter names or duplicates. However, we remember the
819821
// locations of these errors if they occur and produce the errors later.
820822
Scanner::Location eval_args_error_loc = Scanner::Location::invalid();
821823
Scanner::Location dupe_error_loc = Scanner::Location::invalid();
822824
Scanner::Location reserved_error_loc = Scanner::Location::invalid();
825+
826+
bool done = arity_restriction == FunctionLiteral::GETTER_ARITY ||
827+
(peek() == Token::RPAREN &&
828+
arity_restriction != FunctionLiteral::SETTER_ARITY);
823829
while (!done) {
824830
bool is_strict_reserved = false;
825831
Identifier param_name =
@@ -837,10 +843,9 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
837843
dupe_error_loc = scanner()->location();
838844
}
839845

846+
if (arity_restriction == FunctionLiteral::SETTER_ARITY) break;
840847
done = (peek() == Token::RPAREN);
841-
if (!done) {
842-
Expect(Token::COMMA, CHECK_OK);
843-
}
848+
if (!done) Expect(Token::COMMA, CHECK_OK);
844849
}
845850
Expect(Token::RPAREN, CHECK_OK);
846851

‎src/preparser.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1047,6 +1047,7 @@ class PreParserTraits {
10471047
bool is_generator,
10481048
int function_token_position,
10491049
FunctionLiteral::FunctionType type,
1050+
FunctionLiteral::ArityRestriction arity_restriction,
10501051
bool* ok);
10511052

10521053
private:
@@ -1176,6 +1177,7 @@ class PreParser : public ParserBase<PreParserTraits> {
11761177
bool is_generator,
11771178
int function_token_pos,
11781179
FunctionLiteral::FunctionType function_type,
1180+
FunctionLiteral::ArityRestriction arity_restriction,
11791181
bool* ok);
11801182
void ParseLazyFunctionLiteralBody(bool* ok);
11811183

@@ -1556,9 +1558,9 @@ typename ParserBase<Traits>::ExpressionT ParserBase<Traits>::ParseObjectLiteral(
15561558
false, // reserved words are allowed here
15571559
false, // not a generator
15581560
RelocInfo::kNoPosition, FunctionLiteral::ANONYMOUS_EXPRESSION,
1561+
is_getter ? FunctionLiteral::GETTER_ARITY
1562+
: FunctionLiteral::SETTER_ARITY,
15591563
CHECK_OK);
1560-
// Allow any number of parameters for compatibilty with JSC.
1561-
// Specification only allows zero parameters for get and one for set.
15621564
typename Traits::Type::ObjectLiteralProperty property =
15631565
factory()->NewObjectLiteralProperty(is_getter, value, next_pos);
15641566
if (this->IsBoilerplateProperty(property)) {
@@ -2060,6 +2062,7 @@ ParserBase<Traits>::ParseMemberExpression(bool* ok) {
20602062
is_generator,
20612063
function_token_position,
20622064
function_type,
2065+
FunctionLiteral::NORMAL_ARITY,
20632066
CHECK_OK);
20642067
} else {
20652068
result = ParsePrimaryExpression(CHECK_OK);

‎test/cctest/test-parsing.cc‎

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2233,22 +2233,27 @@ TEST(ErrorsObjectLiteralChecking) {
22332233

22342234
const char* statement_data[] = {
22352235
"foo: 1, get foo() {}",
2236-
"foo: 1, set foo() {}",
2236+
"foo: 1, set foo(v) {}",
22372237
"\"foo\": 1, get \"foo\"() {}",
2238-
"\"foo\": 1, set \"foo\"() {}",
2238+
"\"foo\": 1, set \"foo\"(v) {}",
22392239
"1: 1, get 1() {}",
22402240
"1: 1, set 1() {}",
22412241
// It's counter-intuitive, but these collide too (even in classic
22422242
// mode). Note that we can have "foo" and foo as properties in classic mode,
22432243
// but we cannot have "foo" and get foo, or foo and get "foo".
22442244
"foo: 1, get \"foo\"() {}",
2245-
"foo: 1, set \"foo\"() {}",
2245+
"foo: 1, set \"foo\"(v) {}",
22462246
"\"foo\": 1, get foo() {}",
2247-
"\"foo\": 1, set foo() {}",
2247+
"\"foo\": 1, set foo(v) {}",
22482248
"1: 1, get \"1\"() {}",
22492249
"1: 1, set \"1\"() {}",
22502250
"\"1\": 1, get 1() {}"
2251-
"\"1\": 1, set 1() {}"
2251+
"\"1\": 1, set 1(v) {}"
2252+
// Wrong number of parameters
2253+
"get bar(x) {}",
2254+
"get bar(x, y) {}",
2255+
"set bar() {}",
2256+
"set bar(x, y) {}",
22522257
// Parsing FunctionLiteral for getter or setter fails
22532258
"get foo( +",
22542259
"get foo() \"error\"",
@@ -2272,25 +2277,22 @@ TEST(NoErrorsObjectLiteralChecking) {
22722277
"1: 1, 2: 2",
22732278
// Syntax: IdentifierName ':' AssignmentExpression
22742279
"foo: bar = 5 + baz",
2275-
// Syntax: 'get' (IdentifierName | String | Number) FunctionLiteral
2280+
// Syntax: 'get' PropertyName '(' ')' '{' FunctionBody '}'
22762281
"get foo() {}",
22772282
"get \"foo\"() {}",
22782283
"get 1() {}",
2279-
// Syntax: 'set' (IdentifierName | String | Number) FunctionLiteral
2280-
"set foo() {}",
2281-
"set \"foo\"() {}",
2282-
"set 1() {}",
2284+
// Syntax: 'set' PropertyName '(' PropertySetParameterList ')'
2285+
// '{' FunctionBody '}'
2286+
"set foo(v) {}",
2287+
"set \"foo\"(v) {}",
2288+
"set 1(v) {}",
22832289
// Non-colliding getters and setters -> no errors
22842290
"foo: 1, get bar() {}",
2285-
"foo: 1, set bar(b) {}",
2291+
"foo: 1, set bar(v) {}",
22862292
"\"foo\": 1, get \"bar\"() {}",
2287-
"\"foo\": 1, set \"bar\"() {}",
2293+
"\"foo\": 1, set \"bar\"(v) {}",
22882294
"1: 1, get 2() {}",
2289-
"1: 1, set 2() {}",
2290-
// Weird number of parameters -> no errors
2291-
"get bar() {}, set bar() {}",
2292-
"get bar(x) {}, set bar(x) {}",
2293-
"get bar(x, y) {}, set bar(x, y) {}",
2295+
"1: 1, set 2(v) {}",
22942296
// Keywords, future reserved and strict future reserved are also allowed as
22952297
// property names.
22962298
"if: 4",

‎test/mozilla/mozilla.status‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -718,6 +718,13 @@
718718
'js1_5/extensions/regress-361964': [FAIL_OK],
719719
'js1_5/extensions/regress-363988': [FAIL_OK],
720720
'js1_5/extensions/regress-365869': [FAIL_OK],
721+
722+
# Uses non ES5 compatible syntax for setter
723+
'js1_5/extensions/regress-367501-01': [FAIL_OK],
724+
'js1_5/extensions/regress-367501-02': [FAIL_OK],
725+
'js1_5/extensions/regress-367501-03': [FAIL_OK],
726+
'js1_5/extensions/regress-367501-04': [FAIL_OK],
727+
721728
'js1_5/extensions/regress-367630': [FAIL_OK],
722729
'js1_5/extensions/regress-367923': [FAIL_OK],
723730
'js1_5/extensions/regress-368859': [FAIL_OK],

‎test/preparser/duplicate-property.pyt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ def PropertyTest(name, propa, propb, allow_strict = True):
8181
""", replacement, None)
8282

8383
StrictTest("$name-nested-set", """
84-
var o = {set $id1(){}, o: {set $id2(){} } };
84+
var o = {set $id1(v){}, o: {set $id2(v){} } };
8585
""", replacement, None)
8686

8787

‎test/webkit/for-in-cached-expected.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ PASS forIn3({ __proto__: { __proto__: { y3 : 2 } } }) is ['x', 'y3']
3434
PASS forIn4(objectWithArrayAsProto) is []
3535
PASS forIn4(objectWithArrayAsProto) is ['0']
3636
PASS forIn5({get foo() { return 'called getter'} }) is ['foo', 'called getter']
37-
PASS forIn5({set foo() { } }) is ['foo', undefined]
38-
PASS forIn5({get foo() { return 'called getter'}, set foo() { }}) is ['foo', 'called getter']
37+
PASS forIn5({set foo(v) { } }) is ['foo', undefined]
38+
PASS forIn5({get foo() { return 'called getter'}, set foo(v) { }}) is ['foo', 'called getter']
3939
PASS successfullyParsed is true
4040

4141
TEST COMPLETE

‎test/webkit/for-in-cached.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,8 @@ function forIn5(o) {
8484
}
8585

8686
shouldBe("forIn5({get foo() { return 'called getter'} })", "['foo', 'called getter']");
87-
shouldBe("forIn5({set foo() { } })", "['foo', undefined]");
88-
shouldBe("forIn5({get foo() { return 'called getter'}, set foo() { }})", "['foo', 'called getter']");
87+
shouldBe("forIn5({set foo(v) { } })", "['foo', undefined]");
88+
shouldBe("forIn5({get foo() { return 'called getter'}, set foo(v) { }})", "['foo', 'called getter']");
8989

9090
function cacheClearing() {
9191
for(var j=0; j < 10; j++){

0 commit comments

Comments
 (0)