Revert "Flow analysis: track property get targets."

This reverts commit bc6cdf5.

Reason for revert: Broke Golem benchmarks - https://golem.corp.goog/PerformanceChanges?repository=dart&revision=91561

Original change's description:
> Flow analysis: track property get targets.
>
> These targets are needed for "why not promoted" error messages, and
> it's easier to have flow analysis keep track of them than to have the
> analyzer and CFE try to reconstruct them at the time of reporting the
> error.
>
> Bug: #44898
> Change-Id: Ia8ef4a7ce13cc30860e59b7369e6230d233e252d
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/193832
> Commit-Queue: Paul Berry <[email protected]>
> Reviewed-by: Johnni Winther <[email protected]>
> Reviewed-by: Konstantin Shcheglov <[email protected]>

[email protected],[email protected],[email protected]

Change-Id: Ic2b66b1db621c0f4e4c5398acfe2ae61bd7625ca
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: #44898
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/194104
Reviewed-by: Paul Berry <[email protected]>
Commit-Queue: Paul Berry <[email protected]>

Author: stereotype441

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

@@ -736,13 +736,8 @@ abstract class FlowAnalysis<Node extends Object, Statement extends Node,
   /// expression to the left hand side of the `.`, and [propertyName] should be
   /// the identifier to the right hand side of the `.`.  [staticType] should be
   /// the static type of the value returned by the property get.
-  ///
-  /// [propertyMember] should be whatever data structure the client uses to keep
-  /// track of the field or property being accessed.  In the event of
-  /// non-promotion of a property get, this value can be retrieved from
-  /// [PropertyNotPromoted.propertyMember].
   void propertyGet(Expression wholeExpression, Expression target,
-      String propertyName, Object? propertyMember, Type staticType);
+      String propertyName, Type staticType);
 
   /// Retrieves the SSA node associated with [variable], or `null` if [variable]
   /// is not associated with an SSA node because it is write captured.  For
@@ -793,13 +788,8 @@ abstract class FlowAnalysis<Node extends Object, Statement extends Node,
   /// the whole property get, and [propertyName] should be the name of the
   /// property being read.  [staticType] should be the static type of the value
   /// returned by the property get.
-  ///
-  /// [propertyMember] should be whatever data structure the client uses to keep
-  /// track of the field or property being accessed.  In the event of
-  /// non-promotion of a property get, this value can be retrieved from
-  /// [PropertyNotPromoted.propertyMember].
-  void thisOrSuperPropertyGet(Expression expression, String propertyName,
-      Object? propertyMember, Type staticType);
+  void thisOrSuperPropertyGet(
+      Expression expression, String propertyName, Type staticType);
 
   /// Call this method just before visiting the body of a "try/catch" statement.
   ///
@@ -1349,12 +1339,11 @@ class FlowAnalysisDebug<Node extends Object, Statement extends Node,
 
   @override
   void propertyGet(Expression wholeExpression, Expression target,
-      String propertyName, Object? propertyMember, Type staticType) {
+      String propertyName, Type staticType) {
     _wrap(
-        'propertyGet($wholeExpression, $target, $propertyName, '
-        '$propertyMember, $staticType)',
+        'propertyGet($wholeExpression, $target, $propertyName, $staticType)',
         () => _wrapped.propertyGet(
-            wholeExpression, target, propertyName, propertyMember, staticType));
+            wholeExpression, target, propertyName, staticType));
   }
 
   @override
@@ -1389,13 +1378,12 @@ class FlowAnalysisDebug<Node extends Object, Statement extends Node,
   }
 
   @override
-  void thisOrSuperPropertyGet(Expression expression, String propertyName,
-      Object? propertyMember, Type staticType) {
+  void thisOrSuperPropertyGet(
+      Expression expression, String propertyName, Type staticType) {
     _wrap(
-        'thisOrSuperPropertyGet($expression, $propertyName, $propertyMember, '
-        '$staticType)',
+        'thisOrSuperPropertyGet($expression, $propertyName, $staticType)',
         () => _wrapped.thisOrSuperPropertyGet(
-            expression, propertyName, propertyMember, staticType));
+            expression, propertyName, staticType));
   }
 
   @override
