Shared type analysis: add "if-case" support.
Change-Id: I7c956239dd050c3c07ff85508180c451a70ca8a8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/257601
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
diff --git a/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart b/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
index fd7ecd6..2862583 100644
--- a/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
+++ b/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
@@ -379,7 +379,9 @@
/// Call this method after visiting the condition part of an if statement.
/// [condition] should be the if statement's condition. [ifNode] should be
/// the entire `if` statement (or the collection literal entry).
- void ifStatement_thenBegin(Expression condition, Node ifNode);
+ ///
+ /// For an if-case statement, [condition] should be `null`.
+ void ifStatement_thenBegin(Expression? condition, Node ifNode);
/// Call this method after visiting the initializer of a variable declaration,
/// or a variable pattern that is being matched (and hence being initialized
@@ -1040,7 +1042,7 @@
}
@override
- void ifStatement_thenBegin(Expression condition, Node ifNode) {
+ void ifStatement_thenBegin(Expression? condition, Node ifNode) {
_wrap('ifStatement_thenBegin($condition, $ifNode)',
() => _wrapped.ifStatement_thenBegin(condition, ifNode));
}
@@ -3530,7 +3532,7 @@
}
@override
- void ifStatement_thenBegin(Expression condition, Node ifNode) {
+ void ifStatement_thenBegin(Expression? condition, Node ifNode) {
ExpressionInfo<Type> conditionInfo = _expressionEnd(condition);
_stack.add(new _IfContext(conditionInfo));
_current = conditionInfo.ifTrue;
@@ -4010,14 +4012,14 @@
/// be the last expression that was traversed). If there is no
/// [ExpressionInfo] associated with the [expression], then a fresh
/// [ExpressionInfo] is created recording the current flow analysis state.
- ExpressionInfo<Type> _expressionEnd(Expression expression) =>
+ ExpressionInfo<Type> _expressionEnd(Expression? expression) =>
_getExpressionInfo(expression) ?? new _TrivialExpressionInfo(_current);
/// Gets the [ExpressionInfo] associated with the [expression] (which should
/// be the last expression that was traversed). If there is no
/// [ExpressionInfo] associated with the [expression], then `null` is
/// returned.
- ExpressionInfo<Type>? _getExpressionInfo(Expression expression) {
+ ExpressionInfo<Type>? _getExpressionInfo(Expression? expression) {
if (identical(expression, _expressionWithInfo)) {
ExpressionInfo<Type>? expressionInfo = _expressionInfo;
_expressionInfo = null;
@@ -4421,7 +4423,7 @@
}
@override
- void ifStatement_thenBegin(Expression condition, Node ifNode) {
+ void ifStatement_thenBegin(Expression? condition, Node ifNode) {
_conditionalOrIf_thenBegin(condition, ifNode);
}
@@ -4700,7 +4702,7 @@
_writeStackForAnd.last.add(variableKey);
}
- void _conditionalOrIf_thenBegin(Expression condition, Node node) {
+ void _conditionalOrIf_thenBegin(Expression? condition, Node node) {
_contextStack.add(new _LegacyContext<Type>(_knownTypes));
AssignedVariablesNodeInfo info = _assignedVariables.getInfoForNode(node);
Map<int, Type>? newKnownTypes;
@@ -4748,7 +4750,7 @@
/// Gets the [_LegacyExpressionInfo] associated with [expression], if any;
/// otherwise returns `null`.
- _LegacyExpressionInfo<Type>? _getExpressionInfo(Expression expression) {
+ _LegacyExpressionInfo<Type>? _getExpressionInfo(Expression? expression) {
if (identical(expression, _expressionWithInfo)) {
_LegacyExpressionInfo<Type>? expressionInfo = _expressionInfo;
_expressionInfo = null;
diff --git a/pkg/_fe_analyzer_shared/lib/src/type_inference/type_analyzer.dart b/pkg/_fe_analyzer_shared/lib/src/type_inference/type_analyzer.dart
index 9b55ba4..e207f5d 100644
--- a/pkg/_fe_analyzer_shared/lib/src/type_inference/type_analyzer.dart
+++ b/pkg/_fe_analyzer_shared/lib/src/type_inference/type_analyzer.dart
@@ -194,6 +194,65 @@
return result.resolveShorting();
}
+ /// Analyzes a statement of the form `if (expression case pattern) ifTrue` or
+ /// `if (expression case pattern) ifTrue else ifFalse`.
+ ///
+ /// [node] should be the AST node for the entire statement, [expression] for
+ /// the expression, [pattern] for the pattern to match, [ifTrue] for the
+ /// "then" branch, and [ifFalse] for the "else" branch (if present).
+ ///
+ /// Stack effect: pushes (Expression scrutinee, Pattern, Expression guard,
+ /// Statement ifTrue, Statement ifFalse). If there is no `else` clause, the
+ /// representation for `ifFalse` will be pushed by [handleNoStatement]. If
+ /// there is no guard, the representation for `guard` will be pushed by
+ /// [handleNoGuard].
+ void analyzeIfCaseStatement(Statement node, Expression expression,
+ Node pattern, Expression? guard, Statement ifTrue, Statement? ifFalse) {
+ // Stack: ()
+ flow?.ifStatement_conditionBegin();
+ // Note: this follows what's proposed in
+ // https://github.com/dart-lang/language/issues/2458, which is that we use
+ // the unknown type for the type schema.
+ // TODO(paulberry): update this if necessary when that issue is closed.
+ Type initializerType = analyzeExpression(expression, unknownType);
+ // Stack: (Expression)
+ PatternDispatchResult<Node, Expression, Variable, Type>
+ patternDispatchResult = dispatchPattern(pattern);
+ Map<Variable, VariableTypeInfo<Node, Type>> typeInfos = {};
+ // TODO(paulberry): rework handling of isFinal
+ patternDispatchResult.match(initializerType, typeInfos,
+ new MatchContext(isFinal: false, topPattern: pattern));
+ // Stack: (Expression, Pattern)
+ if (guard != null) {
+ _checkGuardType(guard, analyzeExpression(guard, boolType));
+ } else {
+ handleNoGuard(node, 0);
+ }
+ // Stack: (Expression, Pattern, Guard)
+ flow?.ifStatement_thenBegin(null, node);
+ _analyzeIfCommon(node, ifTrue, ifFalse);
+ }
+
+ /// Analyzes a statement of the form `if (condition) ifTrue` or
+ /// `if (condition) ifTrue else ifFalse`.
+ ///
+ /// [node] should be the AST node for the entire statement, [condition] for
+ /// the condition expression, [ifTrue] for the "then" branch, and [ifFalse]
+ /// for the "else" branch (if present).
+ ///
+ /// Stack effect: pushes (Expression condition, Statement ifTrue, Statement
+ /// ifFalse). Note that if there is no `else` clause, the representation for
+ /// `ifFalse` will be pushed by [handleNoStatement].
+ void analyzeIfStatement(Statement node, Expression condition,
+ Statement ifTrue, Statement? ifFalse) {
+ // Stack: ()
+ flow?.ifStatement_conditionBegin();
+ analyzeExpression(condition, boolType);
+ // Stack: (Expression condition)
+ flow?.ifStatement_thenBegin(condition, node);
+ _analyzeIfCommon(node, ifTrue, ifFalse);
+ }
+
/// Analyzes a variable declaration statement of the form
/// `pattern = initializer;`.
///
@@ -562,6 +621,13 @@
/// Stack effect: pushes (Expression).
void handleNoGuard(Node node, int caseIndex);
+ /// Called when visiting a syntactic construct where there is an implicit
+ /// no-op statement. For example, this is called in place of the missing
+ /// `else` part of an `if` statement that lacks an `else` clause.
+ ///
+ /// Stack effect: pushes (Statement).
+ void handleNoStatement(Statement node);
+
/// Called after visiting the scrutinee part of a switch statement or switch
/// expression. This is a hook to allow the client to start exhaustiveness
/// analysis.
@@ -600,6 +666,25 @@
/// whose initializer expression has static type [type].
Type variableTypeFromInitializerType(Type type);
+ /// Common functionality shared by [analyzeIfStatement] and
+ /// [analyzeIfCaseStatement].
+ ///
+ /// Stack effect: pushes (Statement ifTrue, Statement ifFalse).
+ void _analyzeIfCommon(Statement node, Statement ifTrue, Statement? ifFalse) {
+ // Stack: ()
+ dispatchStatement(ifTrue);
+ // Stack: (Statement ifTrue)
+ if (ifFalse == null) {
+ handleNoStatement(node);
+ flow?.ifStatement_end(false);
+ } else {
+ flow?.ifStatement_elseBegin();
+ dispatchStatement(ifFalse);
+ flow?.ifStatement_end(true);
+ }
+ // Stack: (Statement ifTrue, Statement ifFalse)
+ }
+
void _checkGuardType(Expression expression, Type type) {
// TODO(paulberry): harmonize this with analyzer's checkForNonBoolExpression
// TODO(paulberry): spec says the type must be `bool` or `dynamic`. This
@@ -799,10 +884,9 @@
@override
Type get typeSchema {
- // Note: the type schema only matters for patterns that appear in variable
- // declarations, and variable declarations are not allowed to contain
- // constant patterns. So this code should only be reachable during error
- // recovery.
+ // Note: this follows what's proposed in
+ // https://github.com/dart-lang/language/issues/2458.
+ // TODO(paulberry): update this if necessary when that issue is closed.
_typeAnalyzer.errors?.assertInErrorRecovery();
return _typeAnalyzer.unknownType;
}
diff --git a/pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart b/pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart
index 5907faa..e6b1c35 100644
--- a/pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart
+++ b/pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart
@@ -1865,6 +1865,38 @@
]);
});
+ test('ifCase splits control flow', () {
+ var x = Var('x');
+ var y = Var('y');
+ var z = Var('z');
+ var w = Var('w');
+ h.run([
+ declare(x, type: 'int'),
+ declare(y, type: 'int'),
+ declare(z, type: 'int'),
+ ifCase(expr('num'), w.pattern(type: 'int'), [
+ x.write(expr('int')).stmt,
+ y.write(expr('int')).stmt,
+ ], else_: [
+ y.write(expr('int')).stmt,
+ z.write(expr('int')).stmt,
+ ]),
+ checkAssigned(x, false),
+ checkAssigned(y, true),
+ checkAssigned(z, false),
+ ]);
+ });
+
+ test('ifCase does not promote when expression true', () {
+ var x = Var('x');
+ h.run([
+ declare(x, type: 'int?', initializer: expr('int?')),
+ ifCase(x.expr.notEq(nullLiteral), intLiteral(0).pattern, [
+ checkNotPromoted(x),
+ ]),
+ ]);
+ });
+
test('promote promotes to a subtype and sets type of interest', () {
var x = Var('x');
h.run([
diff --git a/pkg/_fe_analyzer_shared/test/mini_ast.dart b/pkg/_fe_analyzer_shared/test/mini_ast.dart
index 8da68f5..9175905 100644
--- a/pkg/_fe_analyzer_shared/test/mini_ast.dart
+++ b/pkg/_fe_analyzer_shared/test/mini_ast.dart
@@ -186,6 +186,19 @@
location: location);
}
+Statement ifCase(
+ Expression expression, CaseHead caseHead, List<Statement> ifTrue,
+ {List<Statement>? else_}) {
+ var location = computeLocation();
+ return new _IfCase(
+ expression,
+ caseHead._pattern!,
+ caseHead._guard,
+ _Block(ifTrue, location: location),
+ else_ == null ? null : _Block(else_, location: location),
+ location: location);
+}
+
Expression intLiteral(int value, {bool? expectConversionToDouble}) =>
new _IntLiteral(value,
expectConversionToDouble: expectConversionToDouble,
@@ -423,6 +436,7 @@
static const Map<String, bool> _coreSubtypes = const {
'bool <: int': false,
'bool <: Object': true,
+ 'double <: bool': false,
'double <: double?': true,
'double <: Object': true,
'double <: Object?': true,
@@ -1727,20 +1741,18 @@
}
}
-class _If extends Statement {
+class _If extends _IfBase {
final Expression condition;
- final Statement ifTrue;
- final Statement? ifFalse;
- _If(this.condition, this.ifTrue, this.ifFalse, {required super.location});
+ _If(this.condition, super.ifTrue, super.ifFalse, {required super.location});
+
+ @override
+ String get _conditionPartString => condition.toString();
@override
void preVisit(PreVisitor visitor) {
condition.preVisit(visitor);
- visitor._assignedVariables.beginNode();
- ifTrue.preVisit(visitor);
- visitor._assignedVariables.endNode(this);
- ifFalse?.preVisit(visitor);
+ super.preVisit(visitor);
}
@override
@@ -1752,6 +1764,68 @@
}
}
+abstract class _IfBase extends Statement {
+ final Statement ifTrue;
+ final Statement? ifFalse;
+
+ _IfBase(this.ifTrue, this.ifFalse, {required super.location});
+
+ String get _conditionPartString;
+
+ @override
+ void preVisit(PreVisitor visitor) {
+ visitor._assignedVariables.beginNode();
+ ifTrue.preVisit(visitor);
+ visitor._assignedVariables.endNode(this);
+ ifFalse?.preVisit(visitor);
+ }
+
+ @override
+ String toString() =>
+ 'if ($_conditionPartString) $ifTrue' +
+ (ifFalse == null ? '' : 'else $ifFalse');
+}
+
+class _IfCase extends _IfBase {
+ final Expression _expression;
+ final Pattern _pattern;
+ final Expression? _guard;
+
+ _IfCase(
+ this._expression, this._pattern, this._guard, super.ifTrue, super.ifFalse,
+ {required super.location});
+
+ @override
+ String get _conditionPartString => '$_expression case $_pattern';
+
+ @override
+ void preVisit(PreVisitor visitor) {
+ _expression.preVisit(visitor);
+ var variableBinder = VariableBinder<Node, Var, Type>(visitor);
+ _pattern.preVisit(visitor, variableBinder);
+ variableBinder.finish();
+ _guard?.preVisit(visitor);
+ super.preVisit(visitor);
+ }
+
+ @override
+ void visit(Harness h) {
+ h.typeAnalyzer.analyzeIfCaseStatement(
+ this, _expression, _pattern, _guard, ifTrue, ifFalse);
+ h.irBuilder.apply(
+ 'ifCase',
+ [
+ Kind.expression,
+ Kind.pattern,
+ Kind.expression,
+ Kind.statement,
+ Kind.statement
+ ],
+ Kind.statement,
+ location: location);
+ }
+}
+
class _IfNull extends Expression {
final Expression lhs;
final Expression rhs;
@@ -2192,22 +2266,6 @@
flow.operations.promoteToNonNull(leftType), rightType));
}
- void analyzeIfStatement(Statement node, Expression condition,
- Statement ifTrue, Statement? ifFalse) {
- flow.ifStatement_conditionBegin();
- analyzeExpression(condition, unknownType);
- flow.ifStatement_thenBegin(condition, node);
- dispatchStatement(ifTrue);
- if (ifFalse == null) {
- handleNoStatement(node);
- flow.ifStatement_end(false);
- } else {
- flow.ifStatement_elseBegin();
- dispatchStatement(ifFalse);
- flow.ifStatement_end(true);
- }
- }
-
void analyzeLabeledStatement(Statement node, Statement body) {
flow.labeledStatement_begin(node);
dispatchStatement(body);
@@ -2441,6 +2499,7 @@
_irBuilder.atom('failure', Kind.expression, location: node.location);
}
+ @override
void handleNoStatement(Node node) {
_irBuilder.atom('noop', Kind.statement, location: node.location);
}
diff --git a/pkg/_fe_analyzer_shared/test/type_inference/type_inference_test.dart b/pkg/_fe_analyzer_shared/test/type_inference/type_inference_test.dart
index 3968b97..11b0601 100644
--- a/pkg/_fe_analyzer_shared/test/type_inference/type_inference_test.dart
+++ b/pkg/_fe_analyzer_shared/test/type_inference/type_inference_test.dart
@@ -176,6 +176,102 @@
});
group('Statements:', () {
+ group('If:', () {
+ test('Condition context', () {
+ h.run([
+ if_(expr('dynamic').checkContext('bool'), [
+ expr('Object').stmt,
+ ]).checkIr('if(expr(dynamic), block(stmt(expr(Object))), noop)'),
+ ]);
+ });
+
+ test('With else', () {
+ h.run([
+ if_(expr('bool'), [
+ expr('Object').stmt,
+ ], [
+ expr('String').stmt,
+ ]).checkIr('if(expr(bool), block(stmt(expr(Object))), '
+ 'block(stmt(expr(String))))'),
+ ]);
+ });
+ });
+
+ group('If-case:', () {
+ test('Type schema', () {
+ var x = Var('x');
+ h.run([
+ ifCase(expr('int').checkContext('?'), x.pattern(type: 'num'), [
+ expr('Object').stmt,
+ ]).checkIr('ifCase(expr(int), '
+ 'varPattern(x, matchedType: int, staticType: num), true, '
+ 'block(stmt(expr(Object))), noop)'),
+ ]);
+ });
+
+ test('With else', () {
+ var x = Var('x');
+ h.run([
+ ifCase(expr('num'), x.pattern(type: 'int'), [
+ expr('Object').stmt,
+ ], else_: [
+ expr('String').stmt,
+ ]).checkIr('ifCase(expr(num), '
+ 'varPattern(x, matchedType: num, staticType: int), true, '
+ 'block(stmt(expr(Object))), block(stmt(expr(String))))'),
+ ]);
+ });
+
+ test('With guard', () {
+ var x = Var('x');
+ h.run([
+ ifCase(expr('num'),
+ x.pattern(type: 'int').when(x.expr.eq(intLiteral(0))), [
+ expr('Object').stmt,
+ ]).checkIr('ifCase(expr(num), '
+ 'varPattern(x, matchedType: num, staticType: int), ==(x, 0), '
+ 'block(stmt(expr(Object))), noop)'),
+ ]);
+ });
+
+ test('Allows refutable patterns', () {
+ var x = Var('x');
+ h.run([
+ ifCase(expr('num').checkContext('?'), x.pattern(type: 'int'), [
+ expr('Object').stmt,
+ ]).checkIr('ifCase(expr(num), '
+ 'varPattern(x, matchedType: num, staticType: int), true, '
+ 'block(stmt(expr(Object))), noop)'),
+ ]);
+ });
+
+ group('Guard not assignable to bool', () {
+ test('int', () {
+ var x = Var('x');
+ h.run([
+ ifCase(expr('int'),
+ x.pattern().when(expr('int')..errorId = 'GUARD'), []),
+ ], expectedErrors: {
+ 'nonBooleanCondition(GUARD)'
+ });
+ });
+
+ test('bool', () {
+ var x = Var('x');
+ h.run([
+ ifCase(expr('int'), x.pattern().when(expr('bool')), []),
+ ], expectedErrors: {});
+ });
+
+ test('dynamic', () {
+ var x = Var('x');
+ h.run([
+ ifCase(expr('int'), x.pattern().when(expr('dynamic')), []),
+ ], expectedErrors: {});
+ });
+ });
+ });
+
group('Switch:', () {
test('Empty', () {
h.run([
diff --git a/pkg/front_end/test/spell_checking_list_code.txt b/pkg/front_end/test/spell_checking_list_code.txt
index 9babcc8..362e822 100644
--- a/pkg/front_end/test/spell_checking_list_code.txt
+++ b/pkg/front_end/test/spell_checking_list_code.txt
@@ -1180,6 +1180,7 @@
reusage
reversible
rewinds
+rework
rewrites
rewrote
rf
