First bits of external debugger API

runtime/vm/dart_dapi_impl.cc

Index: runtime/vm/dart_dapi_impl.cc
===================================================================
--- runtime/vm/dart_dapi_impl.cc	(revision 0)
+++ runtime/vm/dart_dapi_impl.cc	(revision 0)
@@ -0,0 +1,74 @@
+// Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+#include "include/dart_dapi.h"
+
+#include "vm/dart_api_impl.h"
+#include "vm/dart_api_state.h"
+#include "vm/debugger.h"
+#include "vm/isolate.h"
+#include "vm/longjump.h"
+
+
+namespace dart {

[siva, 2011/12/07 05:44:48]

extra blank line?

[hausner, 2011/12/07 22:14:26]

Done.

+
+
+DART_EXPORT void Dart_InitDebugger() {
+  Isolate* isolate = Isolate::Current();
+  DARTSCOPE(isolate);
+  isolate->debugger()->Initialize(isolate);
+}
+
+
+DART_EXPORT Dart_Handle Dart_SetBreakpointAtEntry(
+                            Dart_Handle library_in,
+                            Dart_Handle class_name_in,
+                            Dart_Handle function_name_in,
+                            Dart_Breakpoint* breakpoint) {
+  Isolate* isolate = Isolate::Current();
+  DARTSCOPE(isolate);

[siva, 2011/12/07 05:44:48]

In the dart API functions we normally check for valid input and return errors.
In this case we should check and return an error if breakpoint == NULL.

[hausner, 2011/12/07 22:14:26]

The idea was to allow NULL as a valid value. The caller may not want to get a
pointer to the Breakpoint object.

[siva, 2011/12/07 22:27:08]

Does that mean you will implement the delete API also in terms of
library/class/function name and not by passing in the Dart_Breakpoint object?

On 2011/12/07 22:14:26, hausner wrote:

[hausner, 2011/12/07 22:36:49]

Hmm, good point. I'm pretty sure there will also be a way to enumerate
breakpoints at some time, but maybe it would make sense to require the parameter
to be non-null. I'll leave this for the next change.

+  if (breakpoint != NULL) {
+    *breakpoint = NULL;
+  }
+  const char* msg = CheckIsolateState(isolate);
+  if (msg != NULL) {
+    return Api::Error(msg);
+  }

[siva, 2011/12/07 05:44:48]

Should the isolate state check happen after validating the input params library,
class, function etc.

[hausner, 2011/12/07 22:14:26]

Yes, good point. Rearranged the order of things.

+
+  const Library& library =
+      Library::CheckedHandle(Api::UnwrapHandle(library_in));
+  if (library.IsNull()) {
+    return Api::Error("No library specified");

[siva, 2011/12/07 05:44:48]

The wording of error messages are not consistent with the ones in dart_api.cc,
again Todd has been trying to make the error messages uniform and more
descriptive. In this case it would be:

"%s expects argument 'library' to be non-null", CURRENT_FUNC

[hausner, 2011/12/07 22:14:26]

Ok, adapted the scheme used in dart_api_impl.

+  }
+  const String& class_name =
+      String::CheckedHandle(Api::UnwrapHandle(class_name_in));

[siva, 2011/12/07 05:44:48]

Need similar null checks on class_name_in parameter.

[hausner, 2011/12/07 22:14:26]

Done.

+  const String& function_name =
+      String::CheckedHandle(Api::UnwrapHandle(function_name_in));

[siva, 2011/12/07 05:44:48]

Ditto on check.

[hausner, 2011/12/07 22:14:26]

Done.

+
+  // Resolve the breakpoint target function.
+  const Function& bp_target = Function::Handle(
+      isolate->debugger()->ResolveFunction(
+          library, class_name, function_name));
+  if (bp_target.IsNull()) {
+    return Api::Error("Breakpoint target function does not exist");

[siva, 2011/12/07 05:44:48]

I guess the error message needs to be more descriptive....

[hausner, 2011/12/07 22:14:26]

How? You mean by including the function name?

[siva, 2011/12/07 22:27:08]

Yes, the fully qualified function name (i.e library, class, function).

On 2011/12/07 22:14:26, hausner wrote:

[hausner, 2011/12/07 22:36:49]

Ok, will do in the next CL.

+  }
+
+  LongJump* base = isolate->long_jump_base();
+  LongJump jump;
+  isolate->set_long_jump_base(&jump);
+  Dart_Handle result = Api::True();
+  if (setjmp(*jump.Set()) == 0) {
+    Breakpoint* bpt = isolate->debugger()->SetBreakpointAtEntry(bp_target);

[siva, 2011/12/07 05:44:48]

We seem to be using isolate->debugger() a lot in this function maybe we could
have a local variable debugger.

[hausner, 2011/12/07 22:14:26]

It's only used twice. Done.

+    if (breakpoint != NULL) {

[siva, 2011/12/07 05:44:48]

breakpoint will be non NULL here if we error out earlier.

[hausner, 2011/12/07 22:14:26]

*breakpoint is set to NULL early on. It only has a non-null value if we succeed
setting a breakpoint.

+      *breakpoint = reinterpret_cast<Dart_Breakpoint>(bpt);
+    }
+  } else {
+    SetupErrorResult(&result);
+  }
+  isolate->set_long_jump_base(base);
+  return result;
+}
+
+
+}  // namespace dart