Skip to content

Commit 54f1ee4

Browse files
jensjohaCommit Queue
jensjoha
authored and
Commit Queue
committed
[VM/CFE] Fix expression compilation with record types
Fixes #56859 TEST=pkg/vm_service/test/evaluate_with_record_{1,2}_test.dart + CFE test Change-Id: I4fdd2bc8e6c04fc59aaad28a192fe0b1833f000d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/388841 Commit-Queue: Jens Johansen <[email protected]> Reviewed-by: Ben Konyi <[email protected]> Reviewed-by: Johnni Winther <[email protected]>
1 parent 21a4425 commit 54f1ee4

File tree

7 files changed

+295
-44
lines changed

7 files changed

+295
-44
lines changed

‎pkg/front_end/lib/src/api_prototype/expression_compilation_tools.dart‎

Lines changed: 113 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ import 'package:kernel/ast.dart'
99
DynamicType,
1010
InterfaceType,
1111
Library,
12+
NamedType,
1213
Nullability,
14+
RecordType,
1315
TypeParameter;
1416
import 'package:kernel/library_index.dart' show LibraryIndex;
1517

@@ -91,8 +93,8 @@ List<ParsedType> parseDefinitionTypes(List<String> definitionTypes) {
9193
int i = 0;
9294
List<ParsedType> argumentReceivers = [];
9395
while (i < definitionTypes.length) {
94-
String uriOrNullString = definitionTypes[i];
95-
if (uriOrNullString == "null") {
96+
String uriOrSpecialString = definitionTypes[i];
97+
if (uriOrSpecialString == "null") {
9698
if (argumentReceivers.isEmpty) {
9799
result.add(new ParsedType.nullType());
98100
} else {
@@ -103,14 +105,39 @@ List<ParsedType> parseDefinitionTypes(List<String> definitionTypes) {
103105
}
104106
i++;
105107
continue;
108+
} else if (uriOrSpecialString == "record") {
109+
// Record.
110+
// We expect at least 4 elements: "record", nullability, num fields,
111+
// num positional fields.
112+
if (i + 4 > definitionTypes.length) throw "invalid input";
113+
int nullability = int.parse(definitionTypes[i + 1]);
114+
int numFields = int.parse(definitionTypes[i + 2]);
115+
int numPositionalFields = int.parse(definitionTypes[i + 3]);
116+
i += 4;
117+
118+
List<String?> fieldNames = new List<String?>.filled(numFields, null);
119+
for (int j = numPositionalFields; j < numFields; j++) {
120+
fieldNames[j] = definitionTypes[i++];
121+
}
122+
123+
ParsedType type = new ParsedType.record(nullability, fieldNames);
124+
if (argumentReceivers.isEmpty) {
125+
result.add(type);
126+
} else {
127+
argumentReceivers.removeLast().arguments!.add(type);
128+
}
129+
for (int j = 0; j < numFields; j++) {
130+
argumentReceivers.add(type);
131+
}
106132
} else {
107133
// We expect at least 4 elements: Uri, class name, nullability,
108134
// number of type arguments.
109135
if (i + 4 > definitionTypes.length) throw "invalid input";
110136
String className = definitionTypes[i + 1];
111137
int nullability = int.parse(definitionTypes[i + 2]);
112138
int typeArgumentsCount = int.parse(definitionTypes[i + 3]);
113-
ParsedType type = new ParsedType(uriOrNullString, className, nullability);
139+
ParsedType type =
140+
new ParsedType.interface(uriOrSpecialString, className, nullability);
114141
if (argumentReceivers.isEmpty) {
115142
result.add(type);
116143
} else {
@@ -128,26 +155,44 @@ List<ParsedType> parseDefinitionTypes(List<String> definitionTypes) {
128155
return result;
129156
}
130157

158+
enum ParsedTypeKind {
159+
Null,
160+
Interface,
161+
Record,
162+
}
163+
131164
// Coverage-ignore(suite): Not run.
132165
class ParsedType {
166+
final ParsedTypeKind type;
133167
final String? uri;
134168
final String? className;
135169
final int? nullability;
136170
final List<ParsedType>? arguments;
171+
final List<String?>? recordFieldNames;
137172

138-
bool get isNullType => uri == null;
173+
ParsedType.interface(this.uri, this.className, this.nullability)
174+
: type = ParsedTypeKind.Interface,
175+
arguments = [],
176+
recordFieldNames = null;
139177

140-
ParsedType(this.uri, this.className, this.nullability) : arguments = [];
178+
ParsedType.record(this.nullability, this.recordFieldNames)
179+
: type = ParsedTypeKind.Record,
180+
uri = null,
181+
className = null,
182+
arguments = [];
141183

142184
ParsedType.nullType()
143-
: uri = null,
185+
: type = ParsedTypeKind.Null,
186+
uri = null,
144187
className = null,
145188
nullability = null,
146-
arguments = null;
189+
arguments = null,
190+
recordFieldNames = null;
147191

148192
@override
149193
bool operator ==(Object other) {
150194
if (other is! ParsedType) return false;
195+
if (type != other.type) return false;
151196
if (uri != other.uri) return false;
152197
if (className != other.className) return false;
153198
if (nullability != other.nullability) return false;
@@ -157,42 +202,81 @@ class ParsedType {
157202
if (arguments![i] != other.arguments![i]) return false;
158203
}
159204
}
205+
if (recordFieldNames?.length != other.recordFieldNames?.length) {
206+
return false;
207+
}
208+
if (recordFieldNames != null) {
209+
for (int i = 0; i < recordFieldNames!.length; i++) {
210+
if (recordFieldNames![i] != other.recordFieldNames![i]) return false;
211+
}
212+
}
160213
return true;
161214
}
162215

163216
@override
164217
int get hashCode {
165-
if (isNullType) return 0;
218+
if (type == ParsedTypeKind.Null) return 0;
166219
int hash = 0x3fffffff & uri.hashCode;
167220
hash = 0x3fffffff & (hash * 31 + (hash ^ className.hashCode));
168221
hash = 0x3fffffff & (hash * 31 + (hash ^ nullability.hashCode));
169222
for (ParsedType argument in arguments!) {
170223
hash = 0x3fffffff & (hash * 31 + (hash ^ argument.hashCode));
171224
}
225+
if (recordFieldNames != null) {
226+
for (String? name in recordFieldNames!) {
227+
hash = 0x3fffffff & (hash * 31 + (hash ^ name.hashCode));
228+
}
229+
}
172230
return hash;
173231
}
174232

175233
@override
176234
String toString() {
177-
if (isNullType) return "null-type";
178-
return "$uri[$className] ($nullability) <$arguments>";
235+
switch (type) {
236+
case ParsedTypeKind.Null:
237+
return "null-type";
238+
case ParsedTypeKind.Interface:
239+
return "Record[$recordFieldNames] ($nullability) ($arguments)";
240+
case ParsedTypeKind.Record:
241+
if (arguments?.isEmpty ?? true) {
242+
return "$uri[$className] ($nullability)";
243+
}
244+
return "$uri[$className] ($nullability) <$arguments>";
245+
}
179246
}
180247

181248
DartType createDartType(LibraryIndex libraryIndex) {
182-
if (isNullType) return new DynamicType();
183-
Class? classNode = libraryIndex.tryGetClass(uri!, className!);
184-
if (classNode == null) return new DynamicType();
185-
186-
return new InterfaceType(
187-
classNode,
188-
_getDartNullability(),
189-
arguments
190-
?.map((e) => e.createDartType(libraryIndex))
191-
.toList(growable: false));
249+
switch (type) {
250+
case ParsedTypeKind.Null:
251+
return new DynamicType();
252+
case ParsedTypeKind.Record:
253+
List<DartType> positional = [];
254+
List<NamedType> named = [];
255+
for (int i = 0; i < arguments!.length; i++) {
256+
String? name = recordFieldNames![i];
257+
DartType type = arguments![i].createDartType(libraryIndex);
258+
if (name == null) {
259+
positional.add(type);
260+
} else {
261+
named.add(new NamedType(name, type));
262+
}
263+
}
264+
return new RecordType(positional, named, _getDartNullability());
265+
case ParsedTypeKind.Interface:
266+
Class? classNode = libraryIndex.tryGetClass(uri!, className!);
267+
if (classNode == null) return new DynamicType();
268+
269+
return new InterfaceType(
270+
classNode,
271+
_getDartNullability(),
272+
arguments
273+
?.map((e) => e.createDartType(libraryIndex))
274+
.toList(growable: false));
275+
}
192276
}
193277

194278
Nullability _getDartNullability() {
195-
if (isNullType) throw "No nullability on the null type";
279+
if (type == ParsedTypeKind.Null) throw "No nullability on the null type";
196280
if (nullability == 0) return Nullability.nullable;
197281
if (nullability == 1) return Nullability.nonNullable;
198282
if (nullability == 2) return Nullability.legacy;
@@ -206,9 +290,14 @@ Set<String> collectParsedTypeUris(List<ParsedType> parsedTypes) {
206290
List<ParsedType> workList = new List.from(parsedTypes);
207291
while (workList.isNotEmpty) {
208292
ParsedType type = workList.removeLast();
209-
if (type.isNullType) continue;
210-
result.add(type.uri!);
211-
workList.addAll(type.arguments!);
293+
if (type.arguments != null) workList.addAll(type.arguments!);
294+
switch (type.type) {
295+
case ParsedTypeKind.Null:
296+
case ParsedTypeKind.Record:
297+
continue;
298+
case ParsedTypeKind.Interface:
299+
result.add(type.uri!);
300+
}
212301
}
213302
return result;
214303
}

‎pkg/front_end/test/expression_compilation_tools_test.dart‎

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,12 @@ void main() {
1717
"1",
1818
"0",
1919
]),
20-
[new ParsedType("dart:core", "_OneByteString", 1)]);
20+
[new ParsedType.interface("dart:core", "_OneByteString", 1)]);
2121

2222
// List<something it can't represent which thus becomes an explicit
2323
// dynamic/null>, kNonNullable.
2424
expect(parseDefinitionTypes(["dart:core", "List", "1", "1", "null"]), [
25-
new ParsedType("dart:core", "List", 1)
25+
new ParsedType.interface("dart:core", "List", 1)
2626
..arguments!.add(new ParsedType.nullType())
2727
]);
2828

@@ -39,8 +39,8 @@ void main() {
3939
"0",
4040
]),
4141
[
42-
new ParsedType("dart:core", "_GrowableList", 1)
43-
..arguments!.add(new ParsedType("dart:core", "int", 1))
42+
new ParsedType.interface("dart:core", "_GrowableList", 1)
43+
..arguments!.add(new ParsedType.interface("dart:core", "int", 1))
4444
]);
4545

4646
// Map<int, int>, kNonNullable
@@ -60,9 +60,9 @@ void main() {
6060
"0",
6161
]),
6262
[
63-
new ParsedType("dart:core", "Map", 1)
64-
..arguments!.add(new ParsedType("dart:core", "int", 1))
65-
..arguments!.add(new ParsedType("dart:core", "int", 1))
63+
new ParsedType.interface("dart:core", "Map", 1)
64+
..arguments!.add(new ParsedType.interface("dart:core", "int", 1))
65+
..arguments!.add(new ParsedType.interface("dart:core", "int", 1))
6666
]);
6767

