Skip to content

Commit b6e9f62

Browse files
ajkleinCommit bot
ajklein
authored and
Commit bot
committed
[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. [email protected] BUG=v8:4568 LOG=n Review URL: https://codereview.chromium.org/1468143004 Cr-Commit-Position: refs/heads/master@{#32237}
1 parent ceb92eb commit b6e9f62

7 files changed

Lines changed: 65 additions & 18 deletions

File tree

‎src/parser.cc‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4077,7 +4077,11 @@ void ParserTraits::ParseArrowFunctionFormalParameters(
40774077
parser_->scope_, parameters->scope);
40784078
}
40794079

4080-
AddFormalParameter(parameters, expr, initializer, is_rest);
4080+
// TODO(adamk): params_loc.end_pos is not the correct initializer position,
4081+
// but it should be conservative enough to trigger hole checks for variables
4082+
// referenced in the initializer (if any).
4083+
AddFormalParameter(parameters, expr, initializer, params_loc.end_pos,
4084+
is_rest);
40814085
}
40824086

40834087

@@ -4539,7 +4543,15 @@ Block* Parser::BuildParameterInitializationBlock(
45394543
descriptor.is_const = false;
45404544
descriptor.needs_init = true;
45414545
descriptor.declaration_pos = parameter.pattern->position();
4546+
// The position that will be used by the AssignmentExpression
4547+
// which copies from the temp parameter to the pattern.
4548+
//
4549+
// TODO(adamk): Should this be RelocInfo::kNoPosition, since
4550+
// it's just copying from a temp var to the real param var?
45424551
descriptor.initialization_pos = parameter.pattern->position();
4552+
// The initializer position which will end up in,
4553+
// Variable::initializer_position(), used for hole check elimination.
4554+
int initializer_position = parameter.pattern->position();
45434555
Expression* initial_value =
45444556
factory()->NewVariableProxy(parameters.scope->parameter(i));
45454557
if (parameter.initializer != nullptr) {
@@ -4554,6 +4566,7 @@ Block* Parser::BuildParameterInitializationBlock(
45544566
condition, parameter.initializer, initial_value,
45554567
RelocInfo::kNoPosition);
45564568
descriptor.initialization_pos = parameter.initializer->position();
4569+
initializer_position = parameter.initializer_end_position;
45574570
} else if (parameter.is_rest) {
45584571
// $rest = [];
45594572
// for (var $argument_index = $rest_index;
@@ -4565,7 +4578,6 @@ Block* Parser::BuildParameterInitializationBlock(
45654578
DCHECK(parameter.pattern->IsVariableProxy());
45664579
DCHECK_EQ(i, parameters.params.length() - 1);
45674580

4568-
int pos = parameter.pattern->position();
45694581
Variable* temp_var = parameters.scope->parameter(i);
45704582
auto empty_values = new (zone()) ZoneList<Expression*>(0, zone());
45714583
auto empty_array = factory()->NewArrayLiteral(
@@ -4629,8 +4641,6 @@ Block* Parser::BuildParameterInitializationBlock(
46294641
zone());
46304642

46314643
init_block->statements()->Add(loop, zone());
4632-
4633-
descriptor.initialization_pos = pos;
46344644
}
46354645

46364646
Scope* param_scope = scope_;
@@ -4649,7 +4659,7 @@ Block* Parser::BuildParameterInitializationBlock(
46494659
{
46504660
BlockState block_state(&scope_, param_scope);
46514661
DeclarationParsingResult::Declaration decl(
4652-
parameter.pattern, parameter.pattern->position(), initial_value);
4662+
parameter.pattern, initializer_position, initial_value);
46534663
PatternRewriter::DeclareAndInitializeVariables(param_block, &descriptor,
46544664
&decl, nullptr, CHECK_OK);
46554665
}

‎src/parser.h‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -551,12 +551,17 @@ class SingletonLogger;
551551
struct ParserFormalParameters : FormalParametersBase {
552552
struct Parameter {
553553
Parameter(const AstRawString* name, Expression* pattern,
554-
Expression* initializer, bool is_rest)
555-
: name(name), pattern(pattern), initializer(initializer),
554+
Expression* initializer, int initializer_end_position,
555+
bool is_rest)
556+
: name(name),
557+
pattern(pattern),
558+
initializer(initializer),
559+
initializer_end_position(initializer_end_position),
556560
is_rest(is_rest) {}
557561
const AstRawString* name;
558562
Expression* pattern;
559563
Expression* initializer;
564+
int initializer_end_position;
560565
bool is_rest;
561566
bool is_simple() const {
562567
return pattern->IsVariableProxy() && initializer == nullptr && !is_rest;
@@ -793,9 +798,10 @@ class ParserTraits {
793798
V8_INLINE Scope* NewScope(Scope* parent_scope, ScopeType scope_type,
794799
FunctionKind kind = kNormalFunction);
795800

796-
V8_INLINE void AddFormalParameter(
797-
ParserFormalParameters* parameters, Expression* pattern,
798-
Expression* initializer, bool is_rest);
801+
V8_INLINE void AddFormalParameter(ParserFormalParameters* parameters,
802+
Expression* pattern,
803+
Expression* initializer,
804+
int initializer_end_position, bool is_rest);
799805
V8_INLINE void DeclareFormalParameter(
800806
Scope* scope, const ParserFormalParameters::Parameter& parameter,
801807
ExpressionClassifier* classifier);
@@ -1337,9 +1343,11 @@ Expression* ParserTraits::SpreadCallNew(
13371343
}
13381344

13391345

1340-
void ParserTraits::AddFormalParameter(
1341-
ParserFormalParameters* parameters,
1342-
Expression* pattern, Expression* initializer, bool is_rest) {
1346+
void ParserTraits::AddFormalParameter(ParserFormalParameters* parameters,
1347+
Expression* pattern,
1348+
Expression* initializer,
1349+
int initializer_end_position,
1350+
bool is_rest) {
13431351
bool is_simple =
13441352
!is_rest && pattern->IsVariableProxy() && initializer == nullptr;
13451353
DCHECK(parser_->allow_harmony_destructuring_bind() ||
@@ -1349,7 +1357,8 @@ void ParserTraits::AddFormalParameter(
13491357
? pattern->AsVariableProxy()->raw_name()
13501358
: parser_->ast_value_factory()->empty_string();
13511359
parameters->params.Add(
1352-
ParserFormalParameters::Parameter(name, pattern, initializer, is_rest),
1360+
ParserFormalParameters::Parameter(name, pattern, initializer,
1361+
initializer_end_position, is_rest),
13531362
parameters->scope->zone());
13541363
}
13551364

‎src/preparser.h‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1708,9 +1708,10 @@ class PreParserTraits {
17081708
return !tag.IsNoTemplateTag();
17091709
}
17101710

1711-
void AddFormalParameter(
1712-
PreParserFormalParameters* parameters, PreParserExpression pattern,
1713-
PreParserExpression initializer, bool is_rest) {
1711+
void AddFormalParameter(PreParserFormalParameters* parameters,
1712+
PreParserExpression pattern,
1713+
PreParserExpression initializer,
1714+
int initializer_end_position, bool is_rest) {
17141715
++parameters->arity;
17151716
}
17161717
void DeclareFormalParameter(Scope* scope, PreParserIdentifier parameter,
@@ -3839,7 +3840,8 @@ void ParserBase<Traits>::ParseFormalParameter(
38393840
classifier->RecordNonSimpleParameter();
38403841
}
38413842

3842-
Traits::AddFormalParameter(parameters, pattern, initializer, is_rest);
3843+
Traits::AddFormalParameter(parameters, pattern, initializer,
3844+
scanner()->location().end_pos, is_rest);
38433845
}
38443846

38453847

‎test/message/default-parameter-tdz-arrow.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// Copyright 2015 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
//
5+
// Flags: --harmony-default-parameters
6+
7+
((a=-a) => { })();

‎test/message/default-parameter-tdz-arrow.out‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
*%(basename)s:7: ReferenceError: a is not defined
2+
((a=-a) => { })();
3+
^
4+
ReferenceError: a is not defined
5+
at *%(basename)s:7:6
6+
at *%(basename)s:7:16

‎test/message/default-parameter-tdz.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// Copyright 2015 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
//
5+
// Flags: --harmony-default-parameters
6+
7+
(function(a=+a) { })();

‎test/message/default-parameter-tdz.out‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
*%(basename)s:7: ReferenceError: a is not defined
2+
(function(a=+a) { })();
3+
^
4+
ReferenceError: a is not defined
5+
at *%(basename)s:7:14
6+
at *%(basename)s:7:21

0 commit comments

Comments
 (0)