dart-archive / vm_service_client

A Darty client for the VM service protocol
https://pub.dev/packages/vm_service_client
BSD 3-Clause "New" or "Revised" License
12 stars 19 forks source link

Assertion failure on getStack with a BoundVariable of type @TypeArguments #41

Open DanTup opened 6 years ago

DanTup commented 6 years ago

I hit this failed assertion in getStack. It's expecting the type of a BoundVariable to be @Instance or Instance but it's a @TypeArguments:

screen shot 2018-07-31 at 9 29 25 am screen shot 2018-07-31 at 9 28 54 am

Here's the problematic JSON:

{
    "type": "BoundVariable",
    "name": "function_type_arguments_var",
    "value": {
        "type": "@TypeArguments",
        "class": {
            "type": "@Class",
            "fixedId": true,
            "id": "classes\/41",
            "name": "TypeArguments"
        },
        "id": "objects\/137",
        "name": "<FutureOr<dynamic>, bool>"
    },
    "declarationTokenPos": -1,
    "scopeStartTokenPos": -1,
    "scopeEndTokenPos": 50428
}

And here is the full stack response from the VM.

DanTup commented 6 years ago

The code I'm stepping through looks like this:

Future<void> doAsyncStuff() async {
    print("test"); // BREAKPOINT
    await new Future.value(true);
    print("test2");
    await new Future.microtask(() => true);
    print("test3");
    await new Future.delayed(const Duration(milliseconds: 1));
    print("done!");
  }
devoncarew commented 6 years ago

For context, this is blocking us from writing some tests we'd like to have for Flutter.

devoncarew commented 6 years ago

I think for this case, the spec needs to be updated:

class BoundVariable {
  /// [value] can be one of [InstanceRef] or [Sentinel].
  dynamic value;

For above, we should add TypeArguments, or instead reference the parent type, ObjRef.

and, the assert should be updated to allow for TypeArgumentsRef.

DanTup commented 6 years ago

I've opened a PR that updates the spec. Once that's resolved (assuming it's correct) I'll take a look at this - looks like we already have a newVMTypeArgumentsRef so possibly we can handle this in the caller of the function in the first screenshot (which is newVMInstanceRefOrSentinel, but maybe should be renamed with this change).