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

diff --git a/src/scopes.cc b/src/scopes.cc
index d3f54ad..50da1fa 100644 (file)
--- 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 (file)
--- 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());