6868
// [0] = String
@@ -102,33 +102,52 @@ void main() {
102102
]),
103103
<ParsedType>[
104104
// String
105-
new ParsedType("dart:core", "_OneByteString", 1),
105+
new ParsedType.interface("dart:core", "_OneByteString", 1),
106106
// int
107-
new ParsedType("dart:core", "_Smi", 1),
107+
new ParsedType.interface("dart:core", "_Smi", 1),
108108
// List<String>
109-
new ParsedType("dart:core", "_GrowableList", 1)
109+
new ParsedType.interface("dart:core", "_GrowableList", 1)
110110
..arguments!.addAll([
111-
new ParsedType("dart:core", "String", 1),
111+
new ParsedType.interface("dart:core", "String", 1),
112112
]),
113113
// Bar
114-
new ParsedType("file://wherever/t.dart", "Bar", 1),
114+
new ParsedType.interface("file://wherever/t.dart", "Bar", 1),
115115
// null value
116116
new ParsedType.nullType(),
117117
// HashMap<Map<int, List<int>>, List<String>>
118-
new ParsedType("dart:collection", "_InternalLinkedHashMap", 1)
118+
new ParsedType.interface("dart:collection", "_InternalLinkedHashMap", 1)
119119
..arguments!.addAll([
120-
new ParsedType("dart:core", "Map", 1)
120+
new ParsedType.interface("dart:core", "Map", 1)
121121
..arguments!.addAll([
122-
new ParsedType("dart:core", "int", 1),
123-
new ParsedType("dart:core", "List", 1)
122+
new ParsedType.interface("dart:core", "int", 1),
123+
new ParsedType.interface("dart:core", "List", 1)
124124
..arguments!.addAll([
125-
new ParsedType("dart:core", "int", 1),
125+
new ParsedType.interface("dart:core", "int", 1),
126126
]),
127127
]),
128-
new ParsedType("dart:core", "List", 1)
128+
new ParsedType.interface("dart:core", "List", 1)
129129
..arguments!.addAll([
130-
new ParsedType("dart:core", "String", 1),
130+
new ParsedType.interface("dart:core", "String", 1),
131131
]),
132132
]),
133133
]);
134+
135+
// Set<(int, {int foo})>, kNonNullable
136+
expect(
137+
parseDefinitionTypes([
138+
//
139+
/**/ "dart:_compact_hash", "_Set", "1", "1",
140+
/* */ "record", "1", "2", "1", "foo",
141+
/* */ "dart:core", "int", "1", "0",
142+
/* */ "dart:core", "int", "1", "0",
143+
]),
144+
[
145+
new ParsedType.interface("dart:_compact_hash", "_Set", 1)
146+
..arguments!.add(
147+
new ParsedType.record(1, [null, "foo"])
148+
..arguments!.add(new ParsedType.interface("dart:core", "int", 1))
149+
..arguments!.add(new ParsedType.interface("dart:core", "int", 1)),
150+
)
151+
],
152+
);
134153
}

