Move business logic into shared patterns analysis for relational patterns.

Previously, the client had to know that the relational operator should
be looked up in the matched value type.  Now the shared analysis logic
requests the lookup and supplies the appropriate type.

It's a small change, and one might argue that it doesn't really move
around very much business logic, but it makes the tests more readable
so I think it's worth it.

Change-Id: I936d5f5f929f11800efe25974334c898c7e1072d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/275360
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
This commit is contained in:
Paul Berry 2022-12-14 17:25:59 +00:00 committed by Commit Queue
parent ec8918c20c
commit c6d287cc05
6 changed files with 126 additions and 92 deletions

View file

@ -1152,17 +1152,19 @@ mixin TypeAnalyzer<
return recordType(positional: positional, named: named);
}
/// Analyzes a relational pattern. [node] is the pattern itself, [operator]
/// is the resolution of the used relational operator, and [operand] is a
/// constant expression.
/// Analyzes a relational pattern. [node] is the pattern itself, and
/// [operand] is a constant expression that will be passed to the relational
/// operator.
///
/// This method will invoke [resolveRelationalPatternOperator] to obtain
/// information about the operator.
///
/// See [dispatchPattern] for the meaning of [context].
///
/// Stack effect: pushes (Expression).
void analyzeRelationalPattern(
MatchContext<Node, Expression, Pattern, Type, Variable> context,
Node node,
RelationalOperatorResolution<Type>? operator,
Pattern node,
Expression operand) {
// Stack: ()
TypeAnalyzerErrors<Node, Node, Expression, Variable, Type, Pattern>?
@ -1171,6 +1173,9 @@ mixin TypeAnalyzer<
if (irrefutableContext != null) {
errors?.refutablePatternInIrrefutableContext(node, irrefutableContext);
}
Type matchedValueType = flow.getMatchedValueType();
RelationalOperatorResolution<Type>? operator =
resolveRelationalPatternOperator(node, matchedValueType);
Type operandContext = operator?.parameterType ?? unknownType;
Type operandType = analyzeExpression(operand, operandContext);
// Stack: (Expression)
@ -1669,6 +1674,15 @@ mixin TypeAnalyzer<
required RecordPatternField<Node, Pattern> field,
});
/// Resolves the relational operator for [node] assuming that the value being
/// matched has static type [matchedValueType].
///
/// If no operator is found, `null` should be returned. (This could happen
/// either because the code is invalid, or because [matchedValueType] is
/// `dynamic`).
RelationalOperatorResolution<Type>? resolveRelationalPatternOperator(
Pattern node, Type matchedValueType);
/// Records that type inference has assigned a [type] to a [variable]. This
/// is called once per variable, regardless of whether the variable's type is
/// explicit or inferred.

View file