@@ -2346,17 +2334,12 @@ class PropertyNotPromoted<Type extends Object> extends NonPromotionReason {
   /// The name of the property.
   final String propertyName;
 
-  /// The field or property being accessed.  This matches a `propertyMember`
-  /// value that was passed to either [FlowAnalysis.propertyGet] or
-  /// [FlowAnalysis.thisOrSuperPropertyGet].
-  final Object? propertyMember;
-
   /// The static type of the property at the time of the access.  This is the
   /// type that was passed to [FlowAnalysis.whyNotPromoted]; it is provided to
   /// the client as a convenience for ID testing.
   final Type staticType;
 
-  PropertyNotPromoted(this.propertyName, this.propertyMember, this.staticType);
+  PropertyNotPromoted(this.propertyName, this.staticType);
 
   @override
   String get documentationLink => 'http://dart.dev/go/non-promo-property';
@@ -2534,10 +2517,8 @@ abstract class Reference<Variable extends Object, Type extends Object> {
 
   /// Creates a reference representing a get of a property called [propertyName]
   /// on the reference represented by `this`.
-  Reference<Variable, Type> propertyGet(
-          String propertyName, Object? propertyMember) =>
-      new _PropertyGetReference<Variable, Type>(
-          this, propertyName, propertyMember);
+  Reference<Variable, Type> propertyGet(String propertyName) =>
+      new _PropertyGetReference<Variable, Type>(this, propertyName);
 
   /// Stores info for this reference in [variableInfo].
   void storeInfo(Map<Variable?, VariableModel<Variable, Type>> variableInfo,
@@ -3971,14 +3952,14 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
 
   @override
   void propertyGet(Expression wholeExpression, Expression target,
-      String propertyName, Object? propertyMember, Type staticType) {
+      String propertyName, Type staticType) {
     Reference<Variable, Type>? reference =
         _getExpressionReference(target)?.reference;
     if (reference != null) {
       _storeExpressionReference(
           wholeExpression,
           new ReferenceWithType<Variable, Type>(
-              reference.propertyGet(propertyName, propertyMember), staticType));
+              reference.propertyGet(propertyName), staticType));
     }
   }
 
@@ -4035,13 +4016,12 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
   }
 
   @override
-  void thisOrSuperPropertyGet(Expression expression, String propertyName,
-      Object? propertyMember, Type staticType) {
+  void thisOrSuperPropertyGet(
+      Expression expression, String propertyName, Type staticType) {
     _storeExpressionReference(
         expression,
         new ReferenceWithType<Variable, Type>(
-            new _ThisReference<Variable, Type>()
-                .propertyGet(propertyName, propertyMember),
+            new _ThisReference<Variable, Type>().propertyGet(propertyName),
             staticType));
   }
 
@@ -4706,7 +4686,7 @@ class _LegacyTypePromotion<Node extends Object, Statement extends Node,
 
   @override
   void propertyGet(Expression wholeExpression, Expression target,
-      String propertyName, Object? propertyMember, Type staticType) {}
+      String propertyName, Type staticType) {}
 
   @override
   SsaNode<Variable, Type>? ssaNodeForTesting(Variable variable) {
@@ -4726,8 +4706,8 @@ class _LegacyTypePromotion<Node extends Object, Statement extends Node,
   void thisOrSuper(Expression expression, Type staticType) {}
 
   @override
-  void thisOrSuperPropertyGet(Expression expression, String propertyName,
-      Object? propertyMember, Type staticType) {}
+  void thisOrSuperPropertyGet(
+      Expression expression, String propertyName, Type staticType) {}
 
   @override
   void tryCatchStatement_bodyBegin() {}
@@ -4928,12 +4908,7 @@ class _PropertyGetReference<Variable extends Object, Type extends Object>
   /// The name of the property.
   final String propertyName;
 
-  /// The field or property being accessed.  This matches a `propertyMember`
-  /// value that was passed to either [FlowAnalysis.propertyGet] or
-  /// [FlowAnalysis.thisOrSuperPropertyGet].
-  final Object? propertyMember;
-
-  _PropertyGetReference(this.target, this.propertyName, this.propertyMember);
+  _PropertyGetReference(this.target, this.propertyName);
 
   @override
   Map<Type, NonPromotionReason> Function() getNonPromotionReasons(
@@ -4945,8 +4920,7 @@ class _PropertyGetReference<Variable extends Object, Type extends Object>
       return () {
         Map<Type, NonPromotionReason> result = <Type, NonPromotionReason>{};
         for (Type type in promotedTypes) {
-          result[type] =
-              new PropertyNotPromoted(propertyName, propertyMember, staticType);
+          result[type] = new PropertyNotPromoted(propertyName, staticType);
         }
         return result;
       };

pkg/_fe_analyzer_shared/test/mini_ast.dart

@@ -1510,7 +1510,7 @@ class _Property extends LValue {
       Harness h, FlowAnalysis<Node, Statement, Expression, Var, Type> flow) {
     var targetType = target._visit(h, flow);
     var propertyType = h.getMember(targetType, propertyName);
-    flow.propertyGet(this, target, propertyName, propertyName, propertyType);
+    flow.propertyGet(this, target, propertyName, propertyType);
     return propertyType;
   }
 
@@ -1614,7 +1614,7 @@ class _ThisOrSuperPropertyGet extends Expression {
   @override
   Type _visit(
       Harness h, FlowAnalysis<Node, Statement, Expression, Var, Type> flow) {
-    flow.thisOrSuperPropertyGet(this, propertyName, propertyName, type);
+    flow.thisOrSuperPropertyGet(this, propertyName, type);
     return type;
   }
 }

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

@@ -121,7 +121,8 @@ class AssignmentExpressionResolver {
       CompileTimeErrorCode.INVALID_ASSIGNMENT,
       right,
       [rightType, writeType],
-      _resolver.computeWhyNotPromotedMessages(right, whyNotPromoted?.call()),
+      _resolver.computeWhyNotPromotedMessages(
+          right, right, whyNotPromoted?.call()),
     );
   }
 

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

@@ -807,11 +807,7 @@ class MethodInvocationResolver {
         );
       }
       _resolver.flowAnalysis?.flow?.propertyGet(
-          functionExpression,
-          target,
-          node.methodName.name,
-          node.methodName.staticElement,
-          getterReturnType);
+          functionExpression, target, node.methodName.name, getterReturnType);
       functionExpression.staticType = targetType;
     }
 

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

