Skip to content

Commit 0815445

Browse files
stereotype441Commit Queue
authored andcommitted
[_fe_analyzer_shared] Use TypeAnalyzerOptions to configure flow analysis.
Previously, flow analysis was configured by passing a set of named booleans to its constructor, each to enable or disable a separate language feature. This change merges the flow analysis configuration with the `TypeAnalyzerOptions` class (which was already being used for configuring the shared `TypeAnalyzer` class). This should make it easier to add language features to the shared code base in the future, since there will be one common place where all features will be configured. To avoid duplicating the logic that creates `TypeAnalyzerOptions`, I've had to do a bit of minor surgery to the clients: - In the test harness in `_fe_analyzer_shared`, there is a common method (`Harness.computeTypeAnalyzerOptions`) that constructs `TypeAnalyzerOptions` based on the harness configuration. It is called from a few different tests. - In `pkg/analyzer`, there is a common method (`computeTypeAnalyzerOptions`) that constructs `TypeAnalyzerOptions` based on a `FeatureSet`. It is used by `LibraryAnalyzer.analyzeForCompletion`, `LibraryAnalyzer._resolveFile`, and the late variable `AstResolver._typeAnalyzerOptions`. - In `pkg/front_end`, I've moved computation of `TypeAnalyzerOptions` from the `InferenceVisitorImpl` constructor to the `TypeInferrerImpl` constructor; the options are then passed to the `InferenceVisitorImpl` by `_createInferenceVisitor`. I'm doing this work now as preparation for adding support for sound flow analysis (#60438), so that I can add the logic to enable it in a clean way. Bug: #60438 Change-Id: Ib845194adb404b4c0a3feeff17a14ae641d515eb Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/419940 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent 437bf92 commit 0815445

File tree

11 files changed

+139
-137
lines changed

11 files changed

+139
-137
lines changed

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

Lines changed: 24 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
/// @docImport 'package:_fe_analyzer_shared/src/type_inference/null_shorting.dart';
66
library;
77

8+
import 'package:_fe_analyzer_shared/src/type_inference/type_analyzer.dart';
89
import 'package:_fe_analyzer_shared/src/types/shared_type.dart';
910
import 'package:meta/meta.dart';
1011

@@ -195,18 +196,10 @@ abstract class FlowAnalysis<Node extends Object, Statement extends Node,
195196
factory FlowAnalysis(
196197
FlowAnalysisOperations<Variable, Type> operations,
197198
AssignedVariables<Node, Variable> assignedVariables, {
198-
required bool respectImplicitlyTypedVarInitializers,
199-
required bool fieldPromotionEnabled,
200-
required bool inferenceUpdate4Enabled,
199+
required TypeAnalyzerOptions typeAnalyzerOptions,
201200
}) {
202-
return new _FlowAnalysisImpl(
203-
operations,
204-
assignedVariables,
205-
respectImplicitlyTypedVarInitializers:
206-
respectImplicitlyTypedVarInitializers,
207-
fieldPromotionEnabled: fieldPromotionEnabled,
208-
inferenceUpdate4Enabled: inferenceUpdate4Enabled,
209-
);
201+
return new _FlowAnalysisImpl(operations, assignedVariables,
202+
typeAnalyzerOptions: typeAnalyzerOptions);
210203
}
211204

212205
/// Return `true` if the current state is reachable.
@@ -1204,18 +1197,11 @@ class FlowAnalysisDebug<Node extends Object, Statement extends Node,
12041197

12051198
factory FlowAnalysisDebug(FlowAnalysisOperations<Variable, Type> operations,
12061199
AssignedVariables<Node, Variable> assignedVariables,
1207-
{required bool respectImplicitlyTypedVarInitializers,
1208-
required bool fieldPromotionEnabled,
1209-
required bool inferenceUpdate4Enabled}) {
1200+
{required TypeAnalyzerOptions typeAnalyzerOptions}) {
12101201
print('FlowAnalysisDebug()');
12111202
return new FlowAnalysisDebug._(new _FlowAnalysisImpl(
1212-
operations,
1213-
assignedVariables,
1214-
respectImplicitlyTypedVarInitializers:
1215-
respectImplicitlyTypedVarInitializers,
1216-
fieldPromotionEnabled: fieldPromotionEnabled,
1217-
inferenceUpdate4Enabled: inferenceUpdate4Enabled,
1218-
));
1203+
operations, assignedVariables,
1204+
typeAnalyzerOptions: typeAnalyzerOptions));
12191205
}
12201206

