Debugger support for step-into async and async* functions.

Debugger support for step-into async and async* functions.

- [x] Support stepping into an async function.
- [x] Support stepping into the async generator in an await for loop.
- [x] Unit test for async function.
- [x] Unit test for async* function.

BUG=
R=asiva@google.com, rmacnak@google.com

Committed: https://github.com/dart-lang/sdk/commit/07a257573d1a2323ba6cb7a4fec629a167765e0c

[Cutch, 2017-03-22 20:37:30]

Description was changed from

==========
Debugger support for step-into async and async* functions.

- [x] Support stepping into an async function.
- [x] Support stepping into the async generator in an await for loop.
- [x] Unit test for async function.
- [x] Unit test for async* function.

BUG=
==========

to

==========
Debugger support for step-into async and async* functions.

- [x] Support stepping into an async function.
- [x] Support stepping into the async generator in an await for loop.
- [x] Unit test for async function.
- [x] Unit test for async* function.

BUG=
==========

[Cutch, 2017-03-22 20:37:30]

johnmccutchan@google.com changed reviewers:
+ asiva@google.com, rmacnak@google.com

[rmacnak, 2017-03-23 20:20:18]

lgtm w/c

https://codereview.chromium.org/2768103002/diff/20001/runtime/observatory/tes...
File runtime/observatory/tests/service/async_single_step_into_test.dart (right):

https://codereview.chromium.org/2768103002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/async_single_step_into_test.dart:27: }
Add an extra statement after helper() to check the machinery doesn't stop at
anywhere else in the current microtask, which is trivially skipped now because
its in undebuggable libraries.

https://codereview.chromium.org/2768103002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/async_single_step_into_test.dart:32: stepInto,
Consider stepOver for the prints to make this test independent of the debuggable
bit on dart:core.

https://codereview.chromium.org/2768103002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/async_single_step_into_test.dart:38: stepInto,
stepOver

https://codereview.chromium.org/2768103002/diff/20001/runtime/observatory/tes...
File runtime/observatory/tests/service/async_star_single_step_into_test.dart
(right):

https://codereview.chromium.org/2768103002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/async_star_single_step_into_test.dart:37: }
Ditto trailing statement

https://codereview.chromium.org/2768103002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/async_star_single_step_into_test.dart:42:
stepInto,
over

https://codereview.chromium.org/2768103002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/async_star_single_step_into_test.dart:48:
stepInto,
over

https://codereview.chromium.org/2768103002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/async_star_single_step_into_test.dart:58:
stepInto,
over

https://codereview.chromium.org/2768103002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/async_star_single_step_into_test.dart:63: ];
resumeIsolate to let it cleanly exit

https://codereview.chromium.org/2768103002/diff/20001/runtime/vm/debugger.cc
File runtime/vm/debugger.cc (right):

