From 7f43cf0539f48aad005040c8390c0293e6586795 Mon Sep 17 00:00:00 2001
From: "kmillikin@chromium.org"
 <kmillikin@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Thu, 20 Jan 2011 12:32:43 +0000
Subject: [PATCH] Make 'with' mark only variables occurring in the body as
 used.

Before, we conservatively marked every variable in a scope as used if the
scope contained 'with'.  Instead, just mark the variables occurring in the
body of the with.  This avoids marking 'arguments' as used whenever 'with'
occurs, which incurs an extra performance penalty (a use of arguments is
seen as an instruction to redirect all parameter accesses to the arguments
object).

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@6415 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
 src/scopes.cc                         |  5 +++--
 test/mjsunit/debug-evaluate-locals.js | 20 +++++++++-----------
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/src/scopes.cc b/src/scopes.cc
index d3f54ad..50da1fa 100644
--- a/src/scopes.cc
+++ b/src/scopes.cc
@@ -726,6 +726,7 @@ void Scope::ResolveVariable(Scope* global_scope,
     // Note that we must do a lookup anyway, because if we find one,
     // we must mark that variable as potentially accessed from this
     // inner scope (the property may not be in the 'with' object).
+    if (var != NULL) var->set_is_used(true);
     var = NonLocal(proxy->name(), Variable::DYNAMIC);
 
   } else {
@@ -833,8 +834,8 @@ bool Scope::MustAllocate(Variable* var) {
   // visible name.
   if ((var->is_this() || var->name()->length() > 0) &&
       (var->is_accessed_from_inner_scope() ||
-       scope_calls_eval_ || inner_scope_calls_eval_ ||
-       scope_contains_with_)) {
+       scope_calls_eval_ ||
+       inner_scope_calls_eval_)) {
     var->set_is_used(true);
   }
   // Global variables do not need to be allocated.
diff --git a/test/mjsunit/debug-evaluate-locals.js b/test/mjsunit/debug-evaluate-locals.js
index 4b87829..8bc6a61 100644
--- a/test/mjsunit/debug-evaluate-locals.js
+++ b/test/mjsunit/debug-evaluate-locals.js
@@ -34,18 +34,18 @@ exception = false;
 
 
 function checkFrame0(name, value) {
-  assertTrue(name == 'a' || name == 'b');
+  assertTrue(name == 'a' || name == 'b', 'check name');
   if (name == 'a') {
     assertEquals(1, value);
-  }
-  if (name == 'b') {
+  } else if (name == 'b') {
     assertEquals(2, value);
   }
 }
 
 
 function checkFrame1(name, value) {
-  assertTrue(name == '.arguments' || name == 'a');
+  assertTrue(name == '.arguments' || name == 'arguments' || name == 'a',
+             'check name');
   if (name == 'a') {
     assertEquals(3, value);
   }
@@ -53,8 +53,7 @@ function checkFrame1(name, value) {
 
 
 function checkFrame2(name, value) {
-  assertTrue(name == '.arguments' || name == 'a' ||
-             name == 'arguments' || name == 'b');
+  assertTrue(name == 'a' || name == 'b');
   if (name == 'a') {
     assertEquals(5, value);
   }
@@ -73,18 +72,17 @@ function listener(event, exec_state, event_data, data) {
       checkFrame0(frame0.localName(0), frame0.localValue(0).value());
       checkFrame0(frame0.localName(1), frame0.localValue(1).value());
 
-      // Frame 1 has normal variable a (and the .arguments variable).
+      // Frame 1 has normal variables a and arguments (and the .arguments
+      // variable).
       var frame1 = exec_state.frame(1);
       checkFrame1(frame1.localName(0), frame1.localValue(0).value());
       checkFrame1(frame1.localName(1), frame1.localValue(1).value());
+      checkFrame1(frame1.localName(2), frame1.localValue(2).value());
 
-      // Frame 2 has normal variables a and b (and both the .arguments and
-      // arguments variable).
+      // Frame 2 has normal variables a and b.
       var frame2 = exec_state.frame(2);
       checkFrame2(frame2.localName(0), frame2.localValue(0).value());
       checkFrame2(frame2.localName(1), frame2.localValue(1).value());
-      checkFrame2(frame2.localName(2), frame2.localValue(2).value());
-      checkFrame2(frame2.localName(3), frame2.localValue(3).value());
 
       // Evaluating a and b on frames 0, 1 and 2 produces 1, 2, 3, 4, 5 and 6.
       assertEquals(1, exec_state.frame(0).evaluate('a').value());
-- 
2.7.4