Skip to content

Commit 574e54d

Browse files
stereotype441Commit Queue
authored andcommitted
[_fe_analyzer_shared] Add shared logic to support null-shorting.
This change introduces a new mixin, `NullShortingMixin`, with a type parameter `Guard` for the data structure used by the client to desugar null-aware accesses. The mixin maintains a stack of these guards, and provides methods that the client can use to manipulate the stack: - `startNullShorting` adds an entry to the stack; it should be called when the client encounters the `?.` part of a null-aware expression. It also provides two hooks that the client can override if desired: - `handleNullShortingStep`, called whenever an entry is removed from the stack; this will let the CFE know when it should de-sugar a null short using a "let" expression. - `handleNullShortingFinished`, called whenever a sequence of entries is removed from the stack; this will let the analyzer know when it should change the static type of an expression as a result of null-shorting. Also, a new optional parameter, `continueNullShorting`, is added to `TypeAnalyzer.analyzeExpression`. If this parameter is `false` (the default value), then any null shorting that is started during analysis of the expression (due to the client calling `startNullShorting`) will be terminated before returning. If it is `true`, then null shorting won't be terminated, so it will extend to the containing expression. For expression types that are able to extend null shorting that appears in their target subexpression (e.g., method calls and property accesses), the `visit` or `analyze` method should pass `false` for this parameter when making a recursive call to analyze the target. Finally, the `NullShortingMixin` has a getter `nullShortingDepth`, that `TypeAnalyzer.analyzeExpression` uses to determine when null shorting should be terminated, and a method `finishNullShorting`, that actually does the work of terminating null shorting. In principle, clients don't need to invoke these parts of the `NullShortingMixin` API. However, since the CFE doesn't always use `TypeAnalyzer.analyzeExpression` (favoring its own internal methods `InferenceVisitorImpl.inferExpression` and `InferenceVisitorImpl.inferNullAwareExpression`), the CFE will need to use them. The "mini_ast" tests of flow analysis formerly used a method called `nullAwareAccess` to exercise the flow analysis effects of null-aware constructs. This was hacky and confusing, and is now unnecessary, since the shared infrastructure now fully supports null-shorting. So this method has been removed and replaced by the ability to mark a method invocation or property access as null-aware. This change only builds the infrastructure for shared analysis of null-shorting; the analyzer and CFE still handle null shorting on their own. In follow-up CLs I will change the analyzer and CFE to make use of the shared mechanism. Change-Id: Ide25a915c4d06eab751b87c1c2745749d23a8114 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/399480 Reviewed-by: Johnni Winther <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent 5eec65e commit 574e54d

File tree

14 files changed

+537
-264
lines changed

14 files changed

+537
-264
lines changed

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

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5+
/// @docImport 'package:_fe_analyzer_shared/src/type_inference/null_shorting.dart';
6+
library;
7+
58
import 'package:_fe_analyzer_shared/src/types/shared_type.dart';
69
import 'package:meta/meta.dart';
710

