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 2 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
3 changes: 2 additions & 1 deletion dwds/lib/src/debugging/classes.dart
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ class ClassHelper extends Domain {

if (libraryUri == null || classId == null || className == null) return null;

final rawName = className.split('<').first;
var rawName = className.split('<').first;
if (rawName.startsWith('RecordType(')) rawName = 'Record';
final expression = '''
(function() {
${globalLoadStrategy.loadLibrarySnippet(libraryUri)}
Expand Down
77 changes: 77 additions & 0 deletions dwds/lib/src/debugging/instance.dart
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ class InstanceHelper extends Domain {
} else if (metaData.isSystemMap) {
return await _mapInstanceFor(
classRef, remoteObject, properties, offset, count);
} else if (metaData.isRecord) {
return await _recordInstanceFor(
classRef, remoteObject, properties, offset, count);
} else {
return await _plainInstanceFor(classRef, remoteObject, properties);
}
Expand Down Expand Up @@ -152,6 +155,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 @@ -297,6 +301,79 @@ class InstanceHelper extends Domain {
..count = (numberOfProperties == length) ? null : numberOfProperties;
}

/// The associations for a Dart Map or IdentityMap.
Future<List<MapAssociation>> _recordAssociations(
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 names = new Array();
var named = sdkUtils.dloadRepl(shape, "named");
for (var i = 0; i < positionals; i++) {
names.push(i);
}
for (var name in named) {
names.push(name);
}

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

return {
names: names,
values: values
};
}
''';
final keysAndValues = await inspector.jsCallFunctionOn(map, expression, []);
final keys = await inspector.loadField(keysAndValues, 'names');
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 associations = <MapAssociation>[];
final keyElements = keysInstance?.elements;
final valueElements = valuesInstance?.elements;
if (keyElements != null && valueElements != null) {
Map.fromIterables(keyElements, valueElements).forEach((key, value) {
associations.add(MapAssociation(key: key, value: value));
});
}
return associations;
}

/// Create a Map instance with class [classRef] from [remoteObject].
Future<Instance?> _recordInstanceFor(
ClassRef classRef,
RemoteObject remoteObject,
List<Property> _,
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 _recordAssociations(remoteObject, offset, count);
final length = (offset == null && count == null)
? associations.length
: (await instanceRefFor(remoteObject))?.length;
return Instance(
identityHashCode: remoteObject.objectId.hashCode,
kind: InstanceKind.kRecord,
id: objectId,
classRef: classRef)
..length = length
..offset = offset
..count = (associations.length == length) ? null : associations.length
..fields = associations
.map((e) => BoundField(name: e.key.valueAsString, value: e.value))
.toList();
}

/// 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
45 changes: 33 additions & 12 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,12 +59,15 @@ class ClassMetaData {
final String? dartName;

/// The library identifier, which is the URI of the library.
final String? libraryId;
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'));
return ClassMetaData._(
jsName as String?,
libraryId as String? ?? _dartCoreLibrary,
dartName as String?,
int.tryParse('$length'));
}

