From 410219c8f3abf5223292f209425944a7e17adfa6 Mon Sep 17 00:00:00 2001 From: "keuchel@chromium.org" Date: Wed, 7 Dec 2011 16:03:29 +0000 Subject: [PATCH] Sync parser and preparser on do-while and return statements. This CL fixes the preparser to have the same liberal automatic semicolon insertion behaviour as the parser. In the case of a return statement in global code we throw a syntax error at runtime rather than an early error due to compatibility with KJS. However that hack allowed the following syntactically incorrect program in global code in the parser but not in the preparser: if (false) return else {} while the slightly saner version with the obligatory semicolon if (false) return; else {} was disallowed in the parser, but the preparser allowed it. This CL also fixes that issue. BUG=v8:1856 TEST=cctest/test-parsing.cc Review URL: http://codereview.chromium.org/8844002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@10201 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/parser.cc | 28 ++++--- src/preparser.cc | 1 + test/cctest/test-parsing.cc | 198 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 214 insertions(+), 13 deletions(-) diff --git a/src/parser.cc b/src/parser.cc index 35ed47b..c1681cf 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -2158,6 +2158,20 @@ Statement* Parser::ParseReturnStatement(bool* ok) { // reported (underlining). Expect(Token::RETURN, CHECK_OK); + Token::Value tok = peek(); + Statement* result; + if (scanner().HasAnyLineTerminatorBeforeNext() || + tok == Token::SEMICOLON || + tok == Token::RBRACE || + tok == Token::EOS) { + ExpectSemicolon(CHECK_OK); + result = new(zone()) ReturnStatement(GetLiteralUndefined()); + } else { + Expression* expr = ParseExpression(true, CHECK_OK); + ExpectSemicolon(CHECK_OK); + result = new(zone()) ReturnStatement(expr); + } + // An ECMAScript program is considered syntactically incorrect if it // contains a return statement that is not within the body of a // function. See ECMA-262, section 12.9, page 67. @@ -2170,19 +2184,7 @@ Statement* Parser::ParseReturnStatement(bool* ok) { Expression* throw_error = NewThrowSyntaxError(type, Handle::null()); return new(zone()) ExpressionStatement(throw_error); } - - Token::Value tok = peek(); - if (scanner().HasAnyLineTerminatorBeforeNext() || - tok == Token::SEMICOLON || - tok == Token::RBRACE || - tok == Token::EOS) { - ExpectSemicolon(CHECK_OK); - return new(zone()) ReturnStatement(GetLiteralUndefined()); - } - - Expression* expr = ParseExpression(true, CHECK_OK); - ExpectSemicolon(CHECK_OK); - return new(zone()) ReturnStatement(expr); + return result; } diff --git a/src/preparser.cc b/src/preparser.cc index 49cadb6..b36f4fa 100644 --- a/src/preparser.cc +++ b/src/preparser.cc @@ -627,6 +627,7 @@ PreParser::Statement PreParser::ParseDoWhileStatement(bool* ok) { Expect(i::Token::LPAREN, CHECK_OK); ParseExpression(true, CHECK_OK); Expect(i::Token::RPAREN, ok); + if (peek() == i::Token::SEMICOLON) Consume(i::Token::SEMICOLON); return Statement::Default(); } diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc index bd1e24e..6f394b6 100755 --- a/test/cctest/test-parsing.cc +++ b/test/cctest/test-parsing.cc @@ -884,3 +884,201 @@ TEST(ScopePositions) { CHECK_EQ(inner_scope->end_position(), kPrefixLen + kInnerLen); } } + + +void TestParserSync(i::Handle source, int flags) { + uintptr_t stack_limit = i::Isolate::Current()->stack_guard()->real_climit(); + bool harmony_scoping = ((i::kLanguageModeMask & flags) == i::EXTENDED_MODE); + + // Preparse the data. + i::CompleteParserRecorder log; + i::Scanner scanner(i::Isolate::Current()->unicode_cache()); + i::GenericStringUC16CharacterStream stream(source, 0, source->length()); + scanner.SetHarmonyScoping(harmony_scoping); + scanner.Initialize(&stream); + v8::preparser::PreParser::PreParseResult result = + v8::preparser::PreParser::PreParseProgram( + &scanner, &log, flags, stack_limit); + CHECK_EQ(v8::preparser::PreParser::kPreParseSuccess, result); + i::ScriptDataImpl data(log.ExtractData()); + + // Parse the data + i::Handle script = FACTORY->NewScript(source); + bool save_harmony_scoping = i::FLAG_harmony_scoping; + i::FLAG_harmony_scoping = harmony_scoping; + i::Parser parser(script, flags, NULL, NULL); + i::CompilationInfo info(script); + info.MarkAsGlobal(); + i::FunctionLiteral* function = parser.ParseProgram(&info); + i::FLAG_harmony_scoping = save_harmony_scoping; + + i::String* type_string = NULL; + if (function == NULL) { + // Extract exception from the parser. + i::Handle type_symbol = FACTORY->LookupAsciiSymbol("type"); + CHECK(i::Isolate::Current()->has_pending_exception()); + i::MaybeObject* maybe_object = i::Isolate::Current()->pending_exception(); + i::JSObject* exception = NULL; + CHECK(maybe_object->To(&exception)); + + // Get the type string. + maybe_object = exception->GetProperty(*type_symbol); + CHECK(maybe_object->To(&type_string)); + } + + // Check that preparsing fails iff parsing fails. + if (data.has_error() && function != NULL) { + i::OS::Print( + "Preparser failed on:\n" + "\t%s\n" + "with error:\n" + "\t%s\n" + "However, the parser succeeded", + *source->ToCString(), data.BuildMessage()); + CHECK(false); + } else if (!data.has_error() && function == NULL) { + i::OS::Print( + "Parser failed on:\n" + "\t%s\n" + "with error:\n" + "\t%s\n" + "However, the preparser succeeded", + *source->ToCString(), *type_string->ToCString()); + CHECK(false); + } + + // Check that preparser and parser produce the same error. + if (function == NULL) { + if (!type_string->IsEqualTo(i::CStrVector(data.BuildMessage()))) { + i::OS::Print( + "Expected parser and preparser to produce the same error on:\n" + "\t%s\n" + "However, found the following error messages\n" + "\tparser: %s\n" + "\tpreparser: %s\n", + *source->ToCString(), *type_string->ToCString(), data.BuildMessage()); + CHECK(false); + } + } +} + + +void TestParserSyncWithFlags(i::Handle source) { + static const int kFlagsCount = 6; + const int flags[kFlagsCount] = { + i::kNoParsingFlags | i::CLASSIC_MODE, + i::kNoParsingFlags | i::STRICT_MODE, + i::kNoParsingFlags | i::EXTENDED_MODE, + i::kAllowLazy | i::CLASSIC_MODE, + i::kAllowLazy | i::STRICT_MODE, + i::kAllowLazy | i::EXTENDED_MODE + }; + + for (int k = 0; k < kFlagsCount; ++k) { + TestParserSync(source, flags[k]); + } +} + + +TEST(ParserSync) { + const char* context_data[][2] = { + { "", "" }, + { "{", "}" }, + { "if (true) ", " else {}" }, + { "if (true) {} else ", "" }, + { "if (true) ", "" }, + { "do ", " while (false)" }, + { "while (false) ", "" }, + { "for (;;) ", "" }, + { "with ({})", "" }, + { "switch (12) { case 12: ", "}" }, + { "switch (12) { default: ", "}" }, + { "label2: ", "" }, + { NULL, NULL } + }; + + const char* statement_data[] = { + "{}", + "var x", + "var x = 1", + "const x", + "const x = 1", + ";", + "12", + "if (false) {} else ;", + "if (false) {} else {}", + "if (false) {} else 12", + "if (false) ;" + "if (false) {}", + "if (false) 12", + "do {} while (false)", + "for (;;) ;", + "for (;;) {}", + "for (;;) 12", + "continue", + "continue label", + "continue\nlabel", + "break", + "break label", + "break\nlabel", + "return", + "return 12", + "return\n12", + "with ({}) ;", + "with ({}) {}", + "with ({}) 12", + "switch ({}) { default: }" + "label3: " + "throw", + "throw 12", + "throw\n12", + "try {} catch(e) {}", + "try {} finally {}", + "try {} catch(e) {} finally {}", + "debugger", + NULL + }; + + const char* termination_data[] = { + "", + ";", + "\n", + ";\n", + "\n;", + NULL + }; + + v8::HandleScope handles; + v8::Persistent context = v8::Context::New(); + v8::Context::Scope context_scope(context); + + int marker; + i::Isolate::Current()->stack_guard()->SetStackLimit( + reinterpret_cast(&marker) - 128 * 1024); + + for (int i = 0; context_data[i][0] != NULL; ++i) { + for (int j = 0; statement_data[j] != NULL; ++j) { + for (int k = 0; termination_data[k] != NULL; ++k) { + int kPrefixLen = i::StrLength(context_data[i][0]); + int kStatementLen = i::StrLength(statement_data[j]); + int kTerminationLen = i::StrLength(termination_data[k]); + int kSuffixLen = i::StrLength(context_data[i][1]); + int kProgramSize = kPrefixLen + kStatementLen + kTerminationLen + + kSuffixLen + i::StrLength("label: for (;;) { }"); + + // Plug the source code pieces together. + i::Vector program = i::Vector::New(kProgramSize + 1); + int length = i::OS::SNPrintF(program, + "label: for (;;) { %s%s%s%s }", + context_data[i][0], + statement_data[j], + termination_data[k], + context_data[i][1]); + CHECK(length == kProgramSize); + i::Handle source = + FACTORY->NewStringFromAscii(i::CStrVector(program.start())); + TestParserSyncWithFlags(source); + } + } + } +} -- 2.7.4