Mark variables as being accessed from any inner scope, not only function scopes

BUG=96523
TEST=mjsunit/regress/regress-96523.js

Review URL: http://codereview.chromium.org/7890031

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9281 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/scopes.cc b/src/scopes.cc
index e101fb7a97de1572547ccc889a2e987e1ec44541..d5a7a9f9ca8d7b86361856fa92bde29bbf796489 100644 (file)
--- 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 {
@@ -850,10 +850,7 @@ Variable* Scope::LookupRecursive(Handle<String> name,
       var = function_->var();
 
     } else if (outer_scope_ != NULL) {
-      var = outer_scope_->LookupRecursive(
-          name,
-          is_function_scope() || from_inner_function,
-          invalidated_local);
+      var = outer_scope_->LookupRecursive(name, 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
       // be a property introduced via the 'with' statement. Then, the
@@ -870,8 +867,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 +1019,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 +1042,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 +1108,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)) {

diff --git a/src/variables.cc b/src/variables.cc
index 6cf6e0b04a9a62e4bc865414336558bec7eee590..971061b05378cc9be2e2728307fe43b29656038e 100644 (file)
--- 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());

diff --git a/src/variables.h b/src/variables.h
index a4ead51b893653b3f5517d9b6a739aac890067bd..56c8dabd378e033647a5563d63721e1224363047 100644 (file)
--- 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_;
 };
 

diff --git a/test/mjsunit/regress/regress-96523.js b/test/mjsunit/regress/regress-96523.js
new file mode 100644 (file)
index 0000000..e611ce3
--- /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);
+    }
+  })()
+}