Skip to content

Commit f4dc001

Browse files
stereotype441commit-bot@chromium.org
stereotype441
authored and
[email protected]
committed
Migration: begin adding support for LUB computations in conditional expressions.
A number of corner cases still need to be addressed, and I need to generalize the logic to work for `??` expressions. Change-Id: I6998058698c8b293f7d7b99f3c231a9173b9ff58 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/107901 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Dan Rubel <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
1 parent 585794a commit f4dc001

4 files changed

Lines changed: 291 additions & 23 deletions

File tree

‎pkg/nnbd_migration/lib/src/edge_builder.dart‎

Lines changed: 104 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -322,22 +322,10 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType> {
322322
_handleAssignment(node.condition, _notNullType);
323323
// TODO(paulberry): guard anything inside the true and false branches
324324
var thenType = node.thenExpression.accept(this);
325-
assert(_isSimple(thenType)); // TODO(paulberry)
326325
var elseType = node.elseExpression.accept(this);
327-
assert(_isSimple(elseType)); // TODO(paulberry)
328-
329-
DartType staticType = node.staticType;
330-
DecoratedType returnType;
331-
if (staticType is FunctionType) {
332-
DartType functReturnType = staticType.returnType;
333-
returnType = functReturnType.isDynamic || functReturnType.isVoid
334-
// TODO(danrubel): handle LUB for constituent types
335-
? DecoratedType(functReturnType, _graph.always)
336-
: _variables.decoratedElementType(functReturnType.element);
337-
}
338-
var overallType = DecoratedType(
339-
staticType, NullabilityNode.forLUB(thenType.node, elseType.node),
340-
returnType: returnType);
326+
327+
var overallType = _decorateUpperOrLowerBound(
328+
node, node.staticType, thenType, elseType, true);
341329
_variables.recordDecoratedExpressionType(node, overallType);
342330
return overallType;
343331
}
@@ -800,7 +788,8 @@ $stackTrace''');
800788
var staticElement = node.staticElement;
801789
if (staticElement is ParameterElement ||
802790
staticElement is LocalVariableElement ||
803-
staticElement is FunctionElement) {
791+
staticElement is FunctionElement ||
792+
staticElement is MethodElement) {
804793
return getOrComputeElementType(staticElement);
805794
} else if (staticElement is PropertyAccessorElement) {
806795
var elementType = getOrComputeElementType(staticElement);
@@ -978,6 +967,92 @@ $stackTrace''');
978967
assert(name != 'runtimeType');
979968
}
980969

970+
DecoratedType _decorateUpperOrLowerBound(AstNode astNode, DartType type,
971+
DecoratedType left, DecoratedType right, bool isLUB,
972+
{NullabilityNode node}) {
973+
if (type.isDynamic || type.isVoid) {
974+
if (type.isDynamic) {
975+
_unimplemented(astNode, 'LUB/GLB with dynamic');
976+
}
977+
return DecoratedType(type, _graph.always);
978+
}
979+
node ??= isLUB
980+
? NullabilityNode.forLUB(left.node, right.node)
981+
: _nullabilityNodeForGLB(astNode, left.node, right.node);
982+
if (type is InterfaceType) {
983+
if (type.typeArguments.isEmpty) {
984+
return DecoratedType(type, node);
985+
} else {
986+
var leftType = left.type;
987+
var rightType = right.type;
988+
if (leftType is InterfaceType && rightType is InterfaceType) {
989+
if (leftType.element != type.element ||
990+
rightType.element != type.element) {
991+
_unimplemented(astNode, 'LUB/GLB with substitution');
992+
}
993+
List<DecoratedType> newTypeArguments = [];
994+
for (int i = 0; i < type.typeArguments.length; i++) {
995+
newTypeArguments.add(_decorateUpperOrLowerBound(
996+
astNode,
997+
type.typeArguments[i],
998+
left.typeArguments[i],
999+
right.typeArguments[i],
1000+
isLUB));
1001+
}
1002+
return DecoratedType(type, node, typeArguments: newTypeArguments);
1003+
} else {
1004+
_unimplemented(
1005+
astNode,
1006+
'LUB/GLB with unexpected types: ${leftType.runtimeType}/'
1007+
'${rightType.runtimeType}');
1008+
}
1009+
}
1010+
} else if (type is FunctionType) {
1011+
var leftType = left.type;
1012+
var rightType = right.type;
1013+
if (leftType is FunctionType && rightType is FunctionType) {
1014+
var returnType = _decorateUpperOrLowerBound(
1015+
astNode, type.returnType, left.returnType, right.returnType, isLUB);
1016+
List<DecoratedType> positionalParameters = [];
1017+
Map<String, DecoratedType> namedParameters = {};
1018+
int positionalParameterCount = 0;
1019+
for (var parameter in type.parameters) {
1020+
DecoratedType leftParameterType;
1021+
DecoratedType rightParameterType;
1022+
if (parameter.isNamed) {
1023+
leftParameterType = left.namedParameters[parameter.name];
1024+
rightParameterType = right.namedParameters[parameter.name];
1025+
} else {
1026+
leftParameterType =
1027+
left.positionalParameters[positionalParameterCount];
1028+
rightParameterType =
1029+
right.positionalParameters[positionalParameterCount];
1030+
positionalParameterCount++;
1031+
}
1032+
var decoratedParameterType = _decorateUpperOrLowerBound(astNode,
1033+
parameter.type, leftParameterType, rightParameterType, !isLUB);
1034+
if (parameter.isNamed) {
1035+
namedParameters[parameter.name] = decoratedParameterType;
1036+
} else {
1037+
positionalParameters.add(decoratedParameterType);
1038+
}
1039+
}
1040+
return DecoratedType(type, node,
1041+
returnType: returnType,
1042+
positionalParameters: positionalParameters,
1043+
namedParameters: namedParameters);
1044+
} else {
1045+
_unimplemented(
1046+
astNode,
1047+
'LUB/GLB with unexpected types: ${leftType.runtimeType}/'
1048+
'${rightType.runtimeType}');
1049+
}
1050+
} else if (type is TypeParameterType) {
1051+
_unimplemented(astNode, 'LUB/GLB with type parameter types');
1052+
}
1053+
_unimplemented(astNode, '_decorateUpperOrLowerBound');
1054+
}
1055+
9811056
/// Creates the necessary constraint(s) for an assignment of the given
9821057
/// [expression] to a destination whose type is [destinationType].
9831058
DecoratedType _handleAssignment(
@@ -1209,7 +1284,9 @@ $stackTrace''');
12091284
}
12101285
return calleeType.positionalParameters[0];
12111286
} else {
1212-
var expressionType = calleeType.returnType;
1287+
var expressionType = callee is PropertyAccessorElement
1288+
? calleeType.returnType
1289+
: calleeType;
12131290
if (isConditional) {
12141291
expressionType = expressionType.withNode(
12151292
NullabilityNode.forLUB(targetType.node, expressionType.node));
@@ -1276,6 +1353,16 @@ $stackTrace''');
12761353
return false;
12771354
}
12781355

