LiveEdit to mark more closure functions for re-instantiation when scope layout changes

BUG=v8:2872
R=yangguo@chromium.org

Review URL: https://codereview.chromium.org/23783007

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@16868 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/liveedit-debugger.js

@@ -221,7 +221,7 @@ Debug.LiveEdit = new function() {
     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 @@ Debug.LiveEdit = new function() {
       }
     }
 
-    // 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 @@ Debug.LiveEdit = new function() {
     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 @@ Debug.LiveEdit = new function() {
 
     // 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 @@ Debug.LiveEdit = new function() {
                   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 @@ Debug.LiveEdit = new function() {
       }
 
       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 @@ Debug.LiveEdit = new function() {
           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 @@ Debug.LiveEdit = new function() {
     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 @@ Debug.LiveEdit = new function() {
     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 @@ Debug.LiveEdit = new function() {
     }
 
     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;

src/liveedit.cc

@@ -731,8 +731,8 @@ class FunctionInfoWrapper : public JSArrayBasedStruct<FunctionInfoWrapper> {
     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 @@ class FunctionInfoWrapper : public JSArrayBasedStruct<FunctionInfoWrapper> {
   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 @@ class FunctionInfoListener {
 
     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 @@ class FunctionInfoListener {
     // 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 @@ class FunctionInfoListener {
                                          isolate()));
       scope_info_length++;
 
-      outer_scope = outer_scope->outer_scope();
-    } while (outer_scope != NULL);
+      current_scope = current_scope->outer_scope();
+    }
 
     return *scope_info_list;
   }

test/mjsunit/debug-liveedit-4.js

+// 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);