Skip to content

Commit e64c60d

Browse files
stereotype441Commit Queue
authored and
Commit Queue
committed
Flow analysis: replace context toString methods with a less fragile mechanism.
Adds getters `_debugFields` and `_debugType` to the `_FlowContext` base class, and adds a single implementation of `_FlowContext.toString` that builds a representation of the context based on them. This replaces the implementations of `toString` in all the classes derived from `_FlowContext`, which were more difficult to get right and keep synchronized with code changes. Also changes `_TryFinallyContext` from a `late final` variable to a nullable variable. This sacrifices a tiny bit of safety, but has the advantage of allowing `toString` to show the value if it's been initialized, and avoid crashing if it hasn't. Change-Id: I0ca3ddfed21934c54eaba912c84edda79c8eadfc Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/276202 Reviewed-by: Johnni Winther <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent 8e1f576 commit e64c60d

File tree

1 file changed

+131
-43
lines changed

1 file changed

+131
-43
lines changed

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

Lines changed: 131 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -3261,8 +3261,11 @@ class _AssertContext<Type extends Object> extends _SimpleContext<Type> {
32613261
_AssertContext(super.previous);
32623262

32633263
@override
3264-
String toString() =>
3265-
'_AssertContext(previous: $_previous, conditionInfo: $_conditionInfo)';
3264+
Map<String, Object?> get _debugFields =>
3265+
super._debugFields..['conditionInfo'] = _conditionInfo;
3266+
3267+
@override
3268+
String get _debugType => '_AssertContext';
32663269
}
32673270

32683271
/// [_FlowContext] representing a language construct that branches on a boolean
@@ -3275,7 +3278,11 @@ class _BranchContext<Type extends Object> extends _FlowContext {
32753278
_BranchContext(this._branchModel);
32763279

32773280
@override
3278-
String toString() => '_BranchContext(branchModel: $_branchModel)';
3281+
Map<String, Object?> get _debugFields =>
3282+
super._debugFields..['branchModel'] = _branchModel;
3283+
3284+
@override
3285+
String get _debugType => '_BranchContext';
32793286
}
32803287

32813288
/// [_FlowContext] representing a language construct that can be targeted by
@@ -3298,8 +3305,13 @@ class _BranchTargetContext<Type extends Object> extends _FlowContext {
32983305
_BranchTargetContext(this._checkpoint);
32993306

33003307
@override
3301-
String toString() => '_BranchTargetContext(breakModel: $_breakModel, '
3302-
'continueModel: $_continueModel, checkpoint: $_checkpoint)';
3308+
Map<String, Object?> get _debugFields => super._debugFields
3309+
..['breakModel'] = _breakModel
3310+
..['continueModel'] = _continueModel
3311+
..['checkpoint'] = _checkpoint;
3312+
3313+
@override
3314+
String get _debugType => '_BranchTargetContext';
33033315
}
33043316

33053317
/// [_FlowContext] representing a conditional expression.
@@ -3311,8 +3323,11 @@ class _ConditionalContext<Type extends Object> extends _BranchContext<Type> {
33113323
_ConditionalContext(super._branchModel);
33123324

33133325
@override
3314-
String toString() => '_ConditionalContext(branchModel: $_branchModel, '
3315-
'thenInfo: $_thenInfo)';
3326+
Map<String, Object?> get _debugFields =>
3327+
super._debugFields..['thenInfo'] = _thenInfo;
3328+
3329+
@override
3330+
String get _debugType => '_ConditionalContext';
33163331
}
33173332

33183333
/// Data structure representing the result of demoting a variable from one type
@@ -4320,7 +4335,7 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
43204335
_TryFinallyContext<Type> context =
43214336
_stack.removeLast() as _TryFinallyContext<Type>;
43224337
_current = context._afterBodyAndCatches!
4323-
.attachFinally(operations, context._beforeFinally, _current);
4338+
.attachFinally(operations, context._beforeFinally!, _current);
43244339
}
43254340