@ -309,8 +309,7 @@ Pattern objectPattern({
Pattern recordPattern(List<RecordPatternField> fields) =>
_RecordPattern(fields, location: computeLocation());
Pattern relationalPattern(
RelationalOperatorResolution<Type>? operator, Expression operand,
Pattern relationalPattern(String operator, Expression operand,
{String? errorId}) {
var result =
_RelationalPattern(operator, operand, location: computeLocation());
@ -566,6 +565,11 @@ class GuardedPattern extends Node with PossiblyGuardedPattern {
}
class Harness {
static Map<String, Type> _coreMemberTypes = {
'int.>': Type('bool Function(num)'),
'int.>=': Type('bool Function(num)'),
};
final MiniAstOperations _operations = MiniAstOperations();
bool _started = false;
@ -576,7 +580,10 @@ class Harness {
Type? _thisType;
final Map<String, _PropertyElement> _members = {};
late final Map<String, _PropertyElement?> _members = {
for (var entry in _coreMemberTypes.entries)
entry.key: _PropertyElement(entry.value)
};
late final typeAnalyzer = _MiniAstTypeAnalyzer(
this,
@ -642,9 +649,21 @@ class Harness {
/// Updates the harness so that when member [memberName] is looked up on type
/// [targetType], a member is found having the given [type].
void addMember(String targetType, String memberName, String type,
///
/// If [type] is `null`, then an attempt to look up [memberName] on type
/// [targetType] should result in `null` (no such member) rather than a test
/// failure.
void addMember(String targetType, String memberName, String? type,
{bool promotable = false}) {
var query = '$targetType.$memberName';
if (type == null) {
if (promotable) {
fail("It doesn't make sense to specify `promotable: true` "
"when the type is `null`");
}
_members[query] = null;
return;
}
var member = _PropertyElement(Type(type));
_members[query] = member;
if (promotable) {
@ -663,11 +682,44 @@ class Harness {
}
/// Attempts to look up a member named [memberName] in the given [type]. If
/// a member is found, returns its [_PropertyElement] object. Otherwise the
/// test fails.
_PropertyElement getMember(Type type, String memberName) {
/// a member is found, returns its [_PropertyElement] object; otherwise `null`
/// is returned.
///
/// If test hasn't been configured in such a way that the result of the query
/// is known, the test fails.
_PropertyElement? getMember(Type type, String memberName) {
var query = '$type.$memberName';
return _members[query] ?? fail('Unknown member query: $query');
var member = _members[query];
if (member == null && !_members.containsKey(query)) {
fail('Unknown member query: $query');
}
return member;
}
/// See [TypeAnalyzer.resolveRelationalPatternOperator].
RelationalOperatorResolution<Type>? resolveRelationalPatternOperator(
Type matchedValueType, String operator) {
if (operator == '==' || operator == '!=') {
return RelationalOperatorResolution(
isEquality: true,
parameterType: Type('Object'),
returnType: Type('bool'));
}
var member = getMember(matchedValueType, operator);
if (member == null) return null;
var memberType = member._type;
if (memberType is! FunctionType) {
fail('$matchedValueType.operator$operator has type $memberType; '
'must be a function type');
}
if (memberType.positionalParameters.isEmpty) {
fail('$matchedValueType.operator$operator has type $memberType; '
'must accept a parameter');
}
return RelationalOperatorResolution(
isEquality: operator == '==' || operator == '!=',
parameterType: memberType.positionalParameters[0],
returnType: memberType.returnType);
}
/// Runs the given [statements] through flow analysis, checking any assertions
@ -3284,11 +3336,11 @@ class _MiniAstTypeAnalyzer
Expression node, Expression receiver, String propertyName) {
var receiverType = analyzeExpression(receiver, unknownType);
var member = _lookupMember(node, receiverType, propertyName);
var memberType = member?._type ?? dynamicType;
var promotedType =
flow.propertyGet(node, receiver, propertyName, member, member._type);
flow.propertyGet(node, receiver, propertyName, member, memberType);
// TODO(paulberry): handle null shorting
return new SimpleTypeAnalysisResult<Type>(
type: promotedType ?? member._type);
return new SimpleTypeAnalysisResult<Type>(type: promotedType ?? memberType);
}
void analyzeReturnStatement() {
@ -3304,10 +3356,10 @@ class _MiniAstTypeAnalyzer
SimpleTypeAnalysisResult<Type> analyzeThisPropertyGet(
Expression node, String propertyName) {
var member = _lookupMember(node, thisType, propertyName);
var memberType = member?._type ?? dynamicType;
var promotedType =
flow.thisOrSuperPropertyGet(node, propertyName, member, member._type);
return new SimpleTypeAnalysisResult<Type>(
type: promotedType ?? member._type);
flow.thisOrSuperPropertyGet(node, propertyName, member, memberType);
return new SimpleTypeAnalysisResult<Type>(type: promotedType ?? memberType);
}
SimpleTypeAnalysisResult<Type> analyzeThrow(
@ -3719,7 +3771,7 @@ class _MiniAstTypeAnalyzer
@override
Type listType(Type elementType) => PrimaryType('List', args: [elementType]);
_PropertyElement lookupInterfaceMember(
_PropertyElement? lookupInterfaceMember(
Node node, Type receiverType, String memberName) {
return _harness.getMember(receiverType, memberName);
}
@ -3747,7 +3799,14 @@ class _MiniAstTypeAnalyzer
required Type receiverType,
required shared.RecordPatternField<Node, Pattern> field,
}) {
return _harness.getMember(receiverType, field.name!)._type;
return _harness.getMember(receiverType, field.name!)?._type ?? dynamicType;
}
@override
RelationalOperatorResolution<Type>? resolveRelationalPatternOperator(
covariant _RelationalPattern node, Type matchedValueType) {
return _harness.resolveRelationalPatternOperator(
matchedValueType, node.operator);
}
@override
@ -3774,7 +3833,7 @@ class _MiniAstTypeAnalyzer
return type.recursivelyDemote(covariant: true) ?? type;
}
_PropertyElement _lookupMember(
_PropertyElement? _lookupMember(
Expression node, Type receiverType, String memberName) {
return lookupInterfaceMember(node, receiverType, memberName);
}
@ -4055,7 +4114,7 @@ class _Property extends PromotableLValue {
h.typeAnalyzer.analyzeExpression(target, h.typeAnalyzer.unknownType);
var member = h.typeAnalyzer._lookupMember(this, receiverType, propertyName);
return h.flow
.promotedPropertyType(target, propertyName, member, member._type);
.promotedPropertyType(target, propertyName, member, member!._type);
}
@override
@ -4122,7 +4181,7 @@ class _RecordPattern extends Pattern {
}
class _RelationalPattern extends Pattern {
final RelationalOperatorResolution<Type>? operator;
final String operator;
final Expression operand;
_RelationalPattern(this.operator, this.operand, {required super.location})
@ -4141,10 +4200,9 @@ class _RelationalPattern extends Pattern {
@override
void visit(Harness h, SharedMatchContext context) {
var matchedType = h.typeAnalyzer.flow.getMatchedValueType();
h.typeAnalyzer.analyzeRelationalPattern(context, this, operator, operand);
h.typeAnalyzer.analyzeRelationalPattern(context, this, operand);
h.irBuilder.atom(matchedType.type, Kind.type, location: location);
h.irBuilder.apply(
'relationalPattern', [Kind.expression, Kind.type], Kind.pattern,
h.irBuilder.apply(operator, [Kind.expression, Kind.type], Kind.pattern,
names: ['matchedType'], location: location);
}
@ -4413,7 +4471,7 @@ class _ThisOrSuperProperty extends PromotableLValue {
h.irBuilder.atom('this.$propertyName', Kind.expression, location: location);
var member = h.typeAnalyzer._lookupMember(this, h._thisType!, propertyName);
return h.flow
.promotedPropertyType(null, propertyName, member, member._type);
.promotedPropertyType(null, propertyName, member, member!._type);
}
@override

View file

@ -2,12 +2,9 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
import 'package:_fe_analyzer_shared/src/type_inference/type_analyzer.dart'
hide RecordPatternField, NamedType, RecordType;
import 'package:test/test.dart';
import '../mini_ast.dart';
import '../mini_types.dart';
main() {
late Harness h;
@ -2782,31 +2779,26 @@ main() {
test('Refutability', () {
h.run([
(match(
relationalPattern(
RelationalOperatorResolution<Type>(
isEquality: false,
parameterType: Type('num'),
returnType: Type('bool'),
),
intLiteral(0).checkContext('num'),
)..errorId = 'PATTERN',
relationalPattern('>', intLiteral(0).checkContext('num'))
..errorId = 'PATTERN',
intLiteral(1).checkContext('?'),
)..errorId = 'CONTEXT')
.checkIr('match(1, relationalPattern(0, matchedType: int))'),
.checkIr('match(1, >(0, matchedType: int))'),
], expectedErrors: {
'refutablePatternInIrrefutableContext(PATTERN, CONTEXT)'
});
});
test('no operator', () {
h.addMember('C', '>', null);
h.run([
ifCase(
expr('int').checkContext('?'),
expr('C').checkContext('?'),
relationalPattern(
null,
'>',
intLiteral(0).checkContext('?'),
),
[],
).checkIr('ifCase(expr(int), relationalPattern(0, matchedType: int), '
).checkIr('ifCase(expr(C), >(0, matchedType: C), '
'variables(), true, block(), noop)')
]);
});
@ -2815,16 +2807,9 @@ main() {
h.run([
ifCase(
expr('int').checkContext('?'),
relationalPattern(
RelationalOperatorResolution<Type>(
isEquality: false,
parameterType: Type('num'),
returnType: Type('bool'),
),
intLiteral(0).checkContext('num'),
),
relationalPattern('>=', intLiteral(0).checkContext('num')),
[],
).checkIr('ifCase(expr(int), relationalPattern(0, matchedType: '
).checkIr('ifCase(expr(int), >=(0, matchedType: '
'int), variables(), true, block(), noop)')
]);
});
@ -2832,16 +2817,9 @@ main() {
h.run([
ifCase(
expr('Object').checkContext('?'),
relationalPattern(
RelationalOperatorResolution<Type>(
isEquality: true,
parameterType: Type('Object'),
returnType: Type('bool'),
),
expr('int?').checkContext('Object'),
),
relationalPattern('==', expr('int?').checkContext('Object')),
[],
).checkIr('ifCase(expr(Object), relationalPattern(expr(int?), '
).checkIr('ifCase(expr(Object), ==(expr(int?), '
'matchedType: Object), variables(), true, block(), noop)')
]);
});
@ -2849,16 +2827,9 @@ main() {
h.run([
ifCase(
expr('Object').checkContext('?'),
relationalPattern(
RelationalOperatorResolution<Type>(
isEquality: true,
parameterType: Type('Object'),
returnType: Type('bool'),
),
expr('int?').checkContext('Object'),
),
relationalPattern('!=', expr('int?').checkContext('Object')),
[],
).checkIr('ifCase(expr(Object), relationalPattern(expr(int?), '
).checkIr('ifCase(expr(Object), !=(expr(int?), '
'matchedType: Object), variables(), true, block(), noop)')
]);
});
@ -2866,16 +2837,9 @@ main() {
h.run([
ifCase(
expr('int').checkContext('?'),
relationalPattern(
RelationalOperatorResolution<Type>(
isEquality: false,
parameterType: Type('num'),
returnType: Type('bool'),
),
expr('String')..errorId = 'OPERAND',
),
relationalPattern('>', expr('String')..errorId = 'OPERAND'),
[],
).checkIr('ifCase(expr(int), relationalPattern(expr(String), '
).checkIr('ifCase(expr(int), >(expr(String), '
'matchedType: int), variables(), true, block(), noop)')
], expectedErrors: {
'argumentTypeNotAssignable(argument: OPERAND, '
@ -2883,20 +2847,17 @@ main() {
});
});
test('return type is not assignable to bool', () {
h.addMember('A', '>', 'int Function(Object)');
h.run([
ifCase(
expr('A').checkContext('?'),
relationalPattern(
RelationalOperatorResolution(
isEquality: false,
parameterType: Type('Object'),
returnType: Type('int'),
),
'>',
expr('String').checkContext('Object'),
errorId: 'PATTERN',
),
[],
).checkIr('ifCase(expr(A), relationalPattern(expr(String), '
).checkIr('ifCase(expr(A), >(expr(String), '
'matchedType: A), variables(), true, block(), noop)')
], expectedErrors: {
'relationalPatternOperatorReturnTypeNotAssignableToBool('

View file

@ -11165,13 +11165,7 @@ class RelationalPatternImpl extends DartPatternImpl
ResolverVisitor resolverVisitor,
SharedMatchContext context,
) {
resolverVisitor.analyzeRelationalPattern(
context,
this,
resolverVisitor.resolveRelationalPatternOperator(
this, resolverVisitor.flowAnalysis.flow!.getMatchedValueType()),
operand,
);
resolverVisitor.analyzeRelationalPattern(context, this, operand);
resolverVisitor.popRewrite();
}

View file

@ -1557,6 +1557,7 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
return typeProvider.dynamicType;
}
@override
RelationalOperatorResolution<DartType>? resolveRelationalPatternOperator(
covariant RelationalPatternImpl node,
DartType matchedType,

View file

@ -9123,6 +9123,12 @@ class InferenceVisitorImpl extends InferenceVisitorBase
// TODO(scheglov): implement mapType
throw new UnimplementedError('TODO(scheglov)');
}
@override
RelationalOperatorResolution<DartType>? resolveRelationalPatternOperator(
Pattern node, DartType matchedValueType) {
throw new UnimplementedError('TODO(paulberry)');
}
}
/// Offset and type information collection in [InferenceVisitor.inferMapEntry].