@@ -203,8 +203,8 @@ class PropertyElementResolver {
       readElementRequested = readLookup.requested;
       if (readElementRequested is PropertyAccessorElement &&
           !readElementRequested.isStatic) {
-        _resolver.flowAnalysis?.flow?.thisOrSuperPropertyGet(node, node.name,
-            readElementRequested, readElementRequested.returnType);
+        _resolver.flowAnalysis?.flow?.thisOrSuperPropertyGet(
+            node, node.name, readElementRequested.returnType);
       }
       _resolver.checkReadOfNotAssignedLocalVariable(node, readElementRequested);
     }
@@ -373,11 +373,7 @@ class PropertyElementResolver {
       nameErrorEntity: propertyName,
     );
 
-    _resolver.flowAnalysis?.flow?.propertyGet(
-        node,
-        target,
-        propertyName.name,
-        result.getter,
+    _resolver.flowAnalysis?.flow?.propertyGet(node, target, propertyName.name,
         result.getter?.returnType ?? _typeSystem.typeProvider.dynamicType);
 
     if (hasRead && result.needsGetterError) {
@@ -657,7 +653,6 @@ class PropertyElementResolver {
             node,
             target,
             propertyName.name,
-            readElement,
             readElement?.returnType ?? _typeSystem.typeProvider.dynamicType);
       }
 

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

@@ -130,11 +130,11 @@ class TypePropertyResolver {
       if (flow != null) {
         if (receiver != null) {
           messages = _resolver.computeWhyNotPromotedMessages(
-              nameErrorEntity, flow.whyNotPromoted(receiver)());
+              receiver, nameErrorEntity, flow.whyNotPromoted(receiver)());
         } else {
           var thisType = _resolver.thisType;
           if (thisType != null) {
-            messages = _resolver.computeWhyNotPromotedMessages(
+            messages = _resolver.computeWhyNotPromotedMessages(receiver,
                 nameErrorEntity, flow.whyNotPromotedImplicitThis(thisType)());
           }
         }

pkg/analyzer/lib/src/error/bool_expression_verifier.dart

@@ -57,7 +57,7 @@ class BoolExpressionVerifier {
             errorCode: CompileTimeErrorCode
                 .UNCHECKED_USE_OF_NULLABLE_VALUE_AS_CONDITION,
             messages: _resolver.computeWhyNotPromotedMessages(
-                expression, whyNotPromoted?.call()));
+                expression, expression, whyNotPromoted?.call()));
       } else {
         _errorReporter.reportErrorForNode(errorCode, expression, arguments);
       }

pkg/analyzer/lib/src/error/nullable_dereference_verifier.dart

