Skip to content

V
V8
Commit 14ac291a authored by Marja Hölttä's avatar Marja Hölttä Committed by Commit Bot
Browse files
Options

[parser] Skipping inner funcs: simplify rest parameter handling. 

With the params (a, b, ...c) the param / variable declaration order used to be
"temp, temp, c, a, b". Now it is "temp, temp, a, b, c" as you'd expect. This
makes it easier for PreParser to match the parameter order of Parser.

R=verwaest@chromium.org
BUG=v8:5516

Change-Id: I79da04ef3f812bf52c032bed6263c009fecb7988
Reviewed-on: https://chromium-review.googlesource.com/447677Reviewed-by: 's avatarRoss McIlroy <rmcilroy@chromium.org>
Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
Commit-Queue: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43490}
parent a6b20adb
Show whitespace changes
Inline Side-by-side
Showing with 96 additions and 82 deletions
src/parsing/parser.cc
View file @ 14ac291a
......@@ -2913,10 +2913,10 @@ Block* Parser::BuildParameterInitializationBlock(
const ParserFormalParameters& parameters, bool* ok) {
DCHECK(!parameters.is_simple);
DCHECK(scope()->is_function_scope());
DCHECK_EQ(scope(), parameters.scope);
Block* init_block = factory()->NewBlock(NULL, 1, true, kNoSourcePosition);
int index = 0;
for (auto parameter : parameters.params) {
if (parameter->is_nondestructuring_rest()) break;
DeclarationDescriptor descriptor;
descriptor.declaration_kind = DeclarationDescriptor::PARAMETER;
descriptor.scope = scope();
......
src/parsing/parser.h
View file @ 14ac291a
......@@ -157,11 +157,6 @@ struct ParserFormalParameters : FormalParametersBase {
return pattern->IsVariableProxy() && initializer == nullptr && !is_rest;
}
bool is_nondestructuring_rest() const {
DCHECK_IMPLIES(is_rest, initializer == nullptr);
return is_rest && pattern->IsVariableProxy();
}
Parameter** next() { return &next_parameter; }
Parameter* const* next() const { return &next_parameter; }
};
......@@ -1080,11 +1075,14 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
if (!is_simple) scope->SetHasNonSimpleParameters();
for (auto parameter : parameters) {
bool is_duplicate = false;
bool use_name = is_simple || parameter->is_nondestructuring_rest();
bool is_optional = parameter->initializer != nullptr;
// If the parameter list is simple, declare the parameters normally with
// their names. If the parameter list is not simple, declare a temporary
// for each parameter - the corresponding named variable is declared by
// BuildParamerterInitializationBlock.
scope->DeclareParameter(
use_name ? parameter->name : ast_value_factory()->empty_string(),
use_name ? VAR : TEMPORARY, is_optional, parameter->is_rest,
is_simple ? parameter->name : ast_value_factory()->empty_string(),
is_simple ? VAR : TEMPORARY, is_optional, parameter->is_rest,
&is_duplicate, ast_value_factory());
if (is_duplicate &&
classifier()->is_valid_formal_parameter_list_without_duplicates()) {
......
src/parsing/preparser.h
View file @ 14ac291a
......@@ -791,19 +791,13 @@ class PreParserFactory {
struct PreParserFormalParameters : FormalParametersBase {
struct Parameter : public ZoneObject {
Parameter(PreParserExpression pattern, bool is_destructuring, bool is_rest)
: pattern(pattern),
is_destructuring(is_destructuring),
is_rest(is_rest) {}
Parameter(ZoneList<VariableProxy*>* variables, bool is_rest)
: variables_(variables), is_rest(is_rest) {}
Parameter** next() { return &next_parameter; }
Parameter* const* next() const { return &next_parameter; }
bool is_nondestructuring_rest() const {
return is_rest && !is_destructuring;
}
PreParserExpression pattern;
ZoneList<VariableProxy*>* variables_;
Parameter* next_parameter = nullptr;
bool is_destructuring : 1;
bool is_rest : 1;
};
explicit PreParserFormalParameters(DeclarationScope* scope)
......@@ -1386,16 +1380,6 @@ class PreParser : public ParserBase<PreParser> {
V8_INLINE PreParserStatement BuildParameterInitializationBlock(
const PreParserFormalParameters& parameters, bool* ok) {
if (track_unresolved_variables_) {
for (auto parameter : parameters.params) {
if (parameter->is_nondestructuring_rest()) break;
if (parameter->pattern.variables_ != nullptr) {
for (auto variable : *parameter->pattern.variables_) {
scope()->DeclareVariableName(variable->raw_name(), LET);
}
}
}
}
return PreParserStatement::Default();
}
......@@ -1617,7 +1601,7 @@ class PreParser : public ParserBase<PreParser> {
if (track_unresolved_variables_) {
DCHECK(FLAG_lazy_inner_functions);
parameters->params.Add(new (zone()) PreParserFormalParameters::Parameter(
pattern, !IsIdentifier(pattern), is_rest));
pattern.variables_, is_rest));
}
parameters->UpdateArityAndFunctionLength(!initializer.IsEmpty(), is_rest);
}
......@@ -1630,13 +1614,11 @@ class PreParser : public ParserBase<PreParser> {
if (track_unresolved_variables_) {
DCHECK(FLAG_lazy_inner_functions);
for (auto parameter : parameters) {
bool use_name = is_simple || parameter->is_nondestructuring_rest();
if (use_name) {
DCHECK_NOT_NULL(parameter->pattern.variables_);
DCHECK_EQ(parameter->pattern.variables_->length(), 1);
auto variable = (*parameter->pattern.variables_)[0];
scope->DeclareParameterName(variable->raw_name(), parameter->is_rest,
ast_value_factory());
if (parameter->variables_ != nullptr) {
for (auto variable : (*parameter->variables_)) {
scope->DeclareParameterName(
variable->raw_name(), parameter->is_rest, ast_value_factory());
}
}
}
}
......
test/cctest/interpreter/bytecode_expectations/CreateRestParameter.golden
View file @ 14ac291a
......@@ -11,14 +11,17 @@ snippet: "
function f(...restArgs) { return restArgs; }
f();
"
frame size: 1
frame size: 2
parameter count: 1
bytecode array length: 6
bytecode array length: 13
bytecodes: [
B(CreateRestParameter),
B(Star), R(0),
B(LdaTheHole),
B(Star), R(1),
/* 10 E> */ B(StackCheck),
/* 26 S> */ B(Nop),
B(Mov), R(0), R(1),
/* 26 S> */ B(Ldar), R(1),
/* 43 S> */ B(Return),
]
constant pool: [
......@@ -31,17 +34,20 @@ snippet: "
function f(a, ...restArgs) { return restArgs; }
f();
"
frame size: 2
frame size: 3
parameter count: 2
bytecode array length: 13
bytecode array length: 19
bytecodes: [
B(CreateRestParameter),
B(Star), R(0),
B(LdaTheHole),
B(Star), R(1),
B(LdaTheHole),
B(Star), R(2),
/* 10 E> */ B(StackCheck),
B(Mov), R(arg0), R(1),
/* 29 S> */ B(Ldar), R(0),
B(Mov), R(0), R(2),
/* 29 S> */ B(Ldar), R(2),
/* 46 S> */ B(Return),
]
constant pool: [
......@@ -54,18 +60,21 @@ snippet: "
function f(a, ...restArgs) { return restArgs[0]; }
f();
"
frame size: 2
frame size: 3
parameter count: 2
bytecode array length: 15
bytecode array length: 21
bytecodes: [
B(CreateRestParameter),
B(Star), R(0),
B(LdaTheHole),
B(Star), R(1),
B(LdaTheHole),
B(Star), R(2),
/* 10 E> */ B(StackCheck),
B(Mov), R(arg0), R(1),
B(Mov), R(0), R(2),
/* 29 S> */ B(LdaZero),
/* 44 E> */ B(LdaKeyedProperty), R(0), U8(2),
/* 44 E> */ B(LdaKeyedProperty), R(2), U8(2),
/* 49 S> */ B(Return),
]
constant pool: [
......@@ -78,24 +87,27 @@ snippet: "
function f(a, ...restArgs) { return restArgs[0] + arguments[0]; }
f();
"
frame size: 4
frame size: 5
parameter count: 2
bytecode array length: 27
bytecode array length: 33
bytecodes: [
B(CreateUnmappedArguments),
B(Star), R(2),
B(Star), R(3),
B(CreateRestParameter),
B(Star), R(0),
B(LdaTheHole),
B(Star), R(1),
B(LdaTheHole),
B(Star), R(2),
/* 10 E> */ B(StackCheck),
B(Mov), R(arg0), R(1),
B(Mov), R(0), R(2),
/* 29 S> */ B(LdaZero),
/* 44 E> */ B(LdaKeyedProperty), R(0), U8(2),
B(Star), R(3),
/* 44 E> */ B(LdaKeyedProperty), R(2), U8(2),
B(Star), R(4),
B(LdaZero),
/* 59 E> */ B(LdaKeyedProperty), R(2), U8(4),
/* 48 E> */ B(Add), R(3), U8(6),
/* 59 E> */ B(LdaKeyedProperty), R(3), U8(4),
/* 48 E> */ B(Add), R(4), U8(6),
/* 64 S> */ B(Return),
]
constant pool: [
......
test/cctest/interpreter/bytecode_expectations/SuperCallAndSpread.golden
View file @ 14ac291a
......@@ -51,34 +51,37 @@ snippet: "
test = new B(1, 2, 3).constructor;
})();
"
frame size: 6
frame size: 7
parameter count: 1
bytecode array length: 59
bytecode array length: 65
bytecodes: [
B(CreateRestParameter),
B(Star), R(2),
B(Mov), R(closure), R(1),
B(Mov), R(new_target), R(0),
B(Ldar), R(new_target),
B(LdaTheHole),
B(Star), R(3),
/* 128 E> */ B(StackCheck),
B(Mov), R(2), R(3),
/* 140 S> */ B(Ldar), R(1),
B(GetSuperConstructor), R(3),
B(GetSuperConstructor), R(4),
B(LdaSmi), I8(1),
B(Star), R(4),
B(Star), R(5),
B(Ldar), R(0),
B(Mov), R(2), R(5),
/* 140 E> */ B(ConstructWithSpread), R(3), R(4), U8(2),
B(Star), R(3),
B(Mov), R(2), R(6),
/* 140 E> */ B(ConstructWithSpread), R(4), R(5), U8(2),
B(Star), R(4),
B(Ldar), R(this),
B(JumpIfNotHole), U8(4),
B(Jump), U8(7),
/* 140 E> */ B(CallRuntime), U16(Runtime::kThrowSuperAlreadyCalledError), R(0), U8(0),
B(Mov), R(3), R(this),
B(Mov), R(4), R(this),
B(Ldar), R(this),
B(JumpIfNotHole), U8(11),
B(LdaConstant), U8(0),
B(Star), R(3),
B(CallRuntime), U16(Runtime::kThrowReferenceError), R(3), U8(1),
B(Star), R(4),
B(CallRuntime), U16(Runtime::kThrowReferenceError), R(4), U8(1),
/* 159 S> */ B(Return),
]
constant pool: [
......@@ -100,46 +103,49 @@ snippet: "
test = new B(1, 2, 3).constructor;
})();
"
frame size: 9
frame size: 10
parameter count: 1
bytecode array length: 92
bytecode array length: 98
bytecodes: [
B(CreateRestParameter),
B(Star), R(2),
B(Mov), R(closure), R(1),
B(Mov), R(new_target), R(0),
B(Ldar), R(new_target),
B(LdaTheHole),
B(Star), R(3),
/* 128 E> */ B(StackCheck),
B(Mov), R(2), R(3),
/* 140 S> */ B(LdaUndefined),
B(Star), R(3),
/* 140 E> */ B(CallRuntime), U16(Runtime::k_GetSuperConstructor), R(1), U8(1),
B(Star), R(4),
B(LdaUndefined),
/* 140 E> */ B(CallRuntime), U16(Runtime::k_GetSuperConstructor), R(1), U8(1),
B(Star), R(5),
B(CreateArrayLiteral), U8(0), U8(2), U8(9),
B(Star), R(6),
B(LdaUndefined),
B(Star), R(6),
B(CreateArrayLiteral), U8(0), U8(2), U8(9),
B(Star), R(7),
B(Mov), R(2), R(8),
/* 152 E> */ B(CallJSRuntime), U8(%spread_iterable), R(7), U8(2),
B(Star), R(7),
B(CreateArrayLiteral), U8(1), U8(3), U8(9),
B(LdaUndefined),
B(Star), R(8),
B(CallJSRuntime), U8(%spread_arguments), R(5), U8(4),
B(Star), R(5),
B(Mov), R(0), R(6),
/* 140 E> */ B(CallJSRuntime), U8(%reflect_construct), R(3), U8(4),
B(Star), R(3),
B(Mov), R(2), R(9),
/* 152 E> */ B(CallJSRuntime), U8(%spread_iterable), R(8), U8(2),
B(Star), R(8),
B(CreateArrayLiteral), U8(1), U8(3), U8(9),
B(Star), R(9),
B(CallJSRuntime), U8(%spread_arguments), R(6), U8(4),
B(Star), R(6),
B(Mov), R(0), R(7),
/* 140 E> */ B(CallJSRuntime), U8(%reflect_construct), R(4), U8(4),
B(Star), R(4),
B(Ldar), R(this),
B(JumpIfNotHole), U8(4),
B(Jump), U8(7),
/* 140 E> */ B(CallRuntime), U16(Runtime::kThrowSuperAlreadyCalledError), R(0), U8(0),
B(Mov), R(3), R(this),
B(Mov), R(4), R(this),
B(Ldar), R(this),
B(JumpIfNotHole), U8(11),
B(LdaConstant), U8(2),
B(Star), R(3),
B(CallRuntime), U16(Runtime::kThrowReferenceError), R(3), U8(1),
B(Star), R(4),
B(CallRuntime), U16(Runtime::kThrowReferenceError), R(4), U8(1),
/* 162 S> */ B(Return),
]
constant pool: [
......
test/cctest/scope-test-helper.h
View file @ 14ac291a
......@@ -55,7 +55,23 @@ class ScopeTestHelper {
CHECK_EQ(data->backing_store_[index++], local_name->raw_data()[i]);
}
#endif
CHECK_EQ(data->backing_store_[index++], local->location());
// Allow PreParser to not distinguish between parameter / local; that
// information is not relevant for deciding the allocation (potentially
// skipped inner functions don't affect it).
int location = data->backing_store_[index++];
switch (local->location()) {
case PARAMETER:
case LOCAL:
CHECK(location == PARAMETER || location == LOCAL);
break;
case CONTEXT:
case UNALLOCATED:
CHECK_EQ(location, local->location());
break;
default:
CHECK(false);
}
if (precise_maybe_assigned) {
CHECK_EQ(data->backing_store_[index++], local->maybe_assigned());
} else {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment