Revision: 22039
Author: [email protected]
Date: Thu Jun 26 11:59:42 2014 UTC
Log: Infer whether a variable is assigned in inner functions
[email protected]
BUG=
Review URL: https://codereview.chromium.org/345573002
http://code.google.com/p/v8/source/detail?r=22039
Modified:
/branches/bleeding_edge/src/ast-value-factory.h
/branches/bleeding_edge/src/ast.cc
/branches/bleeding_edge/src/ast.h
/branches/bleeding_edge/src/parser.cc
/branches/bleeding_edge/src/parser.h
/branches/bleeding_edge/src/preparser.h
/branches/bleeding_edge/src/scopes.cc
/branches/bleeding_edge/src/scopes.h
/branches/bleeding_edge/src/variables.cc
/branches/bleeding_edge/src/variables.h
/branches/bleeding_edge/test/cctest/test-parsing.cc
=======================================
--- /branches/bleeding_edge/src/ast-value-factory.h Tue Jun 24 14:03:24
2014 UTC
+++ /branches/bleeding_edge/src/ast-value-factory.h Thu Jun 26 11:59:42
2014 UTC
@@ -278,6 +278,10 @@
}
const AstRawString* GetOneByteString(Vector<const uint8_t> literal);
+ const AstRawString* GetOneByteString(const char* string) {
+ return GetOneByteString(Vector<const uint8_t>(
+ reinterpret_cast<const uint8_t*>(string), StrLength(string)));
+ }
const AstRawString* GetTwoByteString(Vector<const uint16_t> literal);
const AstRawString* GetString(Handle<String> literal);
const AstConsString* NewConsString(const AstString* left,
=======================================
--- /branches/bleeding_edge/src/ast.cc Thu Jun 26 11:55:31 2014 UTC
+++ /branches/bleeding_edge/src/ast.cc Thu Jun 26 11:59:42 2014 UTC
@@ -64,8 +64,7 @@
name_(var->raw_name()),
var_(NULL), // Will be set by the call to BindTo.
is_this_(var->is_this()),
- is_trivial_(false),
- is_lvalue_(false),
+ is_assigned_(false),
interface_(var->interface()) {
BindTo(var);
}
@@ -80,8 +79,7 @@
name_(name),
var_(NULL),
is_this_(is_this),
- is_trivial_(false),
- is_lvalue_(false),
+ is_assigned_(false),
interface_(interface) {
}
@@ -97,7 +95,7 @@
// eval() etc. Const-ness and variable declarations are a complete mess
// in JS. Sigh...
var_ = var;
- var->set_is_used(true);
+ var->set_is_used();
}
=======================================
--- /branches/bleeding_edge/src/ast.h Tue Jun 24 14:03:24 2014 UTC
+++ /branches/bleeding_edge/src/ast.h Thu Jun 26 11:59:42 2014 UTC
@@ -1627,8 +1627,6 @@
}
bool IsArguments() const { return var_ != NULL && var_->is_arguments(); }
-
- bool IsLValue() const { return is_lvalue_; }
Handle<String> name() const { return name_->string(); }
const AstRawString* raw_name() const { return name_; }
@@ -1636,9 +1634,8 @@
bool is_this() const { return is_this_; }
Interface* interface() const { return interface_; }
-
- void MarkAsTrivial() { is_trivial_ = true; }
- void MarkAsLValue() { is_lvalue_ = true; }
+ bool is_assigned() const { return is_assigned_; }
+ void set_is_assigned() { is_assigned_ = true; }
// Bind this proxy to the variable var. Interfaces must match.
void BindTo(Variable* var);
@@ -1655,10 +1652,7 @@
const AstRawString* name_;
Variable* var_; // resolved variable, or NULL
bool is_this_;
- bool is_trivial_;
- // True if this variable proxy is being used in an assignment
- // or with a increment/decrement operator.
- bool is_lvalue_;
+ bool is_assigned_;
Interface* interface_;
};
=======================================
--- /branches/bleeding_edge/src/parser.cc Wed Jun 25 15:26:10 2014 UTC
+++ /branches/bleeding_edge/src/parser.cc Thu Jun 26 11:59:42 2014 UTC
@@ -453,11 +453,10 @@
}
-Expression* ParserTraits::MarkExpressionAsLValue(Expression* expression) {
- VariableProxy* proxy = expression != NULL
- ? expression->AsVariableProxy()
- : NULL;
- if (proxy != NULL) proxy->MarkAsLValue();
+Expression* ParserTraits::MarkExpressionAsAssigned(Expression* expression)
{
+ VariableProxy* proxy =
+ expression != NULL ? expression->AsVariableProxy() : NULL;
+ if (proxy != NULL) proxy->set_is_assigned();
return expression;
}
@@ -593,8 +592,7 @@
Zone* zone = parser_->zone();
int argc = arg != NULL ? 1 : 0;
const AstRawString* type =
- parser_->ast_value_factory_->GetOneByteString(Vector<const uint8_t>(
- reinterpret_cast<const uint8_t*>(message), StrLength(message)));
+ parser_->ast_value_factory_->GetOneByteString(message);
ZoneList<const AstRawString*>* array =
new (zone) ZoneList<const AstRawString*>(argc, zone);
if (arg != NULL) {
@@ -1722,6 +1720,8 @@
Expression* expression = NewThrowTypeError(
"var_redeclaration", name, declaration->position());
declaration_scope->SetIllegalRedeclaration(expression);
+ } else if (mode == VAR) {
+ var->set_maybe_assigned();
}
}
=======================================
--- /branches/bleeding_edge/src/parser.h Tue Jun 24 14:03:24 2014 UTC
+++ /branches/bleeding_edge/src/parser.h Thu Jun 26 11:59:42 2014 UTC
@@ -468,9 +468,8 @@
void CheckPossibleEvalCall(Expression* expression, Scope* scope);
// Determine if the expression is a variable proxy and mark it as being
used
- // in an assignment or with a increment/decrement operator. This is
currently
- // used on for the statically checking assignments to harmony const
bindings.
- static Expression* MarkExpressionAsLValue(Expression* expression);
+ // in an assignment or with a increment/decrement operator.
+ static Expression* MarkExpressionAsAssigned(Expression* expression);
// Returns true if we have a binary expression between two numeric
// literals. In that case, *x will be changed to an expression which is
the
=======================================
--- /branches/bleeding_edge/src/preparser.h Wed Jun 25 10:13:10 2014 UTC
+++ /branches/bleeding_edge/src/preparser.h Thu Jun 26 11:59:42 2014 UTC
@@ -973,10 +973,10 @@
static void CheckPossibleEvalCall(PreParserExpression expression,
PreParserScope* scope) {}
- static PreParserExpression MarkExpressionAsLValue(
+ static PreParserExpression MarkExpressionAsAssigned(
PreParserExpression expression) {
// TODO(marja): To be able to produce the same errors, the preparser
needs
- // to start tracking which expressions are variables and which are
lvalues.
+ // to start tracking which expressions are variables and which are
assigned.
return expression;
}
@@ -1753,7 +1753,7 @@
expression = this->CheckAndRewriteReferenceExpression(
expression, lhs_location, "invalid_lhs_in_assignment", CHECK_OK);
- expression = this->MarkExpressionAsLValue(expression);
+ expression = this->MarkExpressionAsAssigned(expression);
Token::Value op = Next(); // Get assignment operator.
int pos = position();
@@ -1915,7 +1915,7 @@
ExpressionT expression = this->ParseUnaryExpression(CHECK_OK);
expression = this->CheckAndRewriteReferenceExpression(
expression, lhs_location, "invalid_lhs_in_prefix_op", CHECK_OK);
- this->MarkExpressionAsLValue(expression);
+ this->MarkExpressionAsAssigned(expression);
return factory()->NewCountOperation(op,
true /* prefix */,
@@ -1940,7 +1940,7 @@
Token::IsCountOp(peek())) {
expression = this->CheckAndRewriteReferenceExpression(
expression, lhs_location, "invalid_lhs_in_postfix_op", CHECK_OK);
- expression = this->MarkExpressionAsLValue(expression);
+ expression = this->MarkExpressionAsAssigned(expression);
Token::Value next = Next();
expression =
=======================================
--- /branches/bleeding_edge/src/scopes.cc Tue Jun 24 14:03:24 2014 UTC
+++ /branches/bleeding_edge/src/scopes.cc Thu Jun 26 11:59:42 2014 UTC
@@ -170,6 +170,7 @@
strict_mode_ = outer_scope != NULL ? outer_scope->strict_mode_ : SLOPPY;
outer_scope_calls_sloppy_eval_ = false;
inner_scope_calls_eval_ = false;
+ inner_scope_contains_with_ = false;
force_eager_compilation_ = false;
force_context_allocation_ = (outer_scope != NULL && !is_function_scope())
? outer_scope->has_forced_context_allocation() : false;
@@ -194,6 +195,7 @@
Scope* current_scope = NULL;
Scope* innermost_scope = NULL;
bool contains_with = false;
+ bool inner_contains_with = false;
while (!context->IsNativeContext()) {
if (context->IsWithContext()) {
Scope* with_scope = new(zone) Scope(current_scope,
@@ -243,7 +245,11 @@
global_scope->ast_value_factory_->GetString(Handle<String>(name)),
global_scope->ast_value_factory_, zone);
}
- if (contains_with) current_scope->RecordWithStatement();
+ if (inner_contains_with) current_scope->inner_scope_contains_with_ =
true;
+ if (contains_with) {
+ current_scope->RecordWithStatement();
+ inner_contains_with = true;
+ }
if (innermost_scope == NULL) innermost_scope = current_scope;
// Forget about a with when we move to a context for a different
function.
@@ -819,9 +825,15 @@
PrintName(var->raw_name());
PrintF("; // ");
PrintLocation(var);
+ bool comma = !var->IsUnallocated();
if (var->has_forced_context_allocation()) {
- if (!var->IsUnallocated()) PrintF(", ");
+ if (comma) PrintF(", ");
PrintF("forced context allocation");
+ comma = true;
+ }
+ if (var->maybe_assigned()) {
+ if (comma) PrintF(", ");
+ PrintF("maybe assigned");
}
PrintF("\n");
}
@@ -875,6 +887,9 @@
}
if (scope_inside_with_) Indent(n1, "// scope inside 'with'\n");
if (scope_contains_with_) Indent(n1, "// scope contains 'with'\n");
+ if (inner_scope_contains_with_) {
+ Indent(n1, "// inner scope contains 'with'\n");
+ }
if (scope_calls_eval_) Indent(n1, "// scope calls 'eval'\n");
if (outer_scope_calls_sloppy_eval_) {
Indent(n1, "// outer scope calls 'eval' in sloppy context\n");
@@ -951,7 +966,7 @@
}
-Variable* Scope::LookupRecursive(const AstRawString* name,
+Variable* Scope::LookupRecursive(VariableProxy* proxy,
BindingKind* binding_kind,
AstNodeFactory<AstNullVisitor>* factory) {
ASSERT(binding_kind != NULL);
@@ -963,7 +978,7 @@
}
// Try to find the variable in this scope.
- Variable* var = LookupLocal(name);
+ Variable* var = LookupLocal(proxy->raw_name());
// We found a variable and we are done. (Even if there is an 'eval' in
// this scope which introduces the same variable again, the resulting
@@ -977,11 +992,11 @@
// if any. We can do this for all scopes, since the function variable is
// only present - if at all - for function scopes.
*binding_kind = UNBOUND;
- var = LookupFunctionVar(name, factory);
+ var = LookupFunctionVar(proxy->raw_name(), factory);
if (var != NULL) {
*binding_kind = BOUND;
} else if (outer_scope_ != NULL) {
- var = outer_scope_->LookupRecursive(name, binding_kind, factory);
+ var = outer_scope_->LookupRecursive(proxy, binding_kind, factory);
if (*binding_kind == BOUND && (is_function_scope() ||
is_with_scope())) {
var->ForceContextAllocation();
}
@@ -997,6 +1012,7 @@
// the associated variable has to be marked as potentially being
accessed
// from inside of an inner with scope (the property may not be in
the 'with'
// object).
+ if (var != NULL && proxy->is_assigned()) var->set_maybe_assigned();
*binding_kind = DYNAMIC_LOOKUP;
return NULL;
} else if (calls_sloppy_eval()) {
@@ -1025,7 +1041,7 @@
// Otherwise, try to resolve the variable.
BindingKind binding_kind;
- Variable* var = LookupRecursive(proxy->raw_name(), &binding_kind,
factory);
+ Variable* var = LookupRecursive(proxy, &binding_kind, factory);
switch (binding_kind) {
case BOUND:
// We found a variable binding.
@@ -1064,9 +1080,10 @@
}
ASSERT(var != NULL);
+ if (proxy->is_assigned()) var->set_maybe_assigned();
if (FLAG_harmony_scoping && strict_mode() == STRICT &&
- var->is_const_mode() && proxy->IsLValue()) {
+ var->is_const_mode() && proxy->is_assigned()) {
// Assignment to const. Throw a syntax error.
MessageLocation location(
info->script(), proxy->position(), proxy->position());
@@ -1140,7 +1157,7 @@
}
-bool Scope::PropagateScopeInfo(bool outer_scope_calls_sloppy_eval ) {
+void Scope::PropagateScopeInfo(bool outer_scope_calls_sloppy_eval ) {
if (outer_scope_calls_sloppy_eval) {
outer_scope_calls_sloppy_eval_ = true;
}
@@ -1148,16 +1165,18 @@
bool calls_sloppy_eval =
this->calls_sloppy_eval() || outer_scope_calls_sloppy_eval_;
for (int i = 0; i < inner_scopes_.length(); i++) {
- Scope* inner_scope = inner_scopes_[i];
- if (inner_scope->PropagateScopeInfo(calls_sloppy_eval)) {
+ Scope* inner = inner_scopes_[i];
+ inner->PropagateScopeInfo(calls_sloppy_eval);
+ if (inner->scope_calls_eval_ || inner->inner_scope_calls_eval_) {
inner_scope_calls_eval_ = true;
}
- if (inner_scope->force_eager_compilation_) {
+ if (inner->scope_contains_with_ || inner->inner_scope_contains_with_) {
+ inner_scope_contains_with_ = true;
+ }
+ if (inner->force_eager_compilation_) {
force_eager_compilation_ = true;
}
}
-
- return scope_calls_eval_ || inner_scope_calls_eval_;
}
@@ -1174,7 +1193,8 @@
is_block_scope() ||
is_module_scope() ||
is_global_scope())) {
- var->set_is_used(true);
+ var->set_is_used();
+ if (scope_calls_eval_ || inner_scope_calls_eval_)
var->set_maybe_assigned();
}
// Global variables do not need to be allocated.
return !var->IsGlobalObjectProperty() && var->is_used();
=======================================
--- /branches/bleeding_edge/src/scopes.h Tue Jun 24 14:03:24 2014 UTC
+++ /branches/bleeding_edge/src/scopes.h Thu Jun 26 11:59:42 2014 UTC
@@ -474,6 +474,7 @@
// Computed via PropagateScopeInfo.
bool outer_scope_calls_sloppy_eval_;
bool inner_scope_calls_eval_;
+ bool inner_scope_contains_with_;
bool force_eager_compilation_;
bool force_context_allocation_;
@@ -551,7 +552,7 @@
// Lookup a variable reference given by name recursively starting with
this
// scope. If the code is executed because of a call to 'eval', the
context
// parameter should be set to the calling context of 'eval'.
- Variable* LookupRecursive(const AstRawString* name,
+ Variable* LookupRecursive(VariableProxy* proxy,
BindingKind* binding_kind,
AstNodeFactory<AstNullVisitor>* factory);
MUST_USE_RESULT
@@ -563,7 +564,7 @@
AstNodeFactory<AstNullVisitor>*
factory);
// Scope analysis.
- bool PropagateScopeInfo(bool outer_scope_calls_sloppy_eval);
+ void PropagateScopeInfo(bool outer_scope_calls_sloppy_eval);
bool HasTrivialContext() const;
// Predicates.
=======================================
--- /branches/bleeding_edge/src/variables.cc Tue Jun 24 14:03:24 2014 UTC
+++ /branches/bleeding_edge/src/variables.cc Thu Jun 26 11:59:42 2014 UTC
@@ -50,6 +50,7 @@
is_valid_ref_(is_valid_ref),
force_context_allocation_(false),
is_used_(false),
+ maybe_assigned_(false),
initialization_flag_(initialization_flag),
interface_(interface) {
// Var declared variables never need initialization.
=======================================
--- /branches/bleeding_edge/src/variables.h Tue Jun 24 14:03:24 2014 UTC
+++ /branches/bleeding_edge/src/variables.h Thu Jun 26 11:59:42 2014 UTC
@@ -82,7 +82,9 @@
force_context_allocation_ = true;
}
bool is_used() { return is_used_; }
- void set_is_used(bool flag) { is_used_ = flag; }
+ void set_is_used() { is_used_ = true; }
+ bool maybe_assigned() { return maybe_assigned_; }
+ void set_maybe_assigned() { maybe_assigned_ = true; }
int initializer_position() { return initializer_position_; }
void set_initializer_position(int pos) { initializer_position_ = pos; }
@@ -157,6 +159,7 @@
// Usage info.
bool force_context_allocation_; // set by variable resolver
bool is_used_;
+ bool maybe_assigned_;
InitializationFlag initialization_flag_;
// Module type info.
=======================================
--- /branches/bleeding_edge/test/cctest/test-parsing.cc Wed Jun 25 10:13:10
2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-parsing.cc Thu Jun 26 11:59:42
2014 UTC
@@ -38,6 +38,7 @@
#include "src/objects.h"
#include "src/parser.h"
#include "src/preparser.h"
+#include "src/rewriter.h"
#include "src/scanner-character-streams.h"
#include "src/token.h"
#include "src/utils.h"
@@ -2614,3 +2615,133 @@
"}\n"
"this_is_lazy();\n");
}
+
+
+TEST(InnerAssignment) {
+ i::Isolate* isolate = CcTest::i_isolate();
+ i::Factory* factory = isolate->factory();
+ i::HandleScope scope(isolate);
+ LocalContext env;
+
+ const char* prefix = "function f() {";
+ const char* midfix = " function g() {";
+ const char* suffix = "}}";
+ struct { const char* source; bool assigned; bool strict; } outers[] = {
+ // Actual assignments.
+ { "var x; var x = 5;", true, false },
+ { "var x; { var x = 5; }", true, false },
+ { "'use strict'; let x; x = 6;", true, true },
+ { "var x = 5; function x() {}", true, false },
+ // Actual non-assignments.
+ { "var x;", false, false },
+ { "var x = 5;", false, false },
+ { "'use strict'; let x;", false, true },
+ { "'use strict'; let x = 6;", false, true },
+ { "'use strict'; var x = 0; { let x = 6; }", false, true },
+ { "'use strict'; var x = 0; { let x; x = 6; }", false, true },
+ { "'use strict'; let x = 0; { let x = 6; }", false, true },
+ { "'use strict'; let x = 0; { let x; x = 6; }", false, true },
+ { "var x; try {} catch (x) { x = 5; }", false, false },
+ { "function x() {}", false, false },
+ // Eval approximation.
+ { "var x; eval('');", true, false },
+ { "eval(''); var x;", true, false },
+ { "'use strict'; let x; eval('');", true, true },
+ { "'use strict'; eval(''); let x;", true, true },
+ // Non-assignments not recognized, because the analysis is
approximative.
+ { "var x; var x;", true, false },
+ { "var x = 5; var x;", true, false },
+ { "var x; { var x; }", true, false },
+ { "var x; function x() {}", true, false },
+ { "function x() {}; var x;", true, false },
+ { "var x; try {} catch (x) { var x = 5; }", true, false },
+ };
+ struct { const char* source; bool assigned; bool with; } inners[] = {
+ // Actual assignments.
+ { "x = 1;", true, false },
+ { "x++;", true, false },
+ { "++x;", true, false },
+ { "x--;", true, false },
+ { "--x;", true, false },
+ { "{ x = 1; }", true, false },
+ { "'use strict'; { let x; }; x = 0;", true, false },
+ { "'use strict'; { const x = 1; }; x = 0;", true, false },
+ { "'use strict'; { function x() {} }; x = 0;", true, false },
+ { "with ({}) { x = 1; }", true, true },
+ { "eval('');", true, false },
+ { "'use strict'; { let y; eval('') }", true, false },
+ { "function h() { x = 0; }", true, false },
+ { "(function() { x = 0; })", true, false },
+ { "(function() { x = 0; })", true, false },
+ { "with ({}) (function() { x = 0; })", true, true },
+ // Actual non-assignments.
+ { "", false, false },
+ { "x;", false, false },
+ { "var x;", false, false },
+ { "var x = 8;", false, false },
+ { "var x; x = 8;", false, false },
+ { "'use strict'; let x;", false, false },
+ { "'use strict'; let x = 8;", false, false },
+ { "'use strict'; let x; x = 8;", false, false },
+ { "'use strict'; const x = 8;", false, false },
+ { "function x() {}", false, false },
+ { "function x() { x = 0; }", false, false },
+ { "function h(x) { x = 0; }", false, false },
+ { "'use strict'; { let x; x = 0; }", false, false },
+ { "{ var x; }; x = 0;", false, false },
+ { "with ({}) {}", false, true },
+ { "var x; { with ({}) { x = 1; } }", false, true },
+ { "try {} catch(x) { x = 0; }", false, false },
+ { "try {} catch(x) { with ({}) { x = 1; } }", false, true },
+ // Eval approximation.
+ { "eval('');", true, false },
+ { "function h() { eval(''); }", true, false },
+ { "(function() { eval(''); })", true, false },
+ // Shadowing not recognized because of eval approximation.
+ { "var x; eval('');", true, false },
+ { "'use strict'; let x; eval('');", true, false },
+ { "try {} catch(x) { eval(''); }", true, false },
+ { "function x() { eval(''); }", true, false },
+ { "(function(x) { eval(''); })", true, false },
+ };
+
+ int prefix_len = Utf8LengthHelper(prefix);
+ int midfix_len = Utf8LengthHelper(midfix);
+ int suffix_len = Utf8LengthHelper(suffix);
+ for (unsigned i = 0; i < ARRAY_SIZE(outers); ++i) {
+ const char* outer = outers[i].source;
+ int outer_len = Utf8LengthHelper(outer);
+ for (unsigned j = 0; j < ARRAY_SIZE(inners); ++j) {
+ if (outers[i].strict && inners[j].with) continue;
+ const char* inner = inners[j].source;
+ int inner_len = Utf8LengthHelper(inner);
+ int len = prefix_len + outer_len + midfix_len + inner_len +
suffix_len;
+ i::ScopedVector<char> program(len + 1);
+ i::SNPrintF(program, "%s%s%s%s%s", prefix, outer, midfix, inner,
suffix);
+ i::Handle<i::String> source =
+ factory->InternalizeUtf8String(program.start());
+ source->PrintOn(stdout);
+ printf("\n");
+
+ i::Handle<i::Script> script = factory->NewScript(source);
+ i::CompilationInfoWithZone info(script);
+ i::Parser parser(&info);
+ parser.set_allow_harmony_scoping(true);
+ CHECK(parser.Parse());
+ CHECK(i::Rewriter::Rewrite(&info));
+ CHECK(i::Scope::Analyze(&info));
+ CHECK(info.function() != NULL);
+
+ i::Scope* scope = info.function()->scope();
+ CHECK_EQ(scope->inner_scopes()->length(), 1);
+ i::Scope* inner_scope = scope->inner_scopes()->at(0);
+ const i::AstRawString* var_name =
+ info.ast_value_factory()->GetOneByteString("x");
+ i::Variable* var = inner_scope->Lookup(var_name);
+ bool expected = outers[i].assigned || inners[j].assigned;
+ CHECK(var != NULL);
+ CHECK(var->is_used() || !expected);
+ CHECK(var->maybe_assigned() == expected);
+ }
+ }
+}
--
--
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.