43264341
@override
@@ -4660,15 +4675,51 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
46604675

46614676
/// Base class for objects representing constructs in the Dart programming
46624677
/// language for which flow analysis information needs to be tracked.
4663-
abstract class _FlowContext {}
4678+
abstract class _FlowContext {
4679+
_FlowContext() {
4680+
assert(() {
4681+
// Check that `_debugType` has been overridden in a way that reflects the
4682+
// class name. Note that this assumes the behavior of `runtimeType` in
4683+
// the VM, but that's ok, because this code is only active when asserts
4684+
// are enabled, and we only run unit tests on the VM.
4685+
String expectedDebugType = runtimeType.toString();
4686+
int lessThanIndex = expectedDebugType.indexOf('<');
4687+
if (lessThanIndex > 0) {
4688+
expectedDebugType = expectedDebugType.substring(0, lessThanIndex);
4689+
}
4690+
assert(_debugType == expectedDebugType,
4691+
'Expected a debug type of $expectedDebugType, got $_debugType');
4692+
return true;
4693+
}());
4694+
}
4695+
4696+
/// Returns a freshly allocated map whose keys are the names of fields in the
4697+
/// class, and whose values are the values of those fields.
4698+
///
4699+
/// This is used by [toString] to print out information for debugging.
4700+
Map<String, Object?> get _debugFields => {};
4701+
4702+
/// Returns a string representation of the class name. This is used by
4703+
/// [toString] to print out information for debugging.
4704+
String get _debugType;
4705+
4706+
@override
4707+
String toString() {
4708+
List<String> fields = [
4709+
for (MapEntry<String, Object?> entry in _debugFields.entries)
4710+
if (entry.value != null) '${entry.key}: ${entry.value}'
4711+
];
4712+
return '$_debugType(${fields.join(', ')})';
4713+
}
4714+
}
46644715

46654716
/// [_FlowContext] representing a function expression.
46664717
class _FunctionExpressionContext<Type extends Object>
46674718
extends _SimpleContext<Type> {
46684719
_FunctionExpressionContext(super.previous);
46694720

46704721
@override
4671-
String toString() => '_FunctionExpressionContext(previous: $_previous)';
4722+
String get _debugType => '_FunctionExpressionContext';
46724723
}
46734724

46744725
/// [_FlowContext] representing an `if` statement.
@@ -4680,8 +4731,11 @@ class _IfContext<Type extends Object> extends _BranchContext<Type> {
46804731
_IfContext(super._branchModel);
46814732

46824733
@override
4683-
String toString() =>
4684-
'_IfContext(branchModel: $_branchModel, afterThen: $_afterThen)';
4734+
Map<String, Object?> get _debugFields =>
4735+
super._debugFields..['afterThen'] = _afterThen;
4736+
4737+
@override
4738+
String get _debugType => '_IfContext';
46854739
}
46864740

46874741
/// [_FlowContext] representing an "if-null" (`??`) expression.
@@ -4693,7 +4747,11 @@ class _IfNullExpressionContext<Type extends Object> extends _FlowContext {
46934747
_IfNullExpressionContext(this._shortcutState);
46944748

46954749
@override
4696-
String toString() => '_IfNullExpressionContext($_shortcutState)';
4750+
Map<String, Object?> get _debugFields =>
4751+
super._debugFields..['shortcutState'] = _shortcutState;
4752+
4753+
@override
4754+
String get _debugType => '_IfNullExpressionContext';
46974755
}
46984756

46994757
/// Contextual information tracked by legacy type promotion about a binary "and"
@@ -5346,7 +5404,7 @@ class _NullAwareAccessContext<Type extends Object>
53465404
_NullAwareAccessContext(super.previous);
53475405

53485406
@override
5349-
String toString() => '_NullAwareAccessContext(previous: $_previous)';
5407+
String get _debugType => '_NullAwareAccessContext';
53505408
}
53515409

