Skip to content

Commit

Permalink
[analyzer] Use StatementImpl when interfacing with shared code
Browse files Browse the repository at this point in the history
Change the analyzer's use of the following generic types so that it
supplies the type parameter `StatementImpl` instead of `Statement` as
the type of AST node it uses to represent statements:

- `FlowAnalysis`
- `TypeAnalyzerErrors`

This required adjusting some `is` tests to test against concrete
statement types instead of abstract public interface types (e.g. `is
SwitchStatementImpl` instead of `is SwitchStatement`). In one place
(in `FlowAnalysisHelper.getLabelTarget`) I changed an if-test to a
switch statement so that I could use pattern matching to avoid type
casts entirely.

In some places, I was able to avoid adding casts by changing method
parameters to accept concrete statement types instead of abstract
public interface types. This required adding the `covariant` keyword
to some methods.

This is part of a larger arc of work to change the analyzer's use of
the shared code so that the type parameters it supplies are not part
of the analyzer public API. See
#59763.

Change-Id: I8910184ff71ccb97b52e5d44fbf0b6bddd6e273b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403180
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
  • Loading branch information
stereotype441 authored and Commit Queue committed Jan 6, 2025
1 parent 1c4d7e1 commit 8191171
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 23 deletions.
34 changes: 19 additions & 15 deletions pkg/analyzer/lib/src/dart/resolver/flow_analysis_visitor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class FlowAnalysisHelper {
final bool inferenceUpdate4Enabled;

/// The current flow, when resolving a function body, or `null` otherwise.
FlowAnalysis<AstNode, Statement, Expression, PromotableElementImpl2,
FlowAnalysis<AstNode, StatementImpl, Expression, PromotableElementImpl2,
SharedTypeView<DartType>>? flow;

FlowAnalysisHelper(bool retainDataForTesting, FeatureSet featureSet,
Expand Down Expand Up @@ -177,17 +177,17 @@ class FlowAnalysisHelper {
as AssignedVariablesForTesting<AstNode, PromotableElementImpl2>;
}
flow = isNonNullableByDefault
? FlowAnalysis<AstNode, Statement, Expression, PromotableElementImpl2,
SharedTypeView<DartType>>(
? FlowAnalysis<AstNode, StatementImpl, Expression,
PromotableElementImpl2, SharedTypeView<DartType>>(
typeOperations,
assignedVariables!,
respectImplicitlyTypedVarInitializers:
respectImplicitlyTypedVarInitializers,
fieldPromotionEnabled: fieldPromotionEnabled,
inferenceUpdate4Enabled: inferenceUpdate4Enabled,
)
: FlowAnalysis<AstNode, Statement, Expression, PromotableElementImpl2,
SharedTypeView<DartType>>.legacy(
: FlowAnalysis<AstNode, StatementImpl, Expression,
PromotableElementImpl2, SharedTypeView<DartType>>.legacy(
typeOperations, assignedVariables!);
}

Expand Down Expand Up @@ -258,7 +258,7 @@ class FlowAnalysisHelper {
}

void for_bodyBegin(AstNode node, Expression? condition) {
flow?.for_bodyBegin(node is Statement ? node : null, condition);
flow?.for_bodyBegin(node is StatementImpl ? node : null, condition);
}

void for_conditionBegin(AstNode node) {
Expand Down Expand Up @@ -309,7 +309,7 @@ class FlowAnalysisHelper {
);
}

void labeledStatement_enter(LabeledStatement node) {
void labeledStatement_enter(LabeledStatementImpl node) {
if (flow == null) return;

flow!.labeledStatement_begin(node);
Expand Down Expand Up @@ -372,18 +372,22 @@ class FlowAnalysisHelper {
/// not specify a label), so the default enclosing target is returned.
///
/// [isBreak] is `true` for `break`, and `false` for `continue`.
static Statement? getLabelTarget(AstNode? node, Element? element,
static StatementImpl? getLabelTarget(AstNode? node, Element? element,
{required bool isBreak}) {
for (; node != null; node = node.parent) {
if (element == null) {
if (node is DoStatement ||
node is ForStatement ||
(isBreak && node is SwitchStatement) ||
node is WhileStatement) {
return node as Statement;
switch (node) {
case DoStatementImpl():
return node;
case ForStatementImpl():
return node;
case SwitchStatementImpl() when isBreak:
return node;
case WhileStatementImpl():
return node;
}
} else {
if (node is LabeledStatement) {
if (node is LabeledStatementImpl) {
if (_hasLabel(node.labels, element)) {
var statement = node.statement;
// The inner statement is returned for labeled loops and
Expand All @@ -399,7 +403,7 @@ class FlowAnalysisHelper {
return statement;
}
}
if (node is SwitchStatement) {
if (node is SwitchStatementImpl) {
for (var member in node.members) {
if (_hasLabel(member.labels, element)) {
return node;
Expand Down
4 changes: 2 additions & 2 deletions pkg/analyzer/lib/src/dart/resolver/shared_type_analyzer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class SharedTypeAnalyzerErrors
implements
shared.TypeAnalyzerErrors<
AstNode,
Statement,
StatementImpl,
Expression,
PromotableElementImpl2,
SharedTypeView<DartType>,
Expand Down Expand Up @@ -238,7 +238,7 @@ class SharedTypeAnalyzerErrors

@override
void switchCaseCompletesNormally(
{required covariant SwitchStatement node, required int caseIndex}) {
{required covariant SwitchStatementImpl node, required int caseIndex}) {
_errorReporter.atToken(
node.members[caseIndex].keyword,
CompileTimeErrorCode.SWITCH_CASE_COMPLETES_NORMALLY,
Expand Down
12 changes: 6 additions & 6 deletions pkg/analyzer/lib/src/generated/resolver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
TypeAnalyzer<
DartType,
AstNode,
Statement,
StatementImpl,
Expression,
PromotableElementImpl2,
DartPattern,
Expand Down Expand Up @@ -437,7 +437,7 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
ExecutableElement? get enclosingFunction => _enclosingFunction;

@override
FlowAnalysis<AstNode, Statement, Expression, PromotableElementImpl2,
FlowAnalysis<AstNode, StatementImpl, Expression, PromotableElementImpl2,
SharedTypeView<DartType>> get flow => flowAnalysis.flow!;

bool get isConstructorTearoffsEnabled =>
Expand Down Expand Up @@ -995,7 +995,7 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
}

@override
SwitchStatementMemberInfo<AstNode, Statement, Expression,
SwitchStatementMemberInfo<AstNode, StatementImpl, Expression,
PromotableElementImpl2> getSwitchStatementMemberInfo(
covariant SwitchStatementImpl node,
int index,
Expand Down Expand Up @@ -2452,7 +2452,7 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
}

@override
void visitDoStatement(DoStatement node) {
void visitDoStatement(covariant DoStatementImpl node) {
inferenceLogWriter?.enterStatement(node);
checkUnreachableNode(node);

Expand Down Expand Up @@ -3124,7 +3124,7 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
void visitLabel(Label node) {}

@override
void visitLabeledStatement(LabeledStatement node) {
void visitLabeledStatement(covariant LabeledStatementImpl node) {
inferenceLogWriter?.enterStatement(node);
flowAnalysis.labeledStatement_enter(node);
checkUnreachableNode(node);
Expand Down Expand Up @@ -3922,7 +3922,7 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
}

@override
void visitWhileStatement(WhileStatement node) {
void visitWhileStatement(covariant WhileStatementImpl node) {
inferenceLogWriter?.enterStatement(node);
checkUnreachableNode(node);

Expand Down

0 comments on commit 8191171

Please sign in to comment.