Skip to content

Commit bc8104f

Browse files
chloestefantsovaCommit Queue
authored and
Commit Queue
committed
[analyzer][cfe] Implement flow analysis for null-aware map entries
Closes #56786 Change-Id: I738c98b6f4e632cfbbe51221bbc3547edbc718fe Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/386800 Commit-Queue: Chloe Stefantsova <[email protected]> Reviewed-by: Paul Berry <[email protected]>
1 parent e3d1fda commit bc8104f

13 files changed

+716
-9
lines changed

pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -703,6 +703,13 @@ abstract class FlowAnalysis<Node extends Object, Statement extends Node,
703703
/// not be called until after processing the method call to `z(x)`.
704704
void nullAwareAccess_rightBegin(Expression? target, Type targetType);
705705

706+
/// Call this method after visiting the value of a null-aware map entry.
707+
void nullAwareMapEntry_end({required bool isKeyNullAware});
708+
709+
/// Call this method after visiting the key of a null-aware map entry.
710+
void nullAwareMapEntry_valueBegin(Expression key, Type keyType,
711+
{required bool isKeyNullAware});
712+
706713
/// Call this method before visiting the subpattern of a null-check or a
707714
/// null-assert pattern. [isAssert] indicates whether the pattern is a
708715
/// null-check or a null-assert pattern.
@@ -1655,6 +1662,22 @@ class FlowAnalysisDebug<Node extends Object, Statement extends Node,
16551662
() => _wrapped.nullAwareAccess_rightBegin(target, targetType));
16561663
}
16571664

1665+
@override
1666+
void nullAwareMapEntry_end({required bool isKeyNullAware}) {
1667+
return _wrap('nullAwareMapEntry_end(isKeyNullAware: $isKeyNullAware)',
1668+
() => _wrapped.nullAwareMapEntry_end(isKeyNullAware: isKeyNullAware));
1669+
}
1670+
1671+
@override
1672+
void nullAwareMapEntry_valueBegin(Expression key, Type keyType,
1673+
{required bool isKeyNullAware}) {
1674+
_wrap(
1675+
'nullAwareMapEntry_valueBegin($key, $keyType, '
1676+
'isKeyNullAware: $isKeyNullAware)',
1677+
() => _wrapped.nullAwareMapEntry_valueBegin(key, keyType,
1678+
isKeyNullAware: isKeyNullAware));
1679+
}
1680+
16581681
@override
16591682
bool nullCheckOrAssertPattern_begin(
16601683
{required bool isAssert, required Type matchedValueType}) {
@@ -5032,6 +5055,35 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
50325055
}
50335056
}
50345057

5058+
@override
5059+
void nullAwareMapEntry_end({required bool isKeyNullAware}) {
5060+
if (!isKeyNullAware) return;
5061+
_NullAwareMapEntryContext<Type> context =
5062+
_stack.removeLast() as _NullAwareMapEntryContext<Type>;
5063+
_current = _join(_current, context._shortcutState).unsplit();
5064+
}
5065+
5066+
@override
5067+
void nullAwareMapEntry_valueBegin(Expression key, Type keyType,
5068+
{required bool isKeyNullAware}) {
5069+
if (!isKeyNullAware) return;
5070+
_Reference<Type>? keyReference = _getExpressionReference(key);
5071+
FlowModel<Type> shortcutState;
5072+
_current = _current.split();
5073+
if (keyReference != null) {
5074+
ExpressionInfo<Type> expressionInfo =
5075+
_current.tryMarkNonNullable(this, keyReference);
5076+
_current = expressionInfo.ifTrue;
5077+
shortcutState = expressionInfo.ifFalse;
5078+
} else {
5079+
shortcutState = _current;
5080+
}
5081+
if (operations.classifyType(keyType) == TypeClassification.nonNullable) {
5082+
shortcutState = shortcutState.setUnreachable();
5083+
}
5084+
_stack.add(new _NullAwareMapEntryContext<Type>(shortcutState));
5085+
}
5086+
50355087
@override
50365088
bool nullCheckOrAssertPattern_begin(
50375089
{required bool isAssert, required Type matchedValueType}) {
@@ -6640,6 +6692,13 @@ class _LegacyTypePromotion<Node extends Object, Statement extends Node,
66406692
@override
66416693
void nullAwareAccess_rightBegin(Expression? target, Type targetType) {}
66426694

6695+
@override
6696+
void nullAwareMapEntry_end({required bool isKeyNullAware}) {}
6697+
6698+
@override
6699+
void nullAwareMapEntry_valueBegin(Expression key, Type keyType,
6700+
{required bool isKeyNullAware}) {}
6701+
66436702
@override
66446703
bool nullCheckOrAssertPattern_begin(
66456704
{required bool isAssert, required Type matchedValueType}) =>
@@ -6922,6 +6981,25 @@ class _NullAwareAccessContext<Type extends Object>
69226981
String get _debugType => '_NullAwareAccessContext';
69236982
}
69246983