53525410
/// [ExpressionInfo] representing a `null` literal.
@@ -5392,10 +5450,12 @@ class _OrPatternContext<Type extends Object> extends _PatternContext<Type> {
53925450
this._previousUnmatched);
53935451

53945452
@override
5395-
String toString() =>
5396-
'_OrPatternContext(matchedValueReference: $_matchedValueReference, '
5397-
'matchedValueType: $_matchedValueType, '
5398-
'previousUnmatched: $_previousUnmatched, lhsMatched: $_lhsMatched)';
5453+
Map<String, Object?> get _debugFields => super._debugFields
5454+
..['previousUnmatched'] = _previousUnmatched
5455+
..['lhsMatched'] = _lhsMatched;
5456+
5457+
@override
5458+
String get _debugType => '_OrPatternContext';
53995459
}
54005460

54015461
/// [_FlowContext] representing a pattern.
@@ -5409,9 +5469,12 @@ class _PatternContext<Type extends Object> extends _FlowContext {
54095469
_PatternContext(this._matchedValueReference, this._matchedValueType);
54105470

54115471
@override
5412-
String toString() =>
5413-
'_PatternContext(matchedValueReference: $_matchedValueReference, '
5414-
'matchedValueType: $_matchedValueType)';
5472+
Map<String, Object?> get _debugFields => super._debugFields
5473+
..['matchedValueReference'] = _matchedValueReference
5474+
..['matchedValueType'] = _matchedValueType;
5475+
5476+
@override
5477+
String get _debugType => '_PatternContext';
54155478
}
54165479

54175480
/// [ReferenceWithType] object representing a property get.
@@ -5451,10 +5514,13 @@ class _ScrutineeContext<Type extends Object> extends _FlowContext {
54515514
required this.previousScrutineeType});
54525515

54535516
@override
5454-
String toString() => '_ScrutineeContext('
5455-
'previousScrutineeReference: $previousScrutineeReference, '
5456-
'previousScrutineeSsaNode: $previousScrutineeSsaNode, '
5457-
'previousScrutineeType: $previousScrutineeType)';
5517+
Map<String, Object?> get _debugFields => super._debugFields
5518+
..['previousScrutineeReference'] = previousScrutineeReference
5519+
..['previousScrutineeSsaNode'] = previousScrutineeSsaNode
5520+
..['previousScrutineeType'] = previousScrutineeType;
5521+
5522+
@override
5523+
String get _debugType => '_ScrutineeContext';
54585524
}
54595525

54605526
/// [_FlowContext] representing a language construct for which flow analysis
@@ -5467,6 +5533,10 @@ abstract class _SimpleContext<Type extends Object> extends _FlowContext {
54675533
final FlowModel<Type> _previous;
54685534

54695535
_SimpleContext(this._previous);
5536+
5537+
@override
5538+
Map<String, Object?> get _debugFields =>
5539+
super._debugFields..['previous'] = _previous;
54705540
}
54715541

54725542
/// [_FlowContext] representing a language construct that can be targeted by
@@ -5483,9 +5553,11 @@ class _SimpleStatementContext<Type extends Object>
54835553
_SimpleStatementContext(super.checkpoint, this._previous);
54845554

54855555
@override
5486-
String toString() => '_SimpleStatementContext(breakModel: $_breakModel, '
5487-
'continueModel: $_continueModel, previous: $_previous, '
5488-
'checkpoint: $_checkpoint)';
5556+
Map<String, Object?> get _debugFields =>
5557+
super._debugFields..['previous'] = _previous;
5558+
5559+
@override
5560+
String get _debugType => '_SimpleStatementContext';
54895561
}
54905562

