Skip to content

Commit

Permalink
Flow analysis: Track expression variables separately from promotion i…
Browse files Browse the repository at this point in the history 
…nfo.

Previously, we used a single class hierarchy, ExpressionInfo, to store
all the information that flow analysis needs to know about a variable,
including:

1. What is known about the program state if the expression evaluates
   to true/false

2. Whether the expression is a `null` literal

3. Whether the expression is a reference to a variable.

However, in order to address
dart-lang/language#1274 (Infer
non-nullability from local boolean variables), we'll need flutter#3 to be
tracked orthogonally from flutter#1, so that when a local boolean is referred
to later, we can track information of type flutter#1 and flutter#3 simultaneously.

However, it makes sense to keep flutter#1 and flutter#2 in the same data structure,
because future work is planned to represent them in a more uniform
way, as part of addressing
dart-lang/language#1224 (Using `if (foo?.bar
== somethingNotNull)` should promote `foo`).

Change-Id: I432f6e2e80543bb1d565b49403180c520eef66a5
Bug: dart-lang/language#1274
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175008
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
  • Loading branch information
@stereotype441
stereotype441 authored and commit-bot@chromium.org committed Dec 7, 2020
1 parent 81c3e8c commit fd2a6c6
Showing 1 changed file with 67 additions and 59 deletions.
126 changes: 67 additions & 59 deletions pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2566,8 +2566,12 @@ class _EqualityOpContext<Variable, Type> extends _BranchContext {
/// The type of the expression on the LHS of `==` or `!=`.
final Type _leftOperandType;

_EqualityOpContext(
ExpressionInfo<Variable, Type> conditionInfo, this._leftOperandType)
/// If the LHS of `==` or `!=` is a variable reference, the variable.
/// Otherwise `null`.
final Variable _leftOperandVariable;

_EqualityOpContext(ExpressionInfo<Variable, Type> conditionInfo,
this._leftOperandType, this._leftOperandVariable)
: super(conditionInfo);

@override
Expand Down Expand Up @@ -2602,6 +2606,14 @@ class _FlowAnalysisImpl<Node, Statement extends Node, Expression, Variable,
/// corresponding to it. Otherwise `null`.
ExpressionInfo<Variable, Type> _expressionInfo;

/// The most recently visited expression which was a variable reference, or
/// `null` if no expression has been visited that was a variable reference.
Expression _expressionWithVariable;

/// If [_expressionVariable] is not `null`, the variable corresponding to it.
/// Otherwise `null`.
Variable _expressionVariable;

int _functionNestingLevel = 0;

final AssignedVariables<Node, Variable> _assignedVariables;
Expand All @@ -2615,14 +2627,8 @@ class _FlowAnalysisImpl<Node, Statement extends Node, Expression, Variable,

@override
void asExpression_end(Expression subExpression, Type type) {
ExpressionInfo<Variable, Type> subExpressionInfo =
_getExpressionInfo(subExpression);
Variable variable;
if (subExpressionInfo is _VariableReadInfo<Variable, Type>) {
variable = subExpressionInfo._variable;
} else {
return;
}
Variable variable = _getExpressionVariable(subExpression);
if (variable == null) return;
_current = _current.tryPromoteForTypeCast(typeOperations, variable, type);
}

Expand Down Expand Up @@ -2730,8 +2736,10 @@ class _FlowAnalysisImpl<Node, Statement extends Node, Expression, Variable,
_EqualityOpContext<Variable, Type> context =
_stack.removeLast() as _EqualityOpContext<Variable, Type>;
ExpressionInfo<Variable, Type> lhsInfo = context._conditionInfo;
Variable lhsVariable = context._leftOperandVariable;
Type leftOperandType = context._leftOperandType;
ExpressionInfo<Variable, Type> rhsInfo = _getExpressionInfo(rightOperand);
Variable rhsVariable = _getExpressionVariable(rightOperand);
TypeClassification leftOperandTypeClassification =
typeOperations.classifyType(leftOperandType);
TypeClassification rightOperandTypeClassification =
Expand All @@ -2749,18 +2757,16 @@ class _FlowAnalysisImpl<Node, Statement extends Node, Expression, Variable,
// but weak mode it might produce an "equal" result. We don't want flow
// analysis behavior to depend on mode, so we conservatively assume that
// either result is possible.
} else if (lhsInfo is _NullInfo<Variable, Type> &&
rhsInfo is _VariableReadInfo<Variable, Type>) {
} else if (lhsInfo is _NullInfo<Variable, Type> && rhsVariable != null) {
assert(
leftOperandTypeClassification == TypeClassification.nullOrEquivalent);
ExpressionInfo<Variable, Type> equalityInfo =
_current.tryMarkNonNullable(typeOperations, rhsInfo._variable);
_current.tryMarkNonNullable(typeOperations, rhsVariable);
_storeExpressionInfo(wholeExpression,
notEqual ? equalityInfo : ExpressionInfo.invert(equalityInfo));
} else if (rhsInfo is _NullInfo<Variable, Type> &&
lhsInfo is _VariableReadInfo<Variable, Type>) {
} else if (rhsInfo is _NullInfo<Variable, Type> && lhsVariable != null) {
ExpressionInfo<Variable, Type> equalityInfo =
_current.tryMarkNonNullable(typeOperations, lhsInfo._variable);
_current.tryMarkNonNullable(typeOperations, lhsVariable);
_storeExpressionInfo(wholeExpression,
notEqual ? equalityInfo : ExpressionInfo.invert(equalityInfo));
}
Expand All @@ -2769,7 +2775,9 @@ class _FlowAnalysisImpl<Node, Statement extends Node, Expression, Variable,
@override
void equalityOp_rightBegin(Expression leftOperand, Type leftOperandType) {
_stack.add(new _EqualityOpContext<Variable, Type>(
_getExpressionInfo(leftOperand), leftOperandType));
_getExpressionInfo(leftOperand),
leftOperandType,
_getExpressionVariable(leftOperand)));
}

@override
Expand Down Expand Up @@ -2844,6 +2852,9 @@ class _FlowAnalysisImpl<Node, Statement extends Node, Expression, Variable,
if (identical(_expressionWithInfo, oldExpression)) {
_expressionWithInfo = newExpression;
}
if (identical(_expressionWithVariable, oldExpression)) {
_expressionWithVariable = newExpression;
}
}

@override
Expand Down Expand Up @@ -2901,12 +2912,12 @@ class _FlowAnalysisImpl<Node, Statement extends Node, Expression, Variable,
@override
void ifNullExpression_rightBegin(
Expression leftHandSide, Type leftHandSideType) {
ExpressionInfo<Variable, Type> lhsInfo = _getExpressionInfo(leftHandSide);
Variable lhsVariable = _getExpressionVariable(leftHandSide);
FlowModel<Variable, Type> promoted;
_current = _current.split();
if (lhsInfo is _VariableReadInfo<Variable, Type>) {
if (lhsVariable != null) {
ExpressionInfo<Variable, Type> promotionInfo =
_current.tryMarkNonNullable(typeOperations, lhsInfo._variable);
_current.tryMarkNonNullable(typeOperations, lhsVariable);
_current = promotionInfo.ifFalse;
promoted = promotionInfo.ifTrue;
} else {
Expand Down Expand Up @@ -2959,12 +2970,10 @@ class _FlowAnalysisImpl<Node, Statement extends Node, Expression, Variable,
@override
void isExpression_end(Expression isExpression, Expression subExpression,
bool isNot, Type type) {
ExpressionInfo<Variable, Type> subExpressionInfo =
_getExpressionInfo(subExpression);
if (subExpressionInfo is _VariableReadInfo<Variable, Type>) {
ExpressionInfo<Variable, Type> expressionInfo =
_current.tryPromoteForTypeCheck(
typeOperations, subExpressionInfo._variable, type);
Variable subExpressionVariable = _getExpressionVariable(subExpression);
if (subExpressionVariable != null) {
ExpressionInfo<Variable, Type> expressionInfo = _current
.tryPromoteForTypeCheck(typeOperations, subExpressionVariable, type);
_storeExpressionInfo(isExpression,
isNot ? ExpressionInfo.invert(expressionInfo) : expressionInfo);
}
Expand Down Expand Up @@ -3054,11 +3063,10 @@ class _FlowAnalysisImpl<Node, Statement extends Node, Expression, Variable,

@override
void nonNullAssert_end(Expression operand) {
ExpressionInfo<Variable, Type> operandInfo = _getExpressionInfo(operand);
if (operandInfo is _VariableReadInfo<Variable, Type>) {
_current = _current
.tryMarkNonNullable(typeOperations, operandInfo._variable)
.ifTrue;
Variable operandVariable = _getExpressionVariable(operand);
if (operandVariable != null) {
_current =
_current.tryMarkNonNullable(typeOperations, operandVariable).ifTrue;
}
}

Expand All @@ -3075,11 +3083,10 @@ class _FlowAnalysisImpl<Node, Statement extends Node, Expression, Variable,
_current = _current.split();
_stack.add(new _NullAwareAccessContext<Variable, Type>(_current));
if (target != null) {
ExpressionInfo<Variable, Type> targetInfo = _getExpressionInfo(target);
if (targetInfo is _VariableReadInfo<Variable, Type>) {
_current = _current
.tryMarkNonNullable(typeOperations, targetInfo._variable)
.ifTrue;
Variable targetVariable = _getExpressionVariable(target);
if (targetVariable != null) {
_current =
_current.tryMarkNonNullable(typeOperations, targetVariable).ifTrue;
}
}
}
Expand Down Expand Up @@ -3226,7 +3233,7 @@ class _FlowAnalysisImpl<Node, Statement extends Node, Expression, Variable,

@override
Type variableRead(Expression expression, Variable variable) {
_storeExpressionInfo(expression, new _VariableReadInfo(_current, variable));
_storeExpressionVariable(expression, variable);
return _current.infoFor(variable).promotedTypes?.last;
}

Expand Down Expand Up @@ -3273,6 +3280,8 @@ class _FlowAnalysisImpl<Node, Statement extends Node, Expression, Variable,
print(' current: $_current');
print(' expressionWithInfo: $_expressionWithInfo');
print(' expressionInfo: $_expressionInfo');
print(' expressionWithVariable: $_expressionWithVariable');
print(' expressionVariable: $_expressionVariable');
print(' stack:');
for (_FlowContext stackEntry in _stack.reversed) {
print(' $stackEntry');
Expand Down Expand Up @@ -3301,6 +3310,19 @@ class _FlowAnalysisImpl<Node, Statement extends Node, Expression, Variable,
}
}

/// Gets the [Variable] associated with the [expression] (which should be the
/// last expression that was traversed). If there is no [Variable] associated
/// with the [expression], then `null` is returned.
Variable _getExpressionVariable(Expression expression) {
if (identical(expression, _expressionWithVariable)) {
Variable expressionVariable = _expressionVariable;
_expressionVariable = null;
return expressionVariable;
} else {
return null;
}
}

FlowModel<Variable, Type> _join(
FlowModel<Variable, Type> first, FlowModel<Variable, Type> second) =>
FlowModel.join(typeOperations, first, second, _current._emptyVariableMap);
Expand All @@ -3319,6 +3341,14 @@ class _FlowAnalysisImpl<Node, Statement extends Node, Expression, Variable,
_expressionInfo = expressionInfo;
_current = expressionInfo.after;
}

/// Associates [expression], which should be the most recently visited
/// expression, with the given [Variable] object.
void _storeExpressionVariable(
Expression expression, Variable expressionVariable) {
_expressionWithVariable = expression;
_expressionVariable = expressionVariable;
}
}

/// Base class for objects representing constructs in the Dart programming
Expand Down Expand Up @@ -3437,28 +3467,6 @@ class _TryContext<Variable, Type> extends _SimpleContext<Variable, Type> {
'afterBodyAndCatches: $_afterBodyAndCatches)';
}

/// [ExpressionInfo] representing an expression that reads the value of a
/// variable.
class _VariableReadInfo<Variable, Type>
implements ExpressionInfo<Variable, Type> {
@override
final FlowModel<Variable, Type> after;

/// The variable that is being read.
final Variable _variable;

_VariableReadInfo(this.after, this._variable);

@override
FlowModel<Variable, Type> get ifFalse => after;

@override
FlowModel<Variable, Type> get ifTrue => after;

@override
String toString() => '_VariableReadInfo(after: $after, variable: $_variable)';
}

/// [_FlowContext] representing a `while` loop (or a C-style `for` loop, which
/// is functionally similar).
class _WhileContext<Variable, Type>
Expand Down

0 comments on commit fd2a6c6

Please sign in to comment.