6984+
/// [_FlowContext] representing a null-aware map entry (`{?a: ?b}`).
6985+
///
6986+
/// This context should only be created for a null-aware map entry that has a
6987+
/// null-aware key.
6988+
class _NullAwareMapEntryContext<Type extends Object> extends _FlowContext {
6989+
/// The state if the operation short-cuts (i.e. if the key expression was
6990+
/// `null`.
6991+
final FlowModel<Type> _shortcutState;
6992+
6993+
_NullAwareMapEntryContext(this._shortcutState);
6994+
6995+
@override
6996+
Map<String, Object?> get _debugFields =>
6997+
super._debugFields..['shortcutState'] = _shortcutState;
6998+
6999+
@override
7000+
String get _debugType => '_NullAwareMapEntryContext';
7001+
}
7002+
69257003
/// Specialization of [ExpressionInfo] for the case where the expression is a
69267004
/// `null` literal.
69277005
class _NullInfo<Type extends Object> extends ExpressionInfo<Type> {

pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8606,6 +8606,82 @@ main() {
86068606
});
86078607
});
86088608

8609+
group('Null-aware map entry:', () {
8610+
test('Promotes key within value', () {
8611+
var a = Var('a');
8612+
8613+
h.run([
8614+
declare(a, type: 'String?', initializer: expr('String?')),
8615+
mapLiteral(keyType: 'String', valueType: 'dynamic', [
8616+
mapEntry(a, checkPromoted(a, 'String'), isKeyNullAware: true),
8617+
]),
8618+
checkNotPromoted(a),
8619+
]);
8620+
});
8621+
8622+
test('Non-null-aware key', () {
8623+
var a = Var('a');
8624+
8625+
h.run([
8626+
declare(a, type: 'String?', initializer: expr('String?')),
8627+
mapLiteral(keyType: 'String?', valueType: 'dynamic', [
8628+
mapEntry(a, checkNotPromoted(a), isKeyNullAware: false),
8629+
]),
8630+
checkNotPromoted(a),
8631+
]);
8632+
});
8633+
8634+
test('Promotes', () {
8635+
var a = Var('a');
8636+
var x = Var('x');
8637+
8638+
h.run([
8639+
declare(a, type: 'String', initializer: expr('String')),
8640+
declare(x, type: 'num', initializer: expr('num')),
8641+
mapLiteral(keyType: 'String', valueType: 'dynamic', [
8642+
mapEntry(a, x.as_('int'), isKeyNullAware: true),
8643+
]),
8644+
checkPromoted(x, 'int'),
8645+
]);
8646+
});
8647+
8648+
test('Affects promotion', () {
8649+
var a = Var('a');
8650+
var x = Var('x');
8651+
8652+
h.run([
8653+
declare(a, type: 'String?', initializer: expr('String?')),
8654+
declare(x, type: 'num', initializer: expr('num')),
8655+
mapLiteral(keyType: 'String', valueType: 'dynamic', [
8656+
mapEntry(a, x.as_('int'), isKeyNullAware: true),
8657+
]),
8658+
checkNotPromoted(x),
8659+
]);
8660+
});
8661+
8662+
test('Unreachable', () {
8663+
var a = Var('a');
8664+
h.run([
8665+
declare(a, type: 'String', initializer: expr('String')),
8666+
mapLiteral(keyType: 'String', valueType: 'dynamic', [
8667+
mapEntry(a, throw_(expr('Object')), isKeyNullAware: true),
8668+
]),
8669+
checkReachable(false),
8670+
]);
8671+
});
8672+
8673+
test('Reachable', () {
8674+
var a = Var('a');
8675+
h.run([
8676+
declare(a, type: 'String?', initializer: expr('String?')),
8677+
mapLiteral(keyType: 'String', valueType: 'dynamic', [
8678+
mapEntry(a, throw_(expr('Object')), isKeyNullAware: true),
8679+
]),
8680+
checkReachable(true),
8681+
]);
8682+
});
8683+
});
8684+
86098685
group('Map pattern:', () {
86108686
test('Promotes', () {
86118687
var x = Var('x');

pkg/_fe_analyzer_shared/test/mini_ast.dart

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -299,11 +299,12 @@ Expression localFunction(List<ProtoStatement> body) {
299299
}
300300

301301
/// Creates a map entry containing the given [key] and [value] subexpressions.
302-
CollectionElement mapEntry(ProtoExpression key, ProtoExpression value) {
302+
CollectionElement mapEntry(ProtoExpression key, ProtoExpression value,
303+
{bool isKeyNullAware = false}) {
303304
var location = computeLocation();
304305
return MapEntry._(key.asExpression(location: location),
305306
value.asExpression(location: location),
306-
location: location);
307+
isKeyNullAware: isKeyNullAware, location: location);
307308
}
308309

309310
/// Creates a map literal containing the given [elements].
@@ -2527,8 +2528,10 @@ abstract class LValue extends Expression {
25272528
class MapEntry extends CollectionElement {
25282529
final Expression key;
25292530
final Expression value;
2531+
final bool isKeyNullAware;
25302532

2531-
MapEntry._(this.key, this.value, {required super.location});
2533+
MapEntry._(this.key, this.value,
2534+
{required this.isKeyNullAware, required super.location});
25322535

25332536
@override
25342537
void preVisit(PreVisitor visitor) {
@@ -2537,7 +2540,7 @@ class MapEntry extends CollectionElement {
25372540
}
25382541

25392542
@override
2540-
String toString() => '$key: $value';
2543+
String toString() => '${isKeyNullAware ? '?' : ''}$key: $value';
25412544

25422545
@override
25432546
void visit(Harness h, CollectionElementContext context) {
@@ -2550,8 +2553,11 @@ class MapEntry extends CollectionElement {
25502553
default:
25512554
keySchema = valueSchema = h.operations.unknownType;
25522555
}
2553-
h.typeAnalyzer.analyzeExpression(key, keySchema);
2556+
var keyType = h.typeAnalyzer.analyzeExpression(key, keySchema);
2557+
h.flow.nullAwareMapEntry_valueBegin(key, keyType,
2558+
isKeyNullAware: isKeyNullAware);
25542559
h.typeAnalyzer.analyzeExpression(value, valueSchema);
2560+
h.flow.nullAwareMapEntry_end(isKeyNullAware: isKeyNullAware);
25552561
h.irBuilder.apply(
25562562
'mapEntry', [Kind.expression, Kind.expression], Kind.collectionElement,
25572563
location: location);

pkg/analyzer/lib/src/generated/resolver.dart

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3128,14 +3128,18 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
31283128
{CollectionLiteralContext? context}) {
31293129
inferenceLogWriter?.enterElement(node);
31303130
checkUnreachableNode(node);
3131-
analyzeExpression(node.key,
3131+
var keyType = analyzeExpression(node.key,
31323132
SharedTypeSchemaView(context?.keyType ?? UnknownInferredType.instance));
31333133
popRewrite();
3134+
flowAnalysis.flow?.nullAwareMapEntry_valueBegin(node.key, keyType,
3135+
isKeyNullAware: node.keyQuestion != null);
31343136
analyzeExpression(
31353137
node.value,
31363138
SharedTypeSchemaView(
31373139
context?.valueType ?? UnknownInferredType.instance));
31383140
popRewrite();
3141+
flowAnalysis.flow
3142+
?.nullAwareMapEntry_end(isKeyNullAware: node.keyQuestion != null);
31393143
inferenceLogWriter?.exitElement(node);
31403144
}
31413145

pkg/front_end/lib/src/type_inference/inference_visitor.dart

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4595,9 +4595,6 @@ class InferenceVisitorImpl extends InferenceVisitorBase
45954595
Map<TreeNode, DartType> inferredSpreadTypes,
45964596
Map<Expression, DartType> inferredConditionTypes,
45974597
_MapLiteralEntryOffsets offsets) {
4598-
// TODO(cstefantsova): Make sure flow analysis is invoked here when it's
4599-
// implemented.
4600-
46014598
DartType adjustedInferredKeyType = entry.isKeyNullAware
46024599
? inferredKeyType.withDeclaredNullability(Nullability.nullable)
46034600
: inferredKeyType;
@@ -4610,6 +4607,10 @@ class InferenceVisitorImpl extends InferenceVisitorBase
46104607
.expression;
46114608
entry.key = key..parent = entry;
46124609

4610+
flowAnalysis.nullAwareMapEntry_valueBegin(
4611+
key, new SharedTypeView(keyInferenceResult.inferredType),
4612+
isKeyNullAware: entry.isKeyNullAware);
4613+
46134614
DartType adjustedInferredValueType = entry.isValueNullAware
46144615
? inferredValueType.withDeclaredNullability(Nullability.nullable)
46154616
: inferredValueType;
@@ -4627,6 +4628,8 @@ class InferenceVisitorImpl extends InferenceVisitorBase
46274628

46284629
offsets.mapEntryOffset = entry.fileOffset;
46294630

4631+
flowAnalysis.nullAwareMapEntry_end(isKeyNullAware: entry.isKeyNullAware);
4632+
46304633
return entry;
46314634
}
46324635

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
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+
test1(String a, num x) {
6+
<String, dynamic>{?a: (x as int)};
7+
x..expectStaticType<Exactly<int>>();
8+
}
9+
10+
test2(String? a, num x) {
11+
<String, dynamic>{?a: (x as int)};
12+
x..expectStaticType<Exactly<num>>();
13+
}
14+
15+
test3(String a, bool b, num x) {
16+
if (b) {
17+
x as int;
18+
} else {
19+
<String, dynamic>{?a: (throw 0)};
20+
// Unreachable.
21+
}
22+
x..expectStaticType<Exactly<int>>();
23+
}
24+
25+
test4(String? a, bool b, num x) {
26+
if (b) {
27+
x as int;
28+
} else {
29+
<String, dynamic>{?a: (throw 0)};
30+
// Reachable.
31+
}
32+
x..expectStaticType<Exactly<num>>();
33+
}
34+
35+
test5(String? a) {
36+
return {?a: a..expectStaticType<Exactly<String>>()};
37+
}
38+
39+
test6(String? a) {
40+
return {a: a..expectStaticType<Exactly<String?>>()};
41+
}
42+
43+
extension E<X> on X {
44+
void expectStaticType<Y extends Exactly<X>>() {}
45+
}
46+
47+
typedef Exactly<X> = X Function(X);
48+
49+
void expectThrows(void Function() f) {
50+
bool hasThrown;
51+
try {
52+
f();
53+
hasThrown = false;
54+
} catch (e) {
55+
hasThrown = true;
56+
}
57+
58+
if (!hasThrown) {
59+
throw "Expected the function to throw an exception.";
60+
}
61+
}
62+
63+
main() {
64+
test1("", 0);
65+
66+
test2("", 0);
67+
test2(null, 0);
68+
69+
test3("", true, 0);
70+
expectThrows(() => test3("", false, 0));
71+
72+
test4("", true, 0);
73+
expectThrows(() => test4("", false, 0));
74+
test4(null, true, 0);
75+
test4(null, false, 0);
76+
}

0 commit comments

Comments
 (0)