Skip to content

Commit ed59358

Browse files
alexmarkovCommit Bot
alexmarkov
authored and
Commit Bot
committed
[vm] Record operator== and hashCode
TEST=language/records/simple/equals_and_hashcode_test TEST=co19/LanguageFeatures/Records/equality_* Issue: #49719 Change-Id: I63842f980389d63650d00638f1cb1501db88b025 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/260442 Commit-Queue: Alexander Markov <[email protected]> Reviewed-by: Ryan Macnak <[email protected]>
1 parent 4f87eac commit ed59358

File tree

20 files changed

+178
-6
lines changed

20 files changed

+178
-6
lines changed

‎runtime/vm/compiler/assembler/assembler_arm.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2311,6 +2311,7 @@ OperandSize Address::OperandSizeFor(intptr_t cid) {
23112311
switch (cid) {
23122312
case kArrayCid:
23132313
case kImmutableArrayCid:
2314+
case kRecordCid:
23142315
case kTypeArgumentsCid:
23152316
return kFourBytes;
23162317
case kOneByteStringCid:

‎runtime/vm/compiler/assembler/assembler_arm64.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ class Address : public ValueObject {
252252
switch (cid) {
253253
case kArrayCid:
254254
case kImmutableArrayCid:
255+
case kRecordCid:
255256
case kTypeArgumentsCid:
256257
return kObjectBytes;
257258
case kOneByteStringCid:

‎runtime/vm/compiler/assembler/assembler_riscv.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4140,6 +4140,7 @@ static OperandSize OperandSizeFor(intptr_t cid) {
41404140
switch (cid) {
41414141
case kArrayCid:
41424142
case kImmutableArrayCid:
4143+
case kRecordCid:
41434144
case kTypeArgumentsCid:
41444145
return kObjectBytes;
41454146
case kOneByteStringCid:

‎runtime/vm/compiler/backend/il.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6351,6 +6351,7 @@ Representation LoadIndexedInstr::RepresentationOfArrayElement(
63516351
intptr_t array_cid) {
63526352
switch (array_cid) {
63536353
case kImmutableArrayCid:
6354+
case kRecordCid:
63546355
case kTypeArgumentsCid:
63556356
return kTagged;
63566357
case kExternalOneByteStringCid:

‎runtime/vm/compiler/backend/il_arm.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2162,7 +2162,7 @@ void LoadIndexedInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
21622162
const Register result = locs()->out(0).reg();
21632163
ASSERT(representation() == kTagged);
21642164
ASSERT((class_id() == kArrayCid) || (class_id() == kImmutableArrayCid) ||
2165-
(class_id() == kTypeArgumentsCid));
2165+
(class_id() == kTypeArgumentsCid) || (class_id() == kRecordCid));
21662166
__ ldr(result, element_address);
21672167
break;
21682168
}

‎runtime/vm/compiler/backend/il_arm64.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1902,7 +1902,7 @@ void LoadIndexedInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
19021902
default:
19031903
ASSERT(representation() == kTagged);
19041904
ASSERT((class_id() == kArrayCid) || (class_id() == kImmutableArrayCid) ||
1905-
(class_id() == kTypeArgumentsCid));
1905+
(class_id() == kTypeArgumentsCid) || (class_id() == kRecordCid));
19061906
__ LoadCompressed(result, element_address);
19071907
break;
19081908
}

‎runtime/vm/compiler/backend/il_ia32.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1661,7 +1661,7 @@ void LoadIndexedInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
16611661
const Register result = locs()->out(0).reg();
16621662
ASSERT(representation() == kTagged);
16631663
ASSERT((class_id() == kArrayCid) || (class_id() == kImmutableArrayCid) ||
1664-
(class_id() == kTypeArgumentsCid));
1664+
(class_id() == kTypeArgumentsCid) || (class_id() == kRecordCid));
16651665
__ movl(result, element_address);
16661666
break;
16671667
}

‎runtime/vm/compiler/backend/il_riscv.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2143,7 +2143,7 @@ void LoadIndexedInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
21432143
default: {
21442144
ASSERT(representation() == kTagged);
21452145
ASSERT((class_id() == kArrayCid) || (class_id() == kImmutableArrayCid) ||
2146-
(class_id() == kTypeArgumentsCid));
2146+
(class_id() == kTypeArgumentsCid) || (class_id() == kRecordCid));
21472147
const Register result = locs()->out(0).reg();
21482148
__ lx(result, element_address);
21492149
break;

‎runtime/vm/compiler/backend/il_x64.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1844,7 +1844,7 @@ void LoadIndexedInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
18441844
default:
18451845
ASSERT(representation() == kTagged);
18461846
ASSERT((class_id() == kArrayCid) || (class_id() == kImmutableArrayCid) ||
1847-
(class_id() == kTypeArgumentsCid));
1847+
(class_id() == kTypeArgumentsCid) || (class_id() == kRecordCid));
18481848
__ LoadCompressed(result, element_address);
18491849
break;
18501850
}

