Flow analysis: change the way parenthesized expressions are handled.

Instead of forcing the client to provide a way to de-parenthesize an
expression, the client informs flow analysis when a parenthesized
expression is encountered.  This reduces the runtime overhead to zero
for non-parenthesized expressions, and it saves the front end from
having to worry about support for parenthesized expressions (since its
internal representation doesn't care about parentheses).

Change-Id: I0bb6e91c87acaa05591e1b075da18700b11e4aae
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/121080
Reviewed-by: Mike Fairhurst <mfairhurst@google.com>

Author: stereotype441

pkg/analyzer/lib/src/dart/resolver/flow_analysis_visitor.dart

@@ -11,24 +11,12 @@ import 'package:analyzer/src/dart/element/type.dart';
 import 'package:analyzer/src/generated/variable_type_provider.dart';
 import 'package:front_end/src/fasta/flow_analysis/flow_analysis.dart';
 
-class AnalyzerNodeOperations implements NodeOperations<Expression> {
-  const AnalyzerNodeOperations();
-
-  @override
-  Expression unwrapParenthesized(Expression node) {
-    return node.unParenthesized;
-  }
-}
-
 /// The helper for performing flow analysis during resolution.
 ///
 /// It contains related precomputed data, result, and non-trivial pieces of
 /// code that are independent from visiting AST during resolution, so can
 /// be extracted.
 class FlowAnalysisHelper {
-  /// The reused instance for creating new [FlowAnalysis] instances.
-  final NodeOperations<Expression> _nodeOperations;
-
   /// The reused instance for creating new [FlowAnalysis] instances.
   final TypeSystemTypeOperations _typeOperations;
 
@@ -42,13 +30,11 @@ class FlowAnalysisHelper {
   FlowAnalysis<Statement, Expression, PromotableElement, DartType> flow;
 
   factory FlowAnalysisHelper(TypeSystem typeSystem, bool retainDataForTesting) {
-    return FlowAnalysisHelper._(
-        const AnalyzerNodeOperations(),
-        TypeSystemTypeOperations(typeSystem),
+    return FlowAnalysisHelper._(TypeSystemTypeOperations(typeSystem),
         retainDataForTesting ? FlowAnalysisResult() : null);
   }
 
-  FlowAnalysisHelper._(this._nodeOperations, this._typeOperations, this.result);
+  FlowAnalysisHelper._(this._typeOperations, this.result);
 
   LocalVariableTypeProvider get localVariableTypeProvider {
     return _LocalVariableTypeProvider(this);
@@ -186,7 +172,6 @@ class FlowAnalysisHelper {
     assert(flow == null);
     assignedVariables = computeAssignedVariables(node, parameters);
     flow = FlowAnalysis<Statement, Expression, PromotableElement, DartType>(
-        _nodeOperations,
         _typeOperations,
         assignedVariables.writtenAnywhere,
         assignedVariables.capturedAnywhere);

pkg/analyzer/lib/src/generated/resolver.dart

@@ -4345,6 +4345,7 @@ class ResolverVisitor extends ScopedVisitor {
   void visitParenthesizedExpression(ParenthesizedExpression node) {
     InferenceContext.setTypeFromNode(node.expression, node);
     super.visitParenthesizedExpression(node);
+    _flowAnalysis?.flow?.parenthesizedExpression(node, node.expression);
   }
 
   @override

pkg/front_end/lib/src/fasta/flow_analysis/flow_analysis.dart

@@ -168,9 +168,6 @@ class FlowAnalysis<Statement, Expression, Variable, Type> {
 
   final List<Variable> _variablesCapturedAnywhere;
 
-  /// The [NodeOperations], used to manipulate expressions.
-  final NodeOperations<Expression> nodeOperations;
-
   /// The [TypeOperations], used to access types, and check subtyping.
   final TypeOperations<Variable, Type> typeOperations;
 
@@ -198,16 +195,15 @@ class FlowAnalysis<Statement, Expression, Variable, Type> {
   int _functionNestingLevel = 0;
 
   factory FlowAnalysis(
-      NodeOperations<Expression> nodeOperations,
       TypeOperations<Variable, Type> typeOperations,
       Iterable<Variable> variablesWrittenAnywhere,
       Iterable<Variable> variablesCapturedAnywhere) {
-    return new FlowAnalysis._(nodeOperations, typeOperations,
-        variablesWrittenAnywhere.toList(), variablesCapturedAnywhere.toList());
+    return new FlowAnalysis._(typeOperations, variablesWrittenAnywhere.toList(),
+        variablesCapturedAnywhere.toList());
   }
 
-  FlowAnalysis._(this.nodeOperations, this.typeOperations,
-      this._variablesWrittenAnywhere, this._variablesCapturedAnywhere) {
+  FlowAnalysis._(this.typeOperations, this._variablesWrittenAnywhere,
+      this._variablesCapturedAnywhere) {
     _current = new FlowModel<Variable, Type>(true);
   }
 
@@ -552,6 +548,17 @@ class FlowAnalysis<Statement, Expression, Variable, Type> {
     _storeExpressionInfo(expression, new _NullInfo(_current));
   }
 
+  /// Call this method just after visiting a parenthesized expression.
+  ///
+  /// This is only necessary if the implementation uses a different [Expression]
+  /// object to represent a parenthesized expression and its contents.
+  void parenthesizedExpression(
+      Expression outerExpression, Expression innerExpression) {
+    if (identical(_expressionWithInfo, innerExpression)) {
+      _expressionWithInfo = outerExpression;
+    }
+  }
+
   /// Retrieves the type that the [variable] is promoted to, if the [variable]
   /// is currently promoted.  Otherwise returns `null`.
   ///
@@ -722,7 +729,6 @@ class FlowAnalysis<Statement, Expression, Variable, Type> {
   /// [_ExpressionInfo] associated with the [expression], then `null` is
   /// returned.
   _ExpressionInfo<Variable, Type> _getExpressionInfo(Expression expression) {
-    expression = nodeOperations.unwrapParenthesized(expression);
     if (identical(expression, _expressionWithInfo)) {
       return _expressionInfo;
     } else {
@@ -1087,12 +1093,6 @@ class FlowModel<Variable, Type> {
   }
 }
 
-/// Operations on nodes, abstracted from concrete node interfaces.
-abstract class NodeOperations<Expression> {
-  /// If the [node] is a parenthesized expression, recursively unwrap it.
-  Expression unwrapParenthesized(Expression node);
-}
-
 /// Operations on types, abstracted from concrete type interfaces.
 abstract class TypeOperations<Variable, Type> {
   /// Returns `true` if [type1] and [type2] are the same type.

pkg/front_end/test/fasta/flow_analysis/flow_analysis_test.dart

@@ -768,6 +768,18 @@ main() {
       });
     });
 
+    test('parenthesizedExpression preserves promotion behaviors', () {
+      var h = _Harness();
+      var x = h.addVar('x', 'int?');
+      h.run((flow) {
+        h.if_(
+            h.parenthesized(h.notEqual(h.parenthesized(h.variableRead(x)),
+                h.parenthesized(h.nullLiteral))), () {
+          expect(flow.promotedType(x).type, 'int');
+        });
+      });
+    });
+
     test('promotedType handles not-yet-seen variables', () {
       // Note: this is needed for error recovery in the analyzer.
       var h = _Harness();
@@ -1785,8 +1797,7 @@ typedef _Expression LazyExpression();
 
 class _Expression {}
 
-class _Harness
-    implements NodeOperations<_Expression>, TypeOperations<_Var, _Type> {
+class _Harness implements TypeOperations<_Var, _Type> {
   FlowAnalysis<_Statement, _Expression, _Var, _Type> _flow;
 
   final List<_Var> _variablesWrittenAnywhere = [];
@@ -1797,6 +1808,12 @@ class _Harness
   /// flow analysis semantics.
   LazyExpression get expr => () => _Expression();
 
+  LazyExpression get nullLiteral => () {
+        var expr = _Expression();
+        _flow.nullLiteral(expr);
+        return expr;
+      };
+
   _Var addVar(String name, String type,
       {bool hasWrites: false, bool isCaptured: false}) {
     assert(_flow == null);
@@ -1837,7 +1854,7 @@ class _Harness
 
   FlowAnalysis<_Statement, _Expression, _Var, _Type> createFlow() =>
       FlowAnalysis<_Statement, _Expression, _Var, _Type>(
-          this, this, _variablesWrittenAnywhere, _variablesCapturedAnywhere);
+          this, _variablesWrittenAnywhere, _variablesCapturedAnywhere);
 
   void declare(_Var v, {@required bool initialized}) {
     if (initialized) {
@@ -1860,6 +1877,17 @@ class _Harness
     };
   }
 
+  /// Creates a [LazyExpression] representing an equality check between two
+  /// other expressions.
+  LazyExpression notEqual(LazyExpression lhs, LazyExpression rhs) {
+    return () {
+      var expr = _Expression();
+      _flow.equalityOp_rightBegin(lhs());
+      _flow.equalityOp_end(expr, rhs(), notEqual: true);
+      return expr;
+    };
+  }
+
   /// Invokes flow analysis of a nested function.
   void function(Iterable<_Var> writeCaptured, void body()) {
     _flow.functionExpression_begin(writeCaptured);
@@ -1954,6 +1982,15 @@ class _Harness
     };
   }
 
+  /// Creates a [LazyExpression] representing a parenthesized subexpression.
+  LazyExpression parenthesized(LazyExpression inner) {
+    return () {
+      var expr = _Expression();
+      _flow.parenthesizedExpression(expr, inner());
+      return expr;
+    };
+  }
+
   /// Causes [variable] to be promoted to [type].
   void promote(_Var variable, String type) {
     if_(isNotType(variable, type), _flow.handleExit);
@@ -1976,10 +2013,12 @@ class _Harness
     _flow.finish();
   }
 
-  @override
-  _Expression unwrapParenthesized(_Expression node) {
-    // TODO(paulberry): test cases where this matters
-    return node;
+  LazyExpression variableRead(_Var variable) {
+    return () {
+      var expr = _Expression();
+      _flow.variableRead(expr, variable);
+      return expr;
+    };
   }
 
   @override

pkg/nnbd_migration/lib/src/edge_builder.dart

@@ -966,7 +966,9 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
 
   @override
   DecoratedType visitParenthesizedExpression(ParenthesizedExpression node) {
-    return node.expression.accept(this);
+    var result = node.expression.accept(this);
+    _flowAnalysis.parenthesizedExpression(node, node.expression);
+    return result;
   }
 
   @override
@@ -1454,7 +1456,6 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
         FlowAnalysisHelper.computeAssignedVariables(node, parameters);
     _flowAnalysis =
         FlowAnalysis<Statement, Expression, PromotableElement, DecoratedType>(
-            const AnalyzerNodeOperations(),
             DecoratedTypeOperations(_typeSystem, _variables, _graph),
             _assignedVariables.writtenAnywhere,
             _assignedVariables.capturedAnywhere);

pkg/nnbd_migration/lib/src/fix_builder.dart

@@ -111,7 +111,6 @@ abstract class FixBuilder extends GeneralizingAstVisitor<DartType> {
         FlowAnalysisHelper.computeAssignedVariables(node, parameters);
     _flowAnalysis =
         FlowAnalysis<Statement, Expression, PromotableElement, DartType>(
-            const AnalyzerNodeOperations(),
             TypeSystemTypeOperations(_typeSystem),
             _assignedVariables.writtenAnywhere,
             _assignedVariables.capturedAnywhere);
@@ -252,7 +251,9 @@ abstract class FixBuilder extends GeneralizingAstVisitor<DartType> {
 
   @override
   DartType visitParenthesizedExpression(ParenthesizedExpression node) {
-    return node.expression.accept(this);
+    var result = node.expression.accept(this);
+    _flowAnalysis.parenthesizedExpression(node, node.expression);
+    return result;
   }
 
   @override

pkg/nnbd_migration/test/fix_builder_test.dart

@@ -543,6 +543,13 @@ f() => (1);
     visitSubexpression(findNode.integerLiteral('1'), 'int');
   }
 
+  test_parenthesizedExpression_flow() async {
+    await analyze('''
+_f(bool/*?*/ x) => ((x) != (null)) && x;
+''');
+    visitSubexpression(findNode.binary('&&'), 'bool');
+  }
+
   test_simpleIdentifier_className() async {
     await analyze('''
 _f() => int;