Revision: 16868
Author: [email protected]
Date: Fri Sep 20 13:15:31 2013 UTC
Log: LiveEdit to mark more closure functions for re-instantiation when
scope layout changes
BUG=v8:2872
[email protected]
Review URL: https://codereview.chromium.org/23783007
http://code.google.com/p/v8/source/detail?r=16868
Added:
/branches/bleeding_edge/test/mjsunit/debug-liveedit-4.js
Modified:
/branches/bleeding_edge/src/liveedit-debugger.js
/branches/bleeding_edge/src/liveedit.cc
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/debug-liveedit-4.js Fri Sep 20
13:15:31 2013 UTC
@@ -0,0 +1,69 @@
+// Copyright 2010 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.
+
+// Flags: --expose-debug-as debug
+// Get the Debug object exposed from the debug context global object.
+
+// In this test case we edit a script so that techincally function text
+// hasen't been changed. However actually function became one level more
nested
+// and must be recompiled because it uses variable from outer scope.
+
+
+Debug = debug.Debug
+
+eval(
+"function TestFunction() {\n"
++ " var a = 'a';\n"
++ " var b = 'b';\n"
++ " var c = 'c';\n"
++ " function A() {\n"
++ " return 2013;\n"
++ " }\n"
++ " function B() {\n"
++ " return String([a, c]);\n"
++ " }\n"
++ " return B();\n"
++ "}\n"
+);
+
+var res = TestFunction();
+print(res);
+assertEquals('a,c', res);
+
+var script = Debug.findScript(TestFunction);
+var new_source = script.source.replace("2013", "b");
+print("new source: " + new_source);
+var change_log = new Array();
+var result = Debug.LiveEdit.SetScriptSource(script, new_source, false,
change_log);
+
+print("Result: " + JSON.stringify(result) + "\n");
+print("Change log: " + JSON.stringify(change_log) + "\n");
+
+var res = TestFunction();
+print(res);
+// This might be 'a,b' without a bug fixed.
+assertEquals('a,c', res);
=======================================
--- /branches/bleeding_edge/src/liveedit-debugger.js Mon Dec 3 21:47:39
2012 UTC
+++ /branches/bleeding_edge/src/liveedit-debugger.js Fri Sep 20 13:15:31
2013 UTC
@@ -221,7 +221,7 @@
change_log.push( {position_patched: position_patch_report} );
for (var i = 0; i < update_positions_list.length; i++) {
- // TODO(LiveEdit): take into account wether it's source_changed or
+ // TODO(LiveEdit): take into account whether it's source_changed or
// unchanged and whether positions changed at all.
PatchPositions(update_positions_list[i], diff_array,
position_patch_report);
@@ -288,7 +288,7 @@
}
}
- // After sorting update outer_inder field using old_index_map. Also
+ // After sorting update outer_index field using old_index_map. Also
// set next_sibling_index field.
var current_index = 0;
@@ -692,10 +692,10 @@
ProcessInternals(code_info_tree);
}
- // For ecah old function (if it is not damaged) tries to find a
corresponding
+ // For each old function (if it is not damaged) tries to find a
corresponding
// function in new script. Typically it should succeed (non-damaged
functions
// by definition may only have changes inside their bodies). However
there are
- // reasons for corresponence not to be found; function with unmodified
text
+ // reasons for correspondence not to be found; function with unmodified
text
// in new script may become enclosed into other function; the innocent
change
// inside function body may in fact be something like "} function B() {"
that
// splits a function into 2 functions.
@@ -703,7 +703,13 @@
// A recursive function that tries to find a correspondence for all
// child functions and for their inner functions.
- function ProcessChildren(old_node, new_node) {
+ function ProcessNode(old_node, new_node) {
+ var scope_change_description =
+ IsFunctionContextLocalsChanged(old_node.info, new_node.info);
+ if (scope_change_description) {
+ old_node.status = FunctionStatus.CHANGED;
+ }
+
var old_children = old_node.children;
var new_children = new_node.children;
@@ -729,8 +735,15 @@
new_children[new_index];
old_children[old_index].textual_corresponding_node =
new_children[new_index];
- if (old_children[old_index].status !=
FunctionStatus.UNCHANGED) {
- ProcessChildren(old_children[old_index],
+ if (scope_change_description) {
+ old_children[old_index].status = FunctionStatus.DAMAGED;
+ old_children[old_index].status_explanation =
+ "Enclosing function is now incompatible. " +
+ scope_change_description;
+ old_children[old_index].corresponding_node = void 0;
+ } else if (old_children[old_index].status !=
+ FunctionStatus.UNCHANGED) {
+ ProcessNode(old_children[old_index],
new_children[new_index]);
if (old_children[old_index].status ==
FunctionStatus.DAMAGED) {
unmatched_new_nodes_list.push(
@@ -772,11 +785,10 @@
}
if (old_node.status == FunctionStatus.CHANGED) {
- var why_wrong_expectations =
- WhyFunctionExpectationsDiffer(old_node.info, new_node.info);
- if (why_wrong_expectations) {
+ if (old_node.info.param_num != new_node.info.param_num) {
old_node.status = FunctionStatus.DAMAGED;
- old_node.status_explanation = why_wrong_expectations;
+ old_node.status_explanation = "Changed parameter number: " +
+ old_node.info.param_num + " and " + new_node.info.param_num;
}
}
old_node.unmatched_new_nodes = unmatched_new_nodes_list;
@@ -784,7 +796,7 @@
textually_unmatched_new_nodes_list;
}
- ProcessChildren(old_code_tree, new_code_tree);
+ ProcessNode(old_code_tree, new_code_tree);
old_code_tree.corresponding_node = new_code_tree;
old_code_tree.textual_corresponding_node = new_code_tree;
@@ -856,7 +868,7 @@
this.raw_array = raw_array;
}
- // Changes positions (including all statments) in function.
+ // Changes positions (including all statements) in function.
function PatchPositions(old_info_node, diff_array, report_array) {
if (old_info_node.live_shared_function_infos) {
old_info_node.live_shared_function_infos.forEach(function (info) {
@@ -878,15 +890,9 @@
return script.name + " (old)";
}
- // Compares a function interface old and new version, whether it
+ // Compares a function scope heap structure, old and new version,
whether it
// changed or not. Returns explanation if they differ.
- function WhyFunctionExpectationsDiffer(function_info1, function_info2) {
- // Check that function has the same number of parameters (there may
exist
- // an adapter, that won't survive function parameter number change).
- if (function_info1.param_num != function_info2.param_num) {
- return "Changed parameter number: " + function_info1.param_num +
- " and " + function_info2.param_num;
- }
+ function IsFunctionContextLocalsChanged(function_info1, function_info2) {
var scope_info1 = function_info1.scope_info;
var scope_info2 = function_info2.scope_info;
@@ -905,8 +911,8 @@
}
if (scope_info1_text != scope_info2_text) {
- return "Incompatible variable maps: [" + scope_info1_text +
- "] and [" + scope_info2_text + "]";
+ return "Variable map changed: [" + scope_info1_text +
+ "] => [" + scope_info2_text + "]";
}
// No differences. Return undefined.
return;
=======================================
--- /branches/bleeding_edge/src/liveedit.cc Tue Sep 10 14:30:36 2013 UTC
+++ /branches/bleeding_edge/src/liveedit.cc Fri Sep 20 13:15:31 2013 UTC
@@ -731,8 +731,8 @@
Handle<JSValue> scope_wrapper = WrapInJSValue(code_scope_info);
this->SetField(kCodeScopeInfoOffset_, scope_wrapper);
}
- void SetOuterScopeInfo(Handle<Object> scope_info_array) {
- this->SetField(kOuterScopeInfoOffset_, scope_info_array);
+ void SetFunctionScopeInfo(Handle<Object> scope_info_array) {
+ this->SetField(kFunctionScopeInfoOffset_, scope_info_array);
}
void SetSharedFunctionInfo(Handle<SharedFunctionInfo> info) {
Handle<JSValue> info_holder = WrapInJSValue(info);
@@ -771,7 +771,7 @@
static const int kParamNumOffset_ = 3;
static const int kCodeOffset_ = 4;
static const int kCodeScopeInfoOffset_ = 5;
- static const int kOuterScopeInfoOffset_ = 6;
+ static const int kFunctionScopeInfoOffset_ = 6;
static const int kParentIndexOffset_ = 7;
static const int kSharedFunctionInfoOffset_ = 8;
static const int kLiteralNumOffset_ = 9;
@@ -880,7 +880,7 @@
Handle<Object> scope_info_list(SerializeFunctionScope(scope, zone),
isolate());
- info.SetOuterScopeInfo(scope_info_list);
+ info.SetFunctionScopeInfo(scope_info_list);
}
Handle<JSArray> GetResult() { return result_; }
@@ -897,14 +897,12 @@
// Saves some description of scope. It stores name and indexes of
// variables in the whole scope chain. Null-named slots delimit
// scopes of this chain.
- Scope* outer_scope = scope->outer_scope();
- if (outer_scope == NULL) {
- return isolate()->heap()->undefined_value();
- }
- do {
- ZoneList<Variable*> stack_list(outer_scope->StackLocalCount(), zone);
- ZoneList<Variable*> context_list(outer_scope->ContextLocalCount(),
zone);
- outer_scope->CollectStackAndContextLocals(&stack_list,
&context_list);
+ Scope* current_scope = scope;
+ while (current_scope != NULL) {
+ ZoneList<Variable*> stack_list(current_scope->StackLocalCount(),
zone);
+ ZoneList<Variable*> context_list(
+ current_scope->ContextLocalCount(), zone);
+ current_scope->CollectStackAndContextLocals(&stack_list,
&context_list);
context_list.Sort(&Variable::CompareIndex);
for (int i = 0; i < context_list.length(); i++) {
@@ -924,8 +922,8 @@
isolate()));
scope_info_length++;
- outer_scope = outer_scope->outer_scope();
- } while (outer_scope != NULL);
+ current_scope = current_scope->outer_scope();
+ }
return *scope_info_list;
}
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
