From d017e6ce54cfa2b2f931670a413bb27cb84fde31 Mon Sep 17 00:00:00 2001 From: "marja@chromium.org" Date: Thu, 20 Mar 2014 13:18:15 +0000 Subject: [PATCH] Make PreParser track valid left hand sides. Notes: - This makes PreParser produce invalid_lhs_in_assignment and invalid_lhs_in_prefix_op. Other errors will follow as the corresponding funcs move to ParserBase. - PreParserExpression::IsStrictFunction and StrictFunction() are not needed any more -> removed them. R=rossberg@chromium.org BUG= Review URL: https://codereview.chromium.org/196343033 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20125 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/preparser.cc | 19 ++++++------ src/preparser.h | 28 ++++++++++-------- test/cctest/test-parsing.cc | 70 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 20 deletions(-) diff --git a/src/preparser.cc b/src/preparser.cc index 398f327..474e642 100644 --- a/src/preparser.cc +++ b/src/preparser.cc @@ -874,7 +874,7 @@ PreParser::Expression PreParser::ParseLeftHandSideExpression(bool* ok) { if (result.IsThis()) { result = Expression::ThisProperty(); } else { - result = Expression::Default(); + result = Expression::Property(); } break; } @@ -891,7 +891,7 @@ PreParser::Expression PreParser::ParseLeftHandSideExpression(bool* ok) { if (result.IsThis()) { result = Expression::ThisProperty(); } else { - result = Expression::Default(); + result = Expression::Property(); } break; } @@ -913,13 +913,17 @@ PreParser::Expression PreParser::ParseMemberWithNewPrefixesExpression( if (peek() == Token::NEW) { Consume(Token::NEW); ParseMemberWithNewPrefixesExpression(CHECK_OK); + Expression expression = Expression::Default(); if (peek() == Token::LPAREN) { // NewExpression with arguments. ParseArguments(CHECK_OK); - // The expression can still continue with . or [ after the arguments. - ParseMemberExpressionContinuation(Expression::Default(), CHECK_OK); + // The expression can still continue with . or [ after the arguments. Here + // we need to transmit the "is valid left hand side" property of the + // expression. + expression = + ParseMemberExpressionContinuation(Expression::Default(), CHECK_OK); } - return Expression::Default(); + return expression; } // No 'new' keyword. return ParseMemberExpression(ok); @@ -980,7 +984,7 @@ PreParser::Expression PreParser::ParseMemberExpressionContinuation( if (expression.IsThis()) { expression = Expression::ThisProperty(); } else { - expression = Expression::Default(); + expression = Expression::Property(); } break; } @@ -990,7 +994,7 @@ PreParser::Expression PreParser::ParseMemberExpressionContinuation( if (expression.IsThis()) { expression = Expression::ThisProperty(); } else { - expression = Expression::Default(); + expression = Expression::Property(); } break; } @@ -1102,7 +1106,6 @@ PreParser::Expression PreParser::ParseFunctionLiteral( int end_position = scanner()->location().end_pos; CheckOctalLiteral(start_position, end_position, CHECK_OK); - return Expression::StrictFunction(); } return Expression::Default(); diff --git a/src/preparser.h b/src/preparser.h index 05ff972..b6d97f7 100644 --- a/src/preparser.h +++ b/src/preparser.h @@ -554,8 +554,8 @@ class PreParserExpression { return PreParserExpression(kThisPropertyExpression); } - static PreParserExpression StrictFunction() { - return PreParserExpression(kStrictFunctionExpression); + static PreParserExpression Property() { + return PreParserExpression(kPropertyExpression); } bool IsIdentifier() { return (code_ & kIdentifierFlag) != 0; } @@ -576,7 +576,9 @@ class PreParserExpression { bool IsThisProperty() { return code_ == kThisPropertyExpression; } - bool IsStrictFunction() { return code_ == kStrictFunctionExpression; } + bool IsProperty() { + return code_ == kPropertyExpression || code_ == kThisPropertyExpression; + } // Dummy implementation for making expression->AsCall() work (see below). PreParserExpression* operator->() { return this; } @@ -590,9 +592,11 @@ class PreParserExpression { void set_index(int index) {} // For YieldExpressions private: - // First two/three bits are used as flags. - // Bit 0 and 1 represent identifiers or strings literals, and are - // mutually exclusive, but can both be absent. + // Least significant 2 bits are used as flags. Bits 0 and 1 represent + // identifiers or strings literals, and are mutually exclusive, but can both + // be absent. If the expression is an identifier or a string literal, the + // other bits describe the type (see PreParserIdentifier::Type and string + // literal constants below). enum { kUnknownExpression = 0, // Identifiers @@ -604,10 +608,11 @@ class PreParserExpression { kUseStrictString = kStringLiteralFlag | 8, kStringLiteralMask = kUseStrictString, - // Below here applies if neither identifier nor string literal. - kThisExpression = 4, - kThisPropertyExpression = 8, - kStrictFunctionExpression = 12 + // Below here applies if neither identifier nor string literal. Reserve the + // 2 least significant bits for flags. + kThisExpression = 1 << 2, + kThisPropertyExpression = 2 << 2, + kPropertyExpression = 3 << 2 }; explicit PreParserExpression(int expression_code) : code_(expression_code) {} @@ -830,8 +835,7 @@ class PreParserTraits { // Determine whether the expression is a valid assignment left-hand side. static bool IsValidLeftHandSide(PreParserExpression expression) { - // TODO(marja): check properly; for now, leave it to parser. - return true; + return expression.IsIdentifier() || expression.IsProperty(); } static PreParserExpression MarkExpressionAsLValue( diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc index c88b2dc..cb8802b 100644 --- a/test/cctest/test-parsing.cc +++ b/test/cctest/test-parsing.cc @@ -2526,3 +2526,73 @@ TEST(StrictDelete) { RunParserSyncTest(strict_context_data, bad_statement_data, kError); RunParserSyncTest(sloppy_context_data, bad_statement_data, kError); } + + +TEST(ErrorInvalidLeftHandSide) { + const char* assignment_context_data[][2] = { + // {"", " = 1;"}, + // {"\"use strict\"; ", " = 1;"}, + { NULL, NULL } + }; + + const char* prefix_context_data[][2] = { + {"++", ";"}, + {"\"use strict\"; ++", ";"}, + {NULL, NULL}, + }; + + const char* postfix_context_data[][2] = { + {"", "++;"}, + {"\"use strict\"; ", "++;"}, + { NULL, NULL } + }; + + // Good left hand sides for assigment or prefix / postfix operations. + const char* good_statement_data[] = { + "foo", + "foo.bar", + "foo[bar]", + "foo()[bar]", + "foo().bar", + "this.foo", + "this[foo]", + "new foo()[bar]", + "new foo().bar", + NULL + }; + + // Bad left hand sides for assigment or prefix / postfix operations. + const char* bad_statement_data_common[] = { + "2", + "foo()", + "null", + "if", // Unexpected token + "{x: 1}", // Unexpected token + "this", + "\"bar\"", + "(foo + bar)", + "new new foo()[bar]", // means: new (new foo()[bar]) + "new new foo().bar", // means: new (new foo()[bar]) + NULL + }; + + // These are not okay for assignment, but okay for prefix / postix. + const char* bad_statement_data_for_assignment[] = { + "++foo", + "foo++", + "foo + bar", + NULL + }; + + RunParserSyncTest(assignment_context_data, good_statement_data, kSuccess); + RunParserSyncTest(assignment_context_data, bad_statement_data_common, kError); + RunParserSyncTest(assignment_context_data, bad_statement_data_for_assignment, + kError); + + RunParserSyncTest(prefix_context_data, good_statement_data, kSuccess); + RunParserSyncTest(prefix_context_data, bad_statement_data_common, kError); + + RunParserSyncTest(postfix_context_data, good_statement_data, kSuccess); + // TODO(marja): This doesn't work yet. + // RunParserSyncTest(postfix_context_data, bad_statement_data_common, kError); +} -- 2.7.4