Commit a33fcd66 authored by neis's avatar neis Committed by Commit bot

[parsing] Fix maybe-assigned for loop variables.

Due to hoisting, the value of a 'var'-declared variable may actually change even
if the code contains only the "initial" assignment, namely when that assignment
occurs inside a loop.  For example:

  let i = 10;
  do { var x = i } while (i--):

As a simple and very conservative approximation of this, we explicitly mark
as maybe-assigned any non-lexical variable whose "declaration" does not
syntactically occur in the function scope.  (In the example above, it
occurs in a block scope.)

BUG=v8:5636

Review-Url: https://codereview.chromium.org/2673403003
Cr-Commit-Position: refs/heads/master@{#42989}
parent 9458dc9e
...@@ -181,6 +181,7 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) { ...@@ -181,6 +181,7 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
bool* sloppy_mode_block_scope_function_redefinition, bool* sloppy_mode_block_scope_function_redefinition,
bool* ok); bool* ok);
// The return value is meaningful only if FLAG_preparser_scope_analysis is on.
Variable* DeclareVariableName(const AstRawString* name, VariableMode mode); Variable* DeclareVariableName(const AstRawString* name, VariableMode mode);
// Declarations list. // Declarations list.
......
...@@ -1347,6 +1347,24 @@ class ParserBase { ...@@ -1347,6 +1347,24 @@ class ParserBase {
return expression->IsObjectLiteral() || expression->IsArrayLiteral(); return expression->IsObjectLiteral() || expression->IsArrayLiteral();
} }
// Due to hoisting, the value of a 'var'-declared variable may actually change
// even if the code contains only the "initial" assignment, namely when that
// assignment occurs inside a loop. For example:
//
// let i = 10;
// do { var x = i } while (i--):
//
// As a simple and very conservative approximation of this, we explicitly mark
// as maybe-assigned any non-lexical variable whose initializing "declaration"
// does not syntactically occur in the function scope. (In the example above,
// it occurs in a block scope.)
//
// Note that non-lexical variables include temporaries, which may also get
// assigned inside a loop due to the various rewritings that the parser
// performs.
//
static void MarkLoopVariableAsAssigned(Scope* scope, Variable* var);
// Keep track of eval() calls since they disable all local variable // Keep track of eval() calls since they disable all local variable
// optimizations. This checks if expression is an eval call, and if yes, // optimizations. This checks if expression is an eval call, and if yes,
// forwards the information to scope. // forwards the information to scope.
...@@ -5698,8 +5716,12 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseStandardForLoop( ...@@ -5698,8 +5716,12 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseStandardForLoop(
return loop; return loop;
} }
#undef CHECK_OK template <typename Impl>
#undef CHECK_OK_CUSTOM void ParserBase<Impl>::MarkLoopVariableAsAssigned(Scope* scope, Variable* var) {
if (!IsLexicalVariableMode(var->mode()) && !scope->is_function_scope()) {
var->set_maybe_assigned();
}
}
template <typename Impl> template <typename Impl>
void ParserBase<Impl>::ObjectLiteralChecker::CheckDuplicateProto( void ParserBase<Impl>::ObjectLiteralChecker::CheckDuplicateProto(
...@@ -5751,6 +5773,8 @@ void ParserBase<Impl>::ClassLiteralChecker::CheckClassMethodName( ...@@ -5751,6 +5773,8 @@ void ParserBase<Impl>::ClassLiteralChecker::CheckClassMethodName(
} }
} }
#undef CHECK_OK
#undef CHECK_OK_CUSTOM
#undef CHECK_OK_VOID #undef CHECK_OK_VOID
} // namespace internal } // namespace internal
......
...@@ -177,6 +177,8 @@ void Parser::PatternRewriter::VisitVariableProxy(VariableProxy* pattern) { ...@@ -177,6 +177,8 @@ void Parser::PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
// If there's no initializer, we're done. // If there's no initializer, we're done.
if (value == nullptr) return; if (value == nullptr) return;
MarkLoopVariableAsAssigned(var_init_scope, proxy->var());
// A declaration of the form: // A declaration of the form:
// //
// var v = x; // var v = x;
......
...@@ -310,8 +310,14 @@ void PreParser::DeclareAndInitializeVariables( ...@@ -310,8 +310,14 @@ void PreParser::DeclareAndInitializeVariables(
DCHECK(track_unresolved_variables_); DCHECK(track_unresolved_variables_);
for (auto variable : *(declaration->pattern.variables_)) { for (auto variable : *(declaration->pattern.variables_)) {
declaration_descriptor->scope->RemoveUnresolved(variable); declaration_descriptor->scope->RemoveUnresolved(variable);
scope()->DeclareVariableName(variable->raw_name(), Variable* var = scope()->DeclareVariableName(
declaration_descriptor->mode); variable->raw_name(), declaration_descriptor->mode);
if (FLAG_preparser_scope_analysis) {
MarkLoopVariableAsAssigned(declaration_descriptor->scope, var);
// This is only necessary if there is an initializer, but we don't have
// that information here. Consequently, the preparser sometimes says
// maybe-assigned where the parser (correctly) says never-assigned.
}
if (names) { if (names) {
names->Add(variable->raw_name(), zone()); names->Add(variable->raw_name(), zone());
} }
......
This diff is collapsed.
...@@ -32,7 +32,7 @@ class ScopeTestHelper { ...@@ -32,7 +32,7 @@ class ScopeTestHelper {
} }
static void CompareScopeToData(Scope* scope, const PreParsedScopeData* data, static void CompareScopeToData(Scope* scope, const PreParsedScopeData* data,
size_t& index) { size_t& index, bool precise_maybe_assigned) {
CHECK_EQ(data->backing_store_[index++], scope->scope_type()); CHECK_EQ(data->backing_store_[index++], scope->scope_type());
CHECK_EQ(data->backing_store_[index++], scope->start_position()); CHECK_EQ(data->backing_store_[index++], scope->start_position());
CHECK_EQ(data->backing_store_[index++], scope->end_position()); CHECK_EQ(data->backing_store_[index++], scope->end_position());
...@@ -70,14 +70,19 @@ class ScopeTestHelper { ...@@ -70,14 +70,19 @@ class ScopeTestHelper {
} }
#endif #endif
CHECK_EQ(data->backing_store_[index++], local->location()); CHECK_EQ(data->backing_store_[index++], local->location());
if (precise_maybe_assigned) {
CHECK_EQ(data->backing_store_[index++], local->maybe_assigned()); CHECK_EQ(data->backing_store_[index++], local->maybe_assigned());
} else {
STATIC_ASSERT(kMaybeAssigned > kNotAssigned);
CHECK_GE(data->backing_store_[index++], local->maybe_assigned());
}
} }
} }
for (Scope* inner = scope->inner_scope(); inner != nullptr; for (Scope* inner = scope->inner_scope(); inner != nullptr;
inner = inner->sibling()) { inner = inner->sibling()) {
if (!ScopeTreeIsHidden(inner)) { if (!ScopeTreeIsHidden(inner)) {
CompareScopeToData(inner, data, index); CompareScopeToData(inner, data, index, precise_maybe_assigned);
} }
} }
} }
......
This diff is collapsed.
// Copyright 2016 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: --allow-natives-syntax --function-context-specialization
function f(n) {
var a = [];
function g() { return x }
for (var i = 0; i < n; ++i) {
var x = i;
a[i] = g;
%OptimizeFunctionOnNextCall(g);
g();
}
return a;
}
var a = f(3);
assertEquals(3, a.length);
assertEquals(2, a[0]());
assertEquals(2, a[1]());
assertEquals(2, a[2]());
// Copyright 2016 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: --allow-natives-syntax
function f(n) {
"use asm";
var a = [];
function g() { return x }
for (var i = 0; i < n; ++i) {
var x = i;
a[i] = g;
%OptimizeFunctionOnNextCall(g);
g();
}
return a;
}
var a = f(3);
assertEquals(3, a.length);
assertEquals(2, a[0]());
assertEquals(2, a[1]());
assertEquals(2, a[2]());
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