ClassMetaData._(this.jsName, this.libraryId, this.dartName, this.length);
Expand Down Expand Up @@ -106,4 +124,7 @@ 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 record type.
bool get isRecord => jsName?.startsWith('RecordType(') ?? false;
}
8 changes: 7 additions & 1 deletion dwds/test/fixtures/context.dart
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,8 @@ class TestContext {
required this.nullSafety,
}) {
// Verify that the test fixtures package matches the null-safety mode:
final isSoundPackage = packageName.toLowerCase().contains('sound');
final isSoundPackage = packageName.toLowerCase().contains('sound') ||
packageName.toLowerCase().contains('experiment');
assert(nullSafety == NullSafety.sound ? isSoundPackage : !isSoundPackage);
// Verify that the web assets path has no starting slash:
assert(!webAssetsPath.startsWith('/'));
Expand Down Expand Up @@ -199,6 +200,7 @@ class TestContext {
bool launchChrome = true,
bool isFlutterApp = false,
bool isInternalBuild = false,
List<String> experiments = const <String>[],
}) async {
// Generate missing SDK assets if needed.
final sdkConfigurationProvider =
Expand Down Expand Up @@ -265,6 +267,8 @@ class TestContext {
'build_web_compilers|ddc=generate-full-dill=true',
],
'--verbose',
for (var experiment in experiments)
'--enable-experiment=$experiment',
];
_daemonClient =
await connectClient(workingDirectory, options, (log) {
Expand Down Expand Up @@ -293,6 +297,7 @@ class TestContext {
port,
verbose: verboseCompiler,
sdkConfigurationProvider: sdkConfigurationProvider,
experiments: experiments,
);
expressionCompiler = ddcService;
}
Expand Down Expand Up @@ -328,6 +333,7 @@ class TestContext {
'org-dartlang-app',
outputDir.path,
nullSafety == NullSafety.sound,
experiments,
verboseCompiler,
);

Expand Down
136 changes: 136 additions & 0 deletions dwds/test/records_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

@TestOn('vm')
@Timeout(Duration(minutes: 2))

import 'package:collection/collection.dart';
import 'package:dwds/src/connections/debug_connection.dart';
import 'package:dwds/src/services/chrome_proxy_service.dart';
import 'package:test/test.dart';
import 'package:vm_service/vm_service.dart';

import 'fixtures/context.dart';
import 'fixtures/logging.dart';

class TestSetup {
TestContext context;

TestSetup.sound()
: context = TestContext.withSoundNullSafety(
packageName: '_experiment',
webAssetsPath: 'web',
dartEntryFileName: 'main.dart',
htmlEntryFileName: 'index.html',
);

ChromeProxyService get service =>
fetchChromeProxyService(context.debugConnection);
}

void main() async {
// Enable verbose logging for debugging.
final debug = true;

final setup = TestSetup.sound();
final context = setup.context;

Future<void> onBreakPoint(String isolate, ScriptRef script,
String breakPointId, Future<void> Function() body) async {
Breakpoint? bp;
try {
final line =
await context.findBreakpointLine(breakPointId, isolate, script);
bp = await setup.service
.addBreakpointWithScriptUri(isolate, script.uri!, line);
await body();
} finally {
// Remove breakpoint so it doesn't impact other tests or retries.
if (bp != null) {
await setup.service.removeBreakpoint(isolate, bp.id!);
}
}
}

for (var compilationMode in [CompilationMode.frontendServer]) {
group('$compilationMode |', () {
setUpAll(() async {
setCurrentLogWriter(debug: debug);
});

group('shared context with evaluation |', () {
setUpAll(() async {
setCurrentLogWriter(debug: debug);
await context.setUp(
compilationMode: compilationMode,
enableExpressionEvaluation: true,
verboseCompiler: debug,
experiments: ['records'],
);
});

tearDownAll(() async {
await context.tearDown();
});

setUp(() => setCurrentLogWriter(debug: debug));

group('evaluateInFrame |', () {
late String isolateId;
late ScriptRef mainScript;
late Stream<Event> stream;

setUp(() async {
setCurrentLogWriter(debug: debug);
final vm = await setup.service.getVM();
isolateId = vm.isolates!.first.id!;
final scripts = await setup.service.getScripts(isolateId);

await setup.service.streamListen('Debug');
stream = setup.service.onEvent('Debug');

mainScript = scripts.scripts!
.firstWhere((each) => each.uri!.contains('main.dart'));
});

tearDown(() async {
await setup.service.resume(isolateId);
});

test('can evaluate and display records', () async {
await onBreakPoint(isolateId, mainScript, 'printLocal', () async {
final event = await stream.firstWhere(
(event) => event.kind == EventKind.kPauseBreakpoint);

final result = await setup.service
.evaluateInFrame(isolateId, event.topFrame!.index!, 'record');

// Actual: InstanceRef:<[InstanceRef id: 5614429716467540672.1.54, kind: PlainInstance, identityHashCode: 912649884, classRef: [ClassRef id: classes|null|RecordType(bool, int), name: RecordType(bool, int), library: null]]>
expect(result, isA<InstanceRef>());

final instanceRef = result as InstanceRef;
final instance = await setup.service
.getObject(isolateId, instanceRef.id!) as Instance;

expect(instance.kind, InstanceKind.kRecord);

final classRef = instance.classRef!;
expect(classRef, isNotNull);
expect(classRef.name, 'RecordType(bool, int)');

final fieldNames = instance.fields!
.map((boundField) => boundField.name)
.whereNotNull()
.toList();
expect(fieldNames, [
r'$0',
r'$1',
]);
});
});
});
});
});
}
}
20 changes: 20 additions & 0 deletions fixtures/_experiment/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# This code is for testing that the debugger works with weak null-safety
# so we should not migrate it to null-safety.
# TODO(elliette): Delete this directory post Dart 3.0 (when we no longer
# support weak null-safety).
name: _experiment
version: 1.0.0
description: >-
A fake package used for testing experimental language features.
publish_to: none

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

dependencies:
intl: ^0.17.0
path: ^1.8.2

dev_dependencies:
build_runner: ^2.4.0
build_web_compilers: ^4.0.0
7 changes: 7 additions & 0 deletions fixtures/_experiment/web/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<html>

<head>
<script defer src="main.dart.js"></script>
</head>

</html>
Loading