12211207
FlowAnalysisDebug._(this._wrapped);
@@ -4304,6 +4290,9 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
43044290
implements
43054291
FlowAnalysis<Node, Statement, Expression, Variable, Type>,
43064292
_PropertyTargetHelper<Expression, Type> {
4293+
/// Options affecting the behavior of flow analysis.
4294+
final TypeAnalyzerOptions typeAnalyzerOptions;
4295+
43074296
/// The [FlowAnalysisOperations], used to access types, check subtyping, and
43084297
/// query variable types.
43094298
@override
@@ -4387,17 +4376,6 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
43874376

43884377
final AssignedVariables<Node, Variable> _assignedVariables;
43894378

4390-
/// Indicates whether initializers of implicitly typed variables should be
4391-
/// accounted for by SSA analysis. (In an ideal world, they always would be,
4392-
/// but due to https://github.com/dart-lang/language/issues/1785, they weren't
4393-
/// always, and we need to be able to replicate the old behavior when
4394-
/// analyzing old language versions).
4395-
final bool respectImplicitlyTypedVarInitializers;
4396-
4397-
final bool fieldPromotionEnabled;
4398-
4399-
final bool inferenceUpdate4Enabled;
4400-
44014379
@override
44024380
final PromotionKeyStore<Variable> promotionKeyStore;
44034381

@@ -4417,13 +4395,9 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
44174395
@override
44184396
final List<_Reference<Type>> _cascadeTargetStack = [];
44194397

4420-
_FlowAnalysisImpl(
4421-
this.operations,
4422-
this._assignedVariables, {
4423-
required this.respectImplicitlyTypedVarInitializers,
4424-
required this.fieldPromotionEnabled,
4425-
required this.inferenceUpdate4Enabled,
4426-
}) : promotionKeyStore = _assignedVariables.promotionKeyStore {
4398+
_FlowAnalysisImpl(this.operations, this._assignedVariables,
4399+
{required this.typeAnalyzerOptions})
4400+
: promotionKeyStore = _assignedVariables.promotionKeyStore {
44274401
if (!_assignedVariables.isFinished) {
44284402
_assignedVariables.finish();
44294403
}
@@ -4994,7 +4968,7 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
49944968

49954969
@override
49964970
bool isFinal(int variableKey) {
4997-
if (!inferenceUpdate4Enabled) return false;
4971+
if (!typeAnalyzerOptions.inferenceUpdate4Enabled) return false;
49984972
Variable? variable = promotionKeyStore.variableForKey(variableKey);
49994973
if (variable != null && operations.isFinal(variable)) return true;
50004974
return false;
@@ -5899,12 +5873,14 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
58995873
result[type] = whyNotPromotable == null
59005874
? new PropertyNotPromotedForNonInherentReason(
59015875
reference.propertyName, propertyMember,
5902-
fieldPromotionEnabled: fieldPromotionEnabled)
5876+
fieldPromotionEnabled:
5877+
typeAnalyzerOptions.fieldPromotionEnabled)
59035878
: new PropertyNotPromotedForInherentReason(
59045879
reference.propertyName,
59055880
propertyMember,
59065881
whyNotPromotable,
5907-
fieldPromotionEnabled: fieldPromotionEnabled);
5882+
fieldPromotionEnabled:
5883+
typeAnalyzerOptions.fieldPromotionEnabled);
59085884
}
59095885
}
59105886
return result;
@@ -6032,7 +6008,7 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
60326008
// Find the SSA node for the target of the property access, and figure out
60336009
// whether the property in question is promotable.
60346010
bool isPromotable = propertyMember != null &&
6035-
fieldPromotionEnabled &&
6011+
typeAnalyzerOptions.fieldPromotionEnabled &&
60366012
operations.isPropertyPromotable(propertyMember);
60376013
_PropertySsaNode<Type> propertySsaNode =
60386014
targetSsaNode.getOrCreatePropertyNode(propertyName, promotionKeyStore,
@@ -6063,7 +6039,8 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
60636039
// Don't use expression info for late variables, since we don't know when
60646040
// they'll be initialized.
60656041
expressionInfo = null;
6066-
} else if (isImplicitlyTyped && !respectImplicitlyTypedVarInitializers) {
6042+
} else if (isImplicitlyTyped &&
6043+
!typeAnalyzerOptions.respectImplicitlyTypedVarInitializers) {
60676044
// If the language version is too old, SSA analysis has to ignore
60686045
// initializer expressions for implicitly typed variables, in order to
60696046
// preserve the buggy behavior of
@@ -6285,7 +6262,9 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
62856262
unpromotedType: unpromotedType);
62866263

