From fd2a6c68153bd1d0740e638df8fb8a7b1d7ed70d Mon Sep 17 00:00:00 2001
From: Paul Berry <paulberry@google.com>
Date: Mon, 7 Dec 2020 16:09:57 +0000
Subject: [PATCH] Flow analysis: Track expression variables separately from
 promotion info.

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
https://github.com/dart-lang/language/issues/1274 (Infer
non-nullability from local boolean variables), we'll need #3 to be
tracked orthogonally from #1, so that when a local boolean is referred
to later, we can track information of type #1 and #3 simultaneously.

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

Change-Id: I432f6e2e80543bb1d565b49403180c520eef66a5
Bug: https://github.com/dart-lang/language/issues/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>
---
 .../lib/src/flow_analysis/flow_analysis.dart  | 126 ++++++++++--------
 1 file changed, 67 insertions(+), 59 deletions(-)

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 9752d91ce251a..7610ce4950b18 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
@@ -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
@@ -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;
@@ -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);
   }
 
@@ -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 =
@@ -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));
     }
@@ -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
@@ -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
@@ -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 {
@@ -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);
     }
@@ -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;
     }
   }
 
@@ -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;
       }
     }
   }
@@ -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;
   }
 
@@ -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');
@@ -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);
@@ -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
@@ -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>