Reviewers: Dmitry Lomov (chromium), marja, Michael Starzinger, rossberg,
wingo,
Message:
Apart from avoiding temporary allocations, this should avoid the ASAN
bot failures!
Description:
Avoid temporary Vectors for arrow function parameter lists
Instead of using a Collector to gather the parameters from parameter
lists of arrow functions and then calling Collector::ToVector() on it,
declare the parameters on the function Scope as the AST subtree with
the paramters is traversed. This avoids the memory allocations done
by Collector, and removed the chance of leaking the result obtained
from Collector::ToVector().
BUG=
Please review this at https://codereview.chromium.org/387633003/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+40, -40 lines):
M src/parser.h
M src/parser.cc
M src/preparser.h
Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index
37e265aa4a059e7530186a301ec9a48ad7b49b1e..cf0ffdea6bb7306bb2de214e165e12d703e3ebe4
100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -3272,9 +3272,9 @@ Handle<FixedArray>
CompileTimeValue::GetElements(Handle<FixedArray> value) {
}
-bool CheckAndCollectArrowParameter(ParserTraits* traits,
- Collector<VariableProxy*>* collector,
- Expression* expression) {
+bool CheckAndDeclareArrowParameter(ParserTraits* traits, Expression*
expression,
+ Scope* scope, unsigned* num_params,
+ Scanner::Location* dupe_loc) {
// Case for empty parameter lists:
// () => ...
if (expression == NULL) return true;
@@ -3294,7 +3294,14 @@ bool CheckAndCollectArrowParameter(ParserTraits*
traits,
traits->IsFutureStrictReserved(raw_name))
return false;
- collector->Add(expression->AsVariableProxy());
+ if (scope->IsDeclared(raw_name)) {
+ *dupe_loc = Scanner::Location(
+ expression->position(), expression->position() +
raw_name->length());
+ return false;
+ }
+
+ scope->DeclareParameter(raw_name, VAR);
+ ++(*num_params);
return true;
}
@@ -3306,8 +3313,10 @@ bool CheckAndCollectArrowParameter(ParserTraits*
traits,
binop->right()->is_parenthesized())
return false;
- return CheckAndCollectArrowParameter(traits, collector, binop->left())
&&
- CheckAndCollectArrowParameter(traits, collector,
binop->right());
+ return CheckAndDeclareArrowParameter(traits, binop->left(), scope,
+ num_params, dupe_loc) &&
+ CheckAndDeclareArrowParameter(traits, binop->right(), scope,
+ num_params, dupe_loc);
}
// Any other kind of expression is not a valid parameter list.
@@ -3315,11 +3324,13 @@ bool CheckAndCollectArrowParameter(ParserTraits*
traits,
}
-Vector<VariableProxy*> ParserTraits::ParameterListFromExpression(
- Expression* expression, bool* ok) {
- Collector<VariableProxy*> collector;
- *ok = CheckAndCollectArrowParameter(this, &collector, expression);
- return collector.ToVector();
+unsigned ParserTraits::DeclareArrowParametersFromExpression(
+ Expression* expression, Scope* scope, Scanner::Location* dupe_loc,
+ bool* ok) {
+ unsigned num_params = 0;
+ *ok = CheckAndDeclareArrowParameter(this, expression, scope, &num_params,
+ dupe_loc);
+ return num_params;
}
Index: src/parser.h
diff --git a/src/parser.h b/src/parser.h
index
3b3450c2398d1bbbf89a92cd746e9f1dd12f2010..30527b0406b0cf6ca9345f3edd4d9d16654579ab
100644
--- a/src/parser.h
+++ b/src/parser.h
@@ -550,8 +550,10 @@ class ParserTraits {
V8_INLINE Scope* NewScope(Scope* parent_scope, ScopeType scope_type);
// Utility functions
- Vector<VariableProxy*> ParameterListFromExpression(Expression*
expression,
- bool* ok);
+ unsigned DeclareArrowParametersFromExpression(Expression* expression,
+ Scope* scope,
+ Scanner::Location*
dupe_loc,
+ bool* ok);
V8_INLINE AstValueFactory* ast_value_factory();
// Temporary glue; these functions will move to ParserBase.
Index: src/preparser.h
diff --git a/src/preparser.h b/src/preparser.h
index
156fd64a2e5db9aa2ea3f7cf8bbe5f75ae97e366..6c6a63ade3d30ba7f5c55aee9aa3dd641f8c0c67
100644
--- a/src/preparser.h
+++ b/src/preparser.h
@@ -1249,11 +1249,13 @@ class PreParserTraits {
bool is_generator, bool* ok);
// Utility functions
- Vector<PreParserIdentifier> ParameterListFromExpression(
- PreParserExpression expression, bool* ok) {
+ unsigned DeclareArrowParametersFromExpression(PreParserExpression
expression,
+ PreParserScope* scope,
+ Scanner::Location*
dupe_loc,
+ bool* ok) {
// TODO(aperez): Detect duplicated identifiers in paramlists.
*ok = expression.IsValidArrowParamList();
- return Vector<PreParserIdentifier>::empty();
+ return 0;
}
static AstValueFactory* ast_value_factory() { return NULL; }
@@ -2436,16 +2438,6 @@ template <class Traits>
typename ParserBase<Traits>::ExpressionT ParserBase<
Traits>::ParseArrowFunctionLiteral(int start_pos, ExpressionT
params_ast,
bool* ok) {
- typename Traits::Type::ParameterIdentifierVector params =
- Traits::ParameterListFromExpression(params_ast, ok);
-
- if (!*ok) {
- ReportMessageAt(Scanner::Location(start_pos,
scanner()->location().beg_pos),
- "malformed_arrow_function_parameter_list");
- params.Dispose();
- return this->EmptyExpression();
- }
-
// TODO(aperez): Change this to use ARROW_SCOPE
typename Traits::Type::ScopePtr scope =
this->NewScope(scope_, FUNCTION_SCOPE);
@@ -2453,30 +2445,25 @@ typename ParserBase<Traits>::ExpressionT ParserBase<
FunctionState function_state(&function_state_, &scope_, &scope, zone(),
this->ast_value_factory());
Scanner::Location dupe_error_loc = Scanner::Location::invalid();
+ unsigned num_params = Traits::DeclareArrowParametersFromExpression(
+ params_ast, scope_, &dupe_error_loc, ok);
+ if (!*ok) {
+ ReportMessageAt(Scanner::Location(start_pos,
scanner()->location().beg_pos),
+ "malformed_arrow_function_parameter_list");
+ return this->EmptyExpression();
+ }
- if (params.length() > Code::kMaxArguments) {
+ if (num_params > Code::kMaxArguments) {
ReportMessageAt(Scanner::Location(params_ast->position(), position()),
"too_many_parameters");
*ok = false;
- params.Dispose();
return this->EmptyExpression();
}
- for (int i = 0; i < params.length(); ++i) {
- const IdentifierT param_name = params.at(i)->raw_name();
- if (!dupe_error_loc.IsValid() && scope_->IsDeclared(param_name)) {
- int param_pos = params.at(i)->position();
- dupe_error_loc =
- Scanner::Location(param_pos, param_pos + param_name->length());
- }
- scope_->DeclareParameter(param_name, VAR);
- }
-
ExpressionT expression = ParseArrowFunctionLiteralBody(
- &function_state, scope, params.length(),
Scanner::Location::invalid(),
+ &function_state, scope, num_params, Scanner::Location::invalid(),
dupe_error_loc, Scanner::Location::invalid(),
FunctionLiteral::kNotParenthesized, start_pos, CHECK_OK);
- params.Dispose();
return expression;
}
--
--
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.
