[dds/dap] Improve handling of lists in variables requests

Fixes https://github.com/Dart-Code/Dart-Code/issues/4204. Fixes https://github.com/Dart-Code/Dart-Code/issues/4213. Change-Id: Ibd95a149e6f620efb690c24623bb6b9d04a794b3 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/263620 Reviewed-by: Ben Konyi <bkonyi@google.com> Commit-Queue: Ben Konyi <bkonyi@google.com>
2024-09-05 08:24:39 +00:00 · 2022-10-17 21:51:36 +00:00 · 2022-10-17 21:51:36 +00:00 · ae352fe940
parent 263b163f47
commit ae352fe940
6 changed files with 263 additions and 18 deletions
--- a/pkg/dds/CHANGELOG.md
+++ b/pkg/dds/CHANGELOG.md
@ -1,3 +1,7 @@
+# 2.5.0-dev
+- [DAP] `variables` requests now treat lists from `dart:typed_data` (such as `Uint8List`) like standard `List` instances and return their elements instead of class fields.
+- [DAP] `variables` requests now return information about the number of items in lists to allow the client to page through them.
+
 # 2.4.0
 - [DAP] Added support for sending progress notifications via `DartDebugAdapter.startProgressNotification`.
  Standard progress events are sent when a clients sets `supportsProgressReporting: true` in its capabilities,
