Flow analysis: rework analyzer/CFE handling of initialized variables.

Previously, the analyzer and CFE were responsible for two pieces of
logic that arguably should be in the shared flow analysis engine:

- Deciding whether or not it's necessary to tell flow analysis about
  initializer expressions.

- Deciding whether or not to promote the variable at the time of
  initialization (we do this when the variable is implicitly typed,
  and the initializer is a promoted type variable type).

It's better to just always tell flow analysis about the initializer
expression and let it decide what to do about it.

This paves the way for fixing
dart-lang/language#1785 (which results from
initializer expressions sometimes being ignored when they shouldn't
be), by consolidating the broken logic into the flow_analysis library,
where it will be easy to unit test the fix.

Bug: dart-lang/language#1785
Change-Id: Iec832e92995eb4f8d0c1fbd4e9be6c897e0917b5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/211180
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>

Author: stereotype441

pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart

@@ -634,7 +634,9 @@ abstract class FlowAnalysis<Node extends Object, Statement extends Node,
   /// Call this method after visiting the initializer of a variable declaration.
   void initialize(
       Variable variable, Type initializerType, Expression initializerExpression,
-      {required bool isFinal, required bool isLate});
+      {required bool isFinal,
+      required bool isLate,
+      required bool isImplicitlyTyped});
 
   /// Return whether the [variable] is definitely assigned in the current state.
   bool isAssigned(Variable variable);
@@ -724,11 +726,6 @@ abstract class FlowAnalysis<Node extends Object, Statement extends Node,
   void parenthesizedExpression(
       Expression outerExpression, Expression innerExpression);
 
-  /// Attempt to promote [variable] to [type].  The client may use this to
-  /// ensure that a variable declaration of the form `var x = expr;` promotes
-  /// `x` to type `X&T` in the circumstance where the type of `expr` is `X&T`.
-  void promote(Variable variable, Type type);
-
   /// Retrieves the type that the [variable] is promoted to, if the [variable]
   /// is currently promoted.  Otherwise returns `null`.
   Type? promotedType(Variable variable);
@@ -1225,13 +1222,18 @@ class FlowAnalysisDebug<Node extends Object, Statement extends Node,
   @override
   void initialize(
       Variable variable, Type initializerType, Expression initializerExpression,
-      {required bool isFinal, required bool isLate}) {
+      {required bool isFinal,
+      required bool isLate,
+      required bool isImplicitlyTyped}) {
     _wrap(
         'initialize($variable, $initializerType, $initializerExpression, '
-        'isFinal: $isFinal, isLate: $isLate)',
+        'isFinal: $isFinal, isLate: $isLate, '
+        'isImplicitlyTyped: $isImplicitlyTyped)',
         () => _wrapped.initialize(
             variable, initializerType, initializerExpression,
-            isFinal: isFinal, isLate: isLate));
+            isFinal: isFinal,
+            isLate: isLate,
+            isImplicitlyTyped: isImplicitlyTyped));
   }
 
   @override
@@ -1340,11 +1342,6 @@ class FlowAnalysisDebug<Node extends Object, Statement extends Node,
             _wrapped.parenthesizedExpression(outerExpression, innerExpression));
   }
 