1356+
NullabilityNode _nullabilityNodeForGLB(
1357+
AstNode astNode, NullabilityNode leftNode, NullabilityNode rightNode) {
1358+
var node = NullabilityNode.forGLB();
1359+
var origin = GreatestLowerBoundOrigin(_source, astNode.offset);
1360+
_graph.connect(leftNode, node, origin, guards: [rightNode]);
1361+
_graph.connect(node, leftNode, origin);
1362+
_graph.connect(node, rightNode, origin);
1363+
return node;
1364+
}
1365+
12791366
@alwaysThrows
12801367
void _unimplemented(AstNode node, String message) {
12811368
CompilationUnit unit = node.root as CompilationUnit;

‎pkg/nnbd_migration/lib/src/edge_origin.dart‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,19 @@ class FieldFormalParameterOrigin extends EdgeOriginWithLocation {
4040
FieldFormalParameterOrigin(Source source, int offset) : super(source, offset);
4141
}
4242

43+
/// Edge origin resulting from the use of greatest lower bound.
44+
///
45+
/// For example, in the following code snippet:
46+
/// void Function(int) f(void Function(int) x, void Function(int) y)
47+
/// => x ?? y;
48+
///
49+
/// the `int` in the return type is nullable if both the `int`s in the types of
50+
/// `x` and `y` are nullable, due to the fact that the `int` in the return type
51+
/// is the greatest lower bound of the two other `int`s.
52+
class GreatestLowerBoundOrigin extends EdgeOriginWithLocation {
53+
GreatestLowerBoundOrigin(Source source, int offset) : super(source, offset);
54+
}
55+
4356
/// Edge origin resulting from the presence of a `??` operator.
4457
class IfNullOrigin extends EdgeOriginWithLocation {
4558
IfNullOrigin(Source source, int offset) : super(source, offset);

‎pkg/nnbd_migration/lib/src/nullability_node.dart‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,13 @@ abstract class NullabilityNode {
311311
/// List of edges that have this node as their destination.
312312
final _upstreamEdges = <NullabilityEdge>[];
313313

314+
/// Creates a [NullabilityNode] representing the nullability of an expression
315+
/// which is nullable iff two other nullability nodes are both nullable.
316+
///
317+
/// The caller is required to create the appropriate graph edges to ensure
318+
/// that the appropriate relationship between the nodes' nullabilities holds.
319+
factory NullabilityNode.forGLB() => _NullabilityNodeSimple('GLB');
320+
314321
/// Creates a [NullabilityNode] representing the nullability of a variable
315322
/// whose type is determined by the `??` operator.
316323
factory NullabilityNode.forIfNotNull() =>

0 commit comments

Comments
 (0)