[dart2js] Move parameter narrowing from NarrowTypeInformation into field on ParameterTypeInformation.

Change-Id: I1715e47e73c2d45fbe6e4f8bfc9a336224f53bc3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304161
Reviewed-by: Mayank Patke <[email protected]>
Commit-Queue: Nate Biggs <[email protected]>

Author: natebiggs

pkg/compiler/lib/src/inferrer/builder.dart

@@ -282,9 +282,8 @@ class KernelTypeGraphBuilder extends ir.Visitor<TypeInformation?>
   void handleParameter(ir.VariableDeclaration node,
       {required bool isOptional}) {
     Local local = _localsMap.getLocalVariable(node);
-    DartType type = _localsMap.getLocalType(_elementMap, local);
-    _state.updateLocal(_inferrer, _capturedAndBoxed, local,
-        _inferrer.typeOfParameter(local), type);
+    _state.setLocal(
+        _inferrer, _capturedAndBoxed, local, _inferrer.typeOfParameter(local));
     if (isOptional) {
       TypeInformation type;
       if (node.initializer != null) {
@@ -2502,11 +2501,21 @@ class LocalState {
       {isCast = true,
       excludeNull = false,
       excludeLateSentinel = false}) {
-    type = inferrer.types.narrowType(type, staticType,
-        isCast: isCast,
-        excludeNull: excludeNull,
-        excludeLateSentinel: excludeLateSentinel);
+    setLocal(
+        inferrer,
+        capturedAndBoxed,
+        local,
+        inferrer.types.narrowType(type, staticType,
+            isCast: isCast,
+            excludeNull: excludeNull,
+            excludeLateSentinel: excludeLateSentinel));
+  }
 
+  void setLocal(
+      InferrerEngine inferrer,
+      Map<Local, FieldEntity> capturedAndBoxed,
+      Local local,
+      TypeInformation type) {
     final field = capturedAndBoxed[local];
     if (field != null) {
       inferrer.recordTypeOfField(field, type);

pkg/compiler/lib/src/inferrer/engine.dart

@@ -791,21 +791,7 @@ class InferrerEngine {
           info.type = info.refine(this);
           info.doNotEnqueue = true;
         }
-        for (final user in info.users) {
-          _workQueue.add(user);
-          // Virtual parameters that are part of a cycle can end up stuck in
-          // a refinement pattern that prevents the members of the cycle from
-          // converging. We avoid this by adding the concrete parameter and
-          // corresponding narrowing type to the queue before other members of
-          // the cycle.
-          if (user is ParameterTypeInformation) {
-            final concreteParameterType = user.concreteParameterType;
-            if (concreteParameterType != null) {
-              _workQueue.add(concreteParameterType);
-              _workQueue.addAll(concreteParameterType.users);
-            }
-          }
-        }
+        _workQueue.addAll(info.users);
         if (info.hasStableType(this)) {
           info.stabilize(this);
         }

pkg/compiler/lib/src/inferrer/type_graph_inferrer.dart

@@ -121,7 +121,10 @@ class TypeGraphInferrer implements TypesInferrer {
 
     inferrer.types.forEachParameterType(
         (Local parameter, ParameterTypeInformation typeInformation) {
-      AbstractValue type = typeInformation.type;
+      // We need to get the correct type based on the parameter's check policy.
+      // This will be used to determine if the parameter needs a check at the
+      // beginning of the associated function.
+      AbstractValue type = typeInformation.checkedType(inferrer);
       parameterResults[parameter] = type;
     });
 

pkg/compiler/lib/src/inferrer/type_graph_nodes.dart

@@ -795,19 +795,26 @@ class ParameterTypeInformation extends ElementTypeInformation {
   final Local _parameter;
   final AbstractValue _type;
   final FunctionEntity _method;
+
+  /// The input type is calculated directly from the union of this node's
+  /// inputs (i.e. the actual arguments for this parameter). When this
+  /// parameter's static type is not trusted, an implicit check will be added
+  /// based on this type. [type] on the other hand is the type of this parameter
+  /// within the function body so it is narrowed using the static type.
+  AbstractValue _inputType;
   bool get _isInstanceMemberParameter =>
       _hasFlag(_Flag.isInstanceMemberParameter);
   bool get _isClosureParameter => _hasFlag(_Flag.isClosureParameter);
   bool get _isInitializingFormal => _hasFlag(_Flag.isInitializingFormal);
   bool _isTearOffClosureParameter = false;
-  TypeInformation? get concreteParameterType =>
-      _hasFlag(_Flag.isVirtual) ? users.first : null;
+  bool get _isVirtual => _hasFlag(_Flag.isVirtual);
 
   ParameterTypeInformation.localFunction(super.abstractValueDomain,
       super.context, this._parameter, DartType type, this._method)
       : _type = abstractValueDomain
             .createFromStaticType(type, nullable: true)
             .abstractValue,
+        _inputType = abstractValueDomain.uncomputedType,
         super._internal() {
     _setFlag(_Flag.isClosureParameter);
   }
@@ -822,6 +829,7 @@ class ParameterTypeInformation extends ElementTypeInformation {
       : _type = abstractValueDomain
             .createFromStaticType(type, nullable: true)
             .abstractValue,
+        _inputType = abstractValueDomain.uncomputedType,
         super._internal() {
     _setFlagTo(_Flag.isInitializingFormal, isInitializingFormal);
   }
@@ -831,6 +839,7 @@ class ParameterTypeInformation extends ElementTypeInformation {
       {required bool isVirtual})
       : _type =
             _createInstanceMemberStaticType(abstractValueDomain, type, _method),
+        _inputType = abstractValueDomain.uncomputedType,
         super._withInputs() {
     _setFlag(_Flag.isInstanceMemberParameter);
     _setFlagTo(_Flag.isVirtual, isVirtual);
@@ -916,53 +925,61 @@ class ParameterTypeInformation extends ElementTypeInformation {
 
   AbstractValue potentiallyNarrowType(
       AbstractValue mask, InferrerEngine inferrer) {
-    if (inferrer.closedWorld.annotationsData
-        .getParameterCheckPolicy(method)
-        .isTrusted) {
-      // In checked or strong mode we don't trust the types of the arguments
-      // passed to a parameter. The means that the checking of a parameter is
-      // based on the actual arguments.
-      //
-      // With --trust-type-annotations or --omit-implicit-checks we _do_ trust
-      // the arguments passed to a parameter - and we never check them.
-      //
-      // In all these cases we _do_ trust the static type of a parameter within
-      // the method itself. For instance:
-      //
-      //     method(int i) => i;
-      //     main() {
-      //       dynamic f = method;
-      //       f(0); // valid call
-      //       f(''); // invalid call
-      //     }
-      //
-      // Here, in all cases, we infer the returned value of `method` to be an
-      // `int`. In checked and strong mode we infer the parameter of `method` to
-      // be either `int` or `String` and therefore insert a check at the entry
-      // of 'method'. With --trust-type-annotations or --omit-implicit-checks we
-      // (unsoundly) infer the parameter to be `int` and leave the parameter
-      // unchecked, and `method` will at runtime actually return a `String` from
-      // the second invocation.
-      //
-      // The trusting of the parameter types within the body of the method is
-      // is done through `LocalsHandler.update` called in
-      // `KernelTypeGraphBuilder.handleParameter`.
-      return _narrowType(inferrer.abstractValueDomain, mask, _type);
-    }
-    return mask;
+    return _narrowType(inferrer.abstractValueDomain, mask, _type);
+  }
+
+  AbstractValue checkedType(InferrerEngine inferrer) {
+    // By default we don't trust the types of the arguments passed to a
+    // parameter. This means that the checking of a parameter is based on the
+    // actual arguments.
+    //
+    // With --omit-implicit-checks we _do_ trust the arguments passed to a
+    // parameter - and we never check them.
+    //
+    // In all these cases we _do_ trust the static type of a parameter within
+    // the method itself. For instance:
+    //
+    //     method(int i) => i;
+    //     main() {
+    //       dynamic f = method;
+    //       f(0); // valid call
+    //       f(''); // invalid call
+    //     }
+    //
+    // Here, in all cases, we infer the returned value of `method` to be an
+    // `int`. By default we infer the parameter of `method` to be either
+    // `int` or `String` and therefore insert a check at the entry of 'method'.
+    // With --omit-implicit-checks we (unsoundly) infer the parameter to be
+    // `int` and leave the parameter unchecked, and `method` will at runtime
+    // actually return a `String` from the second invocation.
+    //
+    // The trusting of the parameter types within the body of the method is
+    // handled by `potentiallyNarrowType` on each call to `computeType`.
+    return inferrer.closedWorld.annotationsData
+            .getParameterCheckPolicy(method)
+            .isTrusted
+        ? type
+        : _inputType;
   }
 
   @override
   AbstractValue computeType(InferrerEngine inferrer) {
     final special = handleSpecialCases(inferrer);
     if (special != null) return special;
-    return potentiallyNarrowType(
-        inferrer.types.computeTypeMask(inputs), inferrer);
+    final inputType = _inputType = inferrer.types.computeTypeMask(inputs);
+    // Virtual parameters are only inputs to other parameters (virtual or
+    // concrete) and not function bodies. The user parameters need to know the
+    // full set of inputs passed to the virtual parameter.
+    return _isVirtual ? inputType : potentiallyNarrowType(inputType, inferrer);
   }
 
   @override
   AbstractValue safeType(InferrerEngine inferrer) {
-    return potentiallyNarrowType(super.safeType(inferrer), inferrer);
+    final inputType = _inputType = super.safeType(inferrer);
+    // Virtual parameters are only inputs to other parameters (virtual or
+    // concrete) and not function bodies. The user parameters need to know the
+    // full set of inputs passed to the virtual parameter.
+    return _isVirtual ? inputType : potentiallyNarrowType(inputType, inferrer);
   }
 
   @override