-  @override
-  void promote(Variable variable, Type type) {
-    _wrap('promote($variable, $type', () => _wrapped.promote(variable, type));
-  }
-
   @override
   Type? promotedType(Variable variable) {
     return _wrap(
@@ -2059,12 +2056,14 @@ class FlowModel<Variable extends Object, Type extends Object> {
       Variable variable,
       Type writtenType,
       SsaNode<Variable, Type> newSsaNode,
-      TypeOperations<Variable, Type> typeOperations) {
+      TypeOperations<Variable, Type> typeOperations,
+      {bool promoteToTypeOfInterest = true}) {
     VariableModel<Variable, Type>? infoForVar = variableInfo[variable];
     if (infoForVar == null) return this;
 
     VariableModel<Variable, Type> newInfoForVar = infoForVar.write(
-        nonPromotionReason, variable, writtenType, typeOperations, newSsaNode);
+        nonPromotionReason, variable, writtenType, typeOperations, newSsaNode,
+        promoteToTypeOfInterest: promoteToTypeOfInterest);
     if (identical(newInfoForVar, infoForVar)) return this;
 
     return _updateVariableInfo(new VariableReference(variable), newInfoForVar);
@@ -2663,6 +2662,9 @@ abstract class TypeOperations<Variable extends Object, Type extends Object> {
   /// Return `true` if the [leftType] is a subtype of the [rightType].
   bool isSubtypeOf(Type leftType, Type rightType);
 
+  /// Returns `true` if [type] is a reference to a type parameter.
+  bool isTypeParameterType(Type type);
+
   /// Returns the non-null promoted version of [type].
   ///
   /// Note that some types don't have a non-nullable version (e.g.
@@ -2819,7 +2821,8 @@ class VariableModel<Variable extends Object, Type extends Object> {
       Variable variable,
       Type writtenType,
       TypeOperations<Variable, Type> typeOperations,
-      SsaNode<Variable, Type> newSsaNode) {
+      SsaNode<Variable, Type> newSsaNode,
+      {required bool promoteToTypeOfInterest}) {
     if (writeCaptured) {
       return new VariableModel<Variable, Type>(
           promotedTypes: promotedTypes,
@@ -2834,8 +2837,10 @@ class VariableModel<Variable extends Object, Type extends Object> {
     List<Type>? newPromotedTypes = demotionResult.promotedTypes;
 
     Type declaredType = typeOperations.variableType(variable);
-    newPromotedTypes = _tryPromoteToTypeOfInterest(
-        typeOperations, declaredType, newPromotedTypes, writtenType);
+    if (promoteToTypeOfInterest) {
+      newPromotedTypes = _tryPromoteToTypeOfInterest(
+          typeOperations, declaredType, newPromotedTypes, writtenType);
+    }
     // TODO(paulberry): remove demotions from demotionResult.nonPromotionHistory
     // that are no longer in effect due to re-promotion.
     if (identical(promotedTypes, newPromotedTypes) && assigned) {
@@ -3798,21 +3803,37 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
   @override
   void initialize(
       Variable variable, Type initializerType, Expression initializerExpression,
-      {required bool isFinal, required bool isLate}) {
-    ExpressionInfo<Variable, Type>? expressionInfo =
-        _getExpressionInfo(initializerExpression);
-    SsaNode<Variable, Type> newSsaNode = new SsaNode<Variable, Type>(isLate
-        ? null
-        : expressionInfo is _TrivialExpressionInfo
-            ? null
-            : expressionInfo);
-    if (isFinal) {
-      // We don't promote final variables on initialization, so pretend the
-      // written type is the variable's declared type.
-      initializerType = typeOperations.variableType(variable);
+      {required bool isFinal,
+      required bool isLate,
+      required bool isImplicitlyTyped}) {
+    ExpressionInfo<Variable, Type>? expressionInfo;
+    if (isLate) {
+      // Don't get expression info for late variables, since we don't know when
+      // they'll be initialized.
+    } else if (isImplicitlyTyped) {
+      // We don't get expression info for implicitly typed variables yet (bug
+      // https://github.com/dart-lang/language/issues/1785).
+      // TODO(paulberry): fix this.
+    } else {
+      expressionInfo = _getExpressionInfo(initializerExpression);
     }
+    SsaNode<Variable, Type> newSsaNode = new SsaNode<Variable, Type>(
+        expressionInfo is _TrivialExpressionInfo ? null : expressionInfo);
     _current = _current.write(
-        null, variable, initializerType, newSsaNode, typeOperations);
+        null, variable, initializerType, newSsaNode, typeOperations,
+        promoteToTypeOfInterest: !isImplicitlyTyped && !isFinal);
+    if (isImplicitlyTyped &&
+        typeOperations.isTypeParameterType(initializerType)) {
+      _current = _current
+          .tryPromoteForTypeCheck(
+              typeOperations,
+              new ReferenceWithType<Variable, Type>(
+                  new VariableReference<Variable, Type>(variable),
+                  promotedType(variable) ??
+                      typeOperations.variableType(variable)),
+              initializerType)
+          .ifTrue;
+    }
   }
 
   @override
@@ -3957,19 +3978,6 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
     forwardExpression(outerExpression, innerExpression);
   }
 
-  @override
-  void promote(Variable variable, Type type) {
-    _current = _current
-        .tryPromoteForTypeCheck(
-            typeOperations,
-            new ReferenceWithType<Variable, Type>(
-                new VariableReference<Variable, Type>(variable),
-                promotedType(variable) ??
-                    typeOperations.variableType(variable)),
-            type)
-        .ifTrue;
-  }
-
   @override
   Type? promotedType(Variable variable) {
     return _current.infoFor(variable).promotedTypes?.last;
@@ -4544,7 +4552,9 @@ class _LegacyTypePromotion<Node extends Object, Statement extends Node,
   @override
   void initialize(
       Variable variable, Type initializerType, Expression initializerExpression,
-      {required bool isFinal, required bool isLate}) {}
+      {required bool isFinal,
+      required bool isLate,
+      required bool isImplicitlyTyped}) {}
 
   @override
   bool isAssigned(Variable variable) {
@@ -4703,11 +4713,6 @@ class _LegacyTypePromotion<Node extends Object, Statement extends Node,
     forwardExpression(outerExpression, innerExpression);
   }
 
-  @override
-  void promote(Variable variable, Type type) {
-    throw new UnimplementedError('TODO(paulberry)');
-  }
-
   @override
   Type? promotedType(Variable variable) {
     return _knownTypes[variable];

pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart

@@ -1308,6 +1308,75 @@ main() {
       ]);
     });
 
+    group('initialize() promotes implicitly typed vars to type parameter types',
+        () {
+      test('when not final', () {
+        var h = Harness()
+          ..addSubtype('T&int', 'T', true)
+          ..addFactor('T', 'T&int', 'T');
+        var x = Var('x', 'T', isImplicitlyTyped: true);
+        h.run([
+          declareInitialized(x, expr('T&int')),
+          checkPromoted(x, 'T&int'),
+        ]);
+      });
+
+      test('when final', () {
+        var h = Harness()
+          ..addSubtype('T&int', 'T', true)
+          ..addFactor('T', 'T&int', 'T');
+        var x = Var('x', 'T', isImplicitlyTyped: true);
+        h.run([
+          declareInitialized(x, expr('T&int'), isFinal: true),
+          checkPromoted(x, 'T&int'),
+        ]);
+      });
+    });
+
+    group(
+        "initialize() doesn't promote explicitly typed vars to type "
+        'parameter types', () {
+      test('when not final', () {
+        var h = Harness();
+        var x = Var('x', 'T');
+        h.run([
+          declareInitialized(x, expr('T&int')),
+          checkNotPromoted(x),
+        ]);
+      });
+
+      test('when final', () {
+        var h = Harness();
+        var x = Var('x', 'T');
+        h.run([
+          declareInitialized(x, expr('T&int'), isFinal: true),
+          checkNotPromoted(x),
+        ]);
+      });
+    });
+
+    group(
+        "initialize() doesn't promote implicitly typed vars to ordinary types",
+        () {
+      test('when not final', () {
+        var h = Harness();
+        var x = Var('x', 'dynamic', isImplicitlyTyped: true);
+        h.run([
+          declareInitialized(x, expr('Null')),
+          checkNotPromoted(x),
+        ]);
+      });
+
+      test('when final', () {
+        var h = Harness();
+        var x = Var('x', 'dynamic', isImplicitlyTyped: true);
+        h.run([
+          declareInitialized(x, expr('Null'), isFinal: true),
+          checkNotPromoted(x),
+        ]);
+      });
+    });
+
     test('initialize() stores expressionInfo when not late', () {
       var h = Harness();
       var x = Var('x', 'Object');
@@ -1338,6 +1407,19 @@ main() {
       ]);
     });
 
+    test('initialize() does not store expressionInfo for implicitly typed vars',
+        () {
+      var h = Harness();
+      var x = Var('x', 'Object', isImplicitlyTyped: true);
+      var y = Var('y', 'int?');
+      h.run([
+        declareInitialized(x, y.expr.eq(nullLiteral)),
+        getSsaNodes((nodes) {
+          expect(nodes[x]!.expressionInfo, isNull);
+        }),
+      ]);
+    });
+
     test('initialize() does not store expressionInfo for trivial expressions',
         () {
       var h = Harness();

pkg/_fe_analyzer_shared/test/mini_ast.dart

@@ -459,6 +459,9 @@ class Harness extends TypeOperations<Var, Type> {
     return _subtypes[query] ?? fail('Unknown subtype query: $query');
   }
 
+  @override
+  bool isTypeParameterType(Type type) => type is PromotedTypeVariableType;
+
   @override
   Type promoteToNonNull(Type type) {
     if (type.type.endsWith('?')) {
@@ -649,8 +652,10 @@ abstract class TryStatement extends Statement implements TryBuilder {
 class Var {
   final String name;
   final Type type;
+  final bool isImplicitlyTyped;
 
-  Var(this.name, String typeStr) : type = Type(typeStr);
+  Var(this.name, String typeStr, {this.isImplicitlyTyped = false})
+      : type = Type(typeStr);
 
   /// Creates an L-value representing a reference to this variable.
   LValue get expr => new _VariableReference(this, null);
@@ -1608,7 +1613,9 @@ class _MiniAstTypeAnalyzer {
       var initializerType = analyzeExpression(initializer);
       flow.declare(variable, true);
       flow.initialize(variable, initializerType, initializer,
-          isFinal: isFinal, isLate: isLate);
+          isFinal: isFinal,
+          isLate: isLate,
+          isImplicitlyTyped: variable.isImplicitlyTyped);
     }
   }
 

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

@@ -399,6 +399,9 @@ class TypeSystemTypeOperations
     return typeSystem.isSubtypeOf(leftType, rightType);
   }
 
+  @override
+  bool isTypeParameterType(DartType type) => type is TypeParameterType;
+
   @override
   DartType promoteToNonNull(DartType type) {
     return typeSystem.promoteToNonNull(type);

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

@@ -2014,17 +2014,11 @@ class ResolverVisitor extends ResolverBase with ErrorDetectionHelpers {
     var declaredType = parent.type;
     if (initializer != null) {
       var initializerStaticType = initializer.typeOrThrow;
-      if (declaredType == null) {
-        if (_isNonNullableByDefault &&
-            initializerStaticType is TypeParameterType) {
-          flowAnalysis.flow?.promote(
-              declaredElement as PromotableElement, initializerStaticType);
-        }
-      } else {
-        flowAnalysis.flow?.initialize(declaredElement as PromotableElement,
-            initializerStaticType, initializer,
-            isFinal: parent.isFinal, isLate: parent.isLate);
-      }
+      flowAnalysis.flow?.initialize(declaredElement as PromotableElement,
+          initializerStaticType, initializer,
+          isFinal: parent.isFinal,
+          isLate: parent.isLate,
+          isImplicitlyTyped: declaredType == null);
     }
   }
 

pkg/front_end/lib/src/fasta/kernel/inference_visitor.dart

@@ -6458,20 +6458,15 @@ class InferenceVisitor
     }
     if (initializerResult != null) {
       DartType initializerType = initializerResult.inferredType;
-      if (node.isImplicitlyTyped) {
-        if (inferrer.isNonNullableByDefault &&
-            initializerType is TypeParameterType) {
-          inferrer.flowAnalysis.promote(node, initializerType);
-        }
-      } else {
-        // TODO(paulberry): `initializerType` is sometimes `null` during top
-        // level inference.  Figure out how to prevent this.
-        // ignore: unnecessary_null_comparison
-        if (initializerType != null) {
-          inferrer.flowAnalysis.initialize(
-              node, initializerType, initializerResult.expression,
-              isFinal: node.isFinal, isLate: node.isLate);
-        }
+      // TODO(paulberry): `initializerType` is sometimes `null` during top
+      // level inference.  Figure out how to prevent this.
+      // ignore: unnecessary_null_comparison
+      if (initializerType != null) {
+        inferrer.flowAnalysis.initialize(
+            node, initializerType, initializerResult.expression,
+            isFinal: node.isFinal,
+            isLate: node.isLate,
+            isImplicitlyTyped: node.isImplicitlyTyped);
       }
       Expression initializer = inferrer.ensureAssignableResult(
           node.type, initializerResult,