‎runtime/vm/compiler/backend/range_analysis.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2815,6 +2815,7 @@ void LoadFieldInstr::InferRange(RangeAnalysis* analysis, Range* range) {
28152815
case Slot::Kind::kFunctionType_parameter_types:
28162816
case Slot::Kind::kFunctionType_type_parameters:
28172817
case Slot::Kind::kInstance_native_fields_array:
2818+
case Slot::Kind::kRecord_field_names:
28182819
case Slot::Kind::kSuspendState_function_data:
28192820
case Slot::Kind::kSuspendState_then_callback:
28202821
case Slot::Kind::kSuspendState_error_callback:

‎runtime/vm/compiler/backend/slot.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ bool Slot::IsImmutableLengthSlot() const {
189189
case Slot::Kind::kFunctionType_parameter_types:
190190
case Slot::Kind::kFunctionType_type_parameters:
191191
case Slot::Kind::kRecordField:
192+
case Slot::Kind::kRecord_field_names:
192193
case Slot::Kind::kSuspendState_function_data:
193194
case Slot::Kind::kSuspendState_then_callback:
194195
case Slot::Kind::kSuspendState_error_callback:

‎runtime/vm/compiler/backend/slot.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ class ParsedFunction;
125125
V(ArgumentsDescriptor, UntaggedArray, positional_count, Smi, FINAL) \
126126
V(ArgumentsDescriptor, UntaggedArray, count, Smi, FINAL) \
127127
V(ArgumentsDescriptor, UntaggedArray, size, Smi, FINAL) \
128+
V(Record, UntaggedRecord, field_names, Array, FINAL) \
128129
V(TypeArguments, UntaggedTypeArguments, length, Smi, FINAL) \
129130
V(TypeParameters, UntaggedTypeParameters, names, Array, FINAL) \
130131
V(TypeParameter, UntaggedTypeParameter, bound, Dynamic, FINAL) \
@@ -182,6 +183,7 @@ NONNULLABLE_BOXED_NATIVE_SLOTS_LIST(FOR_EACH_NATIVE_SLOT)
182183
V(FunctionType, UntaggedFunctionType, packed_type_parameter_counts, Uint16, \
183184
FINAL) \
184185
V(PointerBase, UntaggedPointerBase, data, IntPtr, VAR) \
186+
V(Record, UntaggedRecord, num_fields, Int32, FINAL) \
185187
V(TypeParameter, UntaggedTypeParameter, flags, Uint8, FINAL)
186188

187189
// For uses that do not need the exact_type (boxed) or representation (unboxed)

‎runtime/vm/compiler/backend/type_propagator.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1990,6 +1990,9 @@ CompileType LoadIndexedInstr::ComputeType() const {
19901990
case kTypedDataUint64ArrayCid:
19911991
return CompileType::Int();
19921992

1993+
case kRecordCid:
1994+
return CompileType::Dynamic();
1995+
19931996
default:
19941997
UNIMPLEMENTED();
19951998
return CompileType::Dynamic();

‎runtime/vm/compiler/frontend/kernel_to_il.cc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -873,6 +873,8 @@ Fragment FlowGraphBuilder::NativeFunctionBody(const Function& function,
873873
V(LinkedHashBase_getIndex, LinkedHashBase_index) \
874874
V(LinkedHashBase_getUsedData, LinkedHashBase_used_data) \
875875
V(ObjectArrayLength, Array_length) \
876+
V(Record_fieldNames, Record_field_names) \
877+
V(Record_numFields, Record_num_fields) \
876878
V(SuspendState_getFunctionData, SuspendState_function_data) \
877879
V(SuspendState_getThenCallback, SuspendState_then_callback) \
878880
V(SuspendState_getErrorCallback, SuspendState_error_callback) \
@@ -908,6 +910,7 @@ bool FlowGraphBuilder::IsRecognizedMethodForFlowGraph(
908910
const MethodRecognizer::Kind kind = function.recognized_kind();
909911

910912
switch (kind) {
913+
case MethodRecognizer::kRecord_fieldAt:
911914
case MethodRecognizer::kSuspendState_clone:
912915
case MethodRecognizer::kSuspendState_resume:
913916
case MethodRecognizer::kTypedData_ByteDataView_factory:
@@ -1072,6 +1075,13 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfRecognizedMethod(
10721075

10731076
const MethodRecognizer::Kind kind = function.recognized_kind();
10741077
switch (kind) {
1078+
case MethodRecognizer::kRecord_fieldAt:
1079+
ASSERT_EQUAL(function.NumParameters(), 2);
1080+
body += LoadLocal(parsed_function_->RawParameterVariable(0));
1081+
body += LoadLocal(parsed_function_->RawParameterVariable(1));
1082+
body += LoadIndexed(
1083+
kRecordCid, /*index_scale*/ compiler::target::kCompressedWordSize);
1084+
break;
10751085
case MethodRecognizer::kSuspendState_clone: {
10761086
ASSERT_EQUAL(function.NumParameters(), 1);
10771087
body += LoadLocal(parsed_function_->RawParameterVariable(0));

‎runtime/vm/compiler/recognized_methods_list.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ namespace dart {
2020
V(_List, []=, ObjectArraySetIndexed, 0x050cd2ba) \
2121
V(_GrowableList, ._withData, GrowableArrayAllocateWithData, 0x1947d8a1) \
2222
V(_GrowableList, []=, GrowableArraySetIndexed, 0x050cd2ba) \
23+
V(_Record, get:_fieldNames, Record_fieldNames, 0x68e5459d) \
24+
V(_Record, get:_numFields, Record_numFields, 0x7bc20792) \
25+
V(_Record, _fieldAt, Record_fieldAt, 0xb49cb873) \
2326
V(_TypedList, _getInt8, ByteArrayBaseGetInt8, 0x1623dc34) \
2427
V(_TypedList, _getUint8, ByteArrayBaseGetUint8, 0x177ffe2a) \
2528
V(_TypedList, _getInt16, ByteArrayBaseGetInt16, 0x2e40964f) \

‎runtime/vm/compiler/runtime_api.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,8 @@ word Instance::DataOffsetFor(intptr_t cid) {
546546
return OneByteString::data_offset();
547547
case kTwoByteStringCid:
548548
return TwoByteString::data_offset();
549+
case kRecordCid:
550+
return Record::field_offset(0);
549551
default:
550552
UNIMPLEMENTED();
551553
return Array::data_offset();

‎runtime/vm/object.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8304,6 +8304,7 @@ bool Function::RecognizedKindForceOptimize() const {
83048304
case MethodRecognizer::kFfiAsExternalTypedDataFloat:
83058305
case MethodRecognizer::kFfiAsExternalTypedDataDouble:
83068306
case MethodRecognizer::kGetNativeField:
8307+
case MethodRecognizer::kRecord_numFields:
83078308
case MethodRecognizer::kUtf8DecoderScan:
83088309
// Prevent the GC from running so that the operation is atomic from
83098310
// a GC point of view. Always double check implementation in
@@ -20082,6 +20083,8 @@ intptr_t Instance::DataOffsetFor(intptr_t cid) {
2008220083
return OneByteString::data_offset();
2008320084
case kTwoByteStringCid:
2008420085
return TwoByteString::data_offset();
20086+
case kRecordCid:
20087+
return Record::field_offset(0);
2008520088
default:
2008620089
UNIMPLEMENTED();
2008720090
return Array::data_offset();

‎sdk/lib/_internal/vm/lib/core_patch.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import "dart:_internal"
2727
Lists,
2828
POWERS_OF_TEN,
2929
SubListIterable,
30+
SystemHash,
3031
UnmodifiableListMixin,
3132
has63BitSmis,
3233
makeFixedListUnmodifiable,

‎sdk/lib/_internal/vm/lib/object_patch.dart

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,68 @@ dynamic _objectNoSuchMethod(Object? obj, Invocation invocation) =>
8585
obj.noSuchMethod(invocation);
8686

8787
// Base class for record instances.
88-
// TODO(49719): create a separate patch file for this class.
88+
// TODO(dartbug.com/49719): create a separate patch file for this class.
8989
@pragma("vm:entry-point")
9090
class _Record {
9191
factory _Record._uninstantiable() {
9292
throw "Unreachable";
9393
}
94+
95+
// Do not inline to avoid mixing _fieldAt with
96+
// record field accesses.
97+
@pragma("vm:never-inline")
98+
bool operator ==(Object other) {
99+
if (identical(this, other)) {
100+
return true;
101+
}
102+
103+
if (other is! _Record) {
104+
return false;
105+
}
106+
107+
_Record otherRec = unsafeCast<_Record>(other);
108+
final int numFields = _numFields;
109+
if (numFields != otherRec._numFields ||
110+
!identical(_fieldNames, otherRec._fieldNames)) {
111+
return false;
112+
}
113+
114+
for (int i = 0; i < numFields; ++i) {
115+
if (_fieldAt(i) != otherRec._fieldAt(i)) {
116+
return false;
117+
}
118+
}
119+
return true;
120+
}
121+
122+
// Do not inline to avoid mixing _fieldAt with
123+
// record field accesses.
124+
@pragma("vm:never-inline")
125+
int get hashCode {
126+
final int numFields = _numFields;
127+
int hash = numFields;
128+
hash = SystemHash.combine(hash, identityHashCode(_fieldNames));
129+
for (int i = 0; i < numFields; ++i) {
130+
hash = SystemHash.combine(hash, _fieldAt(i).hashCode);
131+
}
132+
return SystemHash.finish(hash);
133+
}
134+
135+
@pragma("vm:recognized", "other")
136+
@pragma("vm:prefer-inline")
137+
external int get _numFields;
138+
139+
@pragma("vm:recognized", "other")
140+
@pragma("vm:prefer-inline")
141+
external _List get _fieldNames;
142+
143+
// Currently compiler does not take into account aliasing
144+
// between access to record fields via _fieldAt and
145+
// via record.foo / record.$n.
146+
// So this method should only be used in methods
147+
// which only access record fields with _fieldAt and
148+
// annotated with @pragma("vm:never-inline").
149+
@pragma("vm:recognized", "other")
150+
@pragma("vm:prefer-inline")
151+
external Object? _fieldAt(int index);
94152
}

‎tests/language/records/simple/equals_and_hashcode_test.dart

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code as governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// SharedOptions=--enable-experiment=records
6+
7+
import "package:expect/expect.dart";
8+
9+
10+
class NotEqual {
11+
bool operator==(Object other) => false;
12+
}
13+
14+
class A {
15+
final int i;
16+
const A(this.i);
17+
18+
bool operator==(Object other) => other is A && i == other.i;
19+
int get hashCode => i;
20+
}
21+
22+
class B {
23+
final int i;
24+
bool equalsCalled = false;
25+
bool hashCodeCalled = false;
26+
27+
B(this.i);
28+
29+
bool operator==(Object other) {
30+
equalsCalled = true;
31+
return other is B && i == other.i;
32+
}
33+
34+
int get hashCode {
35+
hashCodeCalled = true;
36+
return i ^ 42;
37+
}
38+
}
39+
40+
checkEqualsAndHash(Object? a, Object? b) {
41+
Expect.isTrue(a == b);
42+
Expect.equals(a.hashCode, b.hashCode);
43+
}
44+
45+
checkNotEquals(Object? a, Object? b) {
46+
Expect.isFalse(a == b);
47+
}
48+
49+
main() {
50+
checkEqualsAndHash((1, 2), (1, 2));
51+
checkEqualsAndHash((1, 2), const (1, 2));
52+
checkEqualsAndHash(const (42, foo: "hello3"), (foo: "hello${int.parse("3")}", 40 + int.parse("2")));
53+
checkEqualsAndHash((1, 2, 3, foo: 4, bar: 5, baz: 6), (baz: 6, 1, bar: 5, 2, foo: 4, int.parse("3")));
54+
checkEqualsAndHash((foo: 1, 2), (2, foo: 1));
55+
checkEqualsAndHash((foo: 3), (foo: 3));
56+
57+
checkNotEquals((1, 2), (1, 3));
58+
checkNotEquals((1, 2), (3, 2));
59+
checkNotEquals((1, 2), (2, 1));
60+
checkNotEquals((1, foo: 2), (foo: 1, 2));
61+
checkNotEquals((1, foo: 2), (1, bar: 2));
62+
63+
checkEqualsAndHash((A(1), A(2)), (A(1), A(2)));
64+
checkEqualsAndHash((A(1), A(2)), (A(1), const A(2)));
65+
checkEqualsAndHash((A(1), A(2)), const (A(1), A(2)));
66+
checkEqualsAndHash(const (A(1), A(2)), (A(1), A(int.parse("2"))));
67+
68+
checkNotEquals((A(1), A(2)), (A(1), A(3)));
69+
70+
Object? notEqual = NotEqual();
71+
checkNotEquals(notEqual, notEqual);
72+
checkNotEquals((1, notEqual), (1, notEqual));
73+
checkNotEquals((foo: notEqual), (foo: notEqual));
74+
75+
B o1 = B(1);
76+
B o2 = B(2);
77+
checkNotEquals((1, o1), (1, o2));
78+
Expect.isTrue(o1.equalsCalled);
79+
Expect.isFalse(o2.equalsCalled);
80+
81+
checkEqualsAndHash((2, o2), (2, B(int.parse("2"))));
82+
Expect.isTrue(o2.equalsCalled);
83+
Expect.isTrue(o2.hashCodeCalled);
84+
}

0 commit comments

Comments
 (0)