62876264
// Update the type of the variable for looking up the write expression.
6288-
if (inferenceUpdate4Enabled && node is Expression && !isPostfixIncDec) {
6265+
if (typeAnalyzerOptions.inferenceUpdate4Enabled &&
6266+
node is Expression &&
6267+
!isPostfixIncDec) {
62896268
_Reference<Type> reference = _variableReference(
62906269
variableKey,
62916270
unpromotedType,

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

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ mixin TypeAnalyzer<
285285
get operations;
286286

287287
/// Options affecting the behavior of [TypeAnalyzer].
288-
TypeAnalyzerOptions get options;
288+
TypeAnalyzerOptions get typeAnalyzerOptions;
289289

290290
/// Analyzes a non-wildcard variable pattern appearing in an assignment
291291
/// context. [node] is the pattern itself, and [variable] is the variable
@@ -425,11 +425,11 @@ mixin TypeAnalyzer<
425425
SharedTypeView expressionType = analyzeExpression(
426426
expression, operations.typeToSchema(matchedValueType));
427427
flow.constantPattern_end(expression, expressionType,
428-
patternsEnabled: options.patternsEnabled,
428+
patternsEnabled: typeAnalyzerOptions.patternsEnabled,
429429
matchedValueType: matchedValueType);
430430
// Stack: (Expression)
431431
Error? caseExpressionTypeMismatchError;
432-
if (!options.patternsEnabled) {
432+
if (!typeAnalyzerOptions.patternsEnabled) {
433433
Expression? switchScrutinee = context.switchScrutinee;
434434
if (switchScrutinee != null) {
435435
bool matches = operations.isSubtypeOf(expressionType, matchedValueType);
@@ -1848,7 +1848,7 @@ mixin TypeAnalyzer<
18481848
// Stack: (Expression, (i + 1) * ExpressionCase)
18491849
}
18501850
// If `inferenceUpdate3` is not enabled, then the type of `E` is `T`.
1851-
if (!this.options.inferenceUpdate3Enabled) {
1851+
if (!typeAnalyzerOptions.inferenceUpdate3Enabled) {
18521852
staticType = t!;
18531853
} else
18541854
// - If `T <: S`, then the type of `E` is `T`.
@@ -1975,7 +1975,7 @@ mixin TypeAnalyzer<
19751975
// n * Statement), where n = body.length
19761976
lastCaseTerminates = !flow.switchStatement_afterCase();
19771977
if (caseIndex < numCases - 1 &&
1978-
!options.patternsEnabled &&
1978+
!typeAnalyzerOptions.patternsEnabled &&
19791979
!lastCaseTerminates) {
19801980
(switchCaseCompletesNormallyErrors ??= {})[caseIndex] = errors
19811981
.switchCaseCompletesNormally(node: node, caseIndex: caseIndex);
@@ -1990,7 +1990,7 @@ mixin TypeAnalyzer<
19901990
if (hasDefault) {
19911991
isExhaustive = true;
19921992
requiresExhaustivenessValidation = false;
1993-
} else if (options.patternsEnabled) {
1993+
} else if (typeAnalyzerOptions.patternsEnabled) {
19941994
requiresExhaustivenessValidation =
19951995
isExhaustive = operations.isAlwaysExhaustiveType(scrutineeType);
19961996
} else {
@@ -2733,6 +2733,22 @@ class TypeAnalyzerOptions {
27332733

27342734
final bool inferenceUpdate3Enabled;
27352735

2736-
TypeAnalyzerOptions(
2737-
{required this.patternsEnabled, required this.inferenceUpdate3Enabled});
2736+
/// Indicates whether initializers of implicitly typed variables should be
2737+
/// accounted for by SSA analysis. (In an ideal world, they always would be,
2738+
/// but due to https://github.com/dart-lang/language/issues/1785, they weren't
2739+
/// always, and we need to be able to replicate the old behavior when
2740+
/// analyzing old language versions).
2741+
final bool respectImplicitlyTypedVarInitializers;
2742+
2743+
final bool fieldPromotionEnabled;
2744+
2745+
final bool inferenceUpdate4Enabled;
2746+
2747+
TypeAnalyzerOptions({
2748+
required this.patternsEnabled,
2749+
required this.inferenceUpdate3Enabled,
2750+
required this.respectImplicitlyTypedVarInitializers,
2751+
required this.fieldPromotionEnabled,
2752+
required this.inferenceUpdate4Enabled,
2753+
});
27382754
}

pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -637,9 +637,7 @@ main() {
637637
var s = if_(e, []);
638638
var flow = FlowAnalysis<Node, Statement, Expression, Var, SharedTypeView>(
639639
h.typeOperations, AssignedVariables<Node, Var>(),
640-
respectImplicitlyTypedVarInitializers: true,
641-
fieldPromotionEnabled: true,
642-
inferenceUpdate4Enabled: true);
640+
typeAnalyzerOptions: h.computeTypeAnalyzerOptions());
643641
flow.ifStatement_conditionBegin();
644642
flow.ifStatement_thenBegin(e, s);
645643
expect(() => flow.finish(), _asserts);

pkg/_fe_analyzer_shared/test/mini_ast.dart

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1611,11 +1611,8 @@ class Harness {
16111611
isPromotable: false, whyNotPromotable: null)
16121612
};
16131613

1614-
late final typeAnalyzer = _MiniAstTypeAnalyzer(
1615-
this,
1616-
TypeAnalyzerOptions(
1617-
patternsEnabled: patternsEnabled,
1618-
inferenceUpdate3Enabled: inferenceUpdate3Enabled));
1614+
late final typeAnalyzer =
1615+
_MiniAstTypeAnalyzer(this, computeTypeAnalyzerOptions());
16191616

16201617
/// Indicates whether initializers of implicitly typed variables should be
16211618
/// accounted for by SSA analysis. (In an ideal world, they always would be,
@@ -1704,6 +1701,15 @@ class Harness {
17041701
operations.addSuperInterfaces(className, template);
17051702
}
17061703

1704+
shared.TypeAnalyzerOptions computeTypeAnalyzerOptions() =>
1705+
TypeAnalyzerOptions(
1706+
patternsEnabled: patternsEnabled,
1707+
inferenceUpdate3Enabled: inferenceUpdate3Enabled,
1708+
respectImplicitlyTypedVarInitializers:
1709+
_respectImplicitlyTypedVarInitializers,
1710+
fieldPromotionEnabled: _fieldPromotionEnabled,
1711+
inferenceUpdate4Enabled: inferenceUpdate4Enabled);
1712+
17071713
void disableFieldPromotion() {
17081714
assert(!_started);
17091715
_fieldPromotionEnabled = false;
@@ -1798,13 +1804,8 @@ class Harness {
17981804
var b = Block._(statements, location: computeLocation());
17991805
b.preVisit(visitor);
18001806
flow = FlowAnalysis<Node, Statement, Expression, Var, SharedTypeView>(
1801-
operations,
1802-
visitor._assignedVariables,
1803-
respectImplicitlyTypedVarInitializers:
1804-
_respectImplicitlyTypedVarInitializers,
1805-
fieldPromotionEnabled: _fieldPromotionEnabled,
1806-
inferenceUpdate4Enabled: inferenceUpdate4Enabled,
1807-
);
1807+
operations, visitor._assignedVariables,
1808+
typeAnalyzerOptions: computeTypeAnalyzerOptions());
18081809
typeAnalyzer.dispatchStatement(b);
18091810
typeAnalyzer.finish();
18101811
expect(typeAnalyzer.errors._accumulatedErrors, expectedErrors);
@@ -5413,7 +5414,7 @@ class _MiniAstTypeAnalyzer
54135414
final _irBuilder = MiniIRBuilder();
54145415

54155416
@override
5416-
final TypeAnalyzerOptions options;
5417+
final TypeAnalyzerOptions typeAnalyzerOptions;
54175418

54185419
/// The temporary variable used in the IR to represent the target of the
54195420
/// innermost enclosing cascade expression, or `null` if no cascade expression
@@ -5425,7 +5426,7 @@ class _MiniAstTypeAnalyzer
54255426
/// cascade expression is currently being visited.
54265427
SharedTypeView? _currentCascadeTargetType;
54275428

5428-
_MiniAstTypeAnalyzer(this._harness, this.options);
5429+
_MiniAstTypeAnalyzer(this._harness, this.typeAnalyzerOptions);
54295430

54305431
@override
54315432
FlowAnalysis<Node, Statement, Expression, Var, SharedTypeView> get flow =>

pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import 'package:analyzer/src/dart/element/type_provider.dart';
2626
import 'package:analyzer/src/dart/element/type_system.dart';
2727
import 'package:analyzer/src/dart/resolver/flow_analysis_visitor.dart';
2828
import 'package:analyzer/src/dart/resolver/resolution_visitor.dart';
29+
import 'package:analyzer/src/dart/resolver/type_analyzer_options.dart';
2930
import 'package:analyzer/src/error/best_practices_verifier.dart';
3031
import 'package:analyzer/src/error/codes.dart';
3132
import 'package:analyzer/src/error/constructor_fields_verifier.dart';
@@ -196,9 +197,12 @@ class LibraryAnalyzer {
196197
),
197198
);
198199

200+
var featureSet = _libraryElement.featureSet;
201+
var typeAnalyzerOptions = computeTypeAnalyzerOptions(featureSet);
199202
FlowAnalysisHelper flowAnalysisHelper = FlowAnalysisHelper(
200-
_testingData != null, _libraryElement.featureSet,
201-
typeSystemOperations: _typeSystemOperations);
203+
_testingData != null,
204+
typeSystemOperations: _typeSystemOperations,
205+
typeAnalyzerOptions: typeAnalyzerOptions);
202206
_testingData?.recordFlowAnalysisDataForTesting(
203207
file.uri, flowAnalysisHelper.dataForTesting!);
204208

@@ -213,6 +217,7 @@ class LibraryAnalyzer {
213217
analysisOptions: _library.file.analysisOptions,
214218
flowAnalysisHelper: flowAnalysisHelper,
215219
libraryFragment: unitElement,
220+
typeAnalyzerOptions: typeAnalyzerOptions,
216221
);
217222
_testingData?.recordTypeConstraintGenerationDataForTesting(
218223
file.uri, resolverVisitor.inferenceHelper.dataForTesting!);
@@ -864,9 +869,11 @@ class LibraryAnalyzer {
864869
// Nothing for RESOLVED_UNIT9?
865870
// Nothing for RESOLVED_UNIT10?
866871

872+
var typeAnalyzerOptions = computeTypeAnalyzerOptions(unit.featureSet);
867873
FlowAnalysisHelper flowAnalysisHelper = FlowAnalysisHelper(
868-
_testingData != null, unit.featureSet,
869-
typeSystemOperations: _typeSystemOperations);
874+
_testingData != null,
875+
typeSystemOperations: _typeSystemOperations,
876+
typeAnalyzerOptions: typeAnalyzerOptions);
870877
_testingData?.recordFlowAnalysisDataForTesting(
871878
fileAnalysis.file.uri, flowAnalysisHelper.dataForTesting!);
872879

@@ -881,6 +888,7 @@ class LibraryAnalyzer {
881888
featureSet: unit.featureSet,
882889
flowAnalysisHelper: flowAnalysisHelper,
883890
libraryFragment: unitElement,
891+
typeAnalyzerOptions: typeAnalyzerOptions,
884892
);
885893
unit.accept(resolver);
886894
_testingData?.recordTypeConstraintGenerationDataForTesting(

0 commit comments

Comments
 (0)