[es6] Self-assignment in a default parameter initializer should throw
The first bug was that there are two different "initialization positions" passed into PatternRewriter::DeclareAndInitializeVariables, and we weren't setting them all properly for this case. After further code review, it became clear that we weren't even recording the correct position (the end of the initializer expression). The combination of those two bugs caused the hole check elimination code in full-codegen to skip emitting a hole check. This patch takes care of both of those things. A follow-up will try to reduce the number of "initializer positions" we track in the variable declaration code. R=littledan@chromium.org BUG=v8:4568 LOG=n Review URL: https://codereview.chromium.org/1468143004 Cr-Commit-Position: refs/heads/master@{#32237}
This commit is contained in:
parent
ceb92ebfdf
commit
b6e9f625c1
@ -4077,7 +4077,11 @@ void ParserTraits::ParseArrowFunctionFormalParameters(
|
|||||||
parser_->scope_, parameters->scope);
|
parser_->scope_, parameters->scope);
|
||||||
}
|
}
|
||||||
|
|
||||||
AddFormalParameter(parameters, expr, initializer, is_rest);
|
// TODO(adamk): params_loc.end_pos is not the correct initializer position,
|
||||||
|
// but it should be conservative enough to trigger hole checks for variables
|
||||||
|
// referenced in the initializer (if any).
|
||||||
|
AddFormalParameter(parameters, expr, initializer, params_loc.end_pos,
|
||||||
|
is_rest);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
@ -4539,7 +4543,15 @@ Block* Parser::BuildParameterInitializationBlock(
|
|||||||
descriptor.is_const = false;
|
descriptor.is_const = false;
|
||||||
descriptor.needs_init = true;
|
descriptor.needs_init = true;
|
||||||
descriptor.declaration_pos = parameter.pattern->position();
|
descriptor.declaration_pos = parameter.pattern->position();
|
||||||
|
// The position that will be used by the AssignmentExpression
|
||||||
|
// which copies from the temp parameter to the pattern.
|
||||||
|
//
|
||||||
|
// TODO(adamk): Should this be RelocInfo::kNoPosition, since
|
||||||
|
// it's just copying from a temp var to the real param var?
|
||||||
descriptor.initialization_pos = parameter.pattern->position();
|
descriptor.initialization_pos = parameter.pattern->position();
|
||||||
|
// The initializer position which will end up in,
|
||||||
|
// Variable::initializer_position(), used for hole check elimination.
|
||||||
|
int initializer_position = parameter.pattern->position();
|
||||||
Expression* initial_value =
|
Expression* initial_value =
|
||||||
factory()->NewVariableProxy(parameters.scope->parameter(i));
|
factory()->NewVariableProxy(parameters.scope->parameter(i));
|
||||||
if (parameter.initializer != nullptr) {
|
if (parameter.initializer != nullptr) {
|
||||||
@ -4554,6 +4566,7 @@ Block* Parser::BuildParameterInitializationBlock(
|
|||||||
condition, parameter.initializer, initial_value,
|
condition, parameter.initializer, initial_value,
|
||||||
RelocInfo::kNoPosition);
|
RelocInfo::kNoPosition);
|
||||||
descriptor.initialization_pos = parameter.initializer->position();
|
descriptor.initialization_pos = parameter.initializer->position();
|
||||||
|
initializer_position = parameter.initializer_end_position;
|
||||||
} else if (parameter.is_rest) {
|
} else if (parameter.is_rest) {
|
||||||
// $rest = [];
|
// $rest = [];
|
||||||
// for (var $argument_index = $rest_index;
|
// for (var $argument_index = $rest_index;
|
||||||
@ -4565,7 +4578,6 @@ Block* Parser::BuildParameterInitializationBlock(
|
|||||||
DCHECK(parameter.pattern->IsVariableProxy());
|
DCHECK(parameter.pattern->IsVariableProxy());
|
||||||
DCHECK_EQ(i, parameters.params.length() - 1);
|
DCHECK_EQ(i, parameters.params.length() - 1);
|
||||||
|
|
||||||
int pos = parameter.pattern->position();
|
|
||||||
Variable* temp_var = parameters.scope->parameter(i);
|
Variable* temp_var = parameters.scope->parameter(i);
|
||||||
auto empty_values = new (zone()) ZoneList<Expression*>(0, zone());
|
auto empty_values = new (zone()) ZoneList<Expression*>(0, zone());
|
||||||
auto empty_array = factory()->NewArrayLiteral(
|
auto empty_array = factory()->NewArrayLiteral(
|
||||||
@ -4629,8 +4641,6 @@ Block* Parser::BuildParameterInitializationBlock(
|
|||||||
zone());
|
zone());
|
||||||
|
|
||||||
init_block->statements()->Add(loop, zone());
|
init_block->statements()->Add(loop, zone());
|
||||||
|
|
||||||
descriptor.initialization_pos = pos;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
Scope* param_scope = scope_;
|
Scope* param_scope = scope_;
|
||||||
@ -4649,7 +4659,7 @@ Block* Parser::BuildParameterInitializationBlock(
|
|||||||
{
|
{
|
||||||
BlockState block_state(&scope_, param_scope);
|
BlockState block_state(&scope_, param_scope);
|
||||||
DeclarationParsingResult::Declaration decl(
|
DeclarationParsingResult::Declaration decl(
|
||||||
parameter.pattern, parameter.pattern->position(), initial_value);
|
parameter.pattern, initializer_position, initial_value);
|
||||||
PatternRewriter::DeclareAndInitializeVariables(param_block, &descriptor,
|
PatternRewriter::DeclareAndInitializeVariables(param_block, &descriptor,
|
||||||
&decl, nullptr, CHECK_OK);
|
&decl, nullptr, CHECK_OK);
|
||||||
}
|
}
|
||||||
|
27
src/parser.h
27
src/parser.h
@ -551,12 +551,17 @@ class SingletonLogger;
|
|||||||
struct ParserFormalParameters : FormalParametersBase {
|
struct ParserFormalParameters : FormalParametersBase {
|
||||||
struct Parameter {
|
struct Parameter {
|
||||||
Parameter(const AstRawString* name, Expression* pattern,
|
Parameter(const AstRawString* name, Expression* pattern,
|
||||||
Expression* initializer, bool is_rest)
|
Expression* initializer, int initializer_end_position,
|
||||||
: name(name), pattern(pattern), initializer(initializer),
|
bool is_rest)
|
||||||
|
: name(name),
|
||||||
|
pattern(pattern),
|
||||||
|
initializer(initializer),
|
||||||
|
initializer_end_position(initializer_end_position),
|
||||||
is_rest(is_rest) {}
|
is_rest(is_rest) {}
|
||||||
const AstRawString* name;
|
const AstRawString* name;
|
||||||
Expression* pattern;
|
Expression* pattern;
|
||||||
Expression* initializer;
|
Expression* initializer;
|
||||||
|
int initializer_end_position;
|
||||||
bool is_rest;
|
bool is_rest;
|
||||||
bool is_simple() const {
|
bool is_simple() const {
|
||||||
return pattern->IsVariableProxy() && initializer == nullptr && !is_rest;
|
return pattern->IsVariableProxy() && initializer == nullptr && !is_rest;
|
||||||
@ -793,9 +798,10 @@ class ParserTraits {
|
|||||||
V8_INLINE Scope* NewScope(Scope* parent_scope, ScopeType scope_type,
|
V8_INLINE Scope* NewScope(Scope* parent_scope, ScopeType scope_type,
|
||||||
FunctionKind kind = kNormalFunction);
|
FunctionKind kind = kNormalFunction);
|
||||||
|
|
||||||
V8_INLINE void AddFormalParameter(
|
V8_INLINE void AddFormalParameter(ParserFormalParameters* parameters,
|
||||||
ParserFormalParameters* parameters, Expression* pattern,
|
Expression* pattern,
|
||||||
Expression* initializer, bool is_rest);
|
Expression* initializer,
|
||||||
|
int initializer_end_position, bool is_rest);
|
||||||
V8_INLINE void DeclareFormalParameter(
|
V8_INLINE void DeclareFormalParameter(
|
||||||
Scope* scope, const ParserFormalParameters::Parameter& parameter,
|
Scope* scope, const ParserFormalParameters::Parameter& parameter,
|
||||||
ExpressionClassifier* classifier);
|
ExpressionClassifier* classifier);
|
||||||
@ -1337,9 +1343,11 @@ Expression* ParserTraits::SpreadCallNew(
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
void ParserTraits::AddFormalParameter(
|
void ParserTraits::AddFormalParameter(ParserFormalParameters* parameters,
|
||||||
ParserFormalParameters* parameters,
|
Expression* pattern,
|
||||||
Expression* pattern, Expression* initializer, bool is_rest) {
|
Expression* initializer,
|
||||||
|
int initializer_end_position,
|
||||||
|
bool is_rest) {
|
||||||
bool is_simple =
|
bool is_simple =
|
||||||
!is_rest && pattern->IsVariableProxy() && initializer == nullptr;
|
!is_rest && pattern->IsVariableProxy() && initializer == nullptr;
|
||||||
DCHECK(parser_->allow_harmony_destructuring_bind() ||
|
DCHECK(parser_->allow_harmony_destructuring_bind() ||
|
||||||
@ -1349,7 +1357,8 @@ void ParserTraits::AddFormalParameter(
|
|||||||
? pattern->AsVariableProxy()->raw_name()
|
? pattern->AsVariableProxy()->raw_name()
|
||||||
: parser_->ast_value_factory()->empty_string();
|
: parser_->ast_value_factory()->empty_string();
|
||||||
parameters->params.Add(
|
parameters->params.Add(
|
||||||
ParserFormalParameters::Parameter(name, pattern, initializer, is_rest),
|
ParserFormalParameters::Parameter(name, pattern, initializer,
|
||||||
|
initializer_end_position, is_rest),
|
||||||
parameters->scope->zone());
|
parameters->scope->zone());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -1708,9 +1708,10 @@ class PreParserTraits {
|
|||||||
return !tag.IsNoTemplateTag();
|
return !tag.IsNoTemplateTag();
|
||||||
}
|
}
|
||||||
|
|
||||||
void AddFormalParameter(
|
void AddFormalParameter(PreParserFormalParameters* parameters,
|
||||||
PreParserFormalParameters* parameters, PreParserExpression pattern,
|
PreParserExpression pattern,
|
||||||
PreParserExpression initializer, bool is_rest) {
|
PreParserExpression initializer,
|
||||||
|
int initializer_end_position, bool is_rest) {
|
||||||
++parameters->arity;
|
++parameters->arity;
|
||||||
}
|
}
|
||||||
void DeclareFormalParameter(Scope* scope, PreParserIdentifier parameter,
|
void DeclareFormalParameter(Scope* scope, PreParserIdentifier parameter,
|
||||||
@ -3839,7 +3840,8 @@ void ParserBase<Traits>::ParseFormalParameter(
|
|||||||
classifier->RecordNonSimpleParameter();
|
classifier->RecordNonSimpleParameter();
|
||||||
}
|
}
|
||||||
|
|
||||||
Traits::AddFormalParameter(parameters, pattern, initializer, is_rest);
|
Traits::AddFormalParameter(parameters, pattern, initializer,
|
||||||
|
scanner()->location().end_pos, is_rest);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
7
test/message/default-parameter-tdz-arrow.js
Normal file
7
test/message/default-parameter-tdz-arrow.js
Normal file
@ -0,0 +1,7 @@
|
|||||||
|
// Copyright 2015 the V8 project authors. All rights reserved.
|
||||||
|
// Use of this source code is governed by a BSD-style license that can be
|
||||||
|
// found in the LICENSE file.
|
||||||
|
//
|
||||||
|
// Flags: --harmony-default-parameters
|
||||||
|
|
||||||
|
((a=-a) => { })();
|
6
test/message/default-parameter-tdz-arrow.out
Normal file
6
test/message/default-parameter-tdz-arrow.out
Normal file
@ -0,0 +1,6 @@
|
|||||||
|
*%(basename)s:7: ReferenceError: a is not defined
|
||||||
|
((a=-a) => { })();
|
||||||
|
^
|
||||||
|
ReferenceError: a is not defined
|
||||||
|
at *%(basename)s:7:6
|
||||||
|
at *%(basename)s:7:16
|
7
test/message/default-parameter-tdz.js
Normal file
7
test/message/default-parameter-tdz.js
Normal file
@ -0,0 +1,7 @@
|
|||||||
|
// Copyright 2015 the V8 project authors. All rights reserved.
|
||||||
|
// Use of this source code is governed by a BSD-style license that can be
|
||||||
|
// found in the LICENSE file.
|
||||||
|
//
|
||||||
|
// Flags: --harmony-default-parameters
|
||||||
|
|
||||||
|
(function(a=+a) { })();
|
6
test/message/default-parameter-tdz.out
Normal file
6
test/message/default-parameter-tdz.out
Normal file
@ -0,0 +1,6 @@
|
|||||||
|
*%(basename)s:7: ReferenceError: a is not defined
|
||||||
|
(function(a=+a) { })();
|
||||||
|
^
|
||||||
|
ReferenceError: a is not defined
|
||||||
|
at *%(basename)s:7:14
|
||||||
|
at *%(basename)s:7:21
|
Loading…
Reference in New Issue
Block a user