@@ -188,7 +191,8 @@ class ExpressionPropertyTarget<Expression extends Object>
188191
/// analysis knows that the "is" expression is *not* an immediate child of the
189192
/// "if" statement, so therefore no type promotion should occur.
190193
abstract class FlowAnalysis<Node extends Object, Statement extends Node,
191-
Expression extends Node, Variable extends Object, Type extends Object> {
194+
Expression extends Node, Variable extends Object, Type extends Object>
195+
implements FlowAnalysisNullShortingInterface<Expression, Type> {
192196
factory FlowAnalysis(
193197
FlowAnalysisOperations<Variable, Type> operations,
194198
AssignedVariables<Node, Variable> assignedVariables, {
@@ -733,26 +737,6 @@ abstract class FlowAnalysis<Node extends Object, Statement extends Node,
733737
/// expression.
734738
void nonNullAssert_end(Expression operand);
735739

736-
/// Call this method after visiting an expression using `?.`.
737-
void nullAwareAccess_end();
738-
739-
/// Call this method after visiting a null-aware operator such as `?.`,
740-
/// `?..`, `?.[`, or `?..[`.
741-
///
742-
/// [target] should be the expression just before the null-aware operator, or
743-
/// `null` if the null-aware access starts a cascade section.
744-
///
745-
/// [targetType] should be the type of the expression just before the
746-
/// null-aware operator, and should be non-null even if the null-aware access
747-
/// starts a cascade section.
748-
///
749-
/// Note that [nullAwareAccess_end] should be called after the conclusion
750-
/// of any null-shorting that is caused by the `?.`. So, for example, if the
751-
/// code being analyzed is `x?.y?.z(x)`, [nullAwareAccess_rightBegin] should
752-
/// be called once upon reaching each `?.`, but [nullAwareAccess_end] should
753-
/// not be called until after processing the method call to `z(x)`.
754-
void nullAwareAccess_rightBegin(Expression? target, Type targetType);
755-
756740
/// Call this method after visiting the value of a null-aware map entry.
757741
void nullAwareMapEntry_end({required bool isKeyNullAware});
758742

@@ -2115,6 +2099,34 @@ class FlowAnalysisDebug<Node extends Object, Statement extends Node,
21152099
}
21162100
}
21172101

2102+
/// Flow analysis interface methods used by [NullShortingMixin].
2103+
///
2104+
/// These are separated from [FlowAnalysis] in order to isolate
2105+
/// [NullShortingMixin] from the type parameters of [FlowAnalysis] that aren't
2106+
/// relevant to it.
2107+
abstract interface class FlowAnalysisNullShortingInterface<
2108+
Expression extends Object, Type extends Object> {
2109+
/// Call this method after visiting an expression using `?.`.
2110+
void nullAwareAccess_end();
2111+
2112+
/// Call this method after visiting a null-aware operator such as `?.`,
2113+
/// `?..`, `?.[`, or `?..[`.
2114+
///
2115+
/// [target] should be the expression just before the null-aware operator, or
2116+
/// `null` if the null-aware access starts a cascade section.
2117+
///
2118+
/// [targetType] should be the type of the expression just before the
2119+
/// null-aware operator, and should be non-null even if the null-aware access
2120+
/// starts a cascade section.
2121+
///
2122+
/// Note that [nullAwareAccess_end] should be called after the conclusion
2123+
/// of any null-shorting that is caused by the `?.`. So, for example, if the
2124+
/// code being analyzed is `x?.y?.z(x)`, [nullAwareAccess_rightBegin] should
2125+
/// be called once upon reaching each `?.`, but [nullAwareAccess_end] should
2126+
/// not be called until after processing the method call to `z(x)`.
2127+
void nullAwareAccess_rightBegin(Expression? target, Type targetType);
2128+
}
2129+
21182130
/// An instance of the [FlowModel] class represents the information gathered by
21192131
/// flow analysis at a single point in the control flow of the function or
21202132
/// method being analyzed.

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
/// @docImport 'flow_analysis.dart';
66
/// @docImport '../field_promotability.dart';
7+
/// @docImport '../type_inference/type_analyzer_operations.dart';
78
library;
89

910
/// Callback API used by flow analysis to query and manipulate the client's
@@ -84,6 +85,16 @@ abstract interface class FlowAnalysisTypeOperations<Type extends Object> {
8485
/// Returns `true` if [type] is a reference to a type parameter.
8586
bool isTypeParameterType(Type type);
8687

88+
/// Computes the nullable form of [type], in other words the least upper bound
89+
/// of [type] and `Null`.
90+
///
91+
/// The concrete classes implementing [TypeAnalyzerOperations] should mix in
92+
/// [TypeAnalyzerOperationsMixin] and implement
93+
/// [TypeAnalyzerOperations.makeNullableInternal] to receive a concrete
94+
/// implementation of [makeNullable] instead of implementing [makeNullable]
95+
/// directly.
96+
Type makeNullable(Type type);
97+
8798
/// Returns the non-null promoted version of [type].
8899
///
89100
/// Note that some types don't have a non-nullable version (e.g.
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
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+
/// @docImport 'type_analyzer.dart';
6+
/// @docImport 'type_analyzer_operations.dart';
7+
library;
8+
9+
import '../flow_analysis/flow_analysis.dart';
10+
import '../flow_analysis/flow_analysis_operations.dart';
11+
12+
/// Null shorting logic to be shared between the analyzer and the CFE.
13+
///
14+
/// This mixin should be used by the same class that mixes in [TypeAnalyzer].
15+
///
16+
/// The type parameter [Guard] should be instantiated with the data structure
17+
/// used by the client to desugar null-aware accesses. (The analyzer can
18+
/// instantiate this with `Null`, since it doesn't do desugaring.)
19+
mixin NullShortingMixin<Guard, Expression extends Object, Type extends Object>
20+
on TypeAnalysisNullShortingInterface<Expression, Type> {
21+
/// Stack of [Guard] objects associated with null-shorting operations that
22+
/// haven't been terminated yet.
23+
final _guards = <Guard>[];
24+
25+
@override
26+
int get nullShortingDepth => _guards.length;
27+
28+
@override
29+
Type finishNullShorting(int targetDepth, Type inferredType) {
30+
assert(targetDepth < nullShortingDepth);
31+
inferredType = operations.makeNullable(inferredType);
32+
do {
33+
// End non-nullable promotion of the null-aware variable.
34+
flow.nullAwareAccess_end();
35+
handleNullShortingStep(_guards.removeLast(), inferredType);
36+
} while (nullShortingDepth > targetDepth);
37+
handleNullShortingFinished(inferredType);
38+
return inferredType;
39+
}
40+
41+
/// Hook called by [finishNullShorting] after terminating all the null
42+
/// shorting that needs to be terminated for a given expression.
43+
///
44+
/// [inferredType] is the (nullable) type of the final expression.
45+
void handleNullShortingFinished(Type inferredType) {}
46+
47+
/// Hook called by [finishNullShorting] after terminating a single
48+
/// null-shorting operation.
49+
///
50+
/// [guard] is the value that was passed to [startNullShorting] when the
51+
/// null-shorting operation was started.
52+
///
53+
/// [inferredType] is the (nullable) type of the final expression.
54+
void handleNullShortingStep(Guard guard, Type inferredType) {}
55+
56+
/// Starts null shorting for a null-aware expression that participates in
57+
/// null-shorting.
58+
///
59+
/// [target] should be the target of the null-aware operation (the expression
60+
/// to the left of the `?.`), and [targetType] should be its static type.
61+
///
62+
/// [guard] should be the data structure that will be used by the client to
63+
/// desugar the null-aware access. It will be passed to
64+
/// [handleNullShortingStep] when the null shorting for this particular
65+
/// null-aware expression is terminated.
66+
void startNullShorting(Guard guard, Expression target, Type targetType) {
67+
// Ensure the initializer of [_nullAwareVariable] is promoted to
68+
// non-nullable.
69+
flow.nullAwareAccess_rightBegin(target, targetType);
70+
_guards.add(guard);
71+
}
72+
}
73+
74+
abstract interface class TypeAnalysisNullShortingInterface<
75+
Expression extends Object, Type extends Object> {
76+
/// Returns the client's [FlowAnalysis] object.
77+
FlowAnalysisNullShortingInterface<Expression, Type> get flow;
78+
79+
/// Returns the number of null-shorting operations that haven't been
80+
/// terminated yet.
81+
int get nullShortingDepth;
82+
83+
/// The [FlowAnalysisTypeOperations], used to access types and check
84+
/// subtyping.
85+
///
86+
/// Typically this will be an instance of [TypeAnalyzerOperations].
87+
FlowAnalysisTypeOperations<Type> get operations;
88+
89+
/// Terminates one or more null-shorting operations that were previously
90+
/// started using [NullShortingMixin.startNullShorting].
91+
///
92+
/// This method should be called at the point where the null-shorting flow
93+
/// control path rejoins with the main flow control path. For example, when
94+
/// analyzing the expression `i?.toString()`, this method should be called
95+
/// after analyzing the call to `toString()`.
96+
///
97+
/// [targetDepth] should be a value previously returned by
98+
/// [nullShortingDepth]. Null-shorting operations will be terminated until the
99+
/// null-shorting depth returns to this value. Caller is required to pass in
100+
/// a value that is strictly less than [nullShortingDepth] (that is, there
101+
/// must be at least one null-shorting operation that needs to be terminated).
102+
///
103+
/// [inferredType] should be the type of the expression that was just
104+
/// analyzed, prior to termination of null shorting. For example, if the
105+
/// expression that is being analyzed is `i?.toString()`, [inferredType]
106+
/// should represent the non-nullable type `String`.
107+
///
108+
/// The return value represents the type of the full expression, after
109+
/// termination of null shorting. For example, if the expression that is being
110+
/// analyzed is `i?.toString()`, the return value will represent the type
111+
/// `String?`.
112+
Type finishNullShorting(int targetDepth, Type inferredType);
113+
}

pkg/_fe_analyzer_shared/lib/src/type_inference/type_analysis_result.dart

Lines changed: 11 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -65,28 +65,15 @@ class DeclaredVariablePatternResult<
6565

6666
/// Container for the result of running type analysis on an expression.
6767
///
68-
/// This class keeps track of a provisional type of the expression (prior to
69-
/// resolving null shorting) as well as the information necessary to resolve
70-
/// null shorting.
71-
abstract class ExpressionTypeAnalysisResult<
68+
/// This class keeps track of the type of the expression. Derived classes expose
69+
/// other results of type analysis that are specific to certain expression
70+
/// types.
71+
class ExpressionTypeAnalysisResult<
7272
TypeStructure extends SharedTypeStructure<TypeStructure>> {
73-
/// Type of the expression before resolving null shorting.
74-
///
75-
/// For example, if `this` is the result of analyzing `(... as int?)?.isEven`,
76-
/// [provisionalType] will be `bool`, because the `isEven` getter returns
77-
/// `bool`, and it is not yet known (until looking at the surrounding code)
78-
/// whether there will be additional selectors after `isEven` that should act
79-
/// on the `bool` type.
80-
SharedTypeView<TypeStructure> get provisionalType;
81-
82-
/// Resolves any pending null shorting. For example, if `this` is the result
83-
/// of analyzing `(... as int?)?.isEven`, then calling [resolveShorting] will
84-
/// cause the `?.` to be desugared (if code generation is occurring) and will
85-
/// return the type `bool?`.
86-
///
87-
/// TODO(paulberry): document what calls back to the client might be made by
88-
/// invoking this method.
89-
SharedTypeView<TypeStructure> resolveShorting();
73+
/// The static type of the expression.
74+
final SharedTypeView<TypeStructure> type;
75+
76+
ExpressionTypeAnalysisResult({required this.type});
9077
}
9178

9279
/// Result for analyzing an if-case statement or element in
@@ -112,7 +99,7 @@ class IfCaseStatementResult<
11299
/// Container for the result of running type analysis on an integer literal.
113100
class IntTypeAnalysisResult<
114101
TypeStructure extends SharedTypeStructure<TypeStructure>>
115-
extends SimpleTypeAnalysisResult<TypeStructure> {
102+
extends ExpressionTypeAnalysisResult<TypeStructure> {
116103
/// Whether the integer literal was converted to a double.
117104
final bool convertedToDouble;
118105

@@ -316,7 +303,7 @@ class ObjectPatternResult<
316303
/// Container for the result of running type analysis on a pattern assignment.
317304
class PatternAssignmentAnalysisResult<
318305
TypeStructure extends SharedTypeStructure<TypeStructure>>
319-
extends SimpleTypeAnalysisResult<TypeStructure> {
306+
extends ExpressionTypeAnalysisResult<TypeStructure> {
320307
/// The type schema of the pattern on the left hand size of the assignment.
321308
final SharedTypeSchemaView<TypeStructure> patternSchema;
322309

@@ -422,28 +409,11 @@ class RelationalPatternResult<
422409
required super.matchedValueType});
423410
}
424411

425-
/// Container for the result of running type analysis on an expression that does
426-
/// not contain any null shorting.
427-
class SimpleTypeAnalysisResult<
428-
TypeStructure extends SharedTypeStructure<TypeStructure>>
429-
implements ExpressionTypeAnalysisResult<TypeStructure> {
430-
/// The static type of the expression.
431-
final SharedTypeView<TypeStructure> type;
432-
433-
SimpleTypeAnalysisResult({required this.type});
434-
435-
@override
436-
SharedTypeView<TypeStructure> get provisionalType => type;
437-
438-
@override
439-
SharedTypeView<TypeStructure> resolveShorting() => type;
440-
}
441-
442412
/// Result for analyzing a switch expression in
443413
/// [TypeAnalyzer.analyzeSwitchExpression].
444414
class SwitchExpressionResult<
445415
TypeStructure extends SharedTypeStructure<TypeStructure>,
446-
Error> extends SimpleTypeAnalysisResult<TypeStructure> {
416+
Error> extends ExpressionTypeAnalysisResult<TypeStructure> {
447417
/// Errors for non-bool guards.
448418
///
449419
/// The key is the case index of the erroneous guard.

pkg/_fe_analyzer_shared/lib/src/type_inference/type_analyzer.dart

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import '../flow_analysis/flow_analysis.dart';
66
import '../types/shared_type.dart';
7+
import 'null_shorting.dart';
78
import 'type_analysis_result.dart';
89
import 'type_analyzer_operations.dart';
910

@@ -262,25 +263,29 @@ class SwitchStatementMemberInfo<Node extends Object, Statement extends Node,
262263
/// of each entry in order to verify that when an entity is popped, it has the
263264
/// expected kind.
264265
mixin TypeAnalyzer<
265-
TypeStructure extends SharedTypeStructure<TypeStructure>,
266-
Node extends Object,
267-
Statement extends Node,
268-
Expression extends Node,
269-
Variable extends Object,
270-
Pattern extends Node,
271-
Error,
272-
TypeParameterStructure extends SharedTypeParameterStructure<TypeStructure>,
273-
TypeDeclarationType extends Object,
274-
TypeDeclaration extends Object> {
266+
TypeStructure extends SharedTypeStructure<TypeStructure>,
267+
Node extends Object,
268+
Statement extends Node,
269+
Expression extends Node,
270+
Variable extends Object,
271+
Pattern extends Node,
272+
Error,
273+
// Work around https://github.com/dart-lang/dart_style/issues/1568
274+
// ignore: lines_longer_than_80_chars
275+
TypeParameterStructure extends SharedTypeParameterStructure<TypeStructure>,
276+
TypeDeclarationType extends Object,
277+
TypeDeclaration extends Object>
278+
implements
279+
TypeAnalysisNullShortingInterface<Expression,
280+
SharedTypeView<TypeStructure>> {
275281
TypeAnalyzerErrors<Node, Statement, Expression, Variable,
276282
SharedTypeView<TypeStructure>, Pattern, Error> get errors;
277283

278-
/// Returns the client's [FlowAnalysis] object.
284+
@override
279285
FlowAnalysis<Node, Statement, Expression, Variable,
280286
SharedTypeView<TypeStructure>> get flow;
281287

282-
/// The [TypeAnalyzerOperations], used to access types, check subtyping, and
283-
/// query variable types.
288+
@override
284289
TypeAnalyzerOperations<TypeStructure, Variable, TypeParameterStructure,
285290
TypeDeclarationType, TypeDeclaration> get operations;
286291

@@ -553,20 +558,36 @@ mixin TypeAnalyzer<
553558
/// Analyzes an expression. [node] is the expression to analyze, and
554559
/// [schema] is the type schema which should be used for type inference.
555560
///
561+
/// If [continueNullShorting] is `false` (the default), then any null shorting
562+
/// that starts inside [node] will be terminated, and the returned type will
563+
/// be nullable, to reflect the fact that null-aware expressions might
564+
/// evaluate to `null`.
565+
///
566+
/// If [continueNullShorting] is `true`, then null shorting that starts inside
567+
/// [node] will be allowed to continue into the containing expression.
568+
///
556569
/// Stack effect: pushes (Expression).
557570
SharedTypeView<TypeStructure> analyzeExpression(
558-
Expression node, SharedTypeSchemaView<TypeStructure> schema) {
571+
Expression node, SharedTypeSchemaView<TypeStructure> schema,
572+
{bool continueNullShorting = false}) {
573+
int? nullShortingTargetDepth;
574+
if (!continueNullShorting) nullShortingTargetDepth = nullShortingDepth;
559575
// Stack: ()
560576
if (schema is SharedDynamicTypeSchemaView<TypeStructure>) {
561577
schema = operations.unknownType;
562578
}
563579
ExpressionTypeAnalysisResult<TypeStructure> result =
564580
dispatchExpression(node, schema);
565581
// Stack: (Expression)
566-
if (operations.isNever(result.provisionalType)) {
582+
if (operations.isNever(result.type)) {
567583
flow.handleExit();
568584
}
569-
return result.resolveShorting();
585+
SharedTypeView<TypeStructure> type = result.type;
586+
if (nullShortingTargetDepth != null &&
587+
nullShortingDepth > nullShortingTargetDepth) {
588+
type = finishNullShorting(nullShortingTargetDepth, type);
589+
}
590+
return type;
570591
}
571592

572593
/// Analyzes a collection element of the form

0 commit comments

Comments
 (0)