Disagreement about the necessity and behavior of a cast between analyzer and CFE/backends

[natebosch]

There is a disagreement between the analyzer and the CFE (or the backends) around type promotion. My first instinct was that the analyzer was incorrect and the unnecessary_cast should not be reported, since the cast does impact behavior in the CFE.

We can see the difference in behavior without the cast by printing the static type that was filled in by inference.
I also comparing the runtime behavior of printing the static type to the IDE hover type information.

class Foo<T> {}

class C<T extends Object> {
  void something(T? Function() read) {
    var argumentValue = read();
    if (argumentValue is! Foo<String>) {
      return;
    }
    printStaticType(argumentValue);
    var fooValue = argumentValue as Foo<String>;
    //             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unnecessary_cast
    printStaticType(fooValue);
    printStaticFooType(fooValue);
  }
}

void printStaticType<T>(T t) {
  print(T);
}

void printStaticFooType<T>(Foo<T> f) {
  print(T);
}

void main() {
  C<Foo<String>>().something(() => Foo<String>());
}

This prints

Foo<String>?
Foo<String>
String

Using hover information in the IDE, analyzer reports the type of argumentValue where it is printed as T & Foo<String>, and reports the type of fooValue as Foo<String>

If I remove the "unnecessary" cast by running the quick fix, the behavior changes, and it prints:

Foo<String>?
Foo<String>?
dynamic

This CFE behavior is a little confusing, because if fooValue is statically inferred as Foo<String>?, there should have been an error at the call to printStaticFooType which takes a non-nullable Foo.

[natebosch]

I'm also confused by the impact of adding an && in the conditional. Is there an easy to understand explanation for why the extra condition impacts inference in this way?

diff --git a/foo.dart b/foo.dart
index ebc2e52..923fdd8 100644
--- a/foo.dart
+++ b/foo.dart
@@ -3,7 +3,7 @@ class Foo<T> {}
 class C<T extends Object> {
   void something(T? Function() read) {
     var argumentValue = read();
-    if (argumentValue is! Foo<String>) {
+    if (argumentValue != null && argumentValue is! Foo<String>) {
       return;
     }
     printStaticType(argumentValue);

When I make this change and remove the cast, the call to printStaticFooType(fooValue) becomes a static error. "The argument type T? cannot be assigned to the parameter type Foo<dynamic>". The CFE and analyzer agree on this error.

The CFE thinks the variables have type Foo<String>? (by running the code), and the analyzer thinks they have type T? (by hovering in IDE).

[lrhn]

The promotion from T? by is Foo<String> should, by logic, promote to T & Foo<String>.

As such, the "unnecessary cast" warning is consistent, you get that warning for all up-casts, whether they affect type inference or not.

If you don't do the cast, the static type of fooValue is derived from T & Foo<String>. I don't remember if the type becomes T or Foo<String>, but it can't be the intersection type. Probably the former. It's then immediately assignment promoted to T & Foo<String>.

That's what's supposed to happen. I think.

What it actually happening seems to be that the promoted type is T? & Foo<String>. Which is not a type in the Dart type system at all. (I remember reporting something like that before.)

That explains the behaviors.

That intersection type is erased when the static type is captured, so it prints T?, which happens to be Foo<String>?.

The call to printStaticFooType gets confused, and infers dynamic, I can't blame it when faced with s type that doesn't exist, but the argument is a static intersection type where the second half makes it a subtype of Foo<String>, so it's valid as argument.

We need to fix the invalid intersection types. Somehow.

[lrhn]

About the &&, that's on the escape branch. The continuation branch is guarded by the negation of that test, which is effectively

if (argumentValue == null || argumentValue is Foo<String>) ...

We don't do promotion on "or" tests in general (we could here, the LUB is still a type of interest), so you get no promotion, and the type is still T?, which is Foo<String>? at runtime because of the actual type argument.

[lrhn]

Existing related issues are #49691, #50676 and #50677.

[eernstg]

Interesting, @natebosch!

I'd note that it is slightly dangerous to use functions like printStaticType and printStaticFooType, because they receive a reified type argument, and hence we'll never see the promoted (intersection) type of the actual argument, just the erasure (and in situations like this that might be significant). That's the reason why I've used X x = argumentValue; and T t = argumentValue; to verify that argumentValue had the type X & T at some point in a similar situations (where X and T do not have any other common non-bottom subtypes than X & T).

Here is the discrepancy again, in a slightly smaller example:

// ignore_for_file: unused_local_variable

void f<X>(X? arg) {
  if (arg is int) {
    // Check that `arg` has type `X & int`.
    X x = arg;
    int i = arg;
    // Show the run-time value of the erased static type of `arg`.
    var xs = [arg];
    print(xs.runtimeType); // `List<Object?>`.
    // List<X> ys = xs; // CFE reports error, analyzer accepts.
    List<X?> ys = xs; // Accepted by both.
  }
}

void main() {
  f<Object>(1);
}

So the analyzer as well as the CFE accept arg as the initializing expression of both x and i, which is basically proof that arg has the type X & int at that point.

However, when the list literal is created it gets the actual argument type Object?, which is not the erasure of X & int. I would have expected a List<X>, which in the given invocation would have been a List<Object>, but the printed run-time type as well as the following declarations reveal that the CFE considers xs to have type List<X?>.

Looks like the CFE uses the declared type of arg rather than the erasure of the promoted type at that point. Or perhaps this is caused by the representation of nullable types in the CFE, using a bit on the type?

In any case, this issue should probably not use the label 'area-fe-analyzer-shared', considering that the CFE produces the surprising erasure, and the analyzer arrives at the expected result. (I wouldn't say 'correct'/'incorrect', because we don't have a detailed specification of how to erase intersection types in all relevant situations.)

@johnniwinther, do you have further comments on this?

[chloestefantsova]

Side note: this issue initiated an interesting discussion, which resulted in dart-lang/language#3239.