‎pkg/front_end/testcases/expression/evaluate_record_type.expression.yaml‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
# Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
2+
# for details. All rights reserved. Use of this source code is governed by a
3+
# BSD-style license that can be found in the LICENSE file.
4+
5+
sources: |
6+
void stopHere() {
7+
List<(int, {double foo, String bar})> listOfRecord = [(42, foo: 42.42, bar: "fortytwo")];
8+
print(helper(listOfRecord));
9+
}
10+
bool helper(List<(int, {double foo, String bar})> listOfRecord) {
11+
final record = listOfRecord.first;
12+
return record.$1 == 42 && record.foo >= 42.0 && record.bar.length >= 4;
13+
}
14+
definitions: ["listOfRecord"]
15+
# List<(int, {String bar, double foo})> // Note the names being sorted!
16+
definition_types: ["dart:core", "List", "1", "1", "record", "1", "3", "1", "bar", "foo", "dart:core", "int", "1", "0", "dart:core", "String", "1", "0", "dart:core", "double", "1", "0"]
17+
type_definitions: []
18+
type_bounds: []
19+
type_defaults: []
20+
method: "stopHere"
21+
expression: |
22+
helper(listOfRecord)

‎pkg/front_end/testcases/expression/evaluate_record_type.expression.yaml.expect‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Errors: {
2+
}
3+
method /* from org-dartlang-debug:synthetic_debug_expression */ debugExpr(dart.core::List<(dart.core::int, {bar: dart.core::String, foo: dart.core::double})> listOfRecord) → dynamic
4+
return #lib1::helper(listOfRecord);

