Skip to content

Commit c617049

Browse files
johnniwintherCommit Queue
authored and
Commit Queue
committed
[cfe] Use constants in matching expressions
This uses the constant values instead of the constant expressions for the key that determines when caching occurs. Change-Id: I5494d9c3a6af28a9c768a0f2ddfef688d7ba9dba Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/288403 Commit-Queue: Johnni Winther <[email protected]> Reviewed-by: Chloe Stefantsova <[email protected]>
1 parent 787c2ac commit c617049

File tree

56 files changed

+2830
-1456
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

56 files changed

+2830
-1456
lines changed

pkg/front_end/lib/src/fasta/type_inference/matching_cache.dart

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -201,30 +201,33 @@ class MatchingCache {
201201
CacheableExpression? result = _intConstantMap[value];
202202
if (result == null) {
203203
result = _intConstantMap[value] = createConstantExpression(
204-
createIntLiteral(value, fileOffset: fileOffset),
205-
_coreTypes.intNonNullableRawType);
204+
new IntConstant(value), _coreTypes.intNonNullableRawType,
205+
fileOffset: fileOffset);
206206
}
207207
return result;
208208
}
209209

210-
/// Creates a cacheable expression for the constant [expression] of the
211-
/// given [expressionType].
212-
// TODO(johnniwinther): Support using constant value identity to determine
213-
// the cache key.
210+
/// Creates a cacheable expression for the [constant] of the given
211+
/// [expressionType].
214212
CacheableExpression createConstantExpression(
215-
Expression expression, DartType expressionType) {
213+
Constant constant, DartType expressionType,
214+
{required int fileOffset}) {
216215
assert(isKnown(expressionType));
217-
CacheKey cacheKey = new ExpressionKey(expression);
216+
CacheKey cacheKey = new ConstantKey(constant);
218217
Cache? cache = _cacheKeyMap[cacheKey];
219218
if (cache == null) {
220219
cache = _createCacheableExpression(cacheKey,
221220
isLate: false,
222221
isConst: true,
223222
requiresCaching: true,
224-
fileOffset: expression.fileOffset);
223+
fileOffset: fileOffset);
225224
}
226225
return cache.registerAccess(
227-
cacheKey, new FixedExpression(expression, expressionType));
226+
cacheKey,
227+
new FixedExpression(
228+
new ConstantExpression(constant, expressionType)
229+
..fileOffset = fileOffset,
230+
expressionType));
228231
}
229232

230233
/// Creates a cacheable as expression of the [operand] against [type].
@@ -513,7 +516,6 @@ abstract class CacheKey implements AccessKey {
513516
}
514517

515518
/// A key that is defined by the [expression] node that created it.
516-
// TODO(johnniwinther): Handle constant expressions differently.
517519
class ExpressionKey extends CacheKey {
518520
final Expression expression;
519521

@@ -532,6 +534,25 @@ class ExpressionKey extends CacheKey {
532534
}
533535
}
534536

