dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

[ddc] Fix broken named record elements

Browse Source 
Some named elements were colliding with existing properties on
record object instances.

Use a symbol in the runtime library for the shape and values
properties.

Ensure that the `.constructor` and `.prototype` getters are renamed
to match the expectation when compiling the access.

Add regression test for all 4 named elements.

Fixes: https://github.com/dart-lang/sdk/issues/54375
Change-Id: I7f3577455bfcff7dece6350e8c7e3ee7ffdbbac6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/342703
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Mark Zhou <markzipan@google.com>
This commit is contained in:
Nicholas Shahan
2023-12-21 19:23:04 +00:00
committed by Commit Queue
parent b574db4493
commit 4bebe882be
6 changed files with 83 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/debugger.dart
+3 -3
View File
@@ -304,7 +304,7 @@ Object getObjectMetadata(@notNull Object object) {
} else if (object is Function) {
_set(result, 'runtimeKind', RuntimeObjectKind.function);
} else if (object is RecordImpl) {
var shape = object.shape;
var shape = JS<Shape>('!', '#[#]', object, shapeProperty);
var positionalCount = shape.positionals;
var namedCount = shape.named?.length ?? 0;
var length = positionalCount + namedCount;
@@ -452,10 +452,10 @@ Object getTypeFields(@notNull Type type) {
/// ```
@notNull
Object getRecordFields(@notNull RecordImpl record) {
var shape = record.shape;
var shape = JS<Shape>('!', '#[#]', record, shapeProperty);
var positionalCount = shape.positionals;
var named = shape.named?.toList();
var values = record.values;
var values = JS('!', '#[#]', record, valuesProperty);
return _createJsObject({
'positionalCount': positionalCount,
sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/records.dart
+29 -16
View File
@@ -20,16 +20,15 @@ final class Shape {
}
}
/// Used to store a [Shape] on an instance of a record object.
final shapeProperty = JS('', 'Symbol("shape")');
/// Used to store a [JSArray] on an instance of a record object containing all
/// the elements in the record in order.
final valuesProperty = JS('', 'Symbol("values")');
/// Internal base class for all concrete records.
final class RecordImpl implements Record {
final Shape shape;
/// Stores the elements of this record.
///
/// Contains all positional elements followed by all named elements in the
/// order corresponding to names as they appear in [shape].
final List values;
/// Cache for faster access after the first call of [hashCode].
int? _hashCode;
@@ -38,16 +37,17 @@ final class RecordImpl implements Record {
/// NOTE: Does not contain the cached result of the "safe" [_toString] call.
String? _printed;
RecordImpl(this.shape, this.values) {
var valueCount = JS<int>('!', '#.length', values);
RecordImpl(Shape shape, JSArray values) {
// Coerce all undefined values to null because dynamic gets of record
// elements rely on the getter returning undefined to signal that the getter
// does not exist.
for (int i = 0; i < valueCount; i++) {
for (int i = 0; i < values.length; i++) {
if (JS<bool>('!', '#[#] === void 0', values, i)) {
JS('', '#[#] = null', values, i);
}
}
JS('!', '#[#] = #', this, shapeProperty, shape);
JS('!', '#[#] = #', this, valuesProperty, values);
}
@override
@@ -55,12 +55,16 @@ final class RecordImpl implements Record {
if (!(other is RecordImpl)) return false;
// Shapes are canonicalized and stored in a map so there will only ever be
// one instance of the same shape.
if (JS<bool>('!', '# !== #', shape, other.shape)) return false;
if (JS<bool>(
'!', '#[#] !== #[#]', this, shapeProperty, other, shapeProperty))
return false;
// If the shapes are identical then the two records have the same number of
// positional elements and the same named elements.
// This implies: `values.length == other.values.length`.
var values = JS<JSArray>('!', '#[#]', this, valuesProperty);
var otherValues = JS<JSArray>('!', '#[#]', other, valuesProperty);
for (var i = 0; i < values.length; i++) {
if (values[i] != other.values[i]) {
if (values[i] != otherValues[i]) {
return false;
}
}
@@ -71,6 +75,8 @@ final class RecordImpl implements Record {
int get hashCode {
final cachedValue = _hashCode;
if (cachedValue != null) return cachedValue;
var shape = JS<Shape>('!', '#[#]', this, shapeProperty);
var values = JS<JSArray>('!', '#[#]', this, valuesProperty);
return _hashCode = Object.hashAll([shape, ...values]);
}
@@ -85,6 +91,8 @@ final class RecordImpl implements Record {
final cachedValue = _printed;
if (!safe && cachedValue != null) return cachedValue;
var buffer = StringBuffer();
var shape = JS<Shape>('!', '#[#]', this, shapeProperty);
var values = JS<JSArray>('!', '#[#]', this, valuesProperty);
var posCount = shape.positionals;
var count = values.length;
@@ -173,8 +181,8 @@ Object registerRecord(
JS('!', '#.prototype = #.prototype', newRecord, recordClass);
var recordPrototype = JS('', '#.prototype', recordClass);
_recordGet(@notNull int index) =>
JS('!', 'function recordGet() {return this.values[#];}', index);
_recordGet(@notNull int index) => JS(
'!', 'function recordGet() {return this[#][#];}', valuesProperty, index);
// Add convenience getters for accessing the record's field values.
var count = 0;
@@ -185,7 +193,12 @@ Object registerRecord(
count++;
}
if (named != null) {
for (final name in named) {
for (var name in named) {
if (name == 'constructor' || name == 'prototype') {
// This renaming is directly coupled to the renaming logic at compile
// time in js_names.dart `.memberNameForDartMember()`.
name = '_$name';
}
defineAccessor(recordPrototype, name,
get: _recordGet(count), enumerable: true);
count++;
sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/rtti.dart
+1 -1
View File
@@ -96,7 +96,7 @@ getFunctionType(obj) {
RecordType getRecordType(RecordImpl obj) {
var type = JS<RecordType?>('', '#[#]', obj, _runtimeType);
if (type == null) {
var shape = obj.shape;
var shape = JS<Shape>('!', '#[#]', obj, shapeProperty);
var named = shape.named;
var positionals = shape.positionals;
var types = [];
sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/types.dart
+4 -3
View File
@@ -2376,12 +2376,13 @@ class RecordType extends DartType {
@JSExportName('is')
bool is_T(obj) {
if (!(obj is RecordImpl)) return false;
if (shape != obj.shape) return false;
if (types.length != obj.values.length) {
if (shape != JS<Shape>('!', '#[#]', obj, shapeProperty)) return false;
var values = JS<JSArray>('!', '#[#]', obj, valuesProperty);
if (types.length != values.length) {
return false;
}
for (var i = 0; i < types.length; i++) {
if (!JS<bool>('!', '#.is(#)', types[i], obj.values[i])) {
if (!JS<bool>('!', '#.is(#)', types[i], values[i])) {
return false;
}
}
sdk/lib/_internal/js_dev_runtime/private/js_helper.dart
+8 -5
View File
@@ -882,9 +882,10 @@ Object? createRecordTypePredicate(String partialShapeTag, JSArray fieldRtis) {
return (obj) {
return JS<bool>(
'!', '# instanceof #', obj, JS_CLASS_REF(dart.RecordImpl)) &&
JS<dart.RecordImpl>('!', '#', obj).shape ==
JS<dart.Shape>('!', '#[#]', obj, dart.shapeProperty) ==
JS<dart.Shape?>('', '#.get(#)', dart.shapes, shapeKey) &&
rti.pairwiseIsTest(fieldRtis, JS<JSArray>('!', '#.values', obj));
rti.pairwiseIsTest(
fieldRtis, JS<JSArray>('!', '#[#]', obj, dart.valuesProperty));
};
} else {
dart.throwUnimplementedInCurrentRti();
@@ -902,14 +903,16 @@ rti.Rti getRtiForRecord(Object? record) {
if (JS_GET_FLAG('NEW_RUNTIME_TYPES')) {
var recordObj = JS<dart.RecordImpl>('!', '#', record);
var recipeBuffer = StringBuffer('+');
var named = recordObj.shape.named;
var shape = JS<dart.Shape>('!', '#[#]', recordObj, dart.shapeProperty);
var values = JS<JSArray>('!', '#[#]', recordObj, dart.valuesProperty);
var named = shape.named;
if (named != null) recipeBuffer.writeAll(named, ',');
recipeBuffer.write('(');
var elementCount = recordObj.values.length;
var elementCount = values.length;
recipeBuffer.writeAll([for (var i = 1; i <= elementCount; i++) i], ',');
recipeBuffer.write(')');
return rti.evaluateRtiForRecord(recipeBuffer.toString(), recordObj.values);
return rti.evaluateRtiForRecord(recipeBuffer.toString(), values);
} else {
dart.throwUnimplementedInCurrentRti();
}
tests/language/records/regress54375_test.dart
+38
View File
@@ -0,0 +1,38 @@
// Copyright (c) 2023, 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.
/// Regression test for https://github.com/dart-lang/sdk/issues/54375.
///
/// Records should support named elements with the names 'shape', 'values',
/// 'constructor', and 'prototype'.
import 'package:expect/expect.dart';
@pragma('dart2js:noInline')
@pragma('dart2js:assumeDynamic')
confuse(x) {
return x;
}
void main() {
var r = (shape: 123);
Expect.equals(123, r.shape);
var d = confuse(r);
Expect.equals(123, d.shape);
var r2 = (values: 'hello');
Expect.equals('hello', r2.values);
d = confuse(r2);
Expect.equals('hello', d.values);
var r3 = (constructor: Duration.zero);
Expect.equals(Duration.zero, r3.constructor);
d = confuse(r3);
Expect.equals(Duration.zero, d.constructor);
var r4 = (prototype: null);
Expect.equals(null, r4.prototype);
d = confuse(r4);
Expect.equals(null, d.prototype);
}