54915563
class _SwitchAlternativesContext<Type extends Object> extends _FlowContext {
@@ -5496,8 +5568,12 @@ class _SwitchAlternativesContext<Type extends Object> extends _FlowContext {
54965568
_SwitchAlternativesContext(this._previous);
54975569

54985570
@override
5499-
String toString() => '_SwitchAlternativesContext(previous: $_previous, '
5500-
'combinedModel: $_combinedModel)';
5571+
Map<String, Object?> get _debugFields => super._debugFields
5572+
..['previous'] = _previous
5573+
..['combinedModel'] = _combinedModel;
5574+
5575+
@override
5576+
String get _debugType => '_SwitchAlternativesContext';
55015577
}
55025578

55035579
/// [_FlowContext] representing a switch statement.
@@ -5510,19 +5586,19 @@ class _SwitchStatementContext<Type extends Object>
55105586
super.checkpoint, super._previous, this._scrutineeType);
55115587

55125588
@override
5513-
String toString() => '_SwitchStatementContext(breakModel: $_breakModel, '
5514-
'continueModel: $_continueModel, previous: $_previous, '
5515-
'checkpoint: $_checkpoint, scrutineeType: $_scrutineeType)';
5589+
Map<String, Object?> get _debugFields =>
5590+
super._debugFields..['scrutineeType'] = _scrutineeType;
5591+
5592+
@override
5593+
String get _debugType => '_SwitchStatementContext';
55165594
}
55175595

55185596
/// [_FlowContext] representing the top level of a pattern syntax tree.
55195597
class _TopPatternContext<Type extends Object> extends _PatternContext<Type> {
55205598
_TopPatternContext(super._matchedValueReference, super.matchedValueType);
55215599

55225600
@override
5523-
String toString() =>
5524-
'_TopPatternContext(matchedValueReference: $_matchedValueReference, '
5525-
'matchedValueType: $_matchedValueType)';
5601+
String get _debugType => '_TopPatternContext';
55265602
}
55275603

55285604
/// Specialization of [ExpressionInfo] for the case where the information we
@@ -5566,17 +5642,27 @@ class _TryContext<Type extends Object> extends _SimpleContext<Type> {
55665642
_TryContext(super.previous);
55675643

55685644
@override
5569-
String toString() =>
5570-
'_TryContext(previous: $_previous, beforeCatch: $_beforeCatch, '
5571-
'afterBodyAndCatches: $_afterBodyAndCatches)';
5645+
Map<String, Object?> get _debugFields => super._debugFields
5646+
..['beforeCatch'] = _beforeCatch
5647+
..['afterBodyAndCatches'] = '_afterBodyAndCatches';
5648+
5649+
@override
5650+
String get _debugType => '_TryContext';
55725651
}
55735652

55745653
class _TryFinallyContext<Type extends Object> extends _TryContext<Type> {
55755654
/// The flow model representing program state at the top of the `finally`
55765655
/// block.
5577-
late final FlowModel<Type> _beforeFinally;
5656+
FlowModel<Type>? _beforeFinally;
55785657

55795658
_TryFinallyContext(super.previous);
5659+
5660+
@override
5661+
Map<String, Object?> get _debugFields =>
5662+
super._debugFields..['beforeFinally'] = _beforeFinally;
5663+
5664+
@override
5665+
String get _debugType => '_TryFinallyContext';
55805666
}
55815667

55825668
/// [_FlowContext] representing a `while` loop (or a C-style `for` loop, which
@@ -5588,7 +5674,9 @@ class _WhileContext<Type extends Object> extends _BranchTargetContext<Type> {
55885674
_WhileContext(super.checkpoint, this._conditionInfo);
55895675

55905676
@override
5591-
String toString() => '_WhileContext(breakModel: $_breakModel, '
5592-
'continueModel: $_continueModel, conditionInfo: $_conditionInfo, '
5593-
'checkpoint: $_checkpoint)';
5677+
Map<String, Object?> get _debugFields =>
5678+
super._debugFields..['conditionInfo'] = _conditionInfo;
5679+
5680+
@override
5681+
String get _debugType => '_WhileContext';
55945682
}

0 commit comments

Comments
 (0)