@@ -76,8 +76,8 @@ class NullableDereferenceVerifier {
 
     List<DiagnosticMessage>? messages;
     if (errorNode is Expression) {
-      messages = _resolver.computeWhyNotPromotedMessages(
-          errorNode, _resolver.flowAnalysis?.flow?.whyNotPromoted(errorNode)());
+      messages = _resolver.computeWhyNotPromotedMessages(errorNode, errorNode,
+          _resolver.flowAnalysis?.flow?.whyNotPromoted(errorNode)());
     }
     report(errorNode, receiverType, errorCode: errorCode, messages: messages);
     return true;

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

@@ -85,8 +85,8 @@ mixin ErrorDetectionHelpers {
       }
       return;
     }
-    var messages =
-        computeWhyNotPromotedMessages(expression, whyNotPromoted?.call());
+    var messages = computeWhyNotPromotedMessages(
+        expression, expression, whyNotPromoted?.call());
     // report problem
     if (isConstConstructor) {
       // TODO(paulberry): this error should be based on the actual type of the
@@ -229,6 +229,7 @@ mixin ErrorDetectionHelpers {
   /// [whyNotPromoted] should be the non-promotion details returned by the flow
   /// analysis engine.
   List<DiagnosticMessage> computeWhyNotPromotedMessages(
+      Expression? expression,
       SyntacticEntity errorEntity,
       Map<DartType, NonPromotionReason>? whyNotPromoted);
 
@@ -312,7 +313,8 @@ mixin ErrorDetectionHelpers {
         errorCode,
         getErrorNode(expression),
         [actualStaticType, expectedStaticType],
-        computeWhyNotPromotedMessages(expression, whyNotPromoted?.call()),
+        computeWhyNotPromotedMessages(
+            expression, expression, whyNotPromoted?.call()),
       );
       return false;
     }

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

@@ -312,6 +312,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
 
   @override
   List<DiagnosticMessage> computeWhyNotPromotedMessages(
+      Expression? expression,
       SyntacticEntity errorEntity,
       Map<DartType, NonPromotionReason>? whyNotPromoted) {
     return [];

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

@@ -533,13 +533,17 @@ class ResolverVisitor extends ScopedVisitor with ErrorDetectionHelpers {
 
   @override
   List<DiagnosticMessage> computeWhyNotPromotedMessages(
+      Expression? expression,
       SyntacticEntity errorEntity,
       Map<DartType, NonPromotionReason>? whyNotPromoted) {
+    if (expression is NamedExpression) {
+      expression = expression.expression;
+    }
     List<DiagnosticMessage> messages = [];
     if (whyNotPromoted != null) {
       for (var entry in whyNotPromoted.entries) {
         var whyNotPromotedVisitor = _WhyNotPromotedVisitor(
-            source, errorEntity, flowAnalysis!.dataForTesting);
+            source, expression, errorEntity, flowAnalysis!.dataForTesting);
         if (typeSystem.isPotentiallyNullable(entry.key)) continue;
         var message = entry.value.accept(whyNotPromotedVisitor);
         if (message != null) {
@@ -3443,6 +3447,10 @@ class _WhyNotPromotedVisitor
             PromotableElement, DartType> {
   final Source source;
 
+  /// The expression that was not promoted, or `null` if the thing that was not
+  /// promoted was an implicit `this`.
+  final Expression? _expression;
+
   final SyntacticEntity _errorEntity;
 
   final FlowAnalysisDataForTesting? _dataForTesting;
@@ -3451,7 +3459,8 @@ class _WhyNotPromotedVisitor
 
   DartType? propertyType;
 
-  _WhyNotPromotedVisitor(this.source, this._errorEntity, this._dataForTesting);
+  _WhyNotPromotedVisitor(
+      this.source, this._expression, this._errorEntity, this._dataForTesting);
 
   @override
   DiagnosticMessage? visitDemoteViaExplicitWrite(
@@ -3471,7 +3480,18 @@ class _WhyNotPromotedVisitor
   @override
   DiagnosticMessage? visitPropertyNotPromoted(
       PropertyNotPromoted<DartType> reason) {
-    var receiverElement = reason.propertyMember;
+    var expression = _expression;
+    Element? receiverElement;
+    if (expression is SimpleIdentifier) {
+      receiverElement = expression.staticElement;
+    } else if (expression is PropertyAccess) {
+      receiverElement = expression.propertyName.staticElement;
+    } else if (expression is PrefixedIdentifier) {
+      receiverElement = expression.identifier.staticElement;
+    } else {
+      assert(false,
+          'Unrecognized property access expression: ${expression.runtimeType}');
+    }
     if (receiverElement is PropertyAccessorElement) {
       propertyReference = receiverElement;
       propertyType = reason.staticType;