Revision: 8518
Author: [email protected]
Date: Fri Jul 1 07:05:46 2011
Log: Fix an issue with optimization of functions inside catch.
When optimizing a function defined inside a catch, we did not count
the catch context as part of the context chain.
[email protected]
BUG=1521
TEST=regress-1521
Review URL: http://codereview.chromium.org/7285032
http://code.google.com/p/v8/source/detail?r=8518
Added:
/branches/bleeding_edge/test/mjsunit/regress/regress-1521.js
Modified:
/branches/bleeding_edge/src/scopes.cc
/branches/bleeding_edge/src/scopes.h
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-1521.js Fri Jul 1
07:05:46 2011
@@ -0,0 +1,47 @@
+// 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.
+
+// Optimized variable access inside through a catch context should work.
+function test(x) {
+ try {
+ throw new Error();
+ } catch (e) {
+ var y = {f: 1};
+ var f = function () {
+ var z = y;
+ var g = function () {
+ if (y.f === z.f) return x;
+ };
+ %OptimizeFunctionOnNextCall(g);
+ return g;
+ }
+ assertEquals(3, f()());
+ }
+}
+
+test(3);
+
=======================================
--- /branches/bleeding_edge/src/scopes.cc Thu Jun 30 07:37:55 2011
+++ /branches/bleeding_edge/src/scopes.cc Fri Jul 1 07:05:46 2011
@@ -119,9 +119,9 @@
temps_(0),
params_(0),
unresolved_(0),
- decls_(0) {
+ decls_(0),
+ already_resolved_(false) {
SetDefaults(type, NULL, Handle<SerializedScopeInfo>::null());
- ASSERT(!resolved());
}
@@ -131,14 +131,14 @@
temps_(4),
params_(4),
unresolved_(16),
- decls_(4) {
+ decls_(4),
+ already_resolved_(false) {
SetDefaults(type, outer_scope, Handle<SerializedScopeInfo>::null());
// At some point we might want to provide outer scopes to
// eval scopes (by walking the stack and reading the scope info).
// In that case, the ASSERT below needs to be adjusted.
ASSERT((type == GLOBAL_SCOPE || type == EVAL_SCOPE) == (outer_scope ==
NULL));
ASSERT(!HasIllegalRedeclaration());
- ASSERT(!resolved());
}
@@ -148,15 +148,34 @@
temps_(4),
params_(4),
unresolved_(16),
- decls_(4) {
+ decls_(4),
+ already_resolved_(true) {
ASSERT(!scope_info.is_null());
SetDefaults(FUNCTION_SCOPE, NULL, scope_info);
- ASSERT(resolved());
if (scope_info->HasHeapAllocatedLocals()) {
num_heap_slots_ = scope_info_->NumberOfContextSlots();
}
-
+ AddInnerScope(inner_scope);
+}
+
+
+Scope::Scope(Scope* inner_scope, Handle<String> catch_variable_name)
+ : inner_scopes_(1),
+ variables_(),
+ temps_(0),
+ params_(0),
+ unresolved_(0),
+ decls_(0),
+ already_resolved_(true) {
+ SetDefaults(CATCH_SCOPE, NULL, Handle<SerializedScopeInfo>::null());
AddInnerScope(inner_scope);
+ ++num_var_or_const_;
+ Variable* variable = variables_.Declare(this,
+ catch_variable_name,
+ Variable::VAR,
+ true, // Valid left-hand side.
+ Variable::NORMAL);
+ AllocateHeapSlot(variable);
}
@@ -190,30 +209,43 @@
Scope* Scope::DeserializeScopeChain(CompilationInfo* info,
Scope* global_scope) {
+ // Reconstruct the outer scope chain from a closure's context chain.
ASSERT(!info->closure().is_null());
- // If we have a serialized scope info, reuse it.
+ Context* context = info->closure()->context();
+ Scope* current_scope = NULL;
Scope* innermost_scope = NULL;
- Scope* scope = NULL;
-
- SerializedScopeInfo* scope_info =
info->closure()->shared()->scope_info();
- if (scope_info != SerializedScopeInfo::Empty()) {
- JSFunction* current = *info->closure();
- do {
- current = current->context()->closure();
- Handle<SerializedScopeInfo>
scope_info(current->shared()->scope_info());
- if (*scope_info != SerializedScopeInfo::Empty()) {
- scope = new Scope(scope, scope_info);
- if (innermost_scope == NULL) innermost_scope = scope;
+ bool contains_with = false;
+ while (!context->IsGlobalContext()) {
+ if (context->IsWithContext()) {
+ // All the inner scopes are inside a with.
+ contains_with = true;
+ for (Scope* s = innermost_scope; s != NULL; s = s->outer_scope()) {
+ s->scope_inside_with_ = true;
+ }
+ } else {
+ if (context->IsFunctionContext()) {
+ SerializedScopeInfo* scope_info =
+ context->closure()->shared()->scope_info();
+ current_scope =
+ new Scope(current_scope,
Handle<SerializedScopeInfo>(scope_info));
} else {
- ASSERT(current->context()->IsGlobalContext());
- }
- } while (!current->context()->IsGlobalContext());
+ ASSERT(context->IsCatchContext());
+ String* name = String::cast(context->extension());
+ current_scope = new Scope(current_scope, Handle<String>(name));
+ }
+ if (contains_with) current_scope->RecordWithStatement();
+ if (innermost_scope == NULL) innermost_scope = current_scope;
+ }
+
+ // Forget about a with when we move to a context for a different
function.
+ if (context->previous()->closure() != context->closure()) {
+ contains_with = false;
+ }
+ context = context->previous();
}
- global_scope->AddInnerScope(scope);
- if (innermost_scope == NULL) innermost_scope = global_scope;
-
- return innermost_scope;
+ global_scope->AddInnerScope(current_scope);
+ return (innermost_scope == NULL) ? global_scope : innermost_scope;
}
@@ -238,7 +270,7 @@
void Scope::Initialize(bool inside_with) {
- ASSERT(!resolved());
+ ASSERT(!already_resolved());
// Add this scope as a new inner scope of the outer scope.
if (outer_scope_ != NULL) {
@@ -279,11 +311,10 @@
Variable* Scope::LocalLookup(Handle<String> name) {
Variable* result = variables_.Lookup(name);
- if (result != NULL || !resolved()) {
+ if (result != NULL || scope_info_.is_null()) {
return result;
}
- // If the scope is resolved, we can find a variable in serialized scope
- // info.
+ // If we have a serialized scope info, we might find the variable there.
//
// We should never lookup 'arguments' in this scope as it is implicitly
// present in every scope.
@@ -331,7 +362,7 @@
void Scope::DeclareParameter(Handle<String> name) {
- ASSERT(!resolved());
+ ASSERT(!already_resolved());
ASSERT(is_function_scope());
Variable* var =
variables_.Declare(this, name, Variable::VAR, true,
Variable::NORMAL);
@@ -340,7 +371,7 @@
Variable* Scope::DeclareLocal(Handle<String> name, Variable::Mode mode) {
- ASSERT(!resolved());
+ ASSERT(!already_resolved());
// This function handles VAR and CONST modes. DYNAMIC variables are
// introduces during variable allocation, INTERNAL variables are
allocated
// explicitly, and TEMPORARY variables are allocated via NewTemporary().
@@ -363,7 +394,7 @@
// Note that we must not share the unresolved variables with
// the same name because they may be removed selectively via
// RemoveUnresolved().
- ASSERT(!resolved());
+ ASSERT(!already_resolved());
VariableProxy* proxy = new VariableProxy(name, false, inside_with,
position);
unresolved_.Add(proxy);
return proxy;
@@ -383,7 +414,7 @@
Variable* Scope::NewTemporary(Handle<String> name) {
- ASSERT(!resolved());
+ ASSERT(!already_resolved());
Variable* var =
new Variable(this, name, Variable::TEMPORARY, true,
Variable::NORMAL);
temps_.Add(var);
@@ -1022,7 +1053,7 @@
// If scope is already resolved, we still need to allocate
// variables in inner scopes which might not had been resolved yet.
- if (resolved()) return;
+ if (already_resolved()) return;
// The number of slots required for variables.
num_stack_slots_ = 0;
num_heap_slots_ = Context::MIN_CONTEXT_SLOTS;
=======================================
--- /branches/bleeding_edge/src/scopes.h Thu Jun 30 07:37:55 2011
+++ /branches/bleeding_edge/src/scopes.h Fri Jul 1 07:05:46 2011
@@ -364,6 +364,10 @@
bool outer_scope_is_eval_scope_;
bool force_eager_compilation_;
+ // True if it doesn't need scope resolution (e.g., if the scope was
+ // constructed based on a serialized scope info or a catch context).
+ bool already_resolved_;
+
// Computed as variables are declared.
int num_var_or_const_;
@@ -373,7 +377,7 @@
// Serialized scopes support.
Handle<SerializedScopeInfo> scope_info_;
- bool resolved() { return !scope_info_.is_null(); }
+ bool already_resolved() { return already_resolved_; }
// Create a non-local variable with a given name.
// These variables are looked up dynamically at runtime.
@@ -409,8 +413,12 @@
void AllocateVariablesRecursively();
private:
+ // Construct a function scope based on the scope info.
Scope(Scope* inner_scope, Handle<SerializedScopeInfo> scope_info);
+ // Construct a catch scope with a binding for the name.
+ Scope(Scope* inner_scope, Handle<String> catch_variable_name);
+
void AddInnerScope(Scope* inner_scope) {
if (inner_scope != NULL) {
inner_scopes_.Add(inner_scope);
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
