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 14 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
5 changes: 5 additions & 0 deletions dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
- Update SDK constraint to `>=3.0.0-134.0.dev <4.0.0`.
- Update `package:vm_service` constraint to `>=10.1.0 <12.0.0`.
- Fix expression compiler throwing when weak SDK summary is not found.
- Support records:
- 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.

**Breaking changes**
- Include an optional param to `Dwds.start` to indicate whether it is running
Expand Down
2 changes: 1 addition & 1 deletion dwds/lib/src/debugging/debugger.dart
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ class Debugger extends Domain {
{int? offset, int? count, int? length}) async {
String rangeId = objectId;
if (length != null && (offset != null || count != null)) {
final range = await _subrange(objectId, offset ?? 0, count ?? 0, length);
final range = await _subrange(objectId, offset ?? 0, count, length);
rangeId = range.objectId ?? rangeId;
}
final jsProperties = await sendCommandAndValidateResult<List>(
Expand Down
97 changes: 85 additions & 12 deletions dwds/lib/src/debugging/instance.dart
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,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 @@ -123,8 +123,9 @@ class InstanceHelper extends Domain {
return await _listInstanceFor(
classRef, remoteObject, properties, offset, count);
} else if (metaData.isSystemMap) {
return await _mapInstanceFor(
classRef, remoteObject, properties, offset, count);
return await _mapInstanceFor(classRef, remoteObject, offset, count);
} else if (metaData.isRecord) {
return await _recordInstanceFor(classRef, remoteObject, offset, count);
} else {
return await _plainInstanceFor(classRef, remoteObject, properties);
}
Expand Down Expand Up @@ -152,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 @@ -241,17 +243,13 @@ class InstanceHelper extends Domain {
}

/// Create a Map instance with class [classRef] from [remoteObject].
Future<Instance?> _mapInstanceFor(
ClassRef classRef,
RemoteObject remoteObject,
List<Property> _,
int? offset,
int? count) async {
Future<Instance?> _mapInstanceFor(ClassRef classRef,
RemoteObject remoteObject, int? offset, int? count) async {
final objectId = remoteObject.objectId;
if (objectId == null) return null;
// Maps are complicated, do an eval to get keys and values.
final associations = await _mapAssociations(remoteObject, offset, count);
final length = (offset == null && count == null)
final length = count == null
? associations.length
: (await instanceRefFor(remoteObject))?.length;
return Instance(
Expand All @@ -277,9 +275,8 @@ class InstanceHelper extends Domain {
if (objectId == null) return null;

/// TODO(annagrin): split into cases to make the logic clear.
/// TODO(annagrin): make sure we use offset correctly.
final numberOfProperties = _lengthOf(properties) ?? 0;
final length = (offset == null && count == null)
final length = count == null
? numberOfProperties
: (await instanceRefFor(remoteObject))?.length;
final indexed = properties.sublist(
Expand All @@ -297,6 +294,74 @@ class InstanceHelper extends Domain {
..count = (numberOfProperties == length) ? null : numberOfProperties;
}

/// The fields for a Dart Record.
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 positionals = sdkUtils.dloadRepl(shape, "positionals");
var named = sdkUtils.dloadRepl(shape, "named");

var keys = new Array();
for (var i = 0; i < positionals; i++) {
keys.push(i);
}
for (var key in named) {
key.push(key);
}

var values = sdkUtils.dloadRepl(this, "values");
values = sdkUtils.dsendRepl(values, "toList", []);

return {
keys: keys,
values: values
};
}
''';
final keysAndValues = await inspector.jsCallFunctionOn(map, expression, []);
final keys = await inspector.loadField(keysAndValues, 'keys');
final values = await inspector.loadField(keysAndValues, 'values');
final keysInstance = await instanceFor(keys, offset: offset, count: count);
final valuesInstance =
await instanceFor(values, offset: offset, count: count);
final fields = <BoundField>[];
final keyElements = keysInstance?.elements;
final valueElements = valuesInstance?.elements;
if (keyElements != null && valueElements != null) {
Map.fromIterables(keyElements, valueElements).forEach((key, value) {
fields.add(BoundField(name: key.valueAsString, value: value));
});
}
return fields;
}

/// Create a Map instance with class [classRef] from [remoteObject].
Future<Instance?> _recordInstanceFor(ClassRef classRef,
RemoteObject remoteObject, int? offset, int? count) 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, count);
final length = (count == null)
? fields.length
: (await instanceRefFor(remoteObject))?.length;
return Instance(
identityHashCode: remoteObject.objectId.hashCode,
kind: InstanceKind.kRecord,
id: objectId,
classRef: classRef)
..length = length
..offset = offset
..count = (fields.length == length) ? null : fields.length
..fields = fields;
}

/// Return the value of the length attribute from [properties], if present.
///
/// This is only applicable to Lists or Maps, where we expect a length
Expand Down Expand Up @@ -424,6 +489,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
92 changes: 75 additions & 17 deletions dwds/lib/src/debugging/metadata/class.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,37 @@ import '../../debugging/remote_debugger.dart';
import '../../loaders/strategy.dart';
import '../../services/chrome_debug_exception.dart';

const _dartCoreLibrary = 'dart:core';

/// A hard-coded ClassRef for the Closure class.
final classRefForClosure = classRefFor('dart:core', 'Closure');
final classRefForClosure = classRefFor(_dartCoreLibrary, 'Closure');

/// A hard-coded ClassRef for the String class.
final classRefForString = classRefFor('dart:core', InstanceKind.kString);
final classRefForString = classRefFor(_dartCoreLibrary, InstanceKind.kString);

/// A hard-coded ClassRef for a (non-existent) class called Unknown.
final classRefForUnknown = classRefFor('dart:core', 'Unknown');
final classRefForUnknown = classRefFor(_dartCoreLibrary, 'Unknown');

/// A hard-coded LibraryRef for a a dart:core library.
final libraryRefForCore = LibraryRef(
id: _dartCoreLibrary,
name: _dartCoreLibrary,
uri: _dartCoreLibrary,
);

/// Returns a [LibraryRef] for the provided library ID and class name.
LibraryRef libraryRefFor(String libraryId) => LibraryRef(
id: libraryId,
name: libraryId,
uri: libraryId,
);

/// Returns a [ClassRef] for the provided library ID and class name.
ClassRef classRefFor(String? libraryId, String? name) => ClassRef(
id: 'classes|$libraryId|$name',
name: name,
library: libraryId == null
? null
: LibraryRef(id: libraryId, name: libraryId, uri: libraryId));
ClassRef classRefFor(String libraryId, String? name) => ClassRef(
id: 'classes|$libraryId|$name',
name: name,
library: libraryRefFor(libraryId),
);

/// Meta data for a remote Dart class in Chrome.
class ClassMetaData {
Expand All @@ -44,15 +59,34 @@ class ClassMetaData {
final String? dartName;

/// The library identifier, which is the URI of the library.
final String? libraryId;

factory ClassMetaData(
{Object? jsName, Object? libraryId, Object? dartName, Object? length}) {
return ClassMetaData._(jsName as String?, libraryId as String?,
dartName as String?, int.tryParse('$length'));
final String libraryId;

factory ClassMetaData({
Object? jsName,
Object? libraryId,
Object? dartName,
Object? length,
bool isFunction = false,
bool isRecord = false,
}) {
return ClassMetaData._(
jsName as String?,
libraryId as String? ?? _dartCoreLibrary,
dartName as String?,
int.tryParse('$length'),
isFunction,
isRecord,
);
}

ClassMetaData._(this.jsName, this.libraryId, this.dartName, this.length);
ClassMetaData._(
this.jsName,
this.libraryId,
this.dartName,
this.length,
this.isFunction,
this.isRecord,
);

/// Returns the ID of the class.
///
Expand All @@ -70,14 +104,30 @@ class ClassMetaData {
const sdkUtils = ${globalLoadStrategy.loadModuleSnippet}('dart_sdk').dart;
const classObject = sdkUtils.getReifiedType(arg);
const isFunction = sdkUtils.AbstractFunctionType.is(classObject);
const isRecord = sdkUtils.RecordType.is(classObject);
const result = {};
result['name'] = isFunction ? 'Function' : classObject.name;
var name = isFunction ? 'Function' : classObject.name;

result['name'] = name;
result['libraryId'] = sdkUtils.getLibraryUri(classObject);
result['dartName'] = sdkUtils.typeName(classObject);
result['isFunction'] = isFunction;
result['isRecord'] = isRecord;
result['length'] = arg['length'];

if (isRecord) {
result['name'] = 'Record';
var positionals = classObject.shape.positionals;
var named = classObject.shape.named == null
? 0
: classObject.shape.named.length;
result['length'] = positionals + named;
}

return result;
}
''';

final result = await inspector.jsCallFunctionOn(
remoteObject, evalExpression, [remoteObject],
returnByValue: true);
Expand All @@ -86,6 +136,8 @@ class ClassMetaData {
jsName: metadata['name'],
libraryId: metadata['libraryId'],
dartName: metadata['dartName'],
isFunction: metadata['isFunction'],
isRecord: metadata['isRecord'],
length: metadata['length'],
);
} on ChromeDebugException {
Expand All @@ -106,4 +158,10 @@ class ClassMetaData {

/// True if this class refers to system Lists, which are treated specially.
bool get isSystemList => jsName == 'JSArray';

/// True if this class refers to a function type.
bool isFunction;

/// True if this class refers to a record type.
bool isRecord;
}
4 changes: 2 additions & 2 deletions dwds/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ dependencies:
built_value: ^8.3.0
collection: ^1.15.0
crypto: ^3.0.2
dds: ^2.2.5
dds: ^2.7.1
file: ^6.1.3
http: ^0.13.4
http_multi_server: ^3.2.0
Expand All @@ -34,7 +34,7 @@ dependencies:
stack_trace: ^1.10.0
sse: ^4.1.2
uuid: ^3.0.6
vm_service: ">=10.1.0 <12.0.0"
vm_service: ">=10.1.2 <12.0.0"
web_socket_channel: ^2.2.0
webkit_inspection_protocol: ^1.0.1

Expand Down
Loading