537+
/// A key that is defined by the [constant].
538+
class ConstantKey extends CacheKey {
539+
final Constant constant;
540+
541+
ConstantKey(this.constant);
542+
543+
@override
544+
String get name => '${constant.toText(defaultAstTextStrategy)}';
545+
546+
@override
547+
int get hashCode => constant.hashCode;
548+
549+
@override
550+
bool operator ==(Object other) {
551+
if (identical(this, other)) return true;
552+
return other is ConstantKey && constant == other.constant;
553+
}
554+
}
555+
535556
/// A key for a constant integer value.
536557
class IntegerKey extends CacheKey {
537558
final int value;

pkg/front_end/lib/src/fasta/type_inference/matching_expressions.dart

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,9 @@ class MatchingExpressionVisitor
6767
@override
6868
DelayedExpression visitConstantPattern(
6969
ConstantPattern node, CacheableExpression matchedExpression) {
70-
CacheableExpression constExpression = matchingCache
71-
.createConstantExpression(node.expression, node.expressionType);
70+
CacheableExpression constExpression =
71+
matchingCache.createConstantExpression(node.value!, node.expressionType,
72+
fileOffset: node.fileOffset);
7273
return matchingCache.createEqualsExpression(
7374
constExpression,
7475
matchedExpression,
@@ -308,7 +309,8 @@ class MatchingExpressionVisitor
308309
for (MapPatternEntry entry in node.entries) {
309310
if (entry is MapPatternRestEntry) continue;
310311
CacheableExpression keyExpression =
311-
matchingCache.createConstantExpression(entry.key, entry.keyType);
312+
matchingCache.createConstantExpression(entry.keyValue!, entry.keyType,
313+
fileOffset: entry.key.fileOffset);
312314

313315
CacheableExpression containsExpression =
314316
matchingCache.createContainsKeyExpression(
@@ -553,7 +555,8 @@ class MatchingExpressionVisitor
553555
DelayedExpression visitRelationalPattern(
554556
RelationalPattern node, CacheableExpression matchedExpression) {
555557
CacheableExpression constant = matchingCache.createConstantExpression(
556-
node.expression, node.expressionType);
558+
node.expressionValue!, node.expressionType,
559+
fileOffset: node.expression.fileOffset);
557560

558561
switch (node.kind) {
559562
case RelationalPatternKind.equals:

pkg/front_end/testcases/late_lowering/caching.dart.strong.expect

Lines changed: 120 additions & 120 deletions
Large diffs are not rendered by default.

pkg/front_end/testcases/late_lowering/caching.dart.strong.transformed.expect

Lines changed: 120 additions & 120 deletions
Large diffs are not rendered by default.

pkg/front_end/testcases/late_lowering/caching.dart.weak.expect

Lines changed: 120 additions & 120 deletions
Large diffs are not rendered by default.

pkg/front_end/testcases/late_lowering/caching.dart.weak.modular.expect

Lines changed: 120 additions & 120 deletions
Large diffs are not rendered by default.

pkg/front_end/testcases/late_lowering/caching.dart.weak.transformed.expect

Lines changed: 120 additions & 120 deletions
Large diffs are not rendered by default.

pkg/front_end/testcases/patterns/caching.dart.strong.expect

Lines changed: 52 additions & 52 deletions
Large diffs are not rendered by default.

pkg/front_end/testcases/patterns/caching.dart.strong.transformed.expect

Lines changed: 84 additions & 84 deletions
Large diffs are not rendered by default.

pkg/front_end/testcases/patterns/caching.dart.weak.expect

Lines changed: 52 additions & 52 deletions
Large diffs are not rendered by default.

pkg/front_end/testcases/patterns/caching.dart.weak.modular.expect

Lines changed: 52 additions & 52 deletions
Large diffs are not rendered by default.

pkg/front_end/testcases/patterns/caching.dart.weak.transformed.expect

Lines changed: 84 additions & 84 deletions
Large diffs are not rendered by default.
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// Copyright (c) 2023, 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:collection';
6+
7+
class MyMap<K, V> with MapMixin<K, V> {
8+
int containsKeyCount = 0;
9+
int indexGetCount = 0;
10+
11+
final Map<K, V> _map;
12+
13+
MyMap(this._map);
14+
15+
bool containsKey(Object? key) {
16+
containsKeyCount++;
17+
return _map.containsKey(key);
18+
}
19+
20+
V? operator [](Object? key) {
21+
indexGetCount++;
22+
return _map[key];
23+
}
24+
25+
void operator []=(K key, V value) => _map[key] = value;
26+
27+
void clear() => _map.clear();
28+
29+
Iterable<K> get keys => _map.keys;
30+
31+
V? remove(Object? key) => _map.remove(key);
32+
}
33+
34+
int method(Map<int, String> m) {
35+
switch (m) {
36+
case {1: 'foo'}:
37+
return 0;
38+
case {1: 'bar'}:
39+
return 1;
40+
}
41+
return 2;
42+
}
43+
44+
test(Map<int, String> map,
45+
{required int expectedValue,
46+
required int expectedContainsKeyCount,
47+
required int expectedIndexGetCount}) {
48+
MyMap<int, String> myMap = new MyMap(map);
49+
expect(expectedValue, method(myMap), 'Unexpected value for $map.');
50+
expect(expectedContainsKeyCount, myMap.containsKeyCount,
51+
'Unexpected containsKey count for $map.');
52+
expect(expectedIndexGetCount, myMap.indexGetCount,
53+
'Unexpected indexGet count for $map.');
54+
}
55+
56+
main() {
57+
test({0: 'foo'},
58+
expectedValue: 2, expectedContainsKeyCount: 1, expectedIndexGetCount: 0);
59+
test({1: 'foo'},
60+
expectedValue: 0, expectedContainsKeyCount: 1, expectedIndexGetCount: 1);
61+
test({1: 'bar'},
62+
expectedValue: 1, expectedContainsKeyCount: 1, expectedIndexGetCount: 1);
63+
test({1: 'baz'},
64+
expectedValue: 2, expectedContainsKeyCount: 1, expectedIndexGetCount: 1);
65+
}
66+
67+
expect(expected, actual, message) {
68+
if (expected != actual) throw '$message Expected $expected, actual $actual';
69+
}
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
library /*isNonNullableByDefault*/;
2+
import self as self;
3+
import "dart:core" as core;
4+
import "dart:collection" as col;
5+
6+
import "dart:collection";
7+
8+
abstract class _MyMap&Object&MapMixin<K extends core::Object? = dynamic, V extends core::Object? = dynamic> = core::Object with col::MapMixin<self::_MyMap&Object&MapMixin::K%, self::_MyMap&Object&MapMixin::V%> /*isAnonymousMixin,hasConstConstructor*/ {
9+
const synthetic constructor •() → self::_MyMap&Object&MapMixin<self::_MyMap&Object&MapMixin::K%, self::_MyMap&Object&MapMixin::V%>
10+
: super core::Object::•()
11+
;
12+
mixin-super-stub method containsKey(core::Object? key) → core::bool
13+
return super.{col::MapMixin::containsKey}(key);
14+
abstract mixin-stub operator [](core::Object? key) → self::_MyMap&Object&MapMixin::V?; -> col::MapMixin::[]
15+
abstract mixin-stub operator []=(covariant-by-class self::_MyMap&Object&MapMixin::K% key, covariant-by-class self::_MyMap&Object&MapMixin::V% value) → void; -> col::MapMixin::[]=
16+
abstract mixin-stub method clear() → void; -> col::MapMixin::clear
17+
abstract mixin-stub method remove(core::Object? key) → self::_MyMap&Object&MapMixin::V?; -> col::MapMixin::remove
18+
abstract mixin-stub get keys() → core::Iterable<self::_MyMap&Object&MapMixin::K%>; -> col::MapMixin::keys
19+
mixin-super-stub method cast<RK extends core::Object? = dynamic, RV extends core::Object? = dynamic>() → core::Map<self::_MyMap&Object&MapMixin::cast::RK%, self::_MyMap&Object&MapMixin::cast::RV%>
20+
return super.{col::MapMixin::cast}<self::_MyMap&Object&MapMixin::cast::RK%, self::_MyMap&Object&MapMixin::cast::RV%>();
21+
mixin-super-stub method forEach((self::_MyMap&Object&MapMixin::K%, self::_MyMap&Object&MapMixin::V%) → void action) → void
22+
return super.{col::MapMixin::forEach}(action);
23+
mixin-super-stub method addAll(covariant-by-class core::Map<self::_MyMap&Object&MapMixin::K%, self::_MyMap&Object&MapMixin::V%> other) → void
24+
return super.{col::MapMixin::addAll}(other);
25+
mixin-super-stub method containsValue(core::Object? value) → core::bool
26+
return super.{col::MapMixin::containsValue}(value);
27+
mixin-super-stub method putIfAbsent(covariant-by-class self::_MyMap&Object&MapMixin::K% key, covariant-by-class () → self::_MyMap&Object&MapMixin::V% ifAbsent) → self::_MyMap&Object&MapMixin::V%
28+
return super.{col::MapMixin::putIfAbsent}(key, ifAbsent);
29+
mixin-super-stub method update(covariant-by-class self::_MyMap&Object&MapMixin::K% key, covariant-by-class (self::_MyMap&Object&MapMixin::V%) → self::_MyMap&Object&MapMixin::V% update, {covariant-by-class () →? self::_MyMap&Object&MapMixin::V% ifAbsent = #C1}) → self::_MyMap&Object&MapMixin::V%
30+
return super.{col::MapMixin::update}(key, update, ifAbsent: ifAbsent);
31+
mixin-super-stub method updateAll(covariant-by-class (self::_MyMap&Object&MapMixin::K%, self::_MyMap&Object&MapMixin::V%) → self::_MyMap&Object&MapMixin::V% update) → void
32+
return super.{col::MapMixin::updateAll}(update);
33+
mixin-super-stub get entries() → core::Iterable<core::MapEntry<self::_MyMap&Object&MapMixin::K%, self::_MyMap&Object&MapMixin::V%>>
34+
return super.{col::MapMixin::entries};
35+
mixin-super-stub method map<K2 extends core::Object? = dynamic, V2 extends core::Object? = dynamic>((self::_MyMap&Object&MapMixin::K%, self::_MyMap&Object&MapMixin::V%) → core::MapEntry<self::_MyMap&Object&MapMixin::map::K2%, self::_MyMap&Object&MapMixin::map::V2%> transform) → core::Map<self::_MyMap&Object&MapMixin::map::K2%, self::_MyMap&Object&MapMixin::map::V2%>
36+
return super.{col::MapMixin::map}<self::_MyMap&Object&MapMixin::map::K2%, self::_MyMap&Object&MapMixin::map::V2%>(transform);
37+
mixin-super-stub method addEntries(covariant-by-class core::Iterable<core::MapEntry<self::_MyMap&Object&MapMixin::K%, self::_MyMap&Object&MapMixin::V%>> newEntries) → void
38+
return super.{col::MapMixin::addEntries}(newEntries);
39+
mixin-super-stub method removeWhere((self::_MyMap&Object&MapMixin::K%, self::_MyMap&Object&MapMixin::V%) → core::bool test) → void
40+
return super.{col::MapMixin::removeWhere}(test);
41+
mixin-super-stub get length() → core::int
42+
return super.{col::MapMixin::length};
43+
mixin-super-stub get isEmpty() → core::bool
44+
return super.{col::MapMixin::isEmpty};
45+
mixin-super-stub get isNotEmpty() → core::bool
46+
return super.{col::MapMixin::isNotEmpty};
47+
mixin-super-stub get values() → core::Iterable<self::_MyMap&Object&MapMixin::V%>
48+
return super.{col::MapMixin::values};
49+
mixin-super-stub method toString() → core::String
50+
return super.{col::MapMixin::toString}();
51+
}
52+
class MyMap<K extends core::Object? = dynamic, V extends core::Object? = dynamic> extends self::_MyMap&Object&MapMixin<self::MyMap::K%, self::MyMap::V%> {
53+
field core::int containsKeyCount = 0;
54+
field core::int indexGetCount = 0;
55+
final field core::Map<self::MyMap::K%, self::MyMap::V%> _map;
56+
constructor •(core::Map<self::MyMap::K%, self::MyMap::V%> _map) → self::MyMap<self::MyMap::K%, self::MyMap::V%>
57+
: self::MyMap::_map = _map, super self::_MyMap&Object&MapMixin::•()
58+
;
59+
method containsKey(core::Object? key) → core::bool {
60+
this.{self::MyMap::containsKeyCount} = this.{self::MyMap::containsKeyCount}{core::int}.{core::num::+}(1){(core::num) → core::int};
61+
return this.{self::MyMap::_map}{core::Map<self::MyMap::K%, self::MyMap::V%>}.{core::Map::containsKey}(key){(core::Object?) → core::bool};
62+
}
63+
operator [](core::Object? key) → self::MyMap::V? {
64+
this.{self::MyMap::indexGetCount} = this.{self::MyMap::indexGetCount}{core::int}.{core::num::+}(1){(core::num) → core::int};
65+
return this.{self::MyMap::_map}{core::Map<self::MyMap::K%, self::MyMap::V%>}.{core::Map::[]}(key){(core::Object?) → self::MyMap::V?};
66+
}
67+
operator []=(covariant-by-class self::MyMap::K% key, covariant-by-class self::MyMap::V% value) → void
68+
return let final core::Map<self::MyMap::K%, self::MyMap::V%> #t1 = this.{self::MyMap::_map}{core::Map<self::MyMap::K%, self::MyMap::V%>} in let final self::MyMap::K% #t2 = key in let final self::MyMap::V% #t3 = value in let final void #t4 = #t1.{core::Map::[]=}(#t2, #t3){(self::MyMap::K%, self::MyMap::V%) → void} in #t3;
69+
method clear() → void
70+
return this.{self::MyMap::_map}{core::Map<self::MyMap::K%, self::MyMap::V%>}.{core::Map::clear}(){() → void};
71+
get keys() → core::Iterable<self::MyMap::K%>
72+
return this.{self::MyMap::_map}{core::Map<self::MyMap::K%, self::MyMap::V%>}.{core::Map::keys}{core::Iterable<self::MyMap::K%>};
73+
method remove(core::Object? key) → self::MyMap::V?
74+
return this.{self::MyMap::_map}{core::Map<self::MyMap::K%, self::MyMap::V%>}.{core::Map::remove}(key){(core::Object?) → self::MyMap::V?};
75+
}
76+
static method method(core::Map<core::int, core::String> m) → core::int {
77+
#L1:
78+
{
79+
final synthesized core::Map<core::int, core::String> #0#0 = m;
80+
late final synthesized core::bool #0#3 = #0#0.{core::Map::length}{core::int} =={core::num::==}{(core::Object) → core::bool} #C2;
81+
late final synthesized core::bool #0#4 = #0#0.{core::Map::containsKey}(#C2){(core::Object?) → core::bool};
82+
late final synthesized dynamic #0#5 = #0#0.{core::Map::[]}(#C2){(core::Object?) → dynamic};
83+
if(#0#3 && #0#4 && #C3 =={core::String::==}{(core::Object) → core::bool} #0#5{core::String}) {
84+
{
85+
return 0;
86+
}
87+
}
88+
else
89+
if(#0#3 && #0#4 && #C4 =={core::String::==}{(core::Object) → core::bool} #0#5{core::String}) {
90+
{
91+
return 1;
92+
}
93+
}
94+
}
95+
return 2;
96+
}
97+
static method test(core::Map<core::int, core::String> map, {required core::int expectedValue = #C1, required core::int expectedContainsKeyCount = #C1, required core::int expectedIndexGetCount = #C1}) → dynamic {
98+
self::MyMap<core::int, core::String> myMap = new self::MyMap::•<core::int, core::String>(map);
99+
self::expect(expectedValue, self::method(myMap), "Unexpected value for ${map}.");
100+
self::expect(expectedContainsKeyCount, myMap.{self::MyMap::containsKeyCount}{core::int}, "Unexpected containsKey count for ${map}.");
101+
self::expect(expectedIndexGetCount, myMap.{self::MyMap::indexGetCount}{core::int}, "Unexpected indexGet count for ${map}.");
102+
}
103+
static method main() → dynamic {
104+
self::test(<core::int, core::String>{0: "foo"}, expectedValue: 2, expectedContainsKeyCount: 1, expectedIndexGetCount: 0);
105+
self::test(<core::int, core::String>{1: "foo"}, expectedValue: 0, expectedContainsKeyCount: 1, expectedIndexGetCount: 1);
106+
self::test(<core::int, core::String>{1: "bar"}, expectedValue: 1, expectedContainsKeyCount: 1, expectedIndexGetCount: 1);
107+
self::test(<core::int, core::String>{1: "baz"}, expectedValue: 2, expectedContainsKeyCount: 1, expectedIndexGetCount: 1);
108+
}
109+
static method expect(dynamic expected, dynamic actual, dynamic message) → dynamic {
110+
if(!(expected =={core::Object::==}{(core::Object) → core::bool} actual))
111+
throw "${message} Expected ${expected}, actual ${actual}";
112+
}
113+
114+
constants {
115+
#C1 = null
116+
#C2 = 1
117+
#C3 = "foo"
118+
#C4 = "bar"
119+
}

0 commit comments

Comments
 (0)