[parser] RewritableExpressions should keep track of their Scope directly

Previously, the Parser stored a Scope alongside a RewritableExpression
for each potential destructuring assignment. This Scope was later used
during rewriting to set the correct context for the rewriting. But this
approach failed if a new Scope was inserted into the Scope chain between
the time the assignment was parsed and when it was rewritten.

By storing the Scope directly in RewritableExpression,
ReparentExpressionScopes() is able to appropriately re-scope such
expressions prior to their rewriting.

Bug: chromium:779457
Change-Id: Ieb429a3da841f76d5798610af59da4fccb000652
Reviewed-on: https://chromium-review.googlesource.com/767666
Commit-Queue: Adam Klein <adamk@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49368}
This commit is contained in:
Adam Klein 2017-11-14 11:58:55 -08:00 committed by Commit Bot
parent 61e04e2867
commit 082009fc3d
8 changed files with 62 additions and 33 deletions

View File

@ -2012,17 +2012,22 @@ class RewritableExpression final : public Expression {
set_rewritten();
}
Scope* scope() const { return scope_; }
void set_scope(Scope* scope) { scope_ = scope; }
private:
friend class AstNodeFactory;
explicit RewritableExpression(Expression* expression)
RewritableExpression(Expression* expression, Scope* scope)
: Expression(expression->position(), kRewritableExpression),
expr_(expression) {
expr_(expression),
scope_(scope) {
bit_field_ |= IsRewrittenField::encode(false);
DCHECK(!expression->IsRewritableExpression());
}
Expression* expr_;
Scope* scope_;
class IsRewrittenField
: public BitField<bool, Expression::kNextBitFieldIndex, 1> {};
@ -3062,9 +3067,10 @@ class AstNodeFactory final BASE_EMBEDDED {
Conditional(condition, then_expression, else_expression, position);
}
RewritableExpression* NewRewritableExpression(Expression* expression) {
RewritableExpression* NewRewritableExpression(Expression* expression,
Scope* scope) {
DCHECK_NOT_NULL(expression);
return new (zone_) RewritableExpression(expression);
return new (zone_) RewritableExpression(expression, scope);
}
Assignment* NewAssignment(Token::Value op,

View File

@ -27,6 +27,7 @@ class Reparenter final : public AstTraversalVisitor<Reparenter> {
void VisitFunctionLiteral(FunctionLiteral* expr);
void VisitClassLiteral(ClassLiteral* expr);
void VisitVariableProxy(VariableProxy* expr);
void VisitRewritableExpression(RewritableExpression* expr);
void VisitBlock(Block* stmt);
void VisitTryCatchStatement(TryCatchStatement* stmt);
@ -78,6 +79,11 @@ void Reparenter::VisitVariableProxy(VariableProxy* proxy) {
}
}
void Reparenter::VisitRewritableExpression(RewritableExpression* expr) {
Visit(expr->expression());
expr->set_scope(scope_);
}
void Reparenter::VisitBlock(Block* stmt) {
if (stmt->scope() != nullptr)
stmt->scope()->ReplaceOuterScope(scope_);

View File

@ -374,15 +374,6 @@ class ParserBase {
Scope* const outer_scope_;
};
struct DestructuringAssignment {
public:
DestructuringAssignment(ExpressionT expression, Scope* scope)
: assignment(expression), scope(scope) {}
ExpressionT assignment;
Scope* scope;
};
class FunctionState final : public BlockState {
public:
FunctionState(FunctionState** function_state_stack, Scope** scope_stack,
@ -404,12 +395,12 @@ class ParserBase {
void SetDestructuringAssignmentsScope(int pos, Scope* scope) {
for (int i = pos; i < destructuring_assignments_to_rewrite_.length();
++i) {
destructuring_assignments_to_rewrite_[i].scope = scope;
destructuring_assignments_to_rewrite_[i]->set_scope(scope);
}
}
const ZoneList<DestructuringAssignment>&
destructuring_assignments_to_rewrite() const {
const ZoneList<RewritableExpressionT>&
destructuring_assignments_to_rewrite() const {
return destructuring_assignments_to_rewrite_;
}
@ -458,8 +449,8 @@ class ParserBase {
};
private:
void AddDestructuringAssignment(DestructuringAssignment pair) {
destructuring_assignments_to_rewrite_.Add(pair, scope_->zone());
void AddDestructuringAssignment(RewritableExpressionT expr) {
destructuring_assignments_to_rewrite_.Add(expr, scope_->zone());
}
void AddNonPatternForRewriting(RewritableExpressionT expr, bool* ok) {
@ -477,7 +468,7 @@ class ParserBase {
FunctionState* outer_function_state_;
DeclarationScope* scope_;
ZoneList<DestructuringAssignment> destructuring_assignments_to_rewrite_;
ZoneList<RewritableExpressionT> destructuring_assignments_to_rewrite_;
ZoneList<RewritableExpressionT> non_patterns_to_rewrite_;
ZoneList<typename ExpressionClassifier::Error> reported_errors_;
@ -2073,7 +2064,7 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseArrayLiteral(
ExpressionT result =
factory()->NewArrayLiteral(values, first_spread_index, pos);
if (first_spread_index >= 0) {
auto rewritable = factory()->NewRewritableExpression(result);
auto rewritable = factory()->NewRewritableExpression(result, scope());
impl()->QueueNonPatternForRewriting(rewritable, ok);
if (!*ok) {
// If the non-pattern rewriting mechanism is used in the future for
@ -2980,8 +2971,9 @@ ParserBase<Impl>::ParseAssignmentExpression(bool accept_IN, bool* ok) {
ExpressionT result = factory()->NewAssignment(op, expression, right, pos);
if (is_destructuring_assignment) {
result = factory()->NewRewritableExpression(result);
impl()->QueueDestructuringAssignmentForRewriting(result);
auto rewritable = factory()->NewRewritableExpression(result, scope());
impl()->QueueDestructuringAssignmentForRewriting(rewritable);
result = rewritable;
}
return result;

View File

@ -3866,15 +3866,13 @@ void Parser::RewriteDestructuringAssignments() {
for (int i = assignments.length() - 1; i >= 0; --i) {
// Rewrite list in reverse, so that nested assignment patterns are rewritten
// correctly.
const DestructuringAssignment& pair = assignments.at(i);
RewritableExpression* to_rewrite =
pair.assignment->AsRewritableExpression();
RewritableExpression* to_rewrite = assignments[i];
DCHECK_NOT_NULL(to_rewrite);
if (!to_rewrite->is_rewritten()) {
// Since this function is called at the end of parsing the program,
// pair.scope may already have been removed by FinalizeBlockScope in the
// meantime.
Scope* scope = pair.scope->GetUnremovedScope();
Scope* scope = to_rewrite->scope()->GetUnremovedScope();
BlockState block_state(&scope_, scope);
RewriteDestructuringAssignment(to_rewrite);
}
@ -4020,10 +4018,9 @@ Expression* Parser::RewriteSpreads(ArrayLiteral* lit) {
return factory()->NewDoExpression(do_block, result, lit->position());
}
void Parser::QueueDestructuringAssignmentForRewriting(Expression* expr) {
DCHECK(expr->IsRewritableExpression());
function_state_->AddDestructuringAssignment(
DestructuringAssignment(expr, scope()));
void Parser::QueueDestructuringAssignmentForRewriting(
RewritableExpression* expr) {
function_state_->AddDestructuringAssignment(expr);
}
void Parser::QueueNonPatternForRewriting(RewritableExpression* expr, bool* ok) {

View File

@ -564,7 +564,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
V8_INLINE void RewriteNonPattern(bool* ok);
V8_INLINE void QueueDestructuringAssignmentForRewriting(
Expression* assignment);
RewritableExpression* assignment);
V8_INLINE void QueueNonPatternForRewriting(RewritableExpression* expr,
bool* ok);

View File

@ -131,7 +131,7 @@ void Parser::RewriteDestructuringAssignment(RewritableExpression* to_rewrite) {
Expression* Parser::RewriteDestructuringAssignment(Assignment* assignment) {
DCHECK_NOT_NULL(assignment);
DCHECK_EQ(Token::ASSIGN, assignment->op());
auto to_rewrite = factory()->NewRewritableExpression(assignment);
auto to_rewrite = factory()->NewRewritableExpression(assignment, scope());
RewriteDestructuringAssignment(to_rewrite);
return to_rewrite->expression();
}

View File

@ -303,6 +303,7 @@ class PreParserExpression {
int position() const { return kNoSourcePosition; }
void set_function_token_position(int position) {}
void set_scope(Scope* scope) {}
private:
enum Type {
@ -604,7 +605,7 @@ class PreParserFactory {
return PreParserExpression::Default();
}
PreParserExpression NewRewritableExpression(
const PreParserExpression& expression) {
const PreParserExpression& expression, Scope* scope) {
return expression;
}
PreParserExpression NewAssignment(Token::Value op,

View File

@ -0,0 +1,27 @@
// Copyright 2017 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.
(function testEager() {
(function({name = [foo] = eval("[]")}) {})({});
(function([name = [foo] = eval("[]")]) {})([]);
})();
(function testLazy() {
function f({name = [foo] = eval("[]")}) {}
function g([name = [foo] = eval("[]")]) {}
f({});
g([]);
})();
(function testEagerArrow() {
(({name = [foo] = eval("[]")}) => {})({});
(([name = [foo] = eval("[]")]) => {})([]);
})();
(function testLazyArrow() {
var f = ({name = [foo] = eval("[]")}) => {};
var g = ([name = [foo] = eval("[]")]) => {};
f({});
g([]);
})();