From 11b8551f6056eefce51a97ffd2a2370fcf270634 Mon Sep 17 00:00:00 2001 From: "marja@chromium.org" Date: Thu, 15 May 2014 09:44:57 +0000 Subject: [PATCH] Parser / PreParser: Simplify error message arguments. In some places, we pretended that there can be multiple arguments, though in practice there was only one. In other places (most importantly, PreParser), we only handled one argument. (This means that we were not able to produce a multi-argument error inside a lazy function anyway.) This CL makes it clear that there is ever only one argument. R=ulan@chromium.org BUG= Review URL: https://codereview.chromium.org/273653002 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21324 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/parser.cc | 120 ++++++++++++++++---------------------------- src/parser.h | 8 +-- src/preparser.cc | 22 +++----- src/preparser.h | 23 ++++----- test/cctest/test-parsing.cc | 15 +++--- 5 files changed, 69 insertions(+), 119 deletions(-) diff --git a/src/parser.cc b/src/parser.cc index f07f37e..94d6575 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -300,19 +300,18 @@ const char* ScriptData::BuildMessage() const { } -Vector ScriptData::BuildArgs() const { +const char* ScriptData::BuildArg() const { int arg_count = Read(PreparseDataConstants::kMessageArgCountPos); - const char** array = NewArray(arg_count); + ASSERT(arg_count == 0 || arg_count == 1); + if (arg_count == 0) { + return NULL; + } // Position after text found by skipping past length field and // length field content words. int pos = PreparseDataConstants::kMessageTextPos + 1 + Read(PreparseDataConstants::kMessageTextPos); - for (int i = 0; i < arg_count; i++) { - int count = 0; - array[i] = ReadString(ReadAddress(pos), &count); - pos += count + 1; - } - return Vector(array, arg_count); + int count = 0; + return ReadString(ReadAddress(pos), &count); } @@ -623,7 +622,7 @@ Expression* ParserTraits::NewThrowError( void ParserTraits::ReportMessageAt(Scanner::Location source_location, const char* message, - Vector args, + const char* arg, bool is_reference_error) { if (parser_->stack_overflow()) { // Suppress the error message (syntax error or such) in the presence of a @@ -635,11 +634,11 @@ void ParserTraits::ReportMessageAt(Scanner::Location source_location, source_location.beg_pos, source_location.end_pos); Factory* factory = parser_->isolate()->factory(); - Handle elements = factory->NewFixedArray(args.length()); - for (int i = 0; i < args.length(); i++) { + Handle elements = factory->NewFixedArray(arg == NULL ? 0 : 1); + if (arg != NULL) { Handle arg_string = - factory->NewStringFromUtf8(CStrVector(args[i])).ToHandleChecked(); - elements->set(i, *arg_string); + factory->NewStringFromUtf8(CStrVector(arg)).ToHandleChecked(); + elements->set(0, *arg_string); } Handle array = factory->NewJSArrayWithElements(elements); Handle result = is_reference_error @@ -650,16 +649,16 @@ void ParserTraits::ReportMessageAt(Scanner::Location source_location, void ParserTraits::ReportMessage(const char* message, - Vector > args, + MaybeHandle arg, bool is_reference_error) { Scanner::Location source_location = parser_->scanner()->location(); - ReportMessageAt(source_location, message, args, is_reference_error); + ReportMessageAt(source_location, message, arg, is_reference_error); } void ParserTraits::ReportMessageAt(Scanner::Location source_location, const char* message, - Vector > args, + MaybeHandle arg, bool is_reference_error) { if (parser_->stack_overflow()) { // Suppress the error message (syntax error or such) in the presence of a @@ -671,9 +670,9 @@ void ParserTraits::ReportMessageAt(Scanner::Location source_location, source_location.beg_pos, source_location.end_pos); Factory* factory = parser_->isolate()->factory(); - Handle elements = factory->NewFixedArray(args.length()); - for (int i = 0; i < args.length(); i++) { - elements->set(i, *args[i]); + Handle elements = factory->NewFixedArray(arg.is_null() ? 0 : 1); + if (!arg.is_null()) { + elements->set(0, *(arg.ToHandleChecked())); } Handle array = factory->NewJSArrayWithElements(elements); Handle result = is_reference_error @@ -918,7 +917,7 @@ FunctionLiteral* Parser::DoParseProgram(CompilationInfo* info, !body->at(0)->IsExpressionStatement() || !body->at(0)->AsExpressionStatement()-> expression()->IsFunctionLiteral()) { - ReportMessage("single_function_literal", Vector::empty()); + ReportMessage("single_function_literal"); ok = false; } } @@ -1276,9 +1275,7 @@ Module* Parser::ParseModuleLiteral(bool* ok) { for (Interface::Iterator it = interface->iterator(); !it.done(); it.Advance()) { if (scope->LocalLookup(it.name()) == NULL) { - Handle name(it.name()); - ParserTraits::ReportMessage("module_export_undefined", - Vector >(&name, 1)); + ParserTraits::ReportMessage("module_export_undefined", it.name()); *ok = false; return NULL; } @@ -1317,8 +1314,7 @@ Module* Parser::ParseModulePath(bool* ok) { member->interface()->Print(); } #endif - ParserTraits::ReportMessage("invalid_module_path", - Vector >(&name, 1)); + ParserTraits::ReportMessage("invalid_module_path", name); return NULL; } result = member; @@ -1428,8 +1424,7 @@ Block* Parser::ParseImportDeclaration(bool* ok) { module->interface()->Print(); } #endif - ParserTraits::ReportMessage("invalid_module_path", - Vector >(&name, 1)); + ParserTraits::ReportMessage("invalid_module_path", name); return NULL; } VariableProxy* proxy = NewUnresolved(names[i], LET, interface); @@ -1717,10 +1712,7 @@ void Parser::Declare(Declaration* declaration, bool resolve, bool* ok) { if (allow_harmony_scoping() && strict_mode() == STRICT) { // In harmony we treat re-declarations as early errors. See // ES5 16 for a definition of early errors. - SmartArrayPointer c_string = name->ToCString(DISALLOW_NULLS); - const char* elms[1] = { c_string.get() }; - Vector args(elms, 1); - ReportMessage("var_redeclaration", args); + ParserTraits::ReportMessage("var_redeclaration", name); *ok = false; return; } @@ -1812,8 +1804,7 @@ void Parser::Declare(Declaration* declaration, bool resolve, bool* ok) { var->interface()->Print(); } #endif - ParserTraits::ReportMessage("module_type_error", - Vector >(&name, 1)); + ParserTraits::ReportMessage("module_type_error", name); } } } @@ -2032,14 +2023,14 @@ Block* Parser::ParseVariableDeclarations( if (var_context == kStatement) { // In strict mode 'const' declarations are only allowed in source // element positions. - ReportMessage("unprotected_const", Vector::empty()); + ReportMessage("unprotected_const"); *ok = false; return NULL; } mode = CONST; init_op = Token::INIT_CONST; } else { - ReportMessage("strict_const", Vector::empty()); + ReportMessage("strict_const"); *ok = false; return NULL; } @@ -2056,14 +2047,14 @@ Block* Parser::ParseVariableDeclarations( // // TODO(rossberg): make 'let' a legal identifier in sloppy mode. if (!allow_harmony_scoping() || strict_mode() == SLOPPY) { - ReportMessage("illegal_let", Vector::empty()); + ReportMessage("illegal_let"); *ok = false; return NULL; } Consume(Token::LET); if (var_context == kStatement) { // Let declarations are only allowed in source element positions. - ReportMessage("unprotected_let", Vector::empty()); + ReportMessage("unprotected_let"); *ok = false; return NULL; } @@ -2333,10 +2324,7 @@ Statement* Parser::ParseExpressionOrLabelledStatement(ZoneStringList* labels, // structured. However, these are probably changes we want to // make later anyway so we should go back and fix this then. if (ContainsLabel(labels, label) || TargetStackContainsLabel(label)) { - SmartArrayPointer c_string = label->ToCString(DISALLOW_NULLS); - const char* elms[1] = { c_string.get() }; - Vector args(elms, 1); - ReportMessage("label_redeclaration", args); + ParserTraits::ReportMessage("label_redeclaration", label); *ok = false; return NULL; } @@ -2421,12 +2409,10 @@ Statement* Parser::ParseContinueStatement(bool* ok) { if (target == NULL) { // Illegal continue statement. const char* message = "illegal_continue"; - Vector > args; if (!label.is_null()) { message = "unknown_label"; - args = Vector >(&label, 1); } - ParserTraits::ReportMessageAt(scanner()->location(), message, args); + ParserTraits::ReportMessageAt(scanner()->location(), message, label); *ok = false; return NULL; } @@ -2459,12 +2445,10 @@ Statement* Parser::ParseBreakStatement(ZoneStringList* labels, bool* ok) { if (target == NULL) { // Illegal break statement. const char* message = "illegal_break"; - Vector > args; if (!label.is_null()) { message = "unknown_label"; - args = Vector >(&label, 1); } - ParserTraits::ReportMessageAt(scanner()->location(), message, args); + ParserTraits::ReportMessageAt(scanner()->location(), message, label); *ok = false; return NULL; } @@ -2523,7 +2507,7 @@ Statement* Parser::ParseWithStatement(ZoneStringList* labels, bool* ok) { int pos = position(); if (strict_mode() == STRICT) { - ReportMessage("strict_mode_with", Vector::empty()); + ReportMessage("strict_mode_with"); *ok = false; return NULL; } @@ -2556,8 +2540,7 @@ CaseClause* Parser::ParseCaseClause(bool* default_seen_ptr, bool* ok) { } else { Expect(Token::DEFAULT, CHECK_OK); if (*default_seen_ptr) { - ReportMessage("multiple_defaults_in_switch", - Vector::empty()); + ReportMessage("multiple_defaults_in_switch"); *ok = false; return NULL; } @@ -2613,7 +2596,7 @@ Statement* Parser::ParseThrowStatement(bool* ok) { Expect(Token::THROW, CHECK_OK); int pos = position(); if (scanner()->HasAnyLineTerminatorBeforeNext()) { - ReportMessage("newline_after_throw", Vector::empty()); + ReportMessage("newline_after_throw"); *ok = false; return NULL; } @@ -2649,7 +2632,7 @@ TryStatement* Parser::ParseTryStatement(bool* ok) { Token::Value tok = peek(); if (tok != Token::CATCH && tok != Token::FINALLY) { - ReportMessage("no_catch_or_finally", Vector::empty()); + ReportMessage("no_catch_or_finally"); *ok = false; return NULL; } @@ -3079,10 +3062,7 @@ DebuggerStatement* Parser::ParseDebuggerStatement(bool* ok) { void Parser::ReportInvalidCachedData(Handle name, bool* ok) { - SmartArrayPointer name_string = name->ToCString(DISALLOW_NULLS); - const char* element[1] = { name_string.get() }; - ReportMessage("invalid_cached_data_function", - Vector(element, 1)); + ParserTraits::ReportMessage("invalid_cached_data_function", name); *ok = false; } @@ -3452,14 +3432,9 @@ void Parser::SkipLazyFunctionBody(Handle function_name, return; } if (logger.has_error()) { - const char* arg = logger.argument_opt(); - Vector args; - if (arg != NULL) { - args = Vector(&arg, 1); - } ParserTraits::ReportMessageAt( Scanner::Location(logger.start(), logger.end()), - logger.message(), args, logger.is_reference_error()); + logger.message(), logger.argument_opt(), logger.is_reference_error()); *ok = false; return; } @@ -3598,7 +3573,7 @@ Expression* Parser::ParseV8Intrinsic(bool* ok) { if (args->length() == 1 && args->at(0)->AsVariableProxy() != NULL) { return args->at(0); } else { - ReportMessage("not_isvar", Vector::empty()); + ReportMessage("not_isvar"); *ok = false; return NULL; } @@ -3608,15 +3583,14 @@ Expression* Parser::ParseV8Intrinsic(bool* ok) { if (function != NULL && function->nargs != -1 && function->nargs != args->length()) { - ReportMessage("illegal_access", Vector::empty()); + ReportMessage("illegal_access"); *ok = false; return NULL; } // Check that the function is defined if it's an inline runtime call. if (function == NULL && name->Get(0) == '_') { - ParserTraits::ReportMessage("not_defined", - Vector >(&name, 1)); + ParserTraits::ReportMessage("not_defined", name); *ok = false; return NULL; } @@ -3638,14 +3612,11 @@ void Parser::CheckConflictingVarDeclarations(Scope* scope, bool* ok) { // In harmony mode we treat conflicting variable bindinds as early // errors. See ES5 16 for a definition of early errors. Handle name = decl->proxy()->name(); - SmartArrayPointer c_string = name->ToCString(DISALLOW_NULLS); - const char* elms[1] = { c_string.get() }; - Vector args(elms, 1); int position = decl->proxy()->position(); Scanner::Location location = position == RelocInfo::kNoPosition ? Scanner::Location::invalid() : Scanner::Location(position, position + 1); - ParserTraits::ReportMessageAt(location, "var_redeclaration", args); + ParserTraits::ReportMessageAt(location, "var_redeclaration", name); *ok = false; } } @@ -4620,14 +4591,11 @@ bool Parser::Parse() { ScriptData* cached_data = *(info()->cached_data()); Scanner::Location loc = cached_data->MessageLocation(); const char* message = cached_data->BuildMessage(); - Vector args = cached_data->BuildArgs(); - ParserTraits::ReportMessageAt(loc, message, args, + const char* arg = cached_data->BuildArg(); + ParserTraits::ReportMessageAt(loc, message, arg, cached_data->IsReferenceError()); DeleteArray(message); - for (int i = 0; i < args.length(); i++) { - DeleteArray(args[i]); - } - DeleteArray(args.start()); + DeleteArray(arg); ASSERT(info()->isolate()->has_pending_exception()); } else { result = ParseProgram(); diff --git a/src/parser.h b/src/parser.h index 71bbfd1..4f872a6 100644 --- a/src/parser.h +++ b/src/parser.h @@ -89,7 +89,7 @@ class ScriptData { Scanner::Location MessageLocation() const; bool IsReferenceError() const; const char* BuildMessage() const; - Vector BuildArgs() const; + const char* BuildArg() const; int function_count() { int functions_size = @@ -515,14 +515,14 @@ class ParserTraits { // Reporting errors. void ReportMessageAt(Scanner::Location source_location, const char* message, - Vector args, + const char* arg, bool is_reference_error = false); void ReportMessage(const char* message, - Vector > args, + MaybeHandle arg, bool is_reference_error = false); void ReportMessageAt(Scanner::Location source_location, const char* message, - Vector > args, + MaybeHandle arg, bool is_reference_error = false); // "null" return type creators. diff --git a/src/preparser.cc b/src/preparser.cc index 96623b9..3b43e7e 100644 --- a/src/preparser.cc +++ b/src/preparser.cc @@ -35,31 +35,22 @@ namespace internal { void PreParserTraits::ReportMessageAt(Scanner::Location location, const char* message, - Vector args, + const char* arg, bool is_reference_error) { ReportMessageAt(location.beg_pos, location.end_pos, message, - args.length() > 0 ? args[0] : NULL, + arg, is_reference_error); } -void PreParserTraits::ReportMessageAt(Scanner::Location location, - const char* type, - const char* name_opt, - bool is_reference_error) { - pre_parser_->log_->LogMessage(location.beg_pos, location.end_pos, type, - name_opt, is_reference_error); -} - - void PreParserTraits::ReportMessageAt(int start_pos, int end_pos, - const char* type, - const char* name_opt, + const char* message, + const char* arg, bool is_reference_error) { - pre_parser_->log_->LogMessage(start_pos, end_pos, type, name_opt, + pre_parser_->log_->LogMessage(start_pos, end_pos, message, arg, is_reference_error); } @@ -294,8 +285,7 @@ PreParser::Statement PreParser::ParseStatement(bool* ok) { if (strict_mode() == STRICT) { PreParserTraits::ReportMessageAt(start_location.beg_pos, end_location.end_pos, - "strict_function", - NULL); + "strict_function"); *ok = false; return Statement::Default(); } else { diff --git a/src/preparser.h b/src/preparser.h index 6733658..9dee863 100644 --- a/src/preparser.h +++ b/src/preparser.h @@ -348,16 +348,15 @@ class ParserBase : public Traits { bool is_generator() const { return function_state_->is_generator(); } // Report syntax errors. - void ReportMessage(const char* message, Vector args, + void ReportMessage(const char* message, const char* arg = NULL, bool is_reference_error = false) { Scanner::Location source_location = scanner()->location(); - Traits::ReportMessageAt(source_location, message, args, is_reference_error); + Traits::ReportMessageAt(source_location, message, arg, is_reference_error); } void ReportMessageAt(Scanner::Location location, const char* message, bool is_reference_error = false) { - Traits::ReportMessageAt(location, message, Vector::empty(), - is_reference_error); + Traits::ReportMessageAt(location, message, NULL, is_reference_error); } void ReportUnexpectedToken(Token::Value token); @@ -968,16 +967,12 @@ class PreParserTraits { // Reporting errors. void ReportMessageAt(Scanner::Location location, const char* message, - Vector args, - bool is_reference_error = false); - void ReportMessageAt(Scanner::Location location, - const char* type, - const char* name_opt, + const char* arg = NULL, bool is_reference_error = false); void ReportMessageAt(int start_pos, int end_pos, - const char* type, - const char* name_opt, + const char* message, + const char* arg = NULL, bool is_reference_error = false); // "null" return type creators. @@ -1240,7 +1235,7 @@ void ParserBase::ReportUnexpectedToken(Token::Value token) { const char* name = Token::String(token); ASSERT(name != NULL); Traits::ReportMessageAt( - source_location, "unexpected_token", Vector(&name, 1)); + source_location, "unexpected_token", name); } } @@ -1321,7 +1316,7 @@ typename ParserBase::ExpressionT ParserBase::ParseRegExpLiteral( int pos = peek_position(); if (!scanner()->ScanRegExpPattern(seen_equal)) { Next(); - ReportMessage("unterminated_regexp", Vector::empty()); + ReportMessage("unterminated_regexp"); *ok = false; return Traits::EmptyExpression(); } @@ -1864,7 +1859,7 @@ ParserBase::ParseUnaryExpression(bool* ok) { // "delete identifier" is a syntax error in strict mode. if (op == Token::DELETE && strict_mode() == STRICT && this->IsIdentifier(expression)) { - ReportMessage("strict_delete", Vector::empty()); + ReportMessage("strict_delete"); *ok = false; return this->EmptyExpression(); } diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc index 58734d0..80a116f 100644 --- a/test/cctest/test-parsing.cc +++ b/test/cctest/test-parsing.cc @@ -1147,12 +1147,12 @@ i::Handle FormatMessage(i::ScriptData* data) { const char* message = data->BuildMessage(); i::Handle format = v8::Utils::OpenHandle( *v8::String::NewFromUtf8(CcTest::isolate(), message)); - i::Vector args = data->BuildArgs(); - i::Handle args_array = factory->NewJSArray(args.length()); - for (int i = 0; i < args.length(); i++) { + const char* arg = data->BuildArg(); + i::Handle args_array = factory->NewJSArray(arg == NULL ? 0 : 1); + if (arg != NULL) { i::JSArray::SetElement( - args_array, i, v8::Utils::OpenHandle(*v8::String::NewFromUtf8( - CcTest::isolate(), args[i])), + args_array, 0, v8::Utils::OpenHandle(*v8::String::NewFromUtf8( + CcTest::isolate(), arg)), NONE, i::SLOPPY).Check(); } i::Handle builtins(isolate->js_builtins_object()); @@ -1162,11 +1162,8 @@ i::Handle FormatMessage(i::ScriptData* data) { i::Handle result = i::Execution::Call( isolate, format_fun, builtins, 2, arg_handles).ToHandleChecked(); CHECK(result->IsString()); - for (int i = 0; i < args.length(); i++) { - i::DeleteArray(args[i]); - } - i::DeleteArray(args.start()); i::DeleteArray(message); + i::DeleteArray(arg); return i::Handle::cast(result); } -- 2.7.4