Skip to content

Support records #1919

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 32 commits into from
Feb 11, 2023
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
cb7251b
Display records and add tests
Jan 26, 2023
cbe8d96
Add _experiment test fixture
Jan 26, 2023
0e485cf
Update instanceRef kind to kRecord
Jan 26, 2023
c11b71d
Make offsets work
Jan 27, 2023
13a34ba
Merge branch 'master' of github.com:dart-lang/webdev into annagrin/re…
Jan 30, 2023
71c1fe8
Format
Jan 30, 2023
9c34b75
Require min versions of dds and vm_service that support records
Jan 30, 2023
96824c6
Cleanup and add more records tests
Jan 31, 2023
8fd379a
Cleanup names
Jan 31, 2023
56e23f9
Fix lists and maps getObject tests
Jan 31, 2023
5039cab
Fix records test failures
Jan 31, 2023
0adb558
Temporarily disabled failing record type tests
Feb 1, 2023
a0826c9
cleanup comments and formatting
Feb 1, 2023
aa36615
Merge branch 'master' of github.com:dart-lang/webdev into annagrin/re…
Feb 1, 2023
98119ca
Enable rectord type tests on main
Feb 1, 2023
31e23e0
Merge branch 'master' of github.com:dart-lang/webdev into annagrin/re…
Feb 1, 2023
733c7c6
Addressed CR comments, added tests for named parameters, fixed bugs
Feb 2, 2023
491ab13
Merge with master
Feb 7, 2023
0e280ae
Fix list count bug
Feb 7, 2023
9c4e8bf
Merge branch 'master' of github.com:dart-lang/webdev into annagrin/re…
Feb 9, 2023
2cff45a
Update min SDK constrain to support record types and enable skipped t…
Feb 9, 2023
19d64bc
Merge with master
Feb 9, 2023
b5202c7
Fix merge errors
Feb 9, 2023
dd31083
Fix failure on getObject of list with out of range offset
Feb 9, 2023
0b89601
Cleanup
Feb 10, 2023
689d6ba
Use JS logic to check for dart types
Feb 10, 2023
569321a
Addressed CR comments
Feb 10, 2023
bf6e585
Use integers as BoundField names for records
Feb 10, 2023
ded1222
Create positional fields faster, add comments
Feb 10, 2023
614d789
Cleaned up comments
Feb 10, 2023
a8c4700
Addressed comments
Feb 10, 2023
0a18c18
Address CR comments
Feb 10, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
- Cleanup `getObject` code for lists and maps.
- Now works with offset `0` and `null` count.
- Fix failures on edge cases.
- Support records:
- Update SDK constraint to `>=3.0.0-188.0.dev <4.0.0`.
- Update `package:vm_service` constraint to `>=10.1.2 <12.0.0`.
- Update `package:dds` constraint to `^2.7.1`.
- Fill `BoundField.name` for records.
- Display records as a container of fields.

## 17.0.0

Expand Down
2 changes: 1 addition & 1 deletion dwds/debug_extension/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ description: >-
A chrome extension for Dart debugging.

environment:
sdk: ">=3.0.0-134.0.dev <4.0.0"
sdk: ">=3.0.0-188.0.dev <4.0.0"

dependencies:
async: ^2.3.0
Expand Down
2 changes: 1 addition & 1 deletion dwds/debug_extension_mv3/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ description: >-
A Chrome extension for Dart debugging.

environment:
sdk: ">=3.0.0-134.0.dev <4.0.0"
sdk: ">=3.0.0-188.0.dev <4.0.0"

