Reviewers: rossberg,
Message:
PTAL
Description:
[es6] Fix completion values of for loops with lexical variables
Currently, the desugaring of for loops of the form for
(let/const ...; bla; bla) causes them to always have a
completion value of 1, regardless of whether the loop body
is executed or not. This CL fixes this, realigning
initializer blocks as a more general purpose way to avoid
the completion value rewriter (since that's all they really
do anyway).
BUG=
Please review this at https://codereview.chromium.org/1177053006/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+58, -21 lines):
M src/ast.h
M src/parser.cc
M src/prettyprinter.cc
M src/rewriter.cc
M test/mjsunit/es6/block-for.js
Index: src/ast.h
diff --git a/src/ast.h b/src/ast.h
index
0dc9e6066630898e326880092c60b157e4e8e6a7..235b77f7e355bdab0c935252fa07d2631d3e631f
100644
--- a/src/ast.h
+++ b/src/ast.h
@@ -484,7 +484,7 @@ class Block final : public BreakableStatement {
}
ZoneList<Statement*>* statements() { return &statements_; }
- bool is_initializer_block() const { return is_initializer_block_; }
+ bool ignore_completion_values() const { return
ignore_completion_values_; }
static int num_ids() { return parent_num_ids() + 1; }
BailoutId DeclsId() const { return BailoutId(local_id(0)); }
@@ -499,10 +499,10 @@ class Block final : public BreakableStatement {
protected:
Block(Zone* zone, ZoneList<const AstRawString*>* labels, int capacity,
- bool is_initializer_block, int pos)
+ bool ignore_completion_values, int pos)
: BreakableStatement(zone, labels, TARGET_FOR_NAMED_ONLY, pos),
statements_(capacity, zone),
- is_initializer_block_(is_initializer_block),
+ ignore_completion_values_(ignore_completion_values),
scope_(NULL) {}
static int parent_num_ids() { return BreakableStatement::num_ids(); }
@@ -510,7 +510,7 @@ class Block final : public BreakableStatement {
int local_id(int n) const { return base_id() + parent_num_ids() + n; }
ZoneList<Statement*> statements_;
- bool is_initializer_block_;
+ bool ignore_completion_values_;
Scope* scope_;
};
@@ -3271,12 +3271,10 @@ class AstNodeFactory final BASE_EMBEDDED {
return new (zone_) ExportDeclaration(zone_, proxy, scope, pos);
}
- Block* NewBlock(ZoneList<const AstRawString*>* labels,
- int capacity,
- bool is_initializer_block,
- int pos) {
+ Block* NewBlock(ZoneList<const AstRawString*>* labels, int capacity,
+ bool ignore_completion_values, int pos) {
return new (zone_)
- Block(zone_, labels, capacity, is_initializer_block, pos);
+ Block(zone_, labels, capacity, ignore_completion_values, pos);
}
#define
STATEMENT_WITH_LABELS(NodeType) \
Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index
816849748869720029796504e4d282dc6ca1bb2e..4e7333986da84969be8011181dcdfe05968065a8
100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -3222,7 +3222,10 @@ Statement*
Parser::DesugarLexicalBindingsInForStatement(
// let/const x = i;
// temp_x = x;
// first = 1;
+ // undefined;
// outer: for (;;) {
+ // { // Nothing inside this block can affect the normal completion
value.
+ // // No new lexical scope is introduced.
// let/const x = temp_x;
// if (first == 1) {
// first = 0;
@@ -3230,6 +3233,7 @@ Statement*
Parser::DesugarLexicalBindingsInForStatement(
// next;
// }
// flag = 1;
+ // }
// labels: for (; flag == 1; flag = 0, temp_x = x) {
// if (cond) {
// body
@@ -3282,6 +3286,13 @@ Statement*
Parser::DesugarLexicalBindingsInForStatement(
outer_block->AddStatement(assignment_statement, zone());
}
+ // make statement: undefined;
+ outer_block->AddStatement(
+ factory()->NewExpressionStatement(
+ factory()->NewUndefinedLiteral(RelocInfo::kNoPosition),
+ RelocInfo::kNoPosition),
+ zone());
+
// Make statement: outer: for (;;)
// Note that we don't actually create the label, or set this loop up as
an
// explicit break target, instead handing it directly to those nodes that
@@ -3294,8 +3305,10 @@ Statement*
Parser::DesugarLexicalBindingsInForStatement(
outer_block->set_scope(for_scope);
scope_ = inner_scope;
- Block* inner_block = factory()->NewBlock(NULL, names->length() + 4,
false,
- RelocInfo::kNoPosition);
+ Block* inner_block =
+ factory()->NewBlock(NULL, 3, false, RelocInfo::kNoPosition);
+ Block* ignore_completion_block = factory()->NewBlock(
+ NULL, names->length() + 2, true, RelocInfo::kNoPosition);
ZoneList<Variable*> inner_vars(names->length(), zone());
// For each let variable x:
// make statement: let/const x = temp_x.
@@ -3314,7 +3327,7 @@ Statement*
Parser::DesugarLexicalBindingsInForStatement(
factory()->NewExpressionStatement(assignment,
RelocInfo::kNoPosition);
DCHECK(init->position() != RelocInfo::kNoPosition);
proxy->var()->set_initializer_position(init->position());
- inner_block->AddStatement(assignment_statement, zone());
+ ignore_completion_block->AddStatement(assignment_statement, zone());
}
// Make statement: if (first == 1) { first = 0; } else { next; }
@@ -3340,7 +3353,7 @@ Statement*
Parser::DesugarLexicalBindingsInForStatement(
}
Statement* clear_first_or_next = factory()->NewIfStatement(
compare, clear_first, next, RelocInfo::kNoPosition);
- inner_block->AddStatement(clear_first_or_next, zone());
+ ignore_completion_block->AddStatement(clear_first_or_next, zone());
}
Variable* flag = scope_->DeclarationScope()->NewTemporary(temp_name);
@@ -3352,9 +3365,9 @@ Statement*
Parser::DesugarLexicalBindingsInForStatement(
Token::ASSIGN, flag_proxy, const1, RelocInfo::kNoPosition);
Statement* assignment_statement =
factory()->NewExpressionStatement(assignment,
RelocInfo::kNoPosition);
- inner_block->AddStatement(assignment_statement, zone());
+ ignore_completion_block->AddStatement(assignment_statement, zone());
}
-
+ inner_block->AddStatement(ignore_completion_block, zone());
// Make cond expression for main loop: flag == 1.
Expression* flag_cond = NULL;
{
@@ -3402,7 +3415,7 @@ Statement*
Parser::DesugarLexicalBindingsInForStatement(
}
// Make statement: labels: for (; flag == 1; flag = 0, temp_x = x)
- // Note that we re-use the original loop node, which retains it labels
+ // Note that we re-use the original loop node, which retains its labels
// and ensures that any break or continue statements in body point to
// the right place.
loop->Initialize(NULL, flag_cond, compound_next_statement, body_or_stop);
Index: src/prettyprinter.cc
diff --git a/src/prettyprinter.cc b/src/prettyprinter.cc
index
284beadfeedb5a7dc64472a6372a4091e6a42eff..ea273ef6aa89addadbf796a744bba97b9886c52c
100644
--- a/src/prettyprinter.cc
+++ b/src/prettyprinter.cc
@@ -433,10 +433,10 @@ PrettyPrinter::~PrettyPrinter() {
void PrettyPrinter::VisitBlock(Block* node) {
- if (!node->is_initializer_block()) Print("{ ");
+ if (!node->ignore_completion_values()) Print("{ ");
PrintStatements(node->statements());
if (node->statements()->length() > 0) Print(" ");
- if (!node->is_initializer_block()) Print("}");
+ if (!node->ignore_completion_values()) Print("}");
}
@@ -1146,7 +1146,8 @@ void
AstPrinter::PrintArguments(ZoneList<Expression*>* arguments) {
void AstPrinter::VisitBlock(Block* node) {
- const char* block_txt = node->is_initializer_block() ? "BLOCK
INIT" : "BLOCK";
+ const char* block_txt =
+ node->ignore_completion_values() ? "BLOCK NOCOMPLETIONS" : "BLOCK";
IndentedScope indent(this, block_txt);
PrintStatements(node->statements());
}
Index: src/rewriter.cc
diff --git a/src/rewriter.cc b/src/rewriter.cc
index
ee08d885f602f2d110e60a540cf7a0e4281ce241..b3977dd2ab0c6de7cdb0d136b32ade1449db04ec
100644
--- a/src/rewriter.cc
+++ b/src/rewriter.cc
@@ -83,7 +83,7 @@ void Processor::VisitBlock(Block* node) {
// with some JS VMs: For instance, using smjs, print(eval('var x = 7'))
// returns 'undefined'. To obtain the same behavior with v8, we need
// to prevent rewriting in that case.
- if (!node->is_initializer_block()) Process(node->statements());
+ if (!node->ignore_completion_values()) Process(node->statements());
}
Index: test/mjsunit/es6/block-for.js
diff --git a/test/mjsunit/es6/block-for.js b/test/mjsunit/es6/block-for.js
index
b91af0116cadb8a1f68b6240d7c6b494d6e52d16..420c41e610cce619cdfd7e5a549b2d680979a52b
100644
--- a/test/mjsunit/es6/block-for.js
+++ b/test/mjsunit/es6/block-for.js
@@ -158,7 +158,7 @@ closure_in_for_next();
// In a for-in statement the iteration variable is fresh
-// for earch iteration.
+// for each iteration.
function closures3(x) {
let a = [];
for (let p in x) {
@@ -171,3 +171,28 @@ function closures3(x) {
}
}
closures3({a : [0], b : 1, c : {v : 1}, get d() {}, set e(x) {}});
+
+// Check normal for statement completion values.
+assertEquals(1, eval("for (let i = 0; i < 10; i++) { 1; }"));
+assertEquals(9, eval("for (let i = 0; i < 10; i++) { i; }"));
+assertEquals(undefined, eval("for (let i = 0; false;) { }"));
+assertEquals(undefined, eval("for (const i = 0; false;) { }"));
+assertEquals(undefined, eval("for (let i = 0; i < 10; i++) { }"));
+assertEquals(undefined, eval("for (let i = 0; false;) { i; }"));
+assertEquals(undefined, eval("for (const i = 0; false;) { i; }"));
+assertEquals(undefined, eval("for (let i = 0; true;) { break; }"));
+assertEquals(undefined, eval("for (const i = 0; true;) { break; }"));
+assertEquals(undefined, eval("for (let i = 0; i < 10; i++) { continue;
}"));
+assertEquals(undefined, eval("for (let i = 0; true;) { break; i; }"));
+assertEquals(undefined, eval("for (const i = 0; true;) { break; i; }"));
+assertEquals(undefined, eval("for (let i = 0; i < 10; i++) { continue; i;
}"));
+assertEquals(0, eval("for (let i = 0; true;) { i; break; }"));
+assertEquals(0, eval("for (const i = 0; true;) { i; break; }"));
+assertEquals(9, eval("for (let i = 0; i < 10; i++) { i; continue; }"));
+assertEquals(3, eval("for (let i = 0; true; i++) { i; if (i >= 3) break;
}"));
+assertEquals(2, eval("for (let i = 0; true; i++) { if (i >= 3) break; i;
}"));
+assertEquals(
+ 2, eval("for (let i = 0; i < 10; i++) { if (i >= 3) continue; i; }"));
+assertEquals(undefined, eval("foo: for (let i = 0; true;) { break foo;
}"));
+assertEquals(undefined, eval("foo: for (const i = 0; true;) { break foo;
}"));
+assertEquals(3, eval("foo: for (let i = 3; true;) { i; break foo; }"));
--
--
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.
