Reviewers: Kevin Millikin, Rico,
Message:
PTAL.
http://codereview.chromium.org/7890031/diff/1/src/scopes.cc
File src/scopes.cc (left):
http://codereview.chromium.org/7890031/diff/1/src/scopes.cc#oldcode855
src/scopes.cc:855: is_function_scope() || from_inner_function,
The important change.
Description:
Mark variables as being accessed from any inner scope, not only function
scopes
BUG=96523
TEST=mjsunit/regress/regress-96523.js
Please review this at http://codereview.chromium.org/7890031/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/scopes.cc
M src/variables.h
M src/variables.cc
A test/mjsunit/regress/regress-96523.js
Index: src/scopes.cc
diff --git a/src/scopes.cc b/src/scopes.cc
index
e101fb7a97de1572547ccc889a2e987e1ec44541..7e1508ac18fb5336f08279df13e12b1223295176
100644
--- a/src/scopes.cc
+++ b/src/scopes.cc
@@ -695,7 +695,7 @@ static void PrintVar(int indent, Variable* var) {
PrintName(var->name());
PrintF("; // ");
PrintLocation(var);
- if (var->is_accessed_from_inner_function_scope()) {
+ if (var->is_accessed_from_inner_scope()) {
if (!var->IsUnallocated()) PrintF(", ");
PrintF("inner scope access");
}
@@ -818,7 +818,7 @@ Variable* Scope::NonLocal(Handle<String> name,
Variable::Mode mode) {
// another variable that is introduced dynamically via an 'eval' call
// or a 'with' statement).
Variable* Scope::LookupRecursive(Handle<String> name,
- bool from_inner_function,
+ bool from_inner_scope,
Variable** invalidated_local) {
// If we find a variable, but the current scope calls 'eval', the found
// variable may not be the correct one (the 'eval' may introduce a
@@ -834,7 +834,7 @@ Variable* Scope::LookupRecursive(Handle<String> name,
// (Even if there is an 'eval' in this scope which introduces the
// same variable again, the resulting variable remains the same.
// Note that enclosing 'with' statements are handled at the call site.)
- if (!from_inner_function)
+ if (!from_inner_scope)
return var;
} else {
@@ -852,7 +852,7 @@ Variable* Scope::LookupRecursive(Handle<String> name,
} else if (outer_scope_ != NULL) {
var = outer_scope_->LookupRecursive(
name,
- is_function_scope() || from_inner_function,
+ true,
invalidated_local);
// We may have found a variable in an outer scope. However, if
// the current scope is inside a 'with', the actual variable may
@@ -870,8 +870,8 @@ Variable* Scope::LookupRecursive(Handle<String> name,
ASSERT(var != NULL);
// If this is a lookup from an inner scope, mark the variable.
- if (from_inner_function) {
- var->MarkAsAccessedFromInnerFunctionScope();
+ if (from_inner_scope) {
+ var->MarkAsAccessedFromInnerScope();
}
// If the variable we have found is just a guess, invalidate the
@@ -1022,7 +1022,7 @@ bool Scope::MustAllocate(Variable* var) {
// via an eval() call. This is only possible if the variable has a
// visible name.
if ((var->is_this() || var->name()->length() > 0) &&
- (var->is_accessed_from_inner_function_scope() ||
+ (var->is_accessed_from_inner_scope() ||
scope_calls_eval_ ||
inner_scope_calls_eval_ ||
scope_contains_with_ ||
@@ -1045,7 +1045,7 @@ bool Scope::MustAllocateInContext(Variable* var) {
// catch-bound variables are always allocated in a context.
if (var->mode() == Variable::TEMPORARY) return false;
if (is_catch_scope() || is_block_scope()) return true;
- return var->is_accessed_from_inner_function_scope() ||
+ return var->is_accessed_from_inner_scope() ||
scope_calls_eval_ ||
inner_scope_calls_eval_ ||
scope_contains_with_ ||
@@ -1111,7 +1111,7 @@ void Scope::AllocateParameterLocals() {
if (uses_nonstrict_arguments) {
// Give the parameter a use from an inner scope, to force allocation
// to the context.
- var->MarkAsAccessedFromInnerFunctionScope();
+ var->MarkAsAccessedFromInnerScope();
}
if (MustAllocate(var)) {
Index: src/variables.cc
diff --git a/src/variables.cc b/src/variables.cc
index
6cf6e0b04a9a62e4bc865414336558bec7eee590..971061b05378cc9be2e2728307fe43b29656038e
100644
--- a/src/variables.cc
+++ b/src/variables.cc
@@ -66,7 +66,7 @@ Variable::Variable(Scope* scope,
index_(-1),
local_if_not_shadowed_(NULL),
is_valid_LHS_(is_valid_LHS),
- is_accessed_from_inner_function_scope_(false),
+ is_accessed_from_inner_scope_(false),
is_used_(false) {
// names must be canonicalized for fast equality checks
ASSERT(name->IsSymbol());
Index: src/variables.h
diff --git a/src/variables.h b/src/variables.h
index
a4ead51b893653b3f5517d9b6a739aac890067bd..56c8dabd378e033647a5563d63721e1224363047
100644
--- a/src/variables.h
+++ b/src/variables.h
@@ -120,12 +120,12 @@ class Variable: public ZoneObject {
Handle<String> name() const { return name_; }
Mode mode() const { return mode_; }
- bool is_accessed_from_inner_function_scope() const {
- return is_accessed_from_inner_function_scope_;
+ bool is_accessed_from_inner_scope() const {
+ return is_accessed_from_inner_scope_;
}
- void MarkAsAccessedFromInnerFunctionScope() {
+ void MarkAsAccessedFromInnerScope() {
ASSERT(mode_ != TEMPORARY);
- is_accessed_from_inner_function_scope_ = true;
+ is_accessed_from_inner_scope_ = true;
}
bool is_used() { return is_used_; }
void set_is_used(bool flag) { is_used_ = flag; }
@@ -188,7 +188,7 @@ class Variable: public ZoneObject {
bool is_valid_LHS_;
// Usage info.
- bool is_accessed_from_inner_function_scope_; // set by variable resolver
+ bool is_accessed_from_inner_scope_; // set by variable resolver
bool is_used_;
};
Index: test/mjsunit/regress/regress-96523.js
diff --git a/test/mjsunit/regress/regress-96523.js
b/test/mjsunit/regress/regress-96523.js
new file mode 100644
index
0000000000000000000000000000000000000000..e611ce36607594c5b66735958553e126d4e5b522
--- /dev/null
+++ b/test/mjsunit/regress/regress-96523.js
@@ -0,0 +1,37 @@
+// Copyright 2011 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following
+// disclaimer in the documentation and/or other materials provided
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+with ({x:'outer'}) {
+ (function() {
+ var x = 'inner';
+ try {
+ throw 'Exception';
+ } catch (e) {
+ assertEquals('inner', x);
+ }
+ })()
+}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