dependencies:
built_value: ^8.3.0
Expand Down
175 changes: 164 additions & 11 deletions dwds/lib/src/debugging/instance.dart
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ class InstanceHelper extends Domain {

final classRef = metaData?.classRef;
if (metaData == null || classRef == null) return null;
if (metaData.jsName == 'Function') {
if (metaData.isFunction) {
return _closureInstanceFor(remoteObject);
}

Expand All @@ -122,6 +122,9 @@ class InstanceHelper extends Domain {
} else if (metaData.isSystemMap) {
return await _mapInstanceFor(classRef, remoteObject,
offset: offset, count: count, length: metaData.length);
} else if (metaData.isRecord) {
return await _recordInstanceFor(classRef, remoteObject,
offset: offset, count: count, length: metaData.length);
} else {
return await _plainInstanceFor(classRef, remoteObject,
offset: offset, count: count, length: metaData.length);
Expand Down Expand Up @@ -150,6 +153,7 @@ class InstanceHelper extends Domain {
Future<BoundField> _fieldFor(Property property, ClassRef classRef) async {
final instance = await _instanceRefForRemote(property.value);
return BoundField(
name: property.name,
decl: FieldRef(
// TODO(grouma) - Convert JS name to Dart.
name: property.name,
Expand Down Expand Up @@ -207,6 +211,12 @@ class InstanceHelper extends Domain {
}

/// The associations for a Dart Map or IdentityMap.
///
/// Returns a range of [count] associations, if available, starting from
/// the [offset].
///
/// If [offset] is `null`, assumes 0 offset.
/// If [count] is `null`, returns all available fields.
Future<List<MapAssociation>> _mapAssociations(RemoteObject map,
{int? offset, int? count}) async {
// We do this in in awkward way because we want the keys and values, but we
Expand Down Expand Up @@ -247,6 +257,14 @@ class InstanceHelper extends Domain {
}

/// Create a Map instance with class [classRef] from [remoteObject].
///
/// Returns an instance containing [count] associations, if available,
/// starting from the [offset].
///
/// If [offset] is `null`, assumes 0 offset.
/// If [count] is `null`, returns all available fields.
/// [length] is the expected length of the whole object, read from
/// the [ClassMetaData].
Future<Instance?> _mapInstanceFor(
ClassRef classRef,
RemoteObject remoteObject, {
Expand All @@ -273,8 +291,15 @@ class InstanceHelper extends Domain {
..associations = associations;
}

/// Create a List instance of [classRef] from [remoteObject] with the JS
/// properties [properties].
/// Create a List instance of [classRef] from [remoteObject].
///
/// Returns an instance containing [count] elements, if available,
/// starting from the [offset].
///
/// If [offset] is `null`, assumes 0 offset.
/// If [count] is `null`, returns all available fields.
/// [length] is the expected length of the whole object, read from
/// the [ClassMetaData].
Future<Instance?> _listInstanceFor(
ClassRef classRef,
RemoteObject remoteObject, {
Expand All @@ -301,6 +326,14 @@ class InstanceHelper extends Domain {
}

/// The elements for a Dart List.
///
/// Returns a range of [count] elements, if available, starting from
/// the [offset].
///
/// If [offset] is `null`, assumes 0 offset.
/// If [count] is `null`, returns all available fields.
/// [length] is the expected length of the whole object, read from
/// the [ClassMetaData].
Future<List<InstanceRef?>> _listElements(
RemoteObject list, {
int? offset,
Expand All @@ -322,6 +355,126 @@ class InstanceHelper extends Domain {
.map((element) async => await _instanceRefForRemote(element.value)));
}

/// Return the value of the length attribute from [properties], if present.
/// Return elements of the list from [properties].
///
/// Ignore any non-elements like 'length', 'fixed$length', etc.
static List<Property> _indexedListProperties(List<Property> properties) =>
properties
.where((p) => p.name != null && int.tryParse(p.name!) != null)
.toList();

/// The fields for a Dart Record.
///
/// Returns a range of [count] fields, if available, starting from
/// the [offset].
///
/// If [offset] is `null`, assumes 0 offset.
/// If [count] is `null`, returns all available fields.
Future<List<BoundField>> _recordFields(RemoteObject map,
{int? offset, int? count}) async {
// We do this in in awkward way because we want the keys and values, but we
// can't return things by value or some Dart objects will come back as
// values that we need to be RemoteObject, e.g. a List of int.
final expression = '''
function() {
var sdkUtils = ${globalLoadStrategy.loadModuleSnippet}('dart_sdk').dart;
var shape = sdkUtils.dloadRepl(this, "shape");
var positionalCount = sdkUtils.dloadRepl(shape, "positionals");
var named = sdkUtils.dloadRepl(shape, "named");
named = named == null? null: sdkUtils.dsendRepl(named, "toList", []);
var values = sdkUtils.dloadRepl(this, "values");
values = sdkUtils.dsendRepl(values, "toList", []);

return {
positionalCount: positionalCount,
named: named,
values: values
};
}
''';
final result = await inspector.jsCallFunctionOn(map, expression, []);
final positionalCount =
(await inspector.loadField(result, 'positionalCount'))!.value as int;
final named = await inspector.loadField(result, 'named');
final values = await inspector.loadField(result, 'values');

final valuesInstance =
await instanceFor(values, offset: offset, count: count);

int? remaining(int? needed, int collected) {
if (needed == null) return null;
return needed < collected ? 0 : needed - collected;
}

// Create requested positional fields.
final positionalOffset = offset ?? 0;
final positionalAvailable = remaining(positionalCount, positionalOffset)!;
final positionalRangeCount =
min(positionalAvailable, count ?? positionalAvailable);
final positionalElements = [
for (var i = positionalOffset + 1;
i <= positionalOffset + positionalRangeCount;
i++)
i
];

// Account for already collected positional fields when calculating
// the offset and count of fields to be collected from named fields.
final namedRangeOffset = remaining(offset, positionalCount);
final namedRangeCount = remaining(count, positionalRangeCount);
final namedInstance = await instanceFor(named,
offset: namedRangeOffset, count: namedRangeCount);
final namedElements =
namedInstance?.elements?.map((e) => e.valueAsString) ?? [];

final fieldNameElements = [
...positionalElements,
...namedElements,
];

final fields = <BoundField>[];
final valueElements = valuesInstance?.elements ?? [];
Map.fromIterables(fieldNameElements, valueElements).forEach((key, value) {
fields.add(BoundField(name: key, value: value));
});
return fields;
}

/// Create a Record instance with class [classRef] from [remoteObject].
///
/// Returns an instance containing [count] fields, if available,
/// starting from the [offset].
///
/// If [offset] is `null`, assumes 0 offset.
/// If [count] is `null`, returns all available fields.
/// [length] is the expected length of the whole object, read from
/// the [ClassMetaData].
Future<Instance?> _recordInstanceFor(
ClassRef classRef,
RemoteObject remoteObject, {
int? offset,
int? count,
int? length,
}) async {
final objectId = remoteObject.objectId;
if (objectId == null) return null;
// Records are complicated, do an eval to get names and values.
final fields =
await _recordFields(remoteObject, offset: offset, count: count);
final rangeCount = _calculateRangeCount(
count: count, elementCount: fields.length, length: length);
return Instance(
identityHashCode: remoteObject.objectId.hashCode,
kind: InstanceKind.kRecord,
id: objectId,
classRef: classRef)
..length = length
..offset = offset
..count = rangeCount
..fields = fields;
}

/// Return the available count of elements in the requested range.
/// Return `null` if the range includes the whole object.
/// [count] is the range length requested by the `getObject` call.
Expand All @@ -336,14 +489,6 @@ class InstanceHelper extends Domain {
return min(count, elementCount);
}

/// Return elements of the list from [properties].
///
/// Ignore any non-elements like 'length', 'fixed$length', etc.
static List<Property> _indexedListProperties(List<Property> properties) =>
properties
.where((p) => p.name != null && int.tryParse(p.name!) != null)
.toList();

/// Filter [allJsProperties] and return a list containing only those
/// that correspond to Dart fields on [remoteObject].
///
Expand Down Expand Up @@ -461,6 +606,14 @@ class InstanceHelper extends Domain {
classRef: metaData.classRef)
..length = metaData.length;
}
if (metaData.isRecord) {
return InstanceRef(
kind: InstanceKind.kRecord,
id: objectId,
identityHashCode: remoteObject.objectId.hashCode,
classRef: metaData.classRef)
..length = metaData.length;
}
return InstanceRef(
kind: InstanceKind.kPlainInstance,
id: objectId,
Expand Down
Loading