--- a/pkg/dds/lib/src/dap/adapters/dart.dart
+++ b/pkg/dds/lib/src/dap/adapters/dart.dart
@ -1619,8 +1619,12 @@ abstract class DartDebugAdapter<TL extends LaunchRequestArguments,
        ]);
      }
    } else if (vmData is vm.ObjRef) {
-      final object =
-          await _isolateManager.getObject(storedData.thread.isolate, vmData);
+      final object = await _isolateManager.getObject(
+        storedData.thread.isolate,
+        vmData,
+        offset: childStart,
+        count: childCount,
+      );

      if (object is vm.Sentinel) {
        variables.add(Variable(
--- a/pkg/dds/lib/src/dap/isolate_manager.dart
+++ b/pkg/dds/lib/src/dap/isolate_manager.dart
@ -140,8 +140,17 @@ class IsolateManager {
  }

  Future<T> getObject<T extends vm.Response>(
-      vm.IsolateRef isolate, vm.ObjRef object) async {
-    final res = await _adapter.vmService?.getObject(isolate.id!, object.id!);
+    vm.IsolateRef isolate,
+    vm.ObjRef object, {
+    int? offset,
+    int? count,
+  }) async {
+    final res = await _adapter.vmService?.getObject(
+      isolate.id!,
+      object.id!,
+      offset: offset,
+      count: count,
+    );
    return res as T;
  }

--- a/pkg/dds/lib/src/dap/protocol_converter.dart
+++ b/pkg/dds/lib/src/dap/protocol_converter.dart
@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.

 import 'dart:async';
+import 'dart:convert';
 import 'dart:io';

 import 'package:collection/collection.dart';
@ -100,9 +101,9 @@ class ProtocolConverter {
        }
      }
      return stringValue;
-    } else if (ref.kind == 'List') {
-      return 'List (${ref.length} ${ref.length == 1 ? "item" : "items"})';
-    } else if (ref.kind == 'Map') {
+    } else if (_isList(ref)) {
+      return '${ref.kind} (${ref.length} ${ref.length == 1 ? "item" : "items"})';
+    } else if (_isMap(ref)) {
      return 'Map (${ref.length} ${ref.length == 1 ? "item" : "items"})';
    } else if (ref.kind == 'Type') {
      return 'Type (${ref.name})';
@ -114,8 +115,9 @@ class ProtocolConverter {
  /// Converts a [vm.Instance] to a list of [dap.Variable]s, one for each
  /// field/member/element/association.
  ///
-  /// If [startItem] and/or [numItems] are supplied, only a slice of the
-  /// items will be returned to allow the client to page.
+  /// If [startItem] and/or [numItems] are supplied, it is assumed that the
+  /// elements/associations/bytes in [instance] have been restricted to that set
+  /// when fetched from the VM.
  Future<List<dap.Variable>> convertVmInstanceToVariablesList(
    ThreadInfo thread,
    vm.Instance instance, {
@ -142,10 +144,7 @@ class ProtocolConverter {
    } else if (elements != null) {
      // For lists, map each item (in the requested subset) to a variable.
      final start = startItem ?? 0;
-      return Future.wait(elements
-          .cast<vm.Response>()
-          .sublist(start, numItems != null ? start + numItems : null)
-          .mapIndexed(
+      return Future.wait(elements.cast<vm.Response>().mapIndexed(
            (index, response) => convertVmResponseToVariable(
              thread,
              response,
@ -163,9 +162,7 @@ class ProtocolConverter {
      // Both the key and value will be expandable (handled by variablesRequest
      // detecting the MapAssociation type).
      final start = startItem ?? 0;
-      return Future.wait(associations
-          .sublist(start, numItems != null ? start + numItems : null)
-          .mapIndexed((index, mapEntry) async {
+      return Future.wait(associations.mapIndexed((index, mapEntry) async {
        final key = mapEntry.key;
        final value = mapEntry.value;
        final callToString =
@ -192,6 +189,21 @@ class ProtocolConverter {
          variablesReference: thread.storeData(mapEntry),
        );
      }));
+    } else if (_isList(instance) &&
+        instance.length != null &&
+        instance.bytes != null) {
+      final elements = _decodeList(instance);
+
+      final start = startItem ?? 0;
+      return elements
+          .mapIndexed(
+            (index, element) => dap.Variable(
+              name: '[${start + index}]',
+              value: element.toString(),
+              variablesReference: 0,
+            ),
+          )
+          .toList();
    } else if (fields != null) {
      // Otherwise, show the fields from the instance.
      final variables = await Future.wait(fields.mapIndexed(
@ -257,6 +269,45 @@ class ProtocolConverter {
    }
  }

+  /// Decodes the bytes of a list from the base64 encoded string
+  /// [instance.bytes].
+  List<Object?> _decodeList(vm.Instance instance) {
+    final bytes = base64Decode(instance.bytes!);
+    switch (instance.kind) {
+      case 'Uint8ClampedList':
+        return bytes.buffer.asUint8ClampedList();
+      case 'Uint8List':
+        return bytes.buffer.asUint8List();
+      case 'Uint16List':
+        return bytes.buffer.asUint16List();
+      case 'Uint32List':
+        return bytes.buffer.asUint32List();
+      case 'Uint64List':
+        return bytes.buffer.asUint64List();
+      case 'Int8List':
+        return bytes.buffer.asInt8List();
+      case 'Int16List':
+        return bytes.buffer.asInt16List();
+      case 'Int32List':
+        return bytes.buffer.asInt32List();
+      case 'Int64List':
+        return bytes.buffer.asInt64List();
+      case 'Float32List':
+        return bytes.buffer.asFloat32List();
+      case 'Float64List':
+        return bytes.buffer.asFloat64List();
+      case 'Int32x4List':
+        return bytes.buffer.asInt32x4List();
+      case 'Float32x4List':
+        return bytes.buffer.asFloat32x4List();
+      case 'Float64x2List':
+        return bytes.buffer.asFloat64x2List();
+      default:
+        // A list type we don't know how to decode.
+        return [];
+    }
+  }
+
  /// Converts a [vm.Response] into a user-friendly display string.
  ///
  /// This may be shown in the collapsed view of a complex type.
@ -311,6 +362,7 @@ class ProtocolConverter {
          response,
          allowCallingToString: allowCallingToString,
        ),
+        indexedVariables: _isList(response) ? response.length : null,
        variablesReference: variablesReference,
      );
    } else if (response is vm.Sentinel) {
@ -336,6 +388,15 @@ class ProtocolConverter {
    }
  }

+  /// Returns whether [ref] is a List kind.
+  ///
+  /// This includes standard Dart [List], as well as lists from
+  /// `dart:typed_data` such as `Uint8List`.
+  bool _isList(vm.InstanceRef ref) => ref.kind?.endsWith('List') ?? false;
+
+  /// Returns whether [ref] is a Map kind.
+  bool _isMap(vm.InstanceRef ref) => ref.kind == 'Map';
+
  /// Converts a VM Service stack frame to a DAP stack frame.
  Future<dap.StackFrame> convertVmToDapStackFrame(
    ThreadInfo thread,
--- a/pkg/dds/test/dap/integration/debug_variables_test.dart
+++ b/pkg/dds/test/dap/integration/debug_variables_test.dart
@ -50,7 +50,7 @@ void foo() {
        stack.stackFrames[1].id, // Second frame: main
        'Locals',
        '''
-            args: List (0 items), eval: args
+            args: List (0 items), eval: args, 0 items
            myVariable: 1, eval: myVariable
        ''',
      );
@ -185,6 +185,7 @@ void main(List<String> args) {
        stop.threadId!,
        expectedName: 'myVariable',
        expectedDisplayString: 'List (3 items)',
+        expectedIndexedItems: 3,
        expectedVariables: '''
            [0]: "first", eval: myVariable[0]
            [1]: "second", eval: myVariable[1]
@ -208,6 +209,7 @@ void main(List<String> args) {
        stop.threadId!,
        expectedName: 'myVariable',
        expectedDisplayString: 'List (3 items)',
+        expectedIndexedItems: 3,
        expectedVariables: '''
            [1]: "second", eval: myVariable[1]
        ''',
@ -216,6 +218,169 @@ void main(List<String> args) {
      );
    });

+    /// Helper to verify variables types of list.
+    _checkList(
+      String typeName, {
+      required String constructor,
+      required List<String> expectedDisplayStrings,
+    }) {
+      test('renders a $typeName', () async {
+        final client = dap.client;
+        final testFile = dap.createTestFile('''
+import 'dart:typed_data';
+
+void main(List<String> args) {
+  final myVariable = $constructor;
+  print('Hello!'); $breakpointMarker
+}
+    ''');
+        final breakpointLine = lineWith(testFile, breakpointMarker);
+
+        final stop = await client.hitBreakpoint(testFile, breakpointLine);
+        await client.expectLocalVariable(
+          stop.threadId!,
+          expectedName: 'myVariable',
+          expectedDisplayString: '$typeName (3 items)',
+          expectedIndexedItems: 3,
+          expectedVariables: '''
+            [0]: ${expectedDisplayStrings[0]}
+            [1]: ${expectedDisplayStrings[1]}
+            [2]: ${expectedDisplayStrings[2]}
+        ''',
+        );
+      });
+
+      test('renders a $typeName subset', () async {
+        final client = dap.client;
+        final testFile = dap.createTestFile('''
+import 'dart:typed_data';
+
+void main(List<String> args) {
+  final myVariable = $constructor;
+  print('Hello!'); $breakpointMarker
+}
+    ''');
+        final breakpointLine = lineWith(testFile, breakpointMarker);
+
+        final stop = await client.hitBreakpoint(testFile, breakpointLine);
+        await client.expectLocalVariable(
+          stop.threadId!,
+          expectedName: 'myVariable',
+          expectedDisplayString: '$typeName (3 items)',
+          expectedIndexedItems: 3,
+          expectedVariables: '''
+            [1]: ${expectedDisplayStrings[1]}
+        ''',
+          start: 1,
+          count: 1,
+        );
+      });
+    }
+
+    _checkList(
+      'Uint8ClampedList',
+      constructor: 'Uint8ClampedList.fromList([1, 2, 3])',
+      expectedDisplayStrings: ['1', '2', '3'],
+    );
+    _checkList(
+      'Uint8List',
+      constructor: 'Uint8List.fromList([1, 2, 3])',
+      expectedDisplayStrings: ['1', '2', '3'],
+    );
+    _checkList(
+      'Uint16List',
+      constructor: 'Uint16List.fromList([1, 2, 3])',
+      expectedDisplayStrings: ['1', '2', '3'],
+    );
+    _checkList(
+      'Uint32List',
+      constructor: 'Uint32List.fromList([1, 2, 3])',
+      expectedDisplayStrings: ['1', '2', '3'],
+    );
+    _checkList(
+      'Uint64List',
+      constructor: 'Uint64List.fromList([1, 2, 3])',
+      expectedDisplayStrings: ['1', '2', '3'],
+    );
+    _checkList(
+      'Int8List',
+      constructor: 'Int8List.fromList([1, 2, 3])',
+      expectedDisplayStrings: ['1', '2', '3'],
+    );
+    _checkList(
+      'Int16List',
+      constructor: 'Int16List.fromList([1, 2, 3])',
+      expectedDisplayStrings: ['1', '2', '3'],
+    );
+    _checkList(
+      'Int32List',
+      constructor: 'Int32List.fromList([1, 2, 3])',
+      expectedDisplayStrings: ['1', '2', '3'],
+    );
+    _checkList(
+      'Int64List',
+      constructor: 'Int64List.fromList([1, 2, 3])',
+      expectedDisplayStrings: ['1', '2', '3'],
+    );
+    _checkList(
+      'Float32List',
+      constructor: 'Float32List.fromList([1.1, 2.2, 3.3])',
+      expectedDisplayStrings: [
+        // Converting the numbers above to 32bit floats loses precisions and
+        // we're just calling toString() on them.
+        '1.100000023841858',
+        '2.200000047683716',
+        '3.299999952316284',
+      ],
+    );
+    _checkList(
+      'Float64List',
+      constructor: 'Float64List.fromList([1.1, 2.2, 3.3])',
+      expectedDisplayStrings: ['1.1', '2.2', '3.3'],
+    );
+    _checkList(
+      'Int32x4List',
+      constructor: 'Int32x4List.fromList(['
+          'Int32x4(1, 1, 1, 1),'
+          'Int32x4(2, 2, 2, 2),'
+          'Int32x4(3, 3, 3, 3)'
+          '])',
+      expectedDisplayStrings: [
+        // toString()s of Int32x4
+        '[00000001, 00000001, 00000001, 00000001]',
+        '[00000002, 00000002, 00000002, 00000002]',
+        '[00000003, 00000003, 00000003, 00000003]',
+      ],
+    );
+    _checkList(
+      'Float32x4List',
+      constructor: 'Float32x4List.fromList(['
+          'Float32x4(1.1, 1.1, 1.1, 1.1),'
+          'Float32x4(2.2, 2.2, 2.2, 2.2),'
+          'Float32x4(3.3, 3.3, 3.3, 3.3)'
+          '])',
+      expectedDisplayStrings: [
+        // toString()s of Float32x4
+        '[1.100000, 1.100000, 1.100000, 1.100000]',
+        '[2.200000, 2.200000, 2.200000, 2.200000]',
+        '[3.300000, 3.300000, 3.300000, 3.300000]',
+      ],
+    );
+    _checkList(
+      'Float64x2List',
+      constructor: 'Float64x2List.fromList(['
+          'Float64x2(1.1,1.1),'
+          'Float64x2(2.2,2.2),'
+          'Float64x2(3.3,3.3)'
+          '])',
+      expectedDisplayStrings: [
+        // toString()s of Float64x2
+        '[1.100000, 1.100000]',
+        '[2.200000, 2.200000]',
+        '[3.300000, 3.300000]',
+      ],
+    );
+
    test('renders a simple map with keys/values', () async {
      final client = dap.client;
      final testFile = dap.createTestFile('''
--- a/pkg/dds/test/dap/integration/test_client.dart
+++ b/pkg/dds/test/dap/integration/test_client.dart
@ -868,6 +868,7 @@ extension DapTestClientExtension on DapTestClient {
    int threadId, {
    required String expectedName,
    required String expectedDisplayString,
+    int? expectedIndexedItems,
    required String expectedVariables,
    int? start,
    int? count,
@ -887,8 +888,9 @@ extension DapTestClientExtension on DapTestClient {
    final expectedVariable = variables.variables
        .singleWhere((variable) => variable.name == expectedName);

-    // Check the display string.
+    // Check basic variable values.
    expect(expectedVariable.value, equals(expectedDisplayString));
+    expect(expectedVariable.indexedVariables, equals(expectedIndexedItems));

    // Check the child fields.
    return expectVariables(