‎pkg/vm_service/test/evaluate_with_record_1_test.dart‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'dart:developer';
6+
7+
import 'package:test/test.dart';
8+
import 'package:vm_service/vm_service.dart';
9+
10+
import 'common/service_test_common.dart';
11+
import 'common/test_helper.dart';
12+
13+
void testFunction() {
14+
final listOfRecord = [(42, foo: 42.42, bar: 'fortytwo')];
15+
debugger();
16+
print(helper(listOfRecord));
17+
}
18+
19+
bool helper(List<(int, {double foo, String bar})> listOfRecord) {
20+
final record = listOfRecord.first;
21+
return record.$1 == 42 && record.foo >= 42.0 && record.bar.length >= 4;
22+
}
23+
24+
final tests = <IsolateTest>[
25+
hasStoppedAtBreakpoint,
26+
(VmService service, IsolateRef isolateRef) async {
27+
final result = await service.evaluateInFrame(
28+
isolateRef.id!,
29+
0,
30+
'helper(listOfRecord)',
31+
) as InstanceRef;
32+
expect(result.valueAsString, equals('true'));
33+
expect(result.kind, equals(InstanceKind.kBool));
34+
},
35+
];
36+
37+
void main([args = const <String>[]]) => runIsolateTests(
38+
args,
39+
tests,
40+
'evaluate_with_record_1_test.dart',
41+
testeeConcurrent: testFunction,
42+
);

0 commit comments

Comments
 (0)