languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-09-05 00:13:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

Shared type analysis for patterns: update to latest spec.

Browse source 
Make the following changes, based on
8a9b9a8a74:

- Replace `ConstOrLiteralPattern` nomenclature with `ConstantPattern`
  (the spec no longer speaks of "literal patterns").

- It is an error if a guard's type is not assignable to `bool`.

- Variable patterns can now be `final`.

- We now have a separate error condition to cover the case where a
  variable, list, map, record, or extractor pattern appears in an
  irrefutable context and the matched type is not assignable to the
  required type of the pattern.  (Previously such patterns were simply
  called "refutable", leading to a less clear error).

Additionally, we now consistenly use the term "guard" to refer to the
expression after a `when`, consistent with the spec text.

There are a few new TODOs, which I plan to address in follow-up CLs.

Change-Id: Ia0abab9492583f2aa8b59a9b381b90ba11b3e0fc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/259246
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This commit is contained in:
Paul Berry 2022-09-15 15:26:43 +00:00 committed by Commit Bot
parent dc4c7a1a8d
commit a74d3fc01d
5 changed files with 210 additions and 74 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
View file

@ -530,11 +530,11 @@ abstract class FlowAnalysis<Node extends Object, Statement extends Node,
@visibleForTesting
SsaNode<Type>? ssaNodeForTesting(Variable variable);
/// Call this method just after visiting a `when` part of a case clause. See
/// Call this method just after visiting a guard part of a case clause. See
/// [switchStatement_expressionEnd] for details.
///
/// [when] should be the expression following the `when` keyword.
void switchStatement_afterWhen(Expression when);
void switchStatement_afterGuard(Expression when);
/// Call this method just before visiting a sequence of two or more `case` or
/// `default` clauses that share a body. See [switchStatement_expressionEnd]
@ -583,7 +583,7 @@ abstract class FlowAnalysis<Node extends Object, Statement extends Node,
/// to call if there is just one `case` or `default` clause).
/// - For each `case` or `default` clause associated with this case body:
/// - If a `when` clause is present, visit it and then call
/// [switchStatement_afterWhen].
/// [switchStatement_afterGuard].
/// - If [switchStatement_beginAlternatives] was called, call
/// [switchStatement_endAlternative].
/// - If [switchStatement_beginAlternatives] was called, call
@ -1205,9 +1205,9 @@ class FlowAnalysisDebug<Node extends Object, Statement extends Node,
}
@override
void switchStatement_afterWhen(Expression when) {
_wrap('switchStatement_afterWhen($when)',
() => _wrapped.switchStatement_afterWhen(when));
void switchStatement_afterGuard(Expression when) {
_wrap('switchStatement_afterGuard($when)',
() => _wrapped.switchStatement_afterGuard(when));
}
@override
@ -3745,7 +3745,7 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
.variableInfo[promotionKeyStore.keyForVariable(variable)]?.ssaNode;
@override
void switchStatement_afterWhen(Expression when) {
void switchStatement_afterGuard(Expression when) {
ExpressionInfo<Type>? expressionInfo = _getExpressionInfo(when);
if (expressionInfo != null) {
_current = expressionInfo.ifTrue;
@ -4610,7 +4610,7 @@ class _LegacyTypePromotion<Node extends Object, Statement extends Node,
}
@override
void switchStatement_afterWhen(Expression when) {}
void switchStatement_afterGuard(Expression when) {}
@override
void switchStatement_beginAlternatives() {}

110
pkg/_fe_analyzer_shared/lib/src/type_inference/type_analyzer.dart
View file

@ -165,16 +165,15 @@ mixin TypeAnalyzer<Node extends Object, Statement extends Node,
/// Returns the unknown type context (`?`) used in type inference.
Type get unknownType;
/// Analyzes a constant pattern or literal pattern. [node] is the pattern
/// itself, and [expression] is the constant or literal expression. Depending
/// on the client's representation, [node] and [expression] might or might not
/// be identical.
/// Analyzes a constant pattern. [node] is the pattern itself, and
/// [expression] is the constant expression. Depending on the client's
/// representation, [node] and [expression] might or might not be identical.
///
/// Stack effect: none.
PatternDispatchResult<Node, Expression, Variable, Type>
analyzeConstOrLiteralPattern(Node node, Expression expression) {
return new _ConstOrLiteralPatternDispatchResult<Node, Expression, Variable,
Type>(this, node, expression);
analyzeConstantPattern(Node node, Expression expression) {
return new _ConstantPatternDispatchResult<Node, Expression, Variable, Type>(
this, node, expression);
}
/// Analyzes an expression. [node] is the expression to analyze, and
@ -280,14 +279,14 @@ mixin TypeAnalyzer<Node extends Object, Statement extends Node,
switchScrutinee: scrutinee,
topPattern: pattern));
// Stack: (Expression, i * ExpressionCase, Pattern)
Expression? when = caseInfo.when;
bool hasWhen = when != null;
if (hasWhen) {
analyzeExpression(when, boolType);
Expression? guard = caseInfo.when;
bool hasGuard = guard != null;
if (hasGuard) {
_checkGuardType(guard, analyzeExpression(guard, boolType));
// Stack: (Expression, i * ExpressionCase, Pattern, Expression)
flow?.switchStatement_afterWhen(when);
flow?.switchStatement_afterGuard(guard);
} else {
handleNoWhenCondition(node, i);
handleNoGuard(node, i);
// Stack: (Expression, i * ExpressionCase, Pattern, Expression)
}
handleCaseHead(node, caseIndex: i, subIndex: 0);
@ -356,15 +355,15 @@ mixin TypeAnalyzer<Node extends Object, Statement extends Node,
topPattern: pattern));
// Stack: (Expression, numExecutionPaths * StatementCase,
// numHeads * CaseHead, Pattern),
Expression? when = head.when;
bool hasWhen = when != null;
if (hasWhen) {
analyzeExpression(when, boolType);
Expression? guard = head.when;
bool hasGuard = guard != null;
if (hasGuard) {
_checkGuardType(guard, analyzeExpression(guard, boolType));
// Stack: (Expression, numExecutionPaths * StatementCase,
// numHeads * CaseHead, Pattern, Expression),
flow?.switchStatement_afterWhen(when);
flow?.switchStatement_afterGuard(guard);
} else {
handleNoWhenCondition(node, i);
handleNoGuard(node, i);
}
handleCaseHead(node, caseIndex: i, subIndex: j);
} else {
@ -438,14 +437,15 @@ mixin TypeAnalyzer<Node extends Object, Statement extends Node,
}
/// Analyzes a variable pattern. [node] is the pattern itself, [variable] is
/// the variable, and [declaredType] is the explicitly declared type (if
/// present).
/// the variable, [declaredType] is the explicitly declared type (if present),
/// and [isFinal] indicates whether the variable is final.
///
/// Stack effect: none.
PatternDispatchResult<Node, Expression, Variable, Type>
analyzeVariablePattern(Node node, Variable variable, Type? declaredType) {
analyzeVariablePattern(Node node, Variable variable, Type? declaredType,
{required bool isFinal}) {
return new _VariablePatternDispatchResult<Node, Expression, Variable, Type>(
this, node, variable, declaredType);
this, node, variable, declaredType, isFinal);
}
/// Calls the appropriate `analyze` method according to the form of
@ -536,13 +536,13 @@ mixin TypeAnalyzer<Node extends Object, Statement extends Node,
void handleCaseHead(Node node,
{required int caseIndex, required int subIndex});
/// Called when matching a constant pattern or a literal pattern.
/// Called when matching a constant pattern.
///
/// [node] is the AST node for the pattern and [matchedType] is the static
/// type of the expression being matched.
///
/// Stack effect: pops (Expression) and pushes (Pattern).
void handleConstOrLiteralPattern(Node node, {required Type matchedType});
void handleConstantPattern(Node node, {required Type matchedType});
/// Called after visiting a `default` clause.
///
@ -556,11 +556,11 @@ mixin TypeAnalyzer<Node extends Object, Statement extends Node,
/// of a `when` clause is semantically equivalent to `when true`, this method
/// should behave similarly to visiting the boolean literal `true`.
///
/// [node] is the enclosing switch statement or switch expression and
/// [caseIndex] is the index of the `case`.
/// [node] is the enclosing switch statement, switch expression, or `if`, and
/// [caseIndex] is the index of the `case` within [node].
///
/// Stack effect: pushes (Expression).
void handleNoWhenCondition(Node node, int caseIndex);
void handleNoGuard(Node node, int caseIndex);
/// Called after visiting the scrutinee part of a switch statement or switch
/// expression. This is a hook to allow the client to start exhaustiveness
@ -600,6 +600,16 @@ mixin TypeAnalyzer<Node extends Object, Statement extends Node,
/// whose initializer expression has static type [type].
Type variableTypeFromInitializerType(Type type);
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
// logic permits `T extends bool`, `T promoted to bool`, or `Never`. What
// do we want?
if (!typeOperations.isAssignableTo(type, boolType)) {
errors?.nonBooleanCondition(expression);
}
}
/// Records in [typeInfos] that a [pattern] binds a [variable] with a given
/// [staticType], and reports any errors caused by type inconsistency.
/// [isImplicitlyTyped] indicates whether the variable is implicitly typed in
@ -682,6 +692,9 @@ abstract class TypeAnalyzerErrors<
void inconsistentMatchVarExplicitness(
{required Node pattern, required Node previousPattern});
/// Called if the static type of a condition is not assignable to `bool`.
void nonBooleanCondition(Expression node);
/// Called if a pattern is illegally used in a variable declaration statement
/// that is marked `late`, and that pattern is not allowed in such a
/// declaration. The only kind of pattern that may be used in a late variable
@ -690,10 +703,25 @@ abstract class TypeAnalyzerErrors<
/// [pattern] is the AST node of the illegal pattern.
void patternDoesNotAllowLate(Node pattern);
/// Called if, for a pattern in an irrefutable context, the matched type of
/// the pattern is not assignable to the required type.
///
/// [pattern] is the AST node of the pattern with the type error, [context] is
/// the containing AST node that established an irrefutable context,
/// [matchedType] is the matched type, and [requiredType] is the required
/// type.
void patternTypeMismatchInIrrefutableContext(
{required Node pattern,
required Node context,
required Type matchedType,
required Type requiredType});
/// Called if a refutable pattern is illegally used in an irrefutable context.
///
/// [pattern] is the AST node of the refutable pattern, and [context] is the
/// containing AST node that established an irrefutable context.
///
/// TODO(paulberry): move this error reporting to the parser.
void refutablePatternInIrrefutableContext(Node pattern, Node context);
/// Called if one of the case bodies of a switch statement completes normally
@ -756,8 +784,8 @@ class VariableTypeInfo<Node extends Object, Type extends Object> {
}
/// Specialization of [PatternDispatchResult] returned by
/// [TypeAnalyzer.analyzeConstOrLiteralPattern]
class _ConstOrLiteralPatternDispatchResult<Node extends Object,
/// [TypeAnalyzer.analyzeConstantPattern]
class _ConstantPatternDispatchResult<Node extends Object,
Expression extends Node, Variable extends Object, Type extends Object>
extends _PatternDispatchResultImpl<Node, Expression, Variable, Type> {
/// The constant or literal expression.
@ -766,7 +794,7 @@ class _ConstOrLiteralPatternDispatchResult<Node extends Object,
/// identical to [node].
final Expression _expression;
_ConstOrLiteralPatternDispatchResult(
_ConstantPatternDispatchResult(
super.typeAnalyzer, super.node, this._expression);
@override
@ -813,7 +841,7 @@ class _ConstOrLiteralPatternDispatchResult<Node extends Object,
}
}
}
_typeAnalyzer.handleConstOrLiteralPattern(node, matchedType: matchedType);
_typeAnalyzer.handleConstantPattern(node, matchedType: matchedType);
// Stack: (Pattern)
}
}
@ -839,8 +867,10 @@ class _VariablePatternDispatchResult<Node extends Object,
final Type? _declaredType;
_VariablePatternDispatchResult(
super._typeAnalyzer, super.node, this._variable, this._declaredType);
final bool _isFinal;
_VariablePatternDispatchResult(super._typeAnalyzer, super.node,
this._variable, this._declaredType, this._isFinal);
@override
Type get typeSchema => _declaredType ?? _typeAnalyzer.unknownType;
@ -856,8 +886,11 @@ class _VariablePatternDispatchResult<Node extends Object,
Node? irrefutableContext = context.irrefutableContext;
if (irrefutableContext != null &&
!_typeAnalyzer.typeOperations.isAssignableTo(matchedType, staticType)) {
_typeAnalyzer.errors
?.refutablePatternInIrrefutableContext(node, irrefutableContext);
_typeAnalyzer.errors?.patternTypeMismatchInIrrefutableContext(
pattern: node,
context: irrefutableContext,
matchedType: matchedType,
requiredType: staticType);
}
bool isImplicitlyTyped = _declaredType == null;
bool isFirstMatch = _typeAnalyzer._recordTypeInfo(typeInfos,
@ -868,9 +901,12 @@ class _VariablePatternDispatchResult<Node extends Object,
if (isFirstMatch) {
_typeAnalyzer.flow?.declare(_variable, false);
_typeAnalyzer.setVariableType(_variable, staticType);
// TODO(paulberry): are we handling _isFinal correctly?
// TODO(paulberry): do we need to verify that all instances of a
// variable are final or all are not final?
_typeAnalyzer.flow?.initialize(
_variable, matchedType, context.getInitializer(node),
isFinal: context.isFinal,
isFinal: context.isFinal || _isFinal,
isLate: context.isLate,
isImplicitlyTyped: isImplicitlyTyped);
}

70
pkg/_fe_analyzer_shared/test/mini_ast.dart
View file

@ -243,23 +243,22 @@ mixin CaseHead implements CaseHeads, Node {
@override
List<CaseHead> get _caseHeads => [this];
Expression? get _guard;
@override
List<Label> get _labels => const [];
Pattern? get _pattern;
Expression? get _whenExpression;
ExpressionCase thenExpr(Expression body) =>
ExpressionCase._(_pattern, _whenExpression, body,
location: computeLocation());
ExpressionCase._(_pattern, _guard, body, location: computeLocation());
void _preVisit(
PreVisitor visitor, VariableBinder<Node, Var, Type> variableBinder) {
variableBinder.startAlternative(this);
_pattern?.preVisit(visitor, variableBinder);
variableBinder.finishAlternative();
_whenExpression?.preVisit(visitor);
_guard?.preVisit(visitor);
}
}
@ -434,6 +433,7 @@ class Harness
'double <: int?': false,
'double <: String': false,
'dynamic <: int': false,
'int <: bool': false,
'int <: double': false,
'int <: double?': false,
'int <: dynamic': true,
@ -879,10 +879,10 @@ abstract class Pattern extends Node with CaseHead, CaseHeads {
Pattern._({required super.location}) : super._();
@override
Pattern? get _pattern => this;
Expression? get _guard => null;
@override
Expression? get _whenExpression => null;
Pattern? get _pattern => this;
void preVisit(
PreVisitor visitor, VariableBinder<Node, Var, Type> variableBinder);
@ -1002,10 +1002,11 @@ class Var extends Node implements Promotable {
_type = value;
}
Pattern pattern({String? type, String? expectInferredType}) =>
Pattern pattern(
{String? type, String? expectInferredType, bool isFinal = false}) =>
new _VariablePattern(
type == null ? null : Type(type), this, expectInferredType,
location: computeLocation());
isFinal: isFinal, location: computeLocation());
@override
void preVisit(PreVisitor visitor) {}
@ -1409,7 +1410,7 @@ class _ConstantPattern extends Pattern {
@override
PatternDispatchResult<Node, Expression, Var, Type> visit(Harness h) =>
h.typeAnalyzer.analyzeConstOrLiteralPattern(this, constant);
h.typeAnalyzer.analyzeConstantPattern(this, constant);
@override
_debugString({required bool needsKeywordOrType}) => constant.toString();
@ -1498,10 +1499,10 @@ class _Default extends Node with CaseHead, CaseHeads {
_Default({required super.location}) : super._();
@override
Pattern? get _pattern => null;
Expression? get _guard => null;
@override
Expression? get _whenExpression => null;
Pattern? get _pattern => null;
}
class _Do extends Statement {
@ -1983,11 +1984,28 @@ class _MiniAstErrors
_recordError('missingMatchVar(${alternative.errorId}, ${variable.name})');
}
@override
void nonBooleanCondition(Expression node) {
_recordError('nonBooleanCondition(${node.errorId})');
}
@override
void patternDoesNotAllowLate(Node pattern) {
_recordError('patternDoesNotAllowLate(${pattern.errorId})');
}
@override
void patternTypeMismatchInIrrefutableContext(
{required Node pattern,
required Node context,
required Type matchedType,
required Type requiredType}) {
_recordError(
'patternTypeMismatchInIrrefutableContext(pattern: ${pattern.errorId}, '
'context: ${context.errorId}, matchedType: ${matchedType.type}, '
'requiredType: ${requiredType.type})');
}
@override
void refutablePatternInIrrefutableContext(Node pattern, Node context) {
_recordError('refutablePatternInIrrefutableContext(${pattern.errorId}, '
@ -2371,9 +2389,7 @@ class _MiniAstTypeAnalyzer
return StatementCaseInfo([
for (var caseHead in case_._caseHeads._caseHeads)
CaseHeadInfo(
node: caseHead,
pattern: caseHead._pattern,
when: caseHead._whenExpression)
node: caseHead, pattern: caseHead._pattern, when: caseHead._guard)
], case_._body.statements, labels: case_._caseHeads._labels);
}
@ -2397,7 +2413,7 @@ class _MiniAstTypeAnalyzer
}
@override
void handleConstOrLiteralPattern(Node node, {required Type matchedType}) {
void handleConstantPattern(Node node, {required Type matchedType}) {
_irBuilder.atom(matchedType.type, Kind.type, location: node.location);
_irBuilder.apply('const', [Kind.expression, Kind.type], Kind.pattern,
names: ['matchedType'], location: node.location);
@ -2412,6 +2428,11 @@ class _MiniAstTypeAnalyzer
_irBuilder.atom('true', Kind.expression, location: node.location);
}
@override
void handleNoGuard(Node node, int caseIndex) {
_irBuilder.atom('true', Kind.expression, location: node.location);
}
void handleNoInitializer(Node node) {
_irBuilder.atom('uninitialized', Kind.statement, location: node.location);
}
@ -2424,11 +2445,6 @@ class _MiniAstTypeAnalyzer
_irBuilder.atom('noop', Kind.statement, location: node.location);
}
@override
void handleNoWhenCondition(Node node, int caseIndex) {
_irBuilder.atom('true', Kind.expression, location: node.location);
}
@override
void handleSwitchScrutinee(Type type) {}
@ -2961,8 +2977,10 @@ class _VariablePattern extends Pattern {
final String? expectInferredType;
final bool isFinal;
_VariablePattern(this.declaredType, this.variable, this.expectInferredType,
{required super.location})
{this.isFinal = false, required super.location})
: super._();
@override
@ -2975,7 +2993,8 @@ class _VariablePattern extends Pattern {
@override
PatternDispatchResult<Node, Expression, Var, Type> visit(Harness h) {
return h.typeAnalyzer.analyzeVariablePattern(this, variable, declaredType);
return h.typeAnalyzer
.analyzeVariablePattern(this, variable, declaredType, isFinal: isFinal);
}
@override
@ -3030,10 +3049,9 @@ class _When extends Node with CaseHead, CaseHeads {
final Pattern _pattern;
@override
final Expression _whenExpression;
final Expression _guard;
_When(this._pattern, this._whenExpression, {required super.location})
: super._();
_When(this._pattern, this._guard, {required super.location}) : super._();
}
class _While extends Statement {

87
pkg/_fe_analyzer_shared/test/type_inference/type_inference_test.dart
View file

@ -120,7 +120,7 @@ main() {
]);
});
test('when clause', () {
test('guard', () {
var i = Var('i');
h.run([
switchExpr(expr('int'), [
@ -138,6 +138,40 @@ main() {
.stmt,
]);
});
group('Guard not assignable to bool', () {
test('int', () {
var x = Var('x');
h.run([
switchExpr(expr('int'), [
x
.pattern()
.when(expr('int')..errorId = 'GUARD')
.thenExpr(expr('int')),
]).stmt,
], expectedErrors: {
'nonBooleanCondition(GUARD)'
});
});
test('bool', () {
var x = Var('x');
h.run([
switchExpr(expr('int'), [
x.pattern().when(expr('bool')).thenExpr(expr('int')),
]).stmt,
], expectedErrors: {});
});
test('dynamic', () {
var x = Var('x');
h.run([
switchExpr(expr('int'), [
x.pattern().when(expr('dynamic')).thenExpr(expr('int')),
]).stmt,
], expectedErrors: {});
});
});
});
});
@ -369,7 +403,7 @@ main() {
]);
});
test('when clause', () {
test('guard', () {
var i = Var('i');
h.run([
switch_(
@ -863,6 +897,52 @@ main() {
]);
});
});
group('Guard not assignable to bool', () {
test('int', () {
var x = Var('x');
h.run([
switch_(
expr('int'),
[
x.pattern().when(expr('int')..errorId = 'GUARD').then([
break_(),
]),
],
isExhaustive: false),
], expectedErrors: {
'nonBooleanCondition(GUARD)'
});
});
test('bool', () {
var x = Var('x');
h.run([
switch_(
expr('int'),
[
x.pattern().when(expr('bool')).then([
break_(),
]),
],
isExhaustive: false),
], expectedErrors: {});
});
test('dynamic', () {
var x = Var('x');
h.run([
switch_(
expr('int'),
[
x.pattern().when(expr('dynamic')).then([
break_(),
]),
],
isExhaustive: false),
], expectedErrors: {});
});
});
});
group('Variable declaration:', () {
@ -983,7 +1063,8 @@ main() {
(match(x.pattern(type: 'num')..errorId = 'PATTERN', expr('String'))
..errorId = 'CONTEXT'),
], expectedErrors: {
'refutablePatternInIrrefutableContext(PATTERN, CONTEXT)'
'patternTypeMismatchInIrrefutableContext(pattern: PATTERN, '
'context: CONTEXT, matchedType: String, requiredType: num)'
});
});
});

1
pkg/front_end/test/spell_checking_list_code.txt
View file

@ -593,6 +593,7 @@ hang
happening
happy
hardcode
harmonize
harness
hashes
hashing