https://codereview.chromium.org/2768103002/diff/20001/runtime/vm/debugger.cc#...
runtime/vm/debugger.cc:1593: void Debugger::OnIsolateRunnable() {
This patching of well-known functions is similar to
Intrinsifier::InitializeState(), MethodRecognizer::InitializeState() and
ObjectStore::InitKnownObject().  Consider attaching this setup to where they are
called instead of the runnable event.

https://codereview.chromium.org/2768103002/diff/20001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/2768103002/diff/20001/runtime/vm/parser.cc#ne...
runtime/vm/parser.cc:9241: // Call '_asyncStarMoveNextHelper' so that the
debugger can intercept and
if (FLAG_support_debugger) {

[Cutch, 2017-03-23 21:24:51]

https://codereview.chromium.org/2768103002/diff/20001/runtime/observatory/tes...
File runtime/observatory/tests/service/async_single_step_into_test.dart (right):

https://codereview.chromium.org/2768103002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/async_single_step_into_test.dart:27: }
On 2017/03/23 20:20:18, rmacnak wrote:
> Add an extra statement after helper() to check the machinery doesn't stop at
> anywhere else in the current microtask, which is trivially skipped now because
> its in undebuggable libraries.

Done.

https://codereview.chromium.org/2768103002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/async_single_step_into_test.dart:32: stepInto,
On 2017/03/23 20:20:18, rmacnak wrote:
> Consider stepOver for the prints to make this test independent of the
debuggable
> bit on dart:core.

Done.

https://codereview.chromium.org/2768103002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/async_single_step_into_test.dart:38: stepInto,
On 2017/03/23 20:20:17, rmacnak wrote:
> stepOver

Done.

https://codereview.chromium.org/2768103002/diff/20001/runtime/observatory/tes...
File runtime/observatory/tests/service/async_star_single_step_into_test.dart
(right):

https://codereview.chromium.org/2768103002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/async_star_single_step_into_test.dart:37: }
On 2017/03/23 20:20:18, rmacnak wrote:
> Ditto trailing statement

Done.

https://codereview.chromium.org/2768103002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/async_star_single_step_into_test.dart:42:
stepInto,
On 2017/03/23 20:20:18, rmacnak wrote:
> over

Done.

https://codereview.chromium.org/2768103002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/async_star_single_step_into_test.dart:48:
stepInto,
On 2017/03/23 20:20:18, rmacnak wrote:
> over

Done.

https://codereview.chromium.org/2768103002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/async_star_single_step_into_test.dart:58:
stepInto,
On 2017/03/23 20:20:18, rmacnak wrote:
> over

Done.

https://codereview.chromium.org/2768103002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/async_star_single_step_into_test.dart:63: ];
On 2017/03/23 20:20:18, rmacnak wrote:
> resumeIsolate to let it cleanly exit

Done.

https://codereview.chromium.org/2768103002/diff/20001/runtime/vm/debugger.cc
File runtime/vm/debugger.cc (right):

https://codereview.chromium.org/2768103002/diff/20001/runtime/vm/debugger.cc#...
runtime/vm/debugger.cc:1593: void Debugger::OnIsolateRunnable() {
On 2017/03/23 20:20:18, rmacnak wrote:
> This patching of well-known functions is similar to
> Intrinsifier::InitializeState(), MethodRecognizer::InitializeState() and
> ObjectStore::InitKnownObject().  Consider attaching this setup to where they
are
> called instead of the runnable event.

At the time these functions are called we haven't attached a debugger instance
to the isolate yet. I agree it makes sense to do this at the time we call those
other functions but I also think this logic should live in the debugger.

https://codereview.chromium.org/2768103002/diff/20001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/2768103002/diff/20001/runtime/vm/parser.cc#ne...
runtime/vm/parser.cc:9241: // Call '_asyncStarMoveNextHelper' so that the
debugger can intercept and
On 2017/03/23 20:20:18, rmacnak wrote:
> if (FLAG_support_debugger) {

Done.

[siva, 2017-03-23 22:37:01]

lgtm

https://codereview.chromium.org/2768103002/diff/40001/runtime/lib/stacktrace.cc
File runtime/lib/stacktrace.cc (right):

https://codereview.chromium.org/2768103002/diff/40001/runtime/lib/stacktrace....
runtime/lib/stacktrace.cc:134: }
Why did you include this function here and not start a new lib/async.cc file

https://codereview.chromium.org/2768103002/diff/40001/runtime/vm/debugger.cc
File runtime/vm/debugger.cc (right):

https://codereview.chromium.org/2768103002/diff/40001/runtime/vm/debugger.cc#...
runtime/vm/debugger.cc:1611: Function& function = Function::Handle();
could use the 'zone' parameter for these handles.

https://codereview.chromium.org/2768103002/diff/40001/runtime/vm/debugger.cc#...
runtime/vm/debugger.cc:1619: function.set_is_inlinable(false);
maybe

for () {
  function ^= functions.At(i);
  if (function.IsNull()) {
    break;
  }
  function.set_is_debuggable(false);
  function.set_is_inlinable(false);
}

https://codereview.chromium.org/2768103002/diff/40001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/2768103002/diff/40001/runtime/vm/parser.cc#ne...
runtime/vm/parser.cc:9246: }
This call has to be added to the kernel side too right? Can you open an issue so
we don't forget it.

[Cutch, 2017-03-24 14:55:39]

https://codereview.chromium.org/2768103002/diff/40001/runtime/lib/stacktrace.cc
File runtime/lib/stacktrace.cc (right):

https://codereview.chromium.org/2768103002/diff/40001/runtime/lib/stacktrace....
runtime/lib/stacktrace.cc:134: }
On 2017/03/23 22:37:01, siva wrote:
> Why did you include this function here and not start a new lib/async.cc file

Done.

https://codereview.chromium.org/2768103002/diff/40001/runtime/vm/debugger.cc
File runtime/vm/debugger.cc (right):

https://codereview.chromium.org/2768103002/diff/40001/runtime/vm/debugger.cc#...
runtime/vm/debugger.cc:1611: Function& function = Function::Handle();
On 2017/03/23 22:37:01, siva wrote:
> could use the 'zone' parameter for these handles.

Done.

https://codereview.chromium.org/2768103002/diff/40001/runtime/vm/debugger.cc#...
runtime/vm/debugger.cc:1619: function.set_is_inlinable(false);
On 2017/03/23 22:37:01, siva wrote:
> maybe
> 
> for () {
>   function ^= functions.At(i);
>   if (function.IsNull()) {
>     break;
>   }
>   function.set_is_debuggable(false);
>   function.set_is_inlinable(false);
> }

Done.

https://codereview.chromium.org/2768103002/diff/40001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/2768103002/diff/40001/runtime/vm/parser.cc#ne...
runtime/vm/parser.cc:9246: }
On 2017/03/23 22:37:01, siva wrote:
> This call has to be added to the kernel side too right? Can you open an issue
so
> we don't forget it.

Filed https://github.com/dart-lang/sdk/issues/29158

[Cutch, 2017-03-24 14:56:57]

Description was changed from

==========
Debugger support for step-into async and async* functions.

- [x] Support stepping into an async function.
- [x] Support stepping into the async generator in an await for loop.
- [x] Unit test for async function.
- [x] Unit test for async* function.

BUG=
==========

to

==========
Debugger support for step-into async and async* functions.

- [x] Support stepping into an async function.
- [x] Support stepping into the async generator in an await for loop.
- [x] Unit test for async function.
- [x] Unit test for async* function.

BUG=
R=asiva@google.com, rmacnak@google.com

Committed:
https://github.com/dart-lang/sdk/commit/07a257573d1a2323ba6cb7a4fec629a167765e0c
==========

[Cutch, 2017-03-24 14:56:58]

Committed patchset #4 (id:60001) manually as
07a257573d1a2323ba6cb7a4fec629a167765e0c (presubmit successful).