Reviewers: Mads Ager,
Description:
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).
Please review this at http://codereview.chromium.org/6357007/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge/build/ia32
Affected files:
M src/scopes.cc
M test/mjsunit/debug-evaluate-locals.js
Index: src/scopes.cc
diff --git a/src/scopes.cc b/src/scopes.cc
index
d3f54ad3f2d9ad8b47fefae2d89fd692bd3e2eda..50da1faf913cc391ad07f2d672d94e1e2fc241eb
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.
Index: test/mjsunit/debug-evaluate-locals.js
diff --git a/test/mjsunit/debug-evaluate-locals.js
b/test/mjsunit/debug-evaluate-locals.js
index
4b87829169586ed4885bee9236138ecbd51faa87..8bc6a617cfeba27dc5fe8a7954d24a93fd9b590c
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());
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
