From 96de832c8982cc3fe130008025d40336847e9b9d Mon Sep 17 00:00:00 2001 From: "keuchel@chromium.org" Date: Wed, 14 Sep 2011 13:51:29 +0000 Subject: [PATCH] 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 --- src/scopes.cc | 21 +++++++-------- src/variables.cc | 2 +- src/variables.h | 10 ++++---- test/mjsunit/regress/regress-96523.js | 37 +++++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 18 deletions(-) create mode 100644 test/mjsunit/regress/regress-96523.js diff --git a/src/scopes.cc b/src/scopes.cc index e101fb7a9..d5a7a9f9c 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 name, Variable::Mode mode) { // another variable that is introduced dynamically via an 'eval' call // or a 'with' statement). Variable* Scope::LookupRecursive(Handle 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 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 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 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 6cf6e0b04..971061b05 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()); diff --git a/src/variables.h b/src/variables.h index a4ead51b8..56c8dabd3 100644 --- a/src/variables.h +++ b/src/variables.h @@ -120,12 +120,12 @@ class Variable: public ZoneObject { Handle 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 index 000000000..e611ce366 --- /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); + } + })() +} -- 2.34.1