Classes: Add support for stepping through default constructors
authorarv <arv@chromium.org>
Sat, 15 Nov 2014 19:48:32 +0000 (11:48 -0800)
committerCommit bot <commit-bot@chromium.org>
Sat, 15 Nov 2014 19:48:39 +0000 (19:48 +0000)
If a class extends another class and it doesn't provide a constructor,
one is created for them. We therefore need to ensure that stepping into
the constructor steps into the super class constructor.

BUG=v8:3674
LOG=Y
R=dslomov@chromium.org, aandrey , yurys

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

Cr-Commit-Position: refs/heads/master@{#25366}

src/debug.cc
src/debug.h
test/mjsunit/harmony/debug-step-into-class-extends.js [new file with mode: 0644]
test/mjsunit/harmony/debug-step-into-constructor.js [new file with mode: 0644]

index 7fe9064..3029adb 100644 (file)
@@ -1225,7 +1225,48 @@ void Debug::FloodBoundFunctionWithOneShot(Handle<JSFunction> function) {
   if (!bindee.is_null() && bindee->IsJSFunction() &&
       !JSFunction::cast(*bindee)->IsFromNativeScript()) {
     Handle<JSFunction> bindee_function(JSFunction::cast(*bindee));
-    FloodWithOneShot(bindee_function);
+    FloodWithOneShotGeneric(bindee_function);
+  }
+}
+
+
+void Debug::FloodDefaultConstructorWithOneShot(Handle<JSFunction> function) {
+  DCHECK(function->shared()->is_default_constructor());
+  // Instead of stepping into the function we directly step into the super class
+  // constructor.
+  Isolate* isolate = function->GetIsolate();
+  PrototypeIterator iter(isolate, function);
+  Handle<Object> proto = PrototypeIterator::GetCurrent(iter);
+  if (!proto->IsJSFunction()) return;  // Object.prototype
+  Handle<JSFunction> function_proto = Handle<JSFunction>::cast(proto);
+  FloodWithOneShotGeneric(function_proto);
+}
+
+
+void Debug::FloodWithOneShotGeneric(Handle<JSFunction> function,
+                                    Handle<Object> holder) {
+  if (function->shared()->bound()) {
+    FloodBoundFunctionWithOneShot(function);
+  } else if (function->shared()->is_default_constructor()) {
+    FloodDefaultConstructorWithOneShot(function);
+  } else if (!function->IsFromNativeScript()) {
+    Isolate* isolate = function->GetIsolate();
+    // Don't allow step into functions in the native context.
+    if (function->shared()->code() ==
+            isolate->builtins()->builtin(Builtins::kFunctionApply) ||
+        function->shared()->code() ==
+            isolate->builtins()->builtin(Builtins::kFunctionCall)) {
+      // Handle function.apply and function.call separately to flood the
+      // function to be called and not the code for Builtins::FunctionApply or
+      // Builtins::FunctionCall. The receiver of call/apply is the target
+      // function.
+      if (!holder.is_null() && holder->IsJSFunction()) {
+        Handle<JSFunction> js_function = Handle<JSFunction>::cast(holder);
+        FloodWithOneShotGeneric(js_function);
+      }
+    } else {
+      FloodWithOneShot(function);
+    }
   }
 }
 
@@ -1464,13 +1505,7 @@ void Debug::PrepareStep(StepAction step_action,
 
       if (fun->IsJSFunction()) {
         Handle<JSFunction> js_function(JSFunction::cast(fun));
-        if (js_function->shared()->bound()) {
-          FloodBoundFunctionWithOneShot(js_function);
-        } else if (!js_function->IsFromNativeScript()) {
-          // Don't step into builtins.
-          // It will also compile target function if it's not compiled yet.
-          FloodWithOneShot(js_function);
-        }
+        FloodWithOneShotGeneric(js_function);
       }
     }
 
@@ -1612,32 +1647,7 @@ void Debug::HandleStepIn(Handle<Object> function_obj, Handle<Object> holder,
   // Flood the function with one-shot break points if it is called from where
   // step into was requested, or when stepping into a new frame.
   if (fp == thread_local_.step_into_fp_ || step_frame) {
-    if (function->shared()->bound()) {
-      // Handle Function.prototype.bind
-      FloodBoundFunctionWithOneShot(function);
-    } else if (!function->IsFromNativeScript()) {
-      // Don't allow step into functions in the native context.
-      if (function->shared()->code() ==
-          isolate->builtins()->builtin(Builtins::kFunctionApply) ||
-          function->shared()->code() ==
-          isolate->builtins()->builtin(Builtins::kFunctionCall)) {
-        // Handle function.apply and function.call separately to flood the
-        // function to be called and not the code for Builtins::FunctionApply or
-        // Builtins::FunctionCall. The receiver of call/apply is the target
-        // function.
-        if (!holder.is_null() && holder->IsJSFunction()) {
-          Handle<JSFunction> js_function = Handle<JSFunction>::cast(holder);
-          if (!js_function->IsFromNativeScript()) {
-            FloodWithOneShot(js_function);
-          } else if (js_function->shared()->bound()) {
-            // Handle Function.prototype.bind
-            FloodBoundFunctionWithOneShot(js_function);
-          }
-        }
-      } else {
-        FloodWithOneShot(function);
-      }
-    }
+    FloodWithOneShotGeneric(function, holder);
   }
 }
 
index e486d97..b8348fd 100644 (file)
@@ -390,6 +390,9 @@ class Debug {
   void FloodWithOneShot(Handle<JSFunction> function,
                         BreakLocatorType type = ALL_BREAK_LOCATIONS);
   void FloodBoundFunctionWithOneShot(Handle<JSFunction> function);
+  void FloodDefaultConstructorWithOneShot(Handle<JSFunction> function);
+  void FloodWithOneShotGeneric(Handle<JSFunction> function,
+                               Handle<Object> holder = Handle<Object>());
   void FloodHandlerWithOneShot();
   void ChangeBreakOnException(ExceptionBreakType type, bool enable);
   bool IsBreakOnException(ExceptionBreakType type);
diff --git a/test/mjsunit/harmony/debug-step-into-class-extends.js b/test/mjsunit/harmony/debug-step-into-class-extends.js
new file mode 100644 (file)
index 0000000..ffc6fda
--- /dev/null
@@ -0,0 +1,42 @@
+// Copyright 2014 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --expose-debug-as debug --harmony-classes
+
+'use strict';
+
+var Debug = debug.Debug
+
+var done = false;
+var stepCount = 0;
+
+function listener(event, execState, eventData, data) {
+  if (event == Debug.DebugEvent.Break) {
+    if (!done) {
+      execState.prepareStep(Debug.StepAction.StepInto);
+      var s = execState.frame().sourceLineText();
+      assertTrue(s.indexOf('// ' + stepCount + '.') !== -1);
+      stepCount++;
+    }
+  }
+};
+
+Debug.setListener(listener);
+
+function GetBase() {
+  var x = 1;   // 1.
+  var y = 2;   // 2.
+  done = true; // 3.
+  return null;
+}
+
+function f() {
+  class Derived extends GetBase() {} // 0.
+}
+
+var bp = Debug.setBreakPoint(f, 0);
+f();
+assertEquals(4, stepCount);
+
+Debug.setListener(null);
diff --git a/test/mjsunit/harmony/debug-step-into-constructor.js b/test/mjsunit/harmony/debug-step-into-constructor.js
new file mode 100644 (file)
index 0000000..dbef60f
--- /dev/null
@@ -0,0 +1,113 @@
+// Copyright 2014 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --expose-debug-as debug --harmony-classes
+
+'use strict';
+
+var Debug = debug.Debug
+var done, stepCount;
+
+function listener(event, execState, eventData, data) {
+  if (event == Debug.DebugEvent.Break) {
+    if (!done) {
+      execState.prepareStep(Debug.StepAction.StepInto);
+      var s = execState.frame().sourceLineText();
+      assertTrue(s.indexOf('// ' + stepCount + '.') !== -1);
+      stepCount++;
+    }
+  }
+};
+
+Debug.setListener(listener);
+
+
+class Base {
+  constructor() {
+    var x = 1;   // 1.
+    var y = 2;   // 2.
+    done = true; // 3.
+  }
+}
+
+class Derived extends Base {}
+
+
+(function TestBreakPointInConstructor() {
+  done = false;
+  stepCount = 1;
+  var bp = Debug.setBreakPoint(Base, 0);
+
+  new Base();
+  assertEquals(4, stepCount);
+
+  Debug.clearBreakPoint(bp);
+})();
+
+
+(function TestDefaultConstructor() {
+  done = false;
+  stepCount = 1;
+
+  var bp = Debug.setBreakPoint(Base, 0);
+  new Derived();
+  assertEquals(4, stepCount);
+
+  Debug.clearBreakPoint(bp);
+})();
+
+
+(function TestStepInto() {
+  done = false;
+  stepCount = 0;
+
+  function f() {
+    new Derived();  // 0.
+  }
+
+  var bp = Debug.setBreakPoint(f, 0);
+  f();
+  assertEquals(4, stepCount);
+
+  Debug.clearBreakPoint(bp);
+})();
+
+
+(function TestExtraIndirection() {
+  done = false;
+  stepCount = 0;
+
+  class Derived2 extends Derived {}
+
+  function f() {
+    new Derived2();  // 0.
+  }
+
+  var bp = Debug.setBreakPoint(f, 0);
+  f();
+  assertEquals(4, stepCount);
+
+  Debug.clearBreakPoint(bp);
+})();
+
+
+(function TestBoundClass() {
+  done = false;
+  stepCount = 0;
+
+  var bound = Derived.bind(null);
+
+  function f() {
+    new bound();  // 0.
+  }
+
+  var bp = Debug.setBreakPoint(f, 0);
+  f();
+  assertEquals(4, stepCount);
+
+  Debug.clearBreakPoint(bp);
+})();